all: fix all golangci-lint issues and make the lint gate strict

.golangci.yml

@@ -8,11 +8,17 @@ linters:
   enable:
     - misspell    # common misspellings
     - revive      # style/lint replacement for the old golint
+  settings:
+    errcheck:
+      # The renderers write to in-memory bytes.Buffers, where these calls can
+      # never fail; checking each return value would only add noise.
+      exclude-functions:
+        - fmt.Fprint
+        - fmt.Fprintf
+        - fmt.Fprintln
+        - (io.Writer).Write
 
 issues:
   # Report every issue, not just a sample.
   max-issues-per-linter: 0
   max-same-issues: 0
-  # Only flag issues introduced by the code a PR changes; the repo's existing
-  # findings are grandfathered. Drop this once they've been cleaned up.
-  new-from-merge-base: origin/master

diff.go

@@ -7,7 +7,6 @@ package main
 import (
 	"bytes"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -18,13 +17,13 @@ func diff(b1, b2 []byte, filename string) (data []byte, err error) {
 	if err != nil {
 		return
 	}
-	defer os.Remove(f1)
+	defer func() { _ = os.Remove(f1) }()
 
 	f2, err := writeTempFile("", "gofmt", b2)
 	if err != nil {
 		return
 	}
-	defer os.Remove(f2)
+	defer func() { _ = os.Remove(f2) }()
 
 	cmd := "diff"
 	if _, err := exec.LookPath("colordiff"); err == nil {
@@ -70,7 +69,7 @@ func replaceTempFilename(diff []byte, filename string) ([]byte, error) {
 }
 
 func writeTempFile(dir, prefix string, data []byte) (string, error) {
-	file, err := ioutil.TempFile(dir, prefix)
+	file, err := os.CreateTemp(dir, prefix)
 	if err != nil {
 		return "", err
 	}
@@ -79,7 +78,7 @@ func writeTempFile(dir, prefix string, data []byte) (string, error) {
 		err = err1
 	}
 	if err != nil {
-		os.Remove(file.Name())
+		_ = os.Remove(file.Name())
 		return "", err
 	}
 	return file.Name(), nil

internal/parser/comments.go

@@ -73,14 +73,15 @@ func assocFieldListComments(cr *commentListReader, fieldList *ast.FieldList) {
 			// Consider any comment that starts on a line earlier than this
 			// field and ends before the next field (or the closing parenthesis,
 			// in the case of the last field.)
-			if cr.end().Offset < openPos.Offset {
-				// The comment starts too early.
-			} else if cr.end().Line == fp.Line-1 || (cr.pos().Line == fp.Line && cr.end().Offset < fp.Offset) {
-				// The comment immediately precedes the import. Doc comment.
-				f.Doc = cr.comment()
-			} else if cr.pos().Line == fp.Line {
-				// The comment immediately follows the import. Line comment.
-				f.Comment = cr.comment()
+			// Ignore comments that start too early; otherwise classify them.
+			if cr.end().Offset >= openPos.Offset {
+				if cr.end().Line == fp.Line-1 || (cr.pos().Line == fp.Line && cr.end().Offset < fp.Offset) {
+					// The comment immediately precedes the import. Doc comment.
+					f.Doc = cr.comment()
+				} else if cr.pos().Line == fp.Line {
+					// The comment immediately follows the import. Line comment.
+					f.Comment = cr.comment()
+				}
 			}
 			cr.next()
 		}

internal/parser/parser.go

@@ -132,6 +132,7 @@ type ImportDecl struct {
 	Pos, End token.Pos
 }
 
+// ImportDecls returns all the import declarations at the top of the file.
 func (f *File) ImportDecls() []*ImportDecl {
 	var out []*ImportDecl
 	for _, d := range f.Decls {
@@ -161,13 +162,15 @@ func (f *File) ImportSpecs() []ImportSpec {
 	return out
 }
 
+// Path returns the unquoted import path of the spec.
 func (i *ImportSpec) Path() string {
 	if t, err := strconv.Unquote(i.ImportSpec.Path.Value); err == nil {
 		return t
 	}
 	return ""
 }
 
+// FuncDecl represents a function declaration.
 type FuncDecl struct {
 	ast.FuncDecl
 }
@@ -181,18 +184,22 @@ func (f *FuncDecl) BodyEnd() token.Pos {
 	return f.Body.Pos() + 1
 }
 
+// Decl is implemented by the declaration types in this package.
 type Decl interface {
 	decl()
 }
 
+// TypeDecl represents a type declaration.
 type TypeDecl struct {
 	ast.GenDecl
 }
 
+// VarDecl represents a var declaration.
 type VarDecl struct {
 	ast.GenDecl
 }
 
+// ConstDecl represents a const declaration.
 type ConstDecl struct {
 	ast.GenDecl
 }

internal/render/render.go

@@ -12,6 +12,8 @@
 // implied. See the License for the specific language governing
 // permissions and limitations under the License.
 
+// Package render turns parsed Go declarations back into formatted source,
+// wrapping function signatures and grouping imports per crlfmt's style.
 package render
 
 import (
@@ -251,6 +253,8 @@ func Func(
 	w.Write(f.Slice(fn.Type.End(), closing))
 }
 
+// GenDecl renders a generic declaration (const, var, or type) into w,
+// including its doc comment.
 func GenDecl(w io.Writer, f *parser.File, decl ast.GenDecl, wrapDocString int, lastPos token.Pos) {
 	if decl.Doc != nil {
 		DocString(w, f, decl.Doc, wrapDocString, lastPos, decl.TokPos)
@@ -287,14 +291,14 @@ func DocString(
 					if len(tokens[tokenIdx])+1 >= remainingBuf {
 						commentLine.WriteString(" ")
 						commentLine.WriteString(tokens[tokenIdx])
-						tokenIdx += 1
+						tokenIdx++
 					} else {
 						for tokenIdx < len(tokens) && len(tokens[tokenIdx])+1 <= remainingBuf {
 							commentLine.WriteString(" ")
-							remainingBuf -= 1
+							remainingBuf--
 							commentLine.WriteString(tokens[tokenIdx])
 							remainingBuf -= len(tokens[tokenIdx])
-							tokenIdx += 1
+							tokenIdx++
 						}
 					}
 					w.Write(commentLine.Bytes())

internal/render/simplify.go

@@ -14,6 +14,7 @@ import (
 	"unicode/utf8"
 )
 
+// Simplify applies gofmt's -s simplifications to the file in place.
 func Simplify(f *ast.File) {
 	// remove empty declarations such as "const ()", etc
 	removeEmptyDeclGroups(f)
@@ -24,19 +25,16 @@ func Simplify(f *ast.File) {
 
 // Values/types for special cases.
 var (
-	objectPtrNil = reflect.ValueOf((*ast.Object)(nil))
-	scopePtrNil  = reflect.ValueOf((*ast.Scope)(nil))
-
-	identType     = reflect.TypeOf((*ast.Ident)(nil))
+	identType = reflect.TypeOf((*ast.Ident)(nil))
+	//nolint:staticcheck // ast.Object identity check mirrors upstream gofmt.
 	objectPtrType = reflect.TypeOf((*ast.Object)(nil))
 	positionType  = reflect.TypeOf(token.NoPos)
 	callExprType  = reflect.TypeOf((*ast.CallExpr)(nil))
-	scopePtrType  = reflect.TypeOf((*ast.Scope)(nil))
 )
 
 func isWildcard(s string) bool {
-	rune, size := utf8.DecodeRuneInString(s)
-	return size == len(s) && unicode.IsLower(rune)
+	r, size := utf8.DecodeRuneInString(s)
+	return size == len(s) && unicode.IsLower(r)
 }
 
 // match reports whether pattern matches val,
@@ -246,14 +244,6 @@ func isBlank(x ast.Expr) bool {
 	return ok && ident.Name == "_"
 }
 
-func simplify(f *ast.File) {
-	// remove empty declarations such as "const ()", etc
-	removeEmptyDeclGroups(f)
-
-	var s simplifier
-	ast.Walk(s, f)
-}
-
 func removeEmptyDeclGroups(f *ast.File) {
 	i := 0
 	for _, d := range f.Decls {

main.go

@@ -12,6 +12,8 @@
 // implied. See the License for the specific language governing
 // permissions and limitations under the License.
 
+// Command crlfmt formats Go source code according to CockroachDB's style,
+// wrapping long function signatures and grouping imports.
 package main
 
 import (
@@ -137,7 +139,9 @@ func checkPath(path string) error {
 				return fmt.Errorf("computing diff: %s", err)
 			}
 			fmt.Printf("diff -u old/%[1]s new/%[1]s\n", filepath.ToSlash(path))
-			os.Stdout.Write(data)
+			if _, err := os.Stdout.Write(data); err != nil {
+				return fmt.Errorf("writing diff: %s", err)
+			}
 		}
 
 		if *overwrite {
@@ -196,7 +200,9 @@ func checkBuf(path string, src []byte) ([]byte, error) {
 				Mode:     printer.UseSpaces | printer.TabIndent,
 			}
 			var buf bytes.Buffer
-			prCfg.Fprint(&buf, fileSet, f)
+			if err := prCfg.Fprint(&buf, fileSet, f); err != nil {
+				return nil, err
+			}
 			src = buf.Bytes()
 		}
 	}
@@ -290,7 +296,7 @@ func checkBuf(path string, src []byte) ([]byte, error) {
 //
 // The goal is to have just one import declaration, within which imports are
 // grouped standard library imports and non-standard library imports. An
-// exception is made for cgo, whose "C" psuedo-imports are extracted into
+// exception is made for cgo, whose "C" pseudo-imports are extracted into
 // separate import declarations.
 func remapImports(file *parser.File) map[*parser.ImportDecl][]render.ImportBlock {
 	imports := file.ImportSpecs()
@@ -347,11 +353,11 @@ NEXT_IMPORT:
 		}
 		if needMainBlock && len(cImports) != len(imp.Specs) {
 			// The first import declaration we see that contains something other
-			// than "C" psuedo-imports will be our main import block.
+			// than "C" pseudo-imports will be our main import block.
 			blocks = append(blocks, mainBlock)
 			needMainBlock = false
 		}
-		// If there were any "C" psuedo-imports in this declaration, split them
+		// If there were any "C" pseudo-imports in this declaration, split them
 		// out into their own import declarations.
 		for _, imp := range cImports {
 			if imp.Doc == nil {

main_test.go

@@ -42,7 +42,7 @@ func TestCheckPath(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			outFile := strings.Replace(file, ".in.go", ".out.go", -1)
+			outFile := strings.ReplaceAll(file, ".in.go", ".out.go")
 
 			output, err := checkBuf(file, inBytes)
 			if err != nil {