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

exported: handle blank line case differently #1234

Open
ccoVeille opened this issue Feb 12, 2025 · 3 comments
Open

exported: handle blank line case differently #1234

ccoVeille opened this issue Feb 12, 2025 · 3 comments

Comments

@ccoVeille
Copy link
Contributor

package sandbox

// Foo is OK
func Foo() {}

// Bar is not OK because of the extra blank line

func Bar() {}

Image

Here everything is fine with Bar except the blank line. It could have been added randomly.

revive reports the right things yet, but it could be improved.

The godoc is empty in such a case. It could be a real problem in terms of documentation.

But, because golangci-lint exclude EXC0012 by default.
This one is not reported.

We should consider trying to detect it, and report another message that won't be caught by EXC0012

@denisvmedia
Copy link
Collaborator

In our codebase (closed source) we do have such cases, where theere is an intentional comment preceding an exported symbol, which is not a godoc comment for the symbol. What do you propose in such a case?

@ccoVeille
Copy link
Contributor Author

ccoVeille commented Feb 13, 2025

It's something I had already thought about.

I also have the same in my code base or open-source projects

I think it's pretty simple, I hope you will confirm 😅

It's about looking for a block that would validate expected format, if no extra space was found

Right now

package sandbox

// whatever the comment

func Foo() {} // MATCH /should have a comment or be unexported/

// Bar is not OK because of the extra blank line

func Bar() {} // MATCH /should have a comment or be unexported/

// yellow is ...
func Qux() {} // MATCH /should be form Qux.../

Suggestion

package sandbox

// whatever the comment

func Foo() {} // MATCH /Foo should have a comment or be unexported/

// Bar is not OK because of the extra blank line

func Bar() {} // MATCH /extra blank line found blah blah/

// yellow is ...
func Qux() {} // MATCH /should be form Qux.../

Only Bar will be affected.

The amount of error reported by revive will be the same

But by using another message than existing ones, it would be reported by golangci-lint that excludes EXC012 and EXC014

@chavacava
Copy link
Collaborator

The proposal LGTM, though IMO it's low priority and shall be implemented in a way that the added complexity is justified (i.e. should be implemented in a very simple way)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants