All committers have signed the CLA.

Hey, thank you for opening your first Pull Request !

In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.

• The CLA must be signed

Pull Request Description

• It must have a link to the linter repository.
• It must provide a short description of the linter.

Linter

• It must not be a duplicate of another linter or a rule of a linter (the team will help to verify that).
• It must have a valid license (AGPL is not allowed), and the file must contain the required information by the license, ex: author, year, etc.
• It must use Go version <= 1.24.0
• The linter repository must have a CI and tests.
• It must usego/analysis.
• It must have a valid semver tag, ex:v1.0.0,v0.1.0 (0.0.x are not valid).
• It must not containinit().
• It must not containpanic().
• It must not containlog.Fatal(),os.Exit(), or similar.
• It must not modify the AST.
• It must not have false positives/negatives (the team will help to verify that).
• It must have tests inside golangci-lint.

The Linter Tests Inside Golangci-lint

• They must have at least one std lib import.
• They must have integration tests without configuration (default).
• They must have integration tests with configuration (if the linter has a configuration).

.golangci.next.reference.yml

• The file.golangci.next.reference.yml must be updated.
• The file.golangci.reference.yml must NOT be edited.
• The linter must be added to the lists of available linters (alphabetical case-insensitive order).
  • enable anddisable options
• If the linter has a configuration, the exhaustive configuration of the linter must be added (alphabetical case-insensitive order)
  • The values must be different from the default ones.
  • The default values must be defined in a comment.
  • The option must have a short description.

Others Requirements

• The files (tests and linter) inside golangci-lint must have the same name as the linter.
• The.golangci.yml of golangci-lint itself must not be edited and the linter must not be added to this file.
• The linters must be sorted in the alphabetical order (case-insensitive) in thelintersdb/builder_linter.go and.golangci.next.reference.yml.
• The load mode (WithLoadMode(...)):
  • if the linter usesgoanalysis.LoadModeSyntax -> noWithLoadForGoAnalysis() inlintersdb/builder_linter.go
  • if the linter usesgoanalysis.LoadModeTypesInfo, it requiresWithLoadForGoAnalysis() inlintersdb/builder_linter.go
• The version inWithSince(...) must be the next minor version (v2.X.0) of golangci-lint.
• WithURL() must contain the URL of the repository.

Recommendations

• The filejsonschema/golangci.next.jsonschema.json should be updated.
• The filejsonschema/golangci.jsonschema.json must NOT be edited.
• The linter repository should have a readme and linting.
• The linter should be published as a binary (useful for diagnosing bug origins).
• The linter repository should have a.gitignore (IDE files, binaries, OS files, etc. should not be committed)
• A tag should never be recreated.
• usemain as the default branch name.

The golangci-lint team will edit this comment to check the boxes before and during the review.

The code review will start after the completion of those checkboxes (except for the specific items that the team will help to verify).

The reviews should be addressed as commits (no squash).

If the author of the PR is a member of the golangci-lint team, he should not edit this message.

This checklist does not imply that we will accept the linter.

The linter description claims to follow Go recommendations fromthis page, but several rules are not those recommendations (ex, line length).

The linter applies the rules to non-exposed elements, but those elements are not a part of the godoc render (ex, a non-exported constant doesn't need to have a doc beginning with its own name), and so they don't need to follow the same rules as exposed elements.
Also, this is, in some ways, opposed to the recommendations themselves:

Every exported (capitalized) name should have a doc comment.

This can be seen as false positives, IMO, this will be the main criterion for users to not use this linter.

Even the examples of "good" from the linter README don't pass the rules of the linter:

// Check [docs] here. // (Good)
//
// [docs]:https://foo.com/docs
const foo = 0

Thanks for taking the time reviewing the linter,@ldez! 🙏

Let me first provide a categorised table of the rules so that I can refer to them later:

By default, the linter only applies theBasic rules. They're low-effort because they don't require user to add more godocs. Rather, they just check if the existing godocs match the recommended formatting.

TheStrict rules are costly to enable. Although they're recommended byGo Doc Comments, but the linter do not apply them by default, because it will be too much, and in some cases unnecessary. So, that's up to the user to enable them based on their situation.

And finally theExtra rules which are not part of the blog post, are additional rules to help improve godocs. They're not breaking/opposing the recommended rules. These are also not enabled by default.

Now, back to your points:

The linter description claims to follow Go recommendations fromthis page, but several rules are not those recommendations (ex, line length).

True.max-len andno-unused-link areExtra rules, to help further improve godocs. Note that they're opt-in and not enabled by default.

The linter applies the rules to non-exposed elements, but those elements are not a part of the godoc render (ex, a non-exported constant doesn't need to have a doc beginning with its own name), and so they don't need to follow the same rules as exposed elements. Also, this is, in some ways, opposed to the recommendations themselves:

Every exported (capitalized) name should have a doc comment.

This can be seen as false positives, IMO, this will be the main criterion for users to not use this linter.

Good point, and that was one of my initial challenges. Note that godocs are not only used with the renderer or onpkg.go.dev. The Go language server (gopls) uses them to improve the IDE experience. So, godocs on unexported symbols are still visible to developers and it makes sense to apply theBasic rules to them. That's one of the reasons why I called the lintera little opinionated in the README.

That said, I think I'm open to exclude unexported symbols from the default behaviour, of course, in favour of a configuration parameter to enable the rule for them. Please let me know your thoughts.

Even the examples of "good" from the linter README don't pass the rules of the linter:

// Check [docs] here. // (Good)
//
// [docs]:https://foo.com/docs
const foo = 0

Makes sense, and thanks for the feedback. I'll update those examples to avoid confusion for the reader.

That said, I think I'm open to exclude unexported symbols from the default behaviour, of course, in favour of a configuration parameter to enable the rule for them. Please let me know your thoughts.

I think it should be an option ofstart-with-name (and false by default)

Note: golangci-lint will not follow your configuration naming style with/.
The options will be on structures.

I think it should be an option ofstart-with-name (and false by default)

Makes sense. I'll push the changes for this and README good/bad examples, and will then rebase the PR.

Note: golangci-lint will not follow your configuration naming style with/. The options will be on structures.

Just to be clear, you saying this format is not acceptable, right?

linters:settings:godoclint:options:max-len/length:127

And if yes, how about using dashes like this?

linters:settings:godoclint:options:max-len-length:127

Or even dots?

linters:settings:godoclint:options:max-len.length:127

Any particular recommendation?

linters:settings:godoclint:options:max-len:length:127

I'll push the changes for this as well.

I made the following changes and published v0.6.1 of the linter:

• Thestart-with-name rule skips unexported symbol by default.
  • A new option added, namedstart-with-name/include-unexported (defaultfalse), to override the behaviour.
• The examples in the linter README are improved so that "good" cases are actually okay with the linter.
• The same categorised table I mentioned infeat: add godoclint linter #6062 (comment) is now added to the linter's README to proivde a quick overview of different types of rules.
• A few typos are fixed.

And here are the changes I (force) pushed to this PR branch:

• The fine tuning options are now nested structs (rather than being forward-slash separated).
  • The next version JSON schema and config reference are both updated accordingly.
• The test files are renamed with agodoclint_ prefix.

As an improvement of the linter, I think some messages can be more precise.

ex:

// Foo is a thing.typeBarstruct {}

Current message:

foo.go:14:2: godoc should start with symbol name (pattern "((A|a|An|an|THE|The|the) )?%") (godoclint)// Foo is a thing.^

Maybe something like that:

foo.go:14:2: godoc should start with symbol name ('Bar') (godoclint)// Foo is a thing.^

I think I already have a fix for the// Deprecated: case. Just need to add a couple of tests to maje sure ot's the right change.

Regarding the pattern thing, I think I can modify the report message to be different based on the configured pattern.

I'll push a commit when I'm done with the fix. Probably tomorrow.

Regarding the pattern thing, I think I can modify the report message to be different based on the configured pattern.

I think thepattern option is not useful; maybe it can be removed.
Do you have some real cases where this option is useful?

A pattern is needed, because as of the godoc reference godocs should start with a complete senetence, which is okay to start with an article, for example. But I'm not sure there's a point in making the pattern configurable. 🤔

Just released changes, using v0.8.0 of the linter. Thedefault option is added to config, and pattern options are dropped fromstart-with-name andpkg-doc, so the config is much simpler now. Also, deprecations are better handled now (more on that ingodoc-lint/godoc-lint#36).

I'm going to work ongodoc-lint/godoc-lint#41 to detect wrong usages of theDeprecated: marker. Should I just come back and update this PR's branch?

For now, you can update the PR.
I will consider this as the last modification of the PR.

Done, and thanks! 🙏

maybedeprecated should be a part ofbasic? 🤔

I'm torn about that, actually. You think so? I'd like it.

The pattern seems strict enough not to have false positives.
So I'm also torn about that, but I think it can be good.

Yeah, that's my thought, too. Let's do it. I'll update here shortly.

This linter has identical rules to other linters, but:

1. Those linters are meta-linters.
2. This linter is focused on one topic in the right way.
3. This linter has more rules than the others.

In this specific context, the rule about "no duplicate rules" doesn't apply, and then the linter can be accepted.

This is a specific case.
It's neither a rule nor a dogma.
It's only based on the context of this specific situation.

By the way, I have already added validation to thePlainConfig type (here). I doesn't check for conditions like"ifnone, then nodisable", but it mostly matches with what you did in3922fe5, except for it also returns a single error with all issues.

Let me know if we can reuse it, or update it to accommodate with golangci-lint requirements.