From 3d433fe9f15dc6cff400ce7fd03cf15ad55f64fc Mon Sep 17 00:00:00 2001
From: dkegel-fastly <79674949+dkegel-fastly@users.noreply.github.com>
Date: Mon, 22 Apr 2024 11:10:13 -0700
Subject: [PATCH] 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
---
 GNUmakefile               | 10 ++++++----
 internal/tools/tools.go   |  2 ++
 revive.toml               |  5 +++++
 src/os/file.go            |  1 +
 src/os/path_unix.go       |  4 ++--
 src/os/path_windows.go    |  4 ++--
 src/reflect/value_test.go |  4 ++--
 7 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/GNUmakefile b/GNUmakefile
index c15b7724..8987fcaa 100644
--- a/GNUmakefile
+++ b/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}'
diff --git a/internal/tools/tools.go b/internal/tools/tools.go
index c22654a7..60e6a8cb 100644
--- a/internal/tools/tools.go
+++ b/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
diff --git a/revive.toml b/revive.toml
index f09127ca..37778be5 100644
--- a/revive.toml
+++ b/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]
diff --git a/src/os/file.go b/src/os/file.go
index ff62899b..e003d47d 100644
--- a/src/os/file.go
+++ b/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
diff --git a/src/os/path_unix.go b/src/os/path_unix.go
index 97a028c5..9bb5c726 100644
--- a/src/os/path_unix.go
+++ b/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.
diff --git a/src/os/path_windows.go b/src/os/path_windows.go
index a96245f3..7118c45b 100644
--- a/src/os/path_windows.go
+++ b/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.
diff --git a/src/reflect/value_test.go b/src/reflect/value_test.go
index 6641750b..40f0919b 100644
--- a/src/reflect/value_test.go
+++ b/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")