Lint: lint and fix src/{os,reflect} (#4228)

* lint: expand to src/{os,reflect}, fix or suppress what it found

* internal/tools/tools.go: the tools idiom requires a build tag guard to avoid go test complaints

Author: dkegel-fastly

GNUmakefile

@@ -915,7 +915,9 @@ deb: build/release
 endif
 
 lint:
-	# Only run on compiler dir for now, expand as we clean up other dirs
-	# This obviously won't scale, but it's a start, and it's fast
-	go run github.com/mgechev/revive --version
-	go run github.com/mgechev/revive --config revive.toml compiler/...
+	go run github.com/mgechev/revive -version
+	# TODO: lint more directories!
+	# revive.toml isn't flexible enough to filter out just one kind of error from a checker, so do it with grep here.
+	# Can't use grep with friendly formatter.  Plain output isn't too bad, though.
+	# Use 'grep .' to get rid of stray blank line
+	go run github.com/mgechev/revive -config revive.toml compiler/... src/{os,reflect}/*.go | grep -v "should have comment or be unexported" | grep '.' | awk '{print}; END {exit NR>0}'

internal/tools/tools.go

@@ -1,3 +1,5 @@
+//go:build tools
+
 // Install linter versions specified in go.mod
 // See https://marcofranssen.nl/manage-go-tools-via-go-modules for idom
 package tools

revive.toml

@@ -6,15 +6,19 @@ warningCode = 0
 
 # Enable these as we fix them
 [rule.blank-imports]
+  Exclude=["src/os/file_other.go"]
 [rule.context-as-argument]
 [rule.context-keys-type]
 [rule.dot-imports]
+  Exclude=["**/*_test.go"]
 [rule.error-return]
 [rule.error-strings]
 [rule.error-naming]
 [rule.exported]
+  Exclude=["src/reflect/*.go"]
 [rule.increment-decrement]
 [rule.var-naming]
+  Exclude=["src/os/*.go"]
 [rule.var-declaration]
 #[rule.package-comments]
 [rule.range]
@@ -27,4 +31,5 @@ warningCode = 0
 [rule.superfluous-else]
 #[rule.unused-parameter]
 [rule.unreachable-code]
+  Exclude=["src/reflect/visiblefields_test.go", "src/reflect/all_test.go"]
 #[rule.redefines-builtin-id]

src/os/file.go

@@ -310,6 +310,7 @@ func (e *LinkError) Unwrap() error {
 	return e.Err
 }
 
+// OpenFile flag values.
 const (
 	O_RDONLY int = syscall.O_RDONLY
 	O_WRONLY int = syscall.O_WRONLY

src/os/path_unix.go

@@ -7,8 +7,8 @@
 package os
 
 const (
-	PathSeparator     = '/' // OS-specific path separator
-	PathListSeparator = ':' // OS-specific path list separator
+	PathSeparator     = '/' // PathSeparator is the OS-specific path separator
+	PathListSeparator = ':' // PathListSeparator is the OS-specific path list separator
 )
 
 // IsPathSeparator reports whether c is a directory separator character.

src/os/path_windows.go

@@ -5,8 +5,8 @@
 package os
 
 const (
-	PathSeparator     = '\\' // OS-specific path separator
-	PathListSeparator = ';'  // OS-specific path list separator
+	PathSeparator     = '\\' // PathSeparator is the OS-specific path separator
+	PathListSeparator = ';'  // PathListSeparator is the OS-specific path list separator
 )
 
 // IsPathSeparator reports whether c is a directory separator character.

src/reflect/value_test.go

@@ -489,7 +489,7 @@ func TestTinyStruct(t *testing.T) {
 
 func TestTinyZero(t *testing.T) {
 	s := "hello, world"
-	var sptr *string = &s
+	sptr := &s
 	v := ValueOf(&sptr).Elem()
 	v.Set(Zero(v.Type()))
 
@@ -535,7 +535,7 @@ func TestTinyAddr(t *testing.T) {
 }
 
 func TestTinyNilType(t *testing.T) {
-	var a any = nil
+	var a any
 	typ := TypeOf(a)
 	if typ != nil {
 		t.Errorf("Type of any{nil} is not nil")