-
Notifications
You must be signed in to change notification settings - Fork 7
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
Capital mode - false positive on godoc comments on lower-case variables #22
Comments
Most probably the problem is that your code is not at the top level. Go provides only one way to get the list of declaration comments: https://github.com/golang/go/blob/master/src/go/ast/ast.go#L1029 . And it's only for top level. That's why other comments are not checked as declaration comments by godot. And that's why you get this error. This can be fixed just by checking the line after the comment. But it can be quite a big change in code, because currently checkers take only a single comment as an input. I'll take a look a bit later. |
@tetafro Great, thank you! Sorry should have added a link to the source with the initial post: https://github.com/kubernetes-sigs/cluster-api/blob/5310648cc15c5170c207eaf4a245e1634aad674c/cmd/clusterctl/cmd/version_checker.go#L40-L42 I think it is top-level? (but as I have absolutely no idea of the AST, this might be wrong) |
Thanks for clarification, I'll check it. But yes, seems like it should be a "top-level" comment. |
I've finally checked it. It's not a declaration comment in terms of go parser. It just doesn't get to I don't see any way of fixing it without parsing manually source code, which seems too hard, and would significantly increase complexity. So unfortunately this bug will stay unfixed. JFYI This notation doesn't cause linting errors (once again - because of go parser) // gitVersionRegEx matches git versions of style 0.3.7-45-c1aeccb679cd56
// see ./hack/version.sh for more info.
var gitVersionRegEx = regexp.MustCompile(`(.*)-(\d+)-([0-9,a-f]{14})`) |
Thank you for looking into this! |
Hi,
I'm not sure if it's really a false positive. I guess it depends on the definition.
On the following code:
I get the following finding:
Afaik godoc comments of lower case variables should start with lower case. Looks like this is already correctly handled for unexported funcs:
https://github.com/tetafro/godot/blob/master/checks.go#L168
The text was updated successfully, but these errors were encountered: