huacnlee / autocorrect

A linter and formatter to help you to improve copywriting, correct spaces, words, and punctuations between CJK (Chinese, Japanese, Korean).
https://huacnlee.github.io/autocorrect
MIT License
885 stars 29 forks source link

problem with Go directives #205

Open ccoVeille opened 2 months ago

ccoVeille commented 2 months ago

There are a few golang directives that can be used to alter the way Go behaves

These directives are provided in Go comments

Here are some information about them

https://go.dev/doc/comment#syntax https://pkg.go.dev/go/build#hdr-Build_Constraints https://go-review.googlesource.com/c/website/+/442516/1..2/_content/doc/comment.md#494

//go:build linux
// +build:linux
//go:linkname

There is a small problem with the regexp used for detecting Go comments

https://github.com/huacnlee/autocorrect/blob/9602afbbdf8e12c4f00a5d4bf0d6b57980c601e2/autocorrect/grammar/go.pest#L8

For example if I launch autocorrect --lint on this file

https://github.com/Antonboom/testifylint/blob/eb1257817037b6c67234f07f310e7c2b4fc24cf6/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall_linux_ppc64x.go#L5-L7

it would report to change linux to Linux

The whole repository has many examples of code Go directives

while there is no problem to change that on a markdown file, Go string, or in regular Go comment, it would cause issue here

I think the regexp should be fixed.

You can find existing regexp about how to detect them

https://github.com/mgechev/revive/blob/bbe5eb74146a39769ec40e413790dc3975cf8bfe/rule/utils.go#L169 https://github.com/mvdan/gofumpt/blob/52739c56d0efd3647a627598a55e821324f8571b/format/format.go#L320-L334

ccoVeille commented 2 months ago

Here you can find discussions about them in revive and gofumpt

https://github.com/mgechev/revive/pull/986 https://github.com/mgechev/revive/pull/988 https://github.com/mgechev/revive/issues/808

https://github.com/mvdan/gofumpt/issues/7 https://github.com/mvdan/gofumpt/pull/77 https://github.com/mvdan/gofumpt/pull/17 https://github.com/mvdan/gofumpt/pull/268 https://github.com/mvdan/gofumpt/pull/179 https://github.com/mvdan/gofumpt/issues/109

ccoVeille commented 2 months ago

I'm unsure what to do with the +build, but I don't think your tool should suggest things like this

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/pagesize_unix.go:6:1
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris
+// +build aix darwin dragonfly freebsd Linux netbsd openbsd solaris

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall_linux_gccgo_386.go:5:1
-//go:build linux && gccgo && 386
+//go:build Linux && gccgo && 386

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall_linux_mipsx.go:5:1
-//go:build linux && (mips || mipsle)
+//go:build Linux && (mips || mipsle)

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall_linux_mipsx.go:6:1
-// +build linux
+// +build Linux

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall_linux_gccgo_arm.go:5:1
-//go:build linux && gccgo && arm
+//go:build Linux && gccgo && arm

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall.go:5:1
-//go:build aix || darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris || zos
+//go:build aix || darwin || dragonfly || freebsd || Linux || netbsd || openbsd || solaris || zos

antonboom-testifylint/analyzer/testdata/src/vendor/golang.org/x/sys/unix/syscall.go:6:1
-// +build aix darwin dragonfly freebsd linux netbsd openbsd solaris zos
+// +build aix darwin dragonfly freebsd Linux netbsd openbsd solaris zos
huacnlee commented 2 months ago

You can disable spellcheck first, this case is hard to support right now.

ccoVeille commented 2 months ago

Of course, it's what I did.

I also apply fixes then nuke the errors with a git restore -p

It's OK, but I would prefer the issue to be reported and maybe fixed

ccoVeille commented 2 months ago

I'm wondering if the fix couldn't be as simple as replacing // here

https://github.com/huacnlee/autocorrect/blob/main/autocorrect%2Fgrammar%2Fgo.pest#L8

by // so the comment but only when a space is present. This is how to differ a comment from a Go directive after all