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

False positive on package comment is detached lint on CRLF ended files #607

Open
mx-psi opened this issue Oct 25, 2021 · 4 comments
Open
Labels

Comments

@mx-psi
Copy link

mx-psi commented Oct 25, 2021

Describe the bug

revive will incorrectly report package comment is detached on CRLF ended files. This is likely related to golang/go#41197.

To Reproduce
Steps to reproduce the behavior:

  1. I updated revive go get -u github.com/mgechev/revive
  2. I created a main.go file with the following content:
/*
Package main has a multi-line comment.
Its line endings have a carriage return.
*/
package main
  1. I changed the line endings to be CRLF (e.g. one can run sed -e $'s/$/\r/' main.go > main_crlf.go)
  2. I verified that the line endings have been changed (e.g. file main_crlf.go should say ASCII text, with CRLF line terminators and cat -v main_crlf.go should show ^M control codes at the end of lines).
  3. I ran revive main_crlf.go.

Expected behavior

revive should not report anything for that file (i.e. it should have the same behavior as if I skipped steps 3-4).

Logs

revive output is

❯ revive main_crlf.go
main_crlf.go:4:1: package comment is detached; there should be no blank lines between it and the package statement

Desktop (please complete the following information):

  • OS: macOS (doesn't really matter, this is not OS-specific)
  • Version of Go: 1.17.2

Additional context

The Go team deemed the issue above unfixable in the parser side because of backwards-compatibility concerns (see golang/go@a14e7bf). This does not mean the bug is unfixable on revive, but the solution may be a bit more cumbersome.

@chavacava
Copy link
Collaborator

revive relies on the GO parser

f, err := parser.ParseFile(pkg.fset, name, content, parser.ParseComments)

@mx-psi
Copy link
Author

mx-psi commented Oct 27, 2021

gopls also relies on the Go parser, they had a similar-ish issue (golang/go#41057) and fixed it (golang/tools@cf78807) while still relying on the Go parser. I was hoping a similar thing could be achieved here.

@mgechev mgechev added the bug label Dec 3, 2021
@mgechev
Copy link
Owner

mgechev commented Aug 7, 2022

Is this still relevant? The issue seems fixed and we're using the latest parser.

@mx-psi
Copy link
Author

mx-psi commented Aug 8, 2022

This can still be reproduced with the latest version (main.go being the file on my original comment):

❯ go install github.com/mgechev/revive@latest
go: downloading github.com/mgechev/revive v1.2.3
go: downloading github.com/chavacava/garif v0.0.0-20220630083739-93517212f375
❯ revive main.go
<no output>
❯ sed -e $'s/$/\r/' main.go > main_crlf.go
❯ file main_crlf.go
main_crlf.go: ASCII text, with CRLF line terminators
❯ revive main_crlf.go
main_crlf.go:4:1: package comment is detached; there should be no blank lines between it and the package statement

The issue upstream was not fixed by fixing the parser, but by applying a workaround on gopls. The solution would be to apply the same workaround here.

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

No branches or pull requests

3 participants