Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testable examples must have an output comment #3084

Closed
jamietanna opened this issue Aug 9, 2022 · 4 comments
Closed

Testable examples must have an output comment #3084

jamietanna opened this issue Aug 9, 2022 · 4 comments
Labels
enhancement New feature or improvement linter: idea idea of a linter

Comments

@jamietanna
Copy link

Your feature request related to a problem? Please describe.

When writing testable examples, the following code is expected:

func ExampleHello() {
    fmt.Println("hello")
    // Output: hello
}

Or a lowercase output:

func ExampleHello() {
    fmt.Println("hello")
    // output: hello
}

However, when writing the following, without an Output comment:

func ExampleHello() {
    fmt.Println("hello")
}

Then the test compiles correctly, but is not picked up by go test.

We should add a rule which flags when an output comment is missing.

Describe the solution you'd like.

Add a new rule which flags when a testable example - a test that has the test method prefix Example - does not contain a comment with an Output:

Describe alternatives you've considered.

This may not be fit to come into golangci-lint per se, but into one of the other linters.

Additional context.

No response

@jamietanna jamietanna added the enhancement New feature or improvement label Aug 9, 2022
@ldez ldez added the linter: new Support new linter label Aug 9, 2022
@ldez ldez changed the title New rule: testable examples must have an output comment Testable examples must have an output comment Aug 9, 2022
@maratori
Copy link
Contributor

At first glance it's a good idea.
But if I would like to create an example without output, I can't use //nolint and will have to use exclude-rules in .golangci.yml.
That's because //nolint comment will be added to godoc.

func Example_sameLine() { // nolint:testableexamples
	panic("xxx")
}

// nolint:testableexamples
func Example_previousLine() {
	panic("xxx")
}

image

@ldez
Copy link
Member

ldez commented Aug 28, 2022

@maratori you must write nolint with the directive syntax.

The comment syntax is now deprecated.

From my memories, directives are excluded from the generated godoc.


Due to a change of go fmt inside go1.19.
We have to enforce the directive style.

So the option allow-leading-space has been dropped.

The syntax of directives is: [a-zA-Z]+:[a-zA-Z].*.
The spaces around : and after the // must be removed.

// nolint:foo // <-- malformed
//nolint :foo // <-- malformed
//nolint: foo // <-- malformed

//nolint:foo // <-- OK

If you do that your IDE or go fmt will format the directive as a directive and it will not add extra space.

Related to:
#3002
#1658 (comment)

@ldez ldez added linter: idea idea of a linter and removed linter: new Support new linter labels Aug 28, 2022
@maratori
Copy link
Contributor

@ldez you are right. Directives are excluded form godoc.
The only exception if it's placed on the same line.

//nolint:testableexamples
func Example_previousLine() {
	panic("xxx")
}

func Example_sameLine() { //nolint:testableexamples
	panic("xxx")
}

image

Ok, then I'll create a linter.

@ldez
Copy link
Member

ldez commented Oct 3, 2022

closed by #3170

@ldez ldez closed this as completed Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement linter: idea idea of a linter
Projects
None yet
Development

No branches or pull requests

3 participants