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

[go1.19] cmd/gofmt: machine-readable (//nolint) comments should not be reformatted #53835

Closed
thaJeztah opened this issue Jul 13, 2022 · 3 comments

Comments

@thaJeztah
Copy link
Contributor

GoLangCI-lint documentation https://golangci-lint.run/usage/false-positives/

Use //nolint instead of // nolint because machine-readable comments should have no space by Go convention.

I think some improvements may have been made between rc1 and rc2, but I still noticed some diffs

What version of Go are you using (go version)?

$ go version
go version go1.19rc2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
# not relevant

What did you do?

Formatted code with gofmt

What did you expect to see?

Machine-readable comments, such as //nolint: <some linter> or //#nosec XXX preserving their formatting

What did you see instead?

gofmt reformatting those comments to add a leading whitespace (// nolint: <some linter> , // #nosec

The following go file should not produce a diff after running gofmt;

package main

import "fmt"

// Some variables that are defined here
//
//nolint:govet
var (
	var1 int //#nosec G501
	var2 int
)

// foo is something
//nolint: gosimple
var var3 = "foo"

// bar is something
//
//nolint: gosimple
var var4 = "bar"

func main() {
	//#nosec G305 -- Some comment here
	var5 := "hello"

	fmt.Println(var1, var2, var3, var4, var5)
}

// function1 does something.
//nolint
func function1() {}

// function2 does something.
//
//nolint:gocyclo
func function2() {}

// function3 does something.
//
//nolint:gocyclo // Some comment here
func function3() {}

//nolint
func function4() {}

//nolint:gocyclo // Some comment here
func function5() {}

func function6() { //nolint
}

func function7() { //nolint:gocyclo // Some comment here
}

// function1 does something.
//#nosec G305 -- Some comment here
func function8() {}

// function2 does something.
//
//#nosec G305 -- Some comment here
func function9() {}

//#nosec G305 -- Some comment here
func function10() {}

//#nosec G305
func function11() {}

func function12() { //#nosec G305
}

func function13() { //#nosec G305 -- Some comment here
}

type Foo struct{}

// Method1 does something
//
//nolint: gocyclo
func (p *Foo) Method1() {}

// Method2 does something
//
//nolint:gocyclo
func (p *Foo) Method2() {}

// Method3 does something
//nolint:gocyclo
func (p *Foo) Method3() {}

// Method4 does something
//nolint
func (p *Foo) Method4() {}
@mvdan
Copy link
Member

mvdan commented Jul 13, 2022

See golangci/golangci-lint#1658, which seems like a better fix that does not require workarounds in gofmt. Plus, that way we move the broader Go ecosystem further towards the standard comment directive format.

@seankhliao
Copy link
Member

This was raised in #37974 (comment) and following comments. There doesn't appear to be any new info here.

Closing as dup.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2022
@thaJeztah
Copy link
Contributor Author

Ah, thanks for those links (sorry I didn't find those);

right, so IIUC, the requirements are:

  • No space (//nolint)
  • MUST start with alpha-numeric (so //#nosec is invalid because #)
  • MUST have a colon (//nolint:)
  • MUST have text following it, but MUST NOT have whitespace (✅ //nolint:something,foo,bar//nolint: foo, bar)

Which explains why the //nolint (no colon) and //nolint: foo (colon but whitespace) are reformatted as regular comment

diff --git a/./main.go.backup b/./main.go
index e7ca9cb..de570f6 100644
--- a/./main.go.backup
+++ b/./main.go
@@ -11,12 +11,12 @@ var (
 )
 
 // foo is something
-//nolint: gosimple
+// nolint: gosimple
 var var3 = "foo"
 
 // bar is something
 //
-//nolint: gosimple
+// nolint: gosimple
 var var4 = "bar"
 
 func main() {
@@ -27,7 +27,7 @@ func main() {
 }
 
 // function1 does something.
-//nolint
+// nolint
 func function1() {}
 
 // function2 does something.
@@ -40,7 +40,7 @@ func function2() {}
 //nolint:gocyclo // Some comment here
 func function3() {}
 
-//nolint
+// nolint
 func function4() {}
 
 //nolint:gocyclo // Some comment here
@@ -53,18 +53,18 @@ func function7() { //nolint:gocyclo // Some comment here
 }
 
 // function1 does something.
-//#nosec G305 -- Some comment here
+// #nosec G305 -- Some comment here
 func function8() {}
 
 // function2 does something.
 //
-//#nosec G305 -- Some comment here
+// #nosec G305 -- Some comment here
 func function9() {}
 
-//#nosec G305 -- Some comment here
+// #nosec G305 -- Some comment here
 func function10() {}
 
-//#nosec G305
+// #nosec G305
 func function11() {}
 
 func function12() { //#nosec G305
@@ -77,7 +77,7 @@ type Foo struct{}
 
 // Method1 does something
 //
-//nolint: gocyclo
+// nolint: gocyclo
 func (p *Foo) Method1() {}
 
 // Method2 does something
@@ -86,9 +86,10 @@ func (p *Foo) Method1() {}
 func (p *Foo) Method2() {}
 
 // Method3 does something
+//
 //nolint:gocyclo
 func (p *Foo) Method3() {}
 
 // Method4 does something
-//nolint
+// nolint
 func (p *Foo) Method4() {}

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

No branches or pull requests

4 participants