Skip to content

Conversation

@babakks
Copy link
Contributor

@babakks babakks commented Dec 25, 2025

This PR upgrades godoclint to v0.11.1 with the following additions:

  1. Update schema/config to support the new require-stdlib-doclink rule. This rule is part of the extra set (not enabled by default), and is meant to enforce turning plaintext references to Go stdlib declarations into proper Doc links. It doesn't break the fast-ness of the linter (not requiring symbols from dependencies) as it uses a static dataset of Go stdlib declarations.
    (See Enhancement: suggest go doc links godoc-lint/godoc-lint#44)

    • The linter is super careful with this new rule to avoid false positives. For example, only potential doc links of the form pkg.name or pkg.recv.name are detected (single pkg cases are ignored due to high chance of them being normal words, like bytes or encoding). Also, the pkg part is always resolved to the underlying import path to avoid false positives (e.g. when json actually points to something other than encoding/json from stdlib). Other intricate details of the doc links are also covered to make the experience similar to go doc tool.
  2. Minor improvements in godoclint behaviour.
    (See Turn off enforcement for public methods on private structs godoc-lint/godoc-lint#65)

godoc-lint/godoc-lint@v0.10.2...v0.11.1

@ldez ldez changed the title fix: upgrade godoclint to v0.11.0 build(deps): bump github.com/godoc-lint/godoc-lint from 0.10.1 to 0.11.0 Dec 25, 2025
@ldez ldez self-requested a review December 25, 2025 23:12
@ldez ldez added dependencies Relates to an upstream dependency go Pull requests that update Go code linter: update Update the linter implementation inside golangci-lint linter: update version Update version of linter and removed linter: update Update the linter implementation inside golangci-lint labels Dec 25, 2025
@ldez ldez added this to the unreleased milestone Dec 27, 2025
@ldez
Copy link
Member

ldez commented Dec 29, 2025

To explain why I'm not merging this PR quickly: I was sick for 2 weeks, and I need to think about the option ignore-patterns.

There are several elements:

  • inside golangci-lint, we have the exclusion field source but it has some limitations, I need to check if it can work as expected.
  • the examples are, IMHO, not the right examples because it feels like a suggestion to create a kind of "directive" to ignore something.
  • the use case is mainly related to a not standard approach to "annotate" element with kubebuilder.

I know that I cannot ask to kubebuilder to follow the conventional directive syntax, but this is sad to create exception when there is a standard.
Maybe kubebuilder will have to follow the convention at some point.

I'm talking about convention but it's more than a convention: there is dedicated code inside Go to handle directives, and, for example, gofmt doesn't format comments and directives in the same way (// + no space).

@babakks
Copy link
Contributor Author

babakks commented Dec 29, 2025

Thanks for the feedback, @ldez. I hope you feel better soon my friend, and as far as I'm concerned there's no rush to this. 🙏

I agree with you in that such metadata/decoration should be done via directives. As a documentation standpoint, it's not really nice to drop such kubebuilder annotations in godocs, which will later be seen on online docs or in IDE experience.

However, I tend to think the max-len rule is inherently different from other rules of godoclint. I mean, other rules affect the rendered docs (e.g. on web GUI or in IDEs). But max-len is more about textual alignment which would benefit when someone reads the plaintext code. In that sense, I can imagine code authors may want to have the option to allow some lines freely exceed the predefined length as they don't see any good reason to break them over more lines (or sometimes they may not even able to do as such, like in the case of kubebuilder tags).

And reagrding your point about directives, if a comment line starts with a directive, then godoclint will never see it. So, the problem is automatically solved. But in some cases the user may not be able to do that.

@babakks
Copy link
Contributor Author

babakks commented Dec 29, 2025

By the way, thinking about the exclusion by source, seems it's not currently usable because max-len reports issues by range (over the entire godoc), and not by the specific line.

If I can improve this to report the individual line, then maybe there'd be no need for adding ignore-patterns. 🤔 I need to check that, as I remember there was a challenge (See godoc-lint/godoc-lint#55 (comment)).

@babakks
Copy link
Contributor Author

babakks commented Dec 29, 2025

If I can improve this to report the individual line, then maybe there'd be no need for adding ignore-patterns. 🤔 I need to check that, as I remember there was a challenge (See godoc-lint/godoc-lint#55 (comment)).

I think I'm up to something here. Seems like I can improve the reported range/line, although there might be edge cases (e.g. when the entire godoc is a /*...*/ block), but for them the linter can safely fallback to the current behaviour. However, I'll need to keep the max-len/ignore-patterns option in the linter since users who would like to use it out of Golangci-lint would need it. But for here, we wouldn't need to expose the config. I'll wrap up my work, cut a new release, and will then update the PR here.

@babakks
Copy link
Contributor Author

babakks commented Dec 30, 2025

Done. I've just removed the ignore-patterns option. The new v0.11.1 release is now better at reporting the position of the lines exceeding the predefined length. So, users can just exclude the issues by using source-based patterns. I confirmed this works as expected.

Thanks for mentioning source exclusions, @ldez. 🍻

@ldez ldez changed the title build(deps): bump github.com/godoc-lint/godoc-lint from 0.10.1 to 0.11.0 build(deps): bump github.com/godoc-lint/godoc-lint from 0.10.1 to 0.11.1 Dec 30, 2025
@ldez
Copy link
Member

ldez commented Dec 31, 2025

Sorry but I was thinking again about the topic, and I wonder if the approach is the best:
do you really need to allow users to define exclusions?
I mean kubebuilder should always be excluded, and for now, this is the only case.
Maybe it's better to always handle this specific case, to avoid false-positives, and don't provide an option.
WDYT?

(Note: maybe links should also be excluded by default)

@babakks
Copy link
Contributor Author

babakks commented Dec 31, 2025

I get your point, but TBH I prefer to leave it to the users. Also, max-len is a rarely used rule, and I'd rather wait for more feedback to exclude kubebuilder or such. Re links, I think it's best to enforce them so that users get the hint to turn them into proper anchored links.

@ldez
Copy link
Member

ldez commented Dec 31, 2025

I'd rather wait for more feedback to exclude kubebuilder or such.

IMHO, you don't need to wait for more feedback: this is rare and very specific, so a hardcoded rule is enough, it can be extended or converted to an option later.

But anyway, this option will not be a part of the golangci-lint configuration then I don't have to care too much about it.

In the context of golangci-lint, we will not follow your approach.
We will enforce the exclusion of kubebuilder markers to avoid known false-positives.

@ldez ldez changed the title build(deps): bump github.com/godoc-lint/godoc-lint from 0.10.1 to 0.11.1 build(deps): bump github.com/godoc-lint/godoc-lint from 0.10.2 to 0.11.1 Dec 31, 2025
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ldez ldez merged commit 3cebab9 into golangci:main Dec 31, 2025
20 of 24 checks passed
@babakks babakks deleted the babakks/upgrade-godoclint-to-v0.11.0 branch December 31, 2025 16:01
@ldez ldez modified the milestones: unreleased, v2.8 Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Relates to an upstream dependency go Pull requests that update Go code linter: update version Update version of linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants