diff --git a/.golangci.yml b/.golangci.yml index 2137baf..6481e11 100644 --- a/.golangci.yml +++ b/.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 --git a/diff.go b/diff.go index 45fc100..b159e33 100644 --- a/diff.go +++ b/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 diff --git a/internal/parser/comments.go b/internal/parser/comments.go index 6694187..d8c5cd3 100644 --- a/internal/parser/comments.go +++ b/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() } diff --git a/internal/parser/parser.go b/internal/parser/parser.go index 0abc863..e534693 100644 --- a/internal/parser/parser.go +++ b/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,6 +162,7 @@ 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 @@ -168,6 +170,7 @@ func (i *ImportSpec) Path() string { 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 } diff --git a/internal/render/render.go b/internal/render/render.go index 7a351d4..2a5a040 100644 --- a/internal/render/render.go +++ b/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()) diff --git a/internal/render/simplify.go b/internal/render/simplify.go index e98fadd..6001647 100644 --- a/internal/render/simplify.go +++ b/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 { diff --git a/main.go b/main.go index d38307e..c70c352 100644 --- a/main.go +++ b/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 { diff --git a/main_test.go b/main_test.go index 72e7868..c5d9d57 100644 --- a/main_test.go +++ b/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 {