diff --git a/README.md b/README.md index a00bb47..8e4b16d 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,32 @@ Set `-include-generated` to report in them anyway, constable -include-generated ./... ``` +## Pure + +The pure analyzer checks functions annotated with `//constable:pure`. +A pure function has no observable effect beyond its return values. +It forbids every caller-visible mutation that the nonmutating analyzer forbids, and also forbids package-global writes, `go` spawns, and calls that read the clock, random, environment, or perform I/O. + +```go +//constable:pure +func F(p *int) int { + return *p // allowed: read only +} + +//constable:pure +func G() { + // reported: reads the clock + _ = time.Now() +} +``` + +A pure function that reads a global is allowed. +A pure function that writes a global is reported. + +The banned calls are `time.Now` and friends, any `math/rand` call, `os` environment and file calls (except error predicates), `fmt` and `log` printing, and the `print` and `println` builtins. +Reads of `os.Args` and `time.Local` are reported too. +Pure functions are deterministic given their arguments, so they are safe to reorder, parallelize, or test without flaky setup. + ## Example This function is reported because it mutates through a pointer parameter, @@ -100,6 +126,21 @@ if a && isB() { // allowed The check also covers `for` conditions and bool assignments such as `c := a && b`, including negated variables, field and map access, and comparisons. +## Lean + +The lean analyzer reports unused unexported declarations: functions, types, variables, and constants that nothing in the package references. +Deleting dead declarations keeps the package small, so readers and agents hold less context in mind. + +```go +// reported: nothing calls unused +func unused() {} +``` + +The check stays precise on purpose. +Methods, struct fields, unused parameters, and exported symbols are not reported. +Constants in a group that uses `iota` or hides a value are not reported either, because removing one member would renumber the rest. +Unused parameters may exist to satisfy an interface, and exported symbols may be used outside the package, so package-local evidence cannot prove either one dead. + ## Fixes `-fix` applies the fixes that the analyzers suggest, and `-fix -diff` prints the change instead of writing it. @@ -109,8 +150,9 @@ constable -fix ./... constable -fix -diff ./... ``` -Only the methodical analyzer suggests fixes today. -It places every method of a reported type directly after the type declaration, in order, and a method that lives in another file moves into the file that declares its type. +The methodical and lean analyzers suggest fixes. +Methodical places every method of a reported type directly after the type declaration, in order, and a method that lives in another file moves into the file that declares its type. +Lean deletes unused unexported declarations and removes imports that only the deleted declaration used. A move fixes the import blocks of both files: diff --git a/cmd/constable/main.go b/cmd/constable/main.go index 8469b5e..4f08d48 100644 --- a/cmd/constable/main.go +++ b/cmd/constable/main.go @@ -3,10 +3,12 @@ package main import ( "github.com/nnutter/constable/internal/analysis/complexity" "github.com/nnutter/constable/internal/analysis/errcheck" + "github.com/nnutter/constable/internal/analysis/lean" "github.com/nnutter/constable/internal/analysis/methodical" "github.com/nnutter/constable/internal/analysis/nilaway" "github.com/nnutter/constable/internal/analysis/nonmutating" "github.com/nnutter/constable/internal/analysis/predicates" + "github.com/nnutter/constable/internal/analysis/pure" "github.com/nnutter/constable/internal/analysis/sumtype" "github.com/nnutter/constable/internal/analysis/testify" "github.com/nnutter/constable/internal/analysis/testifylint" @@ -23,6 +25,8 @@ func main() { version, driver.Hooks{AfterLoad: nilaway.UseMainModules}, nonmutating.Analyzer, + pure.Analyzer, + lean.Analyzer, methodical.Analyzer, predicates.Analyzer, testify.Analyzer, diff --git a/internal/analysis/lean/fix.go b/internal/analysis/lean/fix.go new file mode 100644 index 0000000..3781c16 --- /dev/null +++ b/internal/analysis/lean/fix.go @@ -0,0 +1,574 @@ +package lean + +import ( + "bytes" + "go/ast" + "go/token" + "go/types" + "path/filepath" + "slices" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + + "github.com/nnutter/constable/internal/generated" +) + +type reporter struct { + offered bool +} + +func newReporter(pass *analysis.Pass) *reporter { + return &reporter{} +} + +func readFixFile(pass *analysis.Pass, file *ast.File) *fixFile { + tf := pass.Fset.File(file.Pos()) + if tf == nil { + return nil + } + src, err := pass.ReadFile(tf.Name()) + if err != nil || generated.Skipped(file) || hasStandaloneComments(pass.Fset, file) { + return nil + } + return &fixFile{file: file, token: tf, source: src} +} + +func containsDirective(src []byte) bool { + return bytes.Contains(src, []byte("//go:")) || bytes.Contains(src, []byte("/*go:")) || bytes.Contains(src, []byte("//line ")) || bytes.Contains(src, []byte("/*line ")) +} + +func isCgoFile(file *ast.File) bool { + return slices.ContainsFunc(file.Imports, isCgoImport) +} + +func isCgoImport(spec *ast.ImportSpec) bool { + path, err := strconv.Unquote(spec.Path.Value) + if err != nil { + return false + } + return path == "C" +} + +func hasStandaloneComments(fset *token.FileSet, file *ast.File) bool { + for _, comment := range file.Comments { + if comment.End() < file.Package { + continue + } + if !commentHasOwner(fset, file, comment) { + return true + } + } + return false +} + +func commentHasOwner(fset *token.FileSet, file *ast.File, comment *ast.CommentGroup) bool { + for _, decl := range file.Decls { + if hasOwner(fset, decl, comment) { + return true + } + } + return false +} + +func hasOwner(fset *token.FileSet, decl ast.Decl, comment *ast.CommentGroup) bool { + if comment.Pos() >= decl.Pos() { + if comment.End() <= decl.End() { + return true + } + } + if comment.Pos() >= decl.End() { + if fset.Position(comment.Pos()).Line == fset.Position(decl.End()).Line { + return true + } + } + switch d := decl.(type) { + case *ast.FuncDecl: + if d.Doc == comment { + return true + } + case *ast.GenDecl: + if d.Doc == comment { + return true + } + } + return false +} + +func candidateDecl(pass *analysis.Pass, sym symbol) (ast.Decl, *ast.GenDecl, ast.Spec) { + if funcDecl, ok := sym.decl.(*ast.FuncDecl); ok { + return funcDecl, nil, nil + } + gen, spec := containingGen(pass, sym) + return nil, gen, spec +} + +func specMatches(spec ast.Spec, sym symbol) bool { + sx, ok := sym.decl.(ast.Spec) + if !ok { + return false + } + return sx == spec +} + +func containingGen(pass *analysis.Pass, sym symbol) (*ast.GenDecl, ast.Spec) { + for _, decl := range sym.file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok { + continue + } + for _, spec := range gen.Specs { + if specMatches(spec, sym) { + return gen, spec + } + } + } + return nil, nil +} + +func fixTarget(pass *analysis.Pass, sym symbol) (start, end token.Pos, whole bool, ok bool) { + decl, gen, spec := candidateDecl(pass, sym) + if decl != nil { + return declStart(decl), decl.End(), true, true + } + if gen != nil { + return specTarget(pass, sym, gen, spec) + } + return 0, 0, false, false +} + +func declStart(node ast.Node) token.Pos { + switch node := node.(type) { + case *ast.FuncDecl: + if node.Doc != nil { + return node.Doc.Pos() + } + return node.Pos() + case *ast.GenDecl: + if node.Doc != nil { + return node.Doc.Pos() + } + return node.Pos() + } + return node.Pos() +} + +func specTarget(pass *analysis.Pass, sym symbol, gen *ast.GenDecl, spec ast.Spec) (token.Pos, token.Pos, bool, bool) { + if spec == nil { + return 0, 0, false, false + } + if len(gen.Specs) == 1 { + return declStart(gen), gen.End(), true, true + } + if gen.Tok == token.CONST { + return 0, 0, false, false + } + return specStart(spec), spec.End(), false, true +} + +func specStart(spec ast.Spec) token.Pos { + switch spec := spec.(type) { + case *ast.TypeSpec: + if spec.Doc != nil { + return spec.Doc.Pos() + } + return spec.Pos() + case *ast.ValueSpec: + if spec.Doc != nil { + return spec.Doc.Pos() + } + return spec.Pos() + } + return spec.Pos() +} + +func suggestedFix(pass *analysis.Pass, sym symbol) *analysis.SuggestedFix { + file := readFixFile(pass, sym.file) + if file == nil { + return nil + } + if !file.plain() || isCgoFile(file.file) { + return nil + } + start, end, whole, ok := fixTarget(pass, sym) + if !ok { + return nil + } + edits, ok := file.removalEdits(pass, sym, start, end, whole) + if !ok { + return nil + } + return &analysis.SuggestedFix{ + Message: "Remove unused unexported declaration " + sym.name, + TextEdits: edits, + } +} + +func endOfLine(src []byte, start int) int { + if n := bytes.IndexByte(src[start:], '\n'); n >= 0 { + return start + n + } + return len(src) +} + +func lineStart(src []byte, offset int) int { + for start := offset; start > 0; start-- { + if src[start-1] == '\n' { + return start + } + } + return 0 +} + +func safeLineSuffix(suffix []byte) bool { + rest := strings.TrimSpace(string(suffix)) + return rest == "" || strings.HasPrefix(rest, "//") +} + +func afterBlankLines(src []byte, lineEnd int) int { + end := lineEnd + for end < len(src) { + lineEnd = endOfLine(src, end) + if !isBlankLine(src[end:lineEnd]) { + break + } + end = lineEnd + 1 + if end > len(src) { + break + } + } + return end +} + +func isBlankLine(line []byte) bool { + return len(bytes.Trim(line, " \t\r")) == 0 +} + +type importRef struct { + name string + path string +} + +func hasCommentsIn(file *fixFile, node ast.Node) bool { + for _, comment := range file.file.Comments { + if comment.Pos() < node.Pos() { + continue + } + if comment.End() > node.End() { + continue + } + return true + } + return false +} + +func endOfLineInclusive(src []byte, offset int) int { + end := endOfLine(src, offset) + if end < len(src) { + return end + 1 + } + return end +} + +func isBlankLineAt(src []byte, offset int) bool { + if offset >= len(src) { + return false + } + lineEnd := endOfLine(src, offset) + return isBlankLine(src[offset:lineEnd]) +} + +func isBlankLineBefore(src []byte, offset int) bool { + if offset == 0 { + return false + } + return isBlankLineAt(src, lineStart(src, offset-1)) +} + +func unusedImports(pass *analysis.Pass, used map[importRef]bool, gen *ast.GenDecl) []*ast.ImportSpec { + var unused []*ast.ImportSpec + for _, spec := range gen.Specs { + importSpec, ok := spec.(*ast.ImportSpec) + if !ok { + continue + } + if isImportUsed(pass, used, importSpec) { + continue + } + unused = append(unused, importSpec) + } + return unused +} + +func isImportUsed(pass *analysis.Pass, used map[importRef]bool, spec *ast.ImportSpec) bool { + pkgName := importPkgName(pass, spec) + if pkgName == nil { + return false + } + if pkgName.Name() == "_" { + return true + } + return used[importRef{name: pkgName.Name(), path: pkgName.Imported().Path()}] +} + +func importPkgName(pass *analysis.Pass, spec *ast.ImportSpec) *types.PkgName { + if spec.Name != nil { + if pkgName, ok := pass.TypesInfo.Defs[spec.Name].(*types.PkgName); ok { + return pkgName + } + return nil + } + if pkgName, ok := pass.TypesInfo.Implicits[spec].(*types.PkgName); ok { + return pkgName + } + return nil +} + +func usedImports(pass *analysis.Pass, file *ast.File, sym symbol, whole bool) map[importRef]bool { + used := make(map[importRef]bool) + for _, decl := range file.Decls { + if isImportDecl(decl) { + continue + } + for _, ref := range remainingImports(pass, decl, sym, whole) { + used[ref] = true + } + } + return used +} + +func remainingImports(pass *analysis.Pass, decl ast.Decl, sym symbol, whole bool) []importRef { + if isRemovedDecl(decl, sym, whole) { + return nil + } + gen, ok := decl.(*ast.GenDecl) + if !ok { + return declImports(pass, decl) + } + return groupRemainingImports(pass, gen, sym, whole) +} + +func isRemovedDecl(decl ast.Decl, sym symbol, whole bool) bool { + if !whole { + return false + } + if decl == sym.decl { + return true + } + gen, ok := decl.(*ast.GenDecl) + if !ok { + return false + } + for _, spec := range gen.Specs { + if specMatches(spec, sym) { + return true + } + } + return false +} + +func groupRemainingImports(pass *analysis.Pass, gen *ast.GenDecl, sym symbol, whole bool) []importRef { + var refs []importRef + seen := make(map[importRef]bool) + for _, spec := range gen.Specs { + if specMatches(spec, sym) { + continue + } + for _, ref := range declImports(pass, spec) { + if seen[ref] { + continue + } + seen[ref] = true + refs = append(refs, ref) + } + } + return refs +} + +func isImportDecl(decl ast.Decl) bool { + gen, ok := decl.(*ast.GenDecl) + if !ok { + return false + } + return gen.Tok == token.IMPORT +} + +func declImports(pass *analysis.Pass, decl ast.Node) []importRef { + var refs []importRef + selectors := make(map[*ast.Ident]bool) + seen := make(map[importRef]bool) + ast.Inspect(decl, func(node ast.Node) bool { + if sel, ok := node.(*ast.SelectorExpr); ok { + selectors[sel.Sel] = true + } + id, ok := node.(*ast.Ident) + if !ok { + return true + } + if selectors[id] { + return true + } + ref, ok := identImport(pass, id) + if !ok { + return true + } + if seen[ref] { + return true + } + seen[ref] = true + refs = append(refs, ref) + return true + }) + return refs +} + +func identImport(pass *analysis.Pass, id *ast.Ident) (importRef, bool) { + obj := pass.TypesInfo.Uses[id] + if pkgName, ok := obj.(*types.PkgName); ok { + return importRef{name: pkgName.Name(), path: pkgName.Imported().Path()}, true + } + if obj == nil { + return importRef{}, false + } + pkg := obj.Pkg() + if pkg == nil { + return importRef{}, false + } + if pkg == pass.Pkg { + return importRef{}, false + } + return importRef{name: ".", path: pkg.Path()}, true +} + +func (f *fixFile) importDeclRemoval(gen *ast.GenDecl) ([]analysis.TextEdit, bool) { + start := gen.Pos() + if gen.Doc != nil { + start = gen.Doc.Pos() + } + end := f.token.Offset(gen.End()) + lineEnd := endOfLine(f.source, end) + if !safeLineSuffix(f.source[end:lineEnd]) { + return nil, false + } + return []analysis.TextEdit{f.removal(f.token.Offset(start), afterBlankLines(f.source, lineEnd))}, true +} + +func (f *fixFile) importRemovals(gen *ast.GenDecl, unused []*ast.ImportSpec) ([]analysis.TextEdit, bool) { + if len(unused) == len(gen.Specs) { + return f.importDeclRemoval(gen) + } + var edits []analysis.TextEdit + for _, spec := range unused { + edit, ok := f.importSpecRemoval(spec) + if !ok { + return nil, false + } + edits = append(edits, edit) + } + return edits, true +} + +func (f *fixFile) importRemovalsFor(pass *analysis.Pass, sym symbol, whole bool) ([]analysis.TextEdit, bool) { + used := usedImports(pass, f.file, sym, whole) + var edits []analysis.TextEdit + for _, decl := range f.file.Decls { + gen, ok := decl.(*ast.GenDecl) + if !ok { + continue + } + if gen.Tok != token.IMPORT { + continue + } + unused := unusedImports(pass, used, gen) + if len(unused) == 0 { + continue + } + if hasCommentsIn(f, gen) { + return nil, false + } + removals, ok := f.importRemovals(gen, unused) + if !ok { + return nil, false + } + edits = append(edits, removals...) + } + return edits, true +} + +func (f *fixFile) importSpecRemoval(spec *ast.ImportSpec) (analysis.TextEdit, bool) { + start := lineStart(f.source, f.token.Offset(spec.Pos())) + end := f.token.Offset(spec.End()) + lineEnd := endOfLine(f.source, end) + if !safeLineSuffix(f.source[end:lineEnd]) { + return analysis.TextEdit{}, false + } + if lineEnd == len(f.source) { + return analysis.TextEdit{}, false + } + next := lineEnd + 1 + if isBlankLineAt(f.source, next) { + return f.removal(start, endOfLineInclusive(f.source, next)), true + } + if isBlankLineBefore(f.source, start) { + start = lineStart(f.source, start-1) + } + return f.removal(start, next), true +} + +func (f *fixFile) plain() bool { + base := strings.TrimSuffix(filepath.Base(f.token.Name()), ".go") + if strings.Contains(base, "_") { + return false + } + return !containsDirective(f.source) && !bytes.Contains(f.source, []byte("// +build")) +} + +func (f *fixFile) removal(start, end int) analysis.TextEdit { + return analysis.TextEdit{Pos: f.token.Pos(start), End: f.token.Pos(end)} +} + +func (f *fixFile) removalEdits(pass *analysis.Pass, sym symbol, start, end token.Pos, whole bool) ([]analysis.TextEdit, bool) { + if !f.startsOwnLine(start) { + return nil, false + } + startOff := f.token.Offset(start) + endOff := f.token.Offset(end) + lineEnd := endOfLine(f.source, endOff) + if !safeLineSuffix(f.source[endOff:lineEnd]) { + return nil, false + } + var edits []analysis.TextEdit + edits = append(edits, f.removal(startOff, afterBlankLines(f.source, lineEnd))) + removals, ok := f.importRemovalsFor(pass, sym, whole) + if !ok { + return nil, false + } + return append(edits, removals...), true +} + +func (f *fixFile) startsOwnLine(pos token.Pos) bool { + off := f.token.Offset(pos) + start := lineStart(f.source, off) + return len(bytes.Trim(f.source[start:off], " \t\r")) == 0 +} + +func (r *reporter) report(pass *analysis.Pass, sym symbol) { + diag := analysis.Diagnostic{ + Pos: sym.pos, + Message: messageFor(sym), + } + if !r.offered { + if fix := suggestedFix(pass, sym); fix != nil { + diag.SuggestedFixes = []analysis.SuggestedFix{*fix} + r.offered = true + } + } + pass.Report(diag) +} + +type fixFile struct { + file *ast.File + token *token.File + source []byte +} diff --git a/internal/analysis/lean/fix_test.go b/internal/analysis/lean/fix_test.go new file mode 100644 index 0000000..dd05730 --- /dev/null +++ b/internal/analysis/lean/fix_test.go @@ -0,0 +1,273 @@ +package lean_test + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/nnutter/constable/internal/analysis/lean" +) + +type capturedDiagnostic struct { + message string + edits []capturedEdit +} + +type capturedEdit struct { + path string + start, end int + text []byte +} + +func inspectFixes(t *testing.T, dir string) []capturedDiagnostic { + t.Helper() + var diagnostics []capturedDiagnostic + analyzer := &analysis.Analyzer{ + Name: "capturelean", + Doc: "capture lean diagnostics", + Run: func(pass *analysis.Pass) (any, error) { + pass.Report = func(d analysis.Diagnostic) { + item := capturedDiagnostic{message: d.Message} + for _, fix := range d.SuggestedFixes { + for _, edit := range fix.TextEdits { + file := pass.Fset.File(edit.Pos) + item.edits = append(item.edits, capturedEdit{path: file.Name(), start: file.Offset(edit.Pos), end: file.Offset(edit.End), text: edit.NewText}) + } + } + diagnostics = append(diagnostics, item) + } + return lean.Analyzer.Run(pass) + }, + } + analysistest.Run(t, dir, analyzer, "p") + return diagnostics +} + +func writeFixture(t *testing.T, dir, name, content string) { + t.Helper() + path := filepath.Join(dir, "src", "p", name) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) +} + +func applyCaptured(t *testing.T, diagnostics []capturedDiagnostic) { + t.Helper() + var edits []capturedEdit + for _, diagnostic := range diagnostics { + edits = append(edits, diagnostic.edits...) + } + slices.SortFunc(edits, func(a, b capturedEdit) int { + if a.path != b.path { + return strings.Compare(a.path, b.path) + } + return b.start - a.start + }) + for _, edit := range edits { + data, err := os.ReadFile(edit.path) + require.NoError(t, err) + data = append(append(append([]byte(nil), data[:edit.start]...), edit.text...), data[edit.end:]...) + require.NoError(t, os.WriteFile(edit.path, data, 0o644)) + } +} + +func findDiagnostic(t *testing.T, diagnostics []capturedDiagnostic, substr string) capturedDiagnostic { + t.Helper() + for i := range diagnostics { + if strings.Contains(diagnostics[i].message, substr) { + return diagnostics[i] + } + } + require.Fail(t, "missing diagnostic", substr) + return capturedDiagnostic{} +} + +func TestFixRemovesSingleDeclaration(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "only.go", `package p + +// unusedHelper reports nothing. +func unusedHelper() int { return 1 } + +func Keep() int { return 2 } +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedHelper") + require.NotEmpty(t, target.edits, "safe diagnostic has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "only.go")) + require.NoError(t, err) + require.Contains(t, string(data), "func Keep()") + require.NotContains(t, string(data), "unusedHelper") + next := inspectFixes(t, dir) + applyCaptured(t, next) + second, err := os.ReadFile(filepath.Join(dir, "src", "p", "only.go")) + require.NoError(t, err) + require.Equal(t, string(data), string(second), "repeated analysis changed the fixed file") + for _, d := range next { + require.NotContains(t, d.message, "unusedHelper", "fixed diagnostic remains") + } +} + +func TestFixRemovesSingleGroupMember(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "group.go", `package p + +var ( + unusedGroup = 1 + usedGroup = 2 +) + +var _ = usedGroup +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedGroup") + require.NotEmpty(t, target.edits, "group member has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "group.go")) + require.NoError(t, err) + require.Contains(t, string(data), "usedGroup") + require.NotContains(t, string(data), "unusedGroup") + next := inspectFixes(t, dir) + applyCaptured(t, next) + second, err := os.ReadFile(filepath.Join(dir, "src", "p", "group.go")) + require.NoError(t, err) + require.Equal(t, string(data), string(second), "repeated analysis changed the fixed file") +} + +func TestFixSkipsGroupedConstant(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "consts.go", `package p + +const ( + unusedConst = 1 + usedConst = 2 +) + +const _ = usedConst +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedConst") + require.Empty(t, target.edits, "grouped constant offered a fix") +} + +func TestFixSkipsStandaloneComments(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "only.go", `package p + +// This section comment has ambiguous ownership. + +func unusedHelper() int { return 1 } + +func Keep() int { return 2 } +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedHelper") + require.Empty(t, target.edits, "offered a fix despite a standalone comment") +} + +func TestFixRemovesPartialImport(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "imps.go", `package p + +import ( + f "fmt" + "strings" +) + +func unused() { + f.Println("x") +} + +func Keep() string { + return strings.ToUpper("x") +} +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unused unexported function unused") + require.NotEmpty(t, target.edits, "safe diagnostic has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "imps.go")) + require.NoError(t, err) + require.NotContains(t, string(data), `"fmt"`) + require.Contains(t, string(data), `"strings"`) + require.Contains(t, string(data), "strings.ToUpper") +} + +func TestFixRemovesGroupedTypeMember(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "types.go", `package p + +type ( + unusedType struct{ x int } + usedType struct{ y int } +) + +var _ = usedType{} +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedType") + require.NotEmpty(t, target.edits, "group member has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "types.go")) + require.NoError(t, err) + require.Contains(t, string(data), "usedType") + require.NotContains(t, string(data), "unusedType") +} + +func TestFixRemovesDocumentedVariable(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "vars.go", `package p + +// unusedCounter says nothing. +var unusedCounter = 1 + +var usedCounter = 2 + +var _ = usedCounter +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedCounter") + require.NotEmpty(t, target.edits, "documented variable has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "vars.go")) + require.NoError(t, err) + require.NotContains(t, string(data), "unusedCounter") + require.Contains(t, string(data), "usedCounter") +} + +func TestFixSkipsGoDirective(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "only.go", "//go:generate echo hi\n\npackage p\n\nfunc unusedHelper() int { return 1 }\n\nfunc Keep() int { return 2 }\n") + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unusedHelper") + require.Empty(t, target.edits, "offered a fix despite a Go directive") +} + +func TestFixRemovesOrphanedImport(t *testing.T) { + dir := t.TempDir() + writeFixture(t, dir, "imp.go", `package p + +import "fmt" + +func unused() { + fmt.Println("x") +} + +func Keep() int { return 2 } +`) + diagnostics := inspectFixes(t, dir) + target := findDiagnostic(t, diagnostics, "unused unexported function unused") + require.NotEmpty(t, target.edits, "safe diagnostic has no fix") + applyCaptured(t, diagnostics) + data, err := os.ReadFile(filepath.Join(dir, "src", "p", "imp.go")) + require.NoError(t, err) + require.NotContains(t, string(data), "fmt") + require.NotContains(t, string(data), "unused") + require.Contains(t, string(data), "func Keep()") +} diff --git a/internal/analysis/lean/lean.go b/internal/analysis/lean/lean.go new file mode 100644 index 0000000..45b9566 --- /dev/null +++ b/internal/analysis/lean/lean.go @@ -0,0 +1,311 @@ +package lean + +import ( + "go/ast" + "go/token" + "go/types" + "slices" + + "golang.org/x/tools/go/analysis" + + "github.com/nnutter/constable/internal/generated" + "github.com/nnutter/constable/internal/report" +) + +var Analyzer = &analysis.Analyzer{ + Name: "lean", + Doc: "reports unused unexported declarations", + Run: run, +} + +func succeeded(ok bool) bool { + return ok +} + +func isNilRecv(recv *ast.FieldList) bool { + return recv == nil +} + +func isBlankIdent(ident *ast.Ident) bool { + return ident.Name == "_" +} + +func isExported(name string) bool { + pos := len(name) + if pos == 0 { + return false + } + return identIsExported(name) +} + +func identIsExported(name string) bool { + first := name[0] + if first < 'A' { + return false + } + return first <= 'Z' +} + +type kind int + +const ( + kindFunction kind = iota + kindType + kindVariable + kindConstant +) + +type symbol struct { + obj types.Object + name string + kind kind + pos token.Pos + decl ast.Node + file *ast.File +} + +func run(pass *analysis.Pass) (any, error) { + symbols := collectSymbols(pass) + used := countUses(pass) + reporter := newReporter(pass) + for _, sym := range symbols { + checkSymbol(pass, reporter, sym, used) + } + + return nil, nil +} + +func collectSymbols(pass *analysis.Pass) []symbol { + var symbols []symbol + for _, file := range pass.Files { + if generated.Skipped(file) { + continue + } + for _, decl := range file.Decls { + symbols = append(symbols, symbolsForDecl(pass, file, decl)...) + } + } + return symbols +} + +func symbolsForDecl(pass *analysis.Pass, file *ast.File, decl ast.Decl) []symbol { + if funcDecl, ok := decl.(*ast.FuncDecl); ok { + return funcSymbol(pass, file, funcDecl) + } + if gen, ok := decl.(*ast.GenDecl); ok { + return genSymbols(pass, file, gen) + } + return nil +} + +func funcSymbol(pass *analysis.Pass, file *ast.File, funcDecl *ast.FuncDecl) []symbol { + if !isNilRecv(funcDecl.Recv) { + return nil + } + if isExported(funcDecl.Name.Name) { + return nil + } + if isEntrypoint(funcDecl.Name.Name) { + return nil + } + obj, ok := pass.TypesInfo.Defs[funcDecl.Name].(*types.Func) + if !succeeded(ok) { + return nil + } + if obj == nil { + return nil + } + return []symbol{{ + obj: obj, + name: funcDecl.Name.Name, + kind: kindFunction, + pos: funcDecl.Name.Pos(), + decl: funcDecl, + file: file, + }} +} + +func isEntrypoint(name string) bool { + switch name { + case "init", "main": + return true + } + return false +} + +func genSymbols(pass *analysis.Pass, file *ast.File, gen *ast.GenDecl) []symbol { + switch gen.Tok { + case token.TYPE: + return typeSymbols(pass, file, gen) + case token.VAR: + return valueSymbols(pass, file, gen, kindVariable) + case token.CONST: + return constSymbols(pass, file, gen) + } + return nil +} + +func constSymbols(pass *analysis.Pass, file *ast.File, gen *ast.GenDecl) []symbol { + if isIotaSensitive(gen) { + return nil + } + return valueSymbols(pass, file, gen, kindConstant) +} + +func isIotaSensitive(gen *ast.GenDecl) bool { + return slices.ContainsFunc(gen.Specs, isIotaSpec) +} + +func isIotaSpec(spec ast.Spec) bool { + valueSpec, ok := spec.(*ast.ValueSpec) + if !ok { + return false + } + if len(valueSpec.Values) == 0 { + return true + } + return containsIota(valueSpec) +} + +func containsIota(spec *ast.ValueSpec) bool { + return slices.ContainsFunc(spec.Values, exprHasIota) +} + +func exprHasIota(value ast.Expr) bool { + found := false + ast.Inspect(value, func(node ast.Node) bool { + if ident, ok := node.(*ast.Ident); ok { + if ident.Name == "iota" { + found = true + return false + } + } + return true + }) + return found +} + +func typeSymbols(pass *analysis.Pass, file *ast.File, gen *ast.GenDecl) []symbol { + var symbols []symbol + for _, spec := range gen.Specs { + symbols = append(symbols, typeSymbol(pass, file, spec)...) + } + return symbols +} + +func typeSymbol(pass *analysis.Pass, file *ast.File, spec ast.Spec) []symbol { + typeSpec, ok := spec.(*ast.TypeSpec) + if !ok { + return nil + } + return typeSpecSymbols(pass, file, typeSpec) +} + +func typeSpecSymbols(pass *analysis.Pass, file *ast.File, spec *ast.TypeSpec) []symbol { + if isExported(spec.Name.Name) { + return nil + } + if isBlankIdent(spec.Name) { + return nil + } + obj, ok := pass.TypesInfo.Defs[spec.Name].(*types.TypeName) + if !succeeded(ok) { + return nil + } + if obj == nil { + return nil + } + return []symbol{{ + obj: obj, + name: spec.Name.Name, + kind: kindType, + pos: spec.Name.Pos(), + decl: spec, + file: file, + }} +} + +func valueSymbols(pass *analysis.Pass, file *ast.File, gen *ast.GenDecl, valueKind kind) []symbol { + var symbols []symbol + for _, spec := range gen.Specs { + symbols = append(symbols, valueSpecSymbols(pass, file, spec, valueKind)...) + } + return symbols +} + +func valueSpecSymbols(pass *analysis.Pass, file *ast.File, spec ast.Spec, valueKind kind) []symbol { + valueSpec, ok := spec.(*ast.ValueSpec) + if !ok { + return nil + } + var symbols []symbol + for _, name := range valueSpec.Names { + symbols = append(symbols, valueNameSymbol(pass, file, valueSpec, name, valueKind)...) + } + return symbols +} + +func valueNameSymbol(pass *analysis.Pass, file *ast.File, spec *ast.ValueSpec, name *ast.Ident, valueKind kind) []symbol { + if isExported(name.Name) || isBlankIdent(name) { + return nil + } + obj := pass.TypesInfo.Defs[name] + if obj == nil { + return nil + } + if !isValueKind(obj, valueKind) { + return nil + } + return []symbol{{ + obj: obj, + name: name.Name, + kind: valueKind, + pos: name.Pos(), + decl: spec, + file: file, + }} +} + +func isValueKind(obj types.Object, valueKind kind) bool { + switch valueKind { + case kindVariable: + _, ok := obj.(*types.Var) + return ok + case kindConstant: + _, ok := obj.(*types.Const) + return ok + } + return false +} + +func countUses(pass *analysis.Pass) map[types.Object]int { + counts := make(map[types.Object]int) + for _, obj := range pass.TypesInfo.Uses { + if obj == nil { + continue + } + counts[obj]++ + } + return counts +} + +func checkSymbol(pass *analysis.Pass, reporter *reporter, sym symbol, used map[types.Object]int) { + if used[sym.obj] != 0 { + return + } + reporter.report(pass, sym) +} + +func messageFor(sym symbol) string { + switch sym.kind { + case kindFunction: + return report.UnusedFunction(sym.name) + case kindType: + return report.UnusedType(sym.name) + case kindVariable: + return report.UnusedVariable(sym.name) + case kindConstant: + return report.UnusedConstant(sym.name) + } + return "unused symbol " + sym.name +} diff --git a/internal/analysis/lean/lean_test.go b/internal/analysis/lean/lean_test.go new file mode 100644 index 0000000..63d8cc8 --- /dev/null +++ b/internal/analysis/lean/lean_test.go @@ -0,0 +1,21 @@ +package lean_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/nnutter/constable/internal/analysis/lean" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), lean.Analyzer, "l") +} + +func TestAnalyzerSkipsGeneratedFiles(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), lean.Analyzer, "g") +} + +func TestSuggestedFixes(t *testing.T) { + analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), lean.Analyzer, "fix") +} diff --git a/internal/analysis/lean/testdata/src/fix/funcs.go b/internal/analysis/lean/testdata/src/fix/funcs.go new file mode 100644 index 0000000..5fa0b27 --- /dev/null +++ b/internal/analysis/lean/testdata/src/fix/funcs.go @@ -0,0 +1,15 @@ +package fix + +import "fmt" + +// unused prints and nothing reads it back. +func unused() { // want "unused unexported function unused is never used" + fmt.Println("x") +} + +func helper() {} + +// Keep exercises the used helper. +func Keep() { + helper() +} diff --git a/internal/analysis/lean/testdata/src/fix/funcs.go.golden b/internal/analysis/lean/testdata/src/fix/funcs.go.golden new file mode 100644 index 0000000..a3766bd --- /dev/null +++ b/internal/analysis/lean/testdata/src/fix/funcs.go.golden @@ -0,0 +1,8 @@ +package fix + +func helper() {} + +// Keep exercises the used helper. +func Keep() { + helper() +} diff --git a/internal/analysis/lean/testdata/src/g/g.go b/internal/analysis/lean/testdata/src/g/g.go new file mode 100644 index 0000000..3b89dc0 --- /dev/null +++ b/internal/analysis/lean/testdata/src/g/g.go @@ -0,0 +1,7 @@ +// Code generated by hand for the test. DO NOT EDIT. + +package g + +// GeneratedUnused is a violation, but a person cannot fix a generated +// file, so the analyzer ignores it. +func generatedUnused() int { return 1 } diff --git a/internal/analysis/lean/testdata/src/l/l.go b/internal/analysis/lean/testdata/src/l/l.go new file mode 100644 index 0000000..78aba5d --- /dev/null +++ b/internal/analysis/lean/testdata/src/l/l.go @@ -0,0 +1,82 @@ +package l + +func unusedHelper() int { return 1 } // want "unused unexported function unusedHelper is never used" + +func usedHelper() int { return 2 } + +func exportedEntry() int { return usedHelper() } + +func init() {} + +type unusedType struct{ n int } // want "unused unexported type unusedType is never used" + +type usedType struct{ n int } + +func makeUsedType() usedType { return usedType{n: 1} } + +var unusedVar = 1 // want "unused unexported variable unusedVar is never used" + +var usedVar = 2 + +const unusedConst = 1 // want "unused unexported constant unusedConst is never used" + +const usedConst = 2 + +var keptByBlank = 3 + +var _ = keptByBlank + +func ReadUsed() (int, int, usedType) { + _ = exportedEntry() + return usedVar, usedConst, makeUsedType() +} + +var ( + unusedGroupedVar = 1 // want "unused unexported variable unusedGroupedVar is never used" + usedGroupedVar = 2 +) + +var _ = usedGroupedVar + +const ( + unusedGroupedConst = 1 // want "unused unexported constant unusedGroupedConst is never used" + usedGroupedConst = 2 +) + +var _ = usedGroupedConst + +type WithMethod struct{ n int } + +func (w WithMethod) unusedMethod() {} + +var _ = WithMethod{} + +type WithField struct{ unusedField int } + +var _ = WithField{unusedField: 1} + +func unusedGeneric[T any](x T) T { return x } // want "unused unexported function unusedGeneric is never used" + +func usedGeneric[T any](x T) T { return x } + +var _ = usedGeneric[int] + +type unusedBox[T any] struct{ v T } // want "unused unexported type unusedBox is never used" + +type usedBox[T any] struct{ v T } + +var _ = usedBox[int]{v: 1} + +func Shadowed() int { + unusedHelper := 1 + return unusedHelper +} + +type code int + +const ( + unusedCode code = iota + usedCode +) + +var _ = usedCode diff --git a/internal/analysis/lean/testdata/src/l/other.go b/internal/analysis/lean/testdata/src/l/other.go new file mode 100644 index 0000000..cf0e243 --- /dev/null +++ b/internal/analysis/lean/testdata/src/l/other.go @@ -0,0 +1,7 @@ +package l + +func ExportedOther() int { return usedHelperOther() } + +func usedHelperOther() int { return 3 } + +func unusedInOther() int { return 4 } // want "unused unexported function unusedInOther is never used" diff --git a/internal/analysis/pure/pure.go b/internal/analysis/pure/pure.go new file mode 100644 index 0000000..7ed2b5a --- /dev/null +++ b/internal/analysis/pure/pure.go @@ -0,0 +1,719 @@ +package pure + +import ( + "go/ast" + "go/token" + "go/types" + + "golang.org/x/tools/go/analysis" + + "github.com/nnutter/constable/internal/directive" + "github.com/nnutter/constable/internal/directive/enum" + "github.com/nnutter/constable/internal/generated" + "github.com/nnutter/constable/internal/report" +) + +var Analyzer = &analysis.Analyzer{ + Name: enum.PureName, + Doc: "reports observable effects in //constable:pure functions", + Run: run, +} + +type origin struct { + name string + typ types.Type + isReceiver bool +} + +type functionState struct { + pass *analysis.Pass + origins map[types.Object]origin + globals map[types.Object]string +} + +func succeeded(ok bool) bool { + return ok +} + +func isNilBody(body *ast.BlockStmt) bool { + return body == nil +} + +func isChanged(changed bool) bool { + return changed +} + +func isDeleteName(name string) bool { + return name == "delete" +} + +func hasNoArgs(call *ast.CallExpr) bool { + return len(call.Args) == 0 +} + +func run(pass *analysis.Pass) (any, error) { + for _, file := range pass.Files { + if generated.Skipped(file) { + continue + } + + for _, decl := range file.Decls { + funcDecl, ok := decl.(*ast.FuncDecl) + if !succeeded(ok) || !hasPureDirective(funcDecl) || isNilBody(funcDecl.Body) { + continue + } + + state := newFunctionState(pass, funcDecl) + state.collectAliases(funcDecl.Body) + state.reportEffects(funcDecl.Body) + } + } + + return nil, nil +} + +func hasPureDirective(funcDecl *ast.FuncDecl) bool { + if funcDecl.Doc == nil { + return false + } + for _, comment := range funcDecl.Doc.List { + if directive.HasPure(comment.Text) { + return true + } + } + return false +} + +func newFunctionState(pass *analysis.Pass, funcDecl *ast.FuncDecl) *functionState { + state := &functionState{ + pass: pass, + origins: make(map[types.Object]origin), + globals: make(map[types.Object]string), + } + + if funcDecl.Recv != nil { + state.addFields(funcDecl.Recv, true) + } + if funcDecl.Type.Params != nil { + state.addFields(funcDecl.Type.Params, false) + } + + return state +} + +func isAddrOp(expr *ast.UnaryExpr) bool { + return expr.Op == token.AND +} + +func isPackageVar(obj types.Object) bool { + if obj == nil { + return false + } + if obj.Pkg() == nil { + return false + } + if _, ok := obj.(*types.Var); !ok { + return false + } + return obj.Parent() == obj.Pkg().Scope() +} + +func qualifiedName(obj types.Object) string { + if obj == nil { + return "" + } + if builtin, ok := obj.(*types.Builtin); ok { + return builtin.Name() + } + if obj.Pkg() == nil { + return obj.Name() + } + return obj.Pkg().Name() + "." + obj.Name() +} + +func isImpureObject(obj types.Object) bool { + if obj == nil { + return false + } + if builtin, ok := obj.(*types.Builtin); ok { + return isPrintBuiltin(builtin.Name()) + } + if obj.Pkg() == nil { + return false + } + return isImpurePackage(obj.Pkg().Path(), obj.Name()) +} + +func isPrintBuiltin(name string) bool { + switch name { + case "print", "println": + return true + } + return false +} + +func isImpurePackage(pkgPath, name string) bool { + switch pkgPath { + case "log": + return true + case "math/rand", "math/rand/v2": + return true + case "fmt": + return isFmtOutput(name) + case "time": + return isClockFunc(name) + case "os": + return isOsEffect(name) + } + return false +} + +func isFmtOutput(name string) bool { + switch name { + case "Print", "Printf", "Println", "Fprint", "Fprintf", "Fprintln": + return true + } + return false +} + +func isClockFunc(name string) bool { + switch name { + case "Now", "Since", "Until", "Sleep", "After", "AfterFunc", "Tick", "NewTicker", "NewTimer": + return true + } + return false +} + +func isOsEffect(name string) bool { + switch name { + case "IsNotExist", "IsExist", "IsPermission", "IsTimeout", "Name": + return false + } + return true +} + +func isAmbientVar(obj types.Object) bool { + if obj == nil { + return false + } + if _, ok := obj.(*types.Var); !ok { + return false + } + if obj.Pkg() == nil { + return false + } + return isAmbientPackageVar(obj.Pkg().Path(), obj.Name()) +} + +func isAmbientPackageVar(pkgPath, name string) bool { + switch pkgPath { + case "os": + return isOsAmbient(name) + case "time": + return name == "Local" + } + return false +} + +func isOsAmbient(name string) bool { + switch name { + case "Args", "Stdout", "Stderr", "Stdin": + return true + } + return false +} + +func ambientName(obj types.Object) string { + if obj == nil { + return "" + } + if obj.Pkg() == nil { + return obj.Name() + } + return obj.Pkg().Name() + "." + obj.Name() +} + +func collectLeftSelectors(body *ast.BlockStmt) map[*ast.SelectorExpr]bool { + lefts := make(map[*ast.SelectorExpr]bool) + ast.Inspect(body, func(node ast.Node) bool { + switch node := node.(type) { + case *ast.AssignStmt: + for _, lhs := range node.Lhs { + if sel, ok := unparen(lhs).(*ast.SelectorExpr); ok { + lefts[sel] = true + } + } + case *ast.IncDecStmt: + if sel, ok := unparen(node.X).(*ast.SelectorExpr); ok { + lefts[sel] = true + } + } + + return true + }) + return lefts +} + +func isReferenceLike(typ types.Type) bool { + typ = dereference(typ) + switch typ.(type) { + case *types.Pointer, *types.Slice, *types.Map: + return true + } + + return false +} + +func isIndexReferenceLike(typ types.Type) bool { + typ = dereference(typ) + switch typ.(type) { + case *types.Slice, *types.Map: + return true + } + + return false +} + +func dereference(typ types.Type) types.Type { + for { + named, ok := typ.(*types.Named) + if !ok { + return typ + } + + typ = named.Underlying() + } +} + +func unparen(expr ast.Expr) ast.Expr { + for { + paren, ok := expr.(*ast.ParenExpr) + if !ok { + return expr + } + + expr = paren.X + } +} + +func (state *functionState) addFields(fields *ast.FieldList, isReceiver bool) { + for _, field := range fields.List { + for _, name := range field.Names { + obj := state.pass.TypesInfo.Defs[name] + if obj == nil { + continue + } + + state.origins[obj] = origin{ + name: name.Name, + typ: obj.Type(), + isReceiver: isReceiver, + } + } + } +} + +func (state *functionState) callObject(call *ast.CallExpr) types.Object { + return state.funObject(call.Fun) +} + +func (state *functionState) callObjectIndex(expr *ast.IndexExpr) types.Object { + inner := unparen(expr.X) + switch inner := inner.(type) { + case *ast.SelectorExpr: + return state.selectorObject(inner) + case *ast.Ident: + return state.pass.TypesInfo.Uses[inner] + } + + return nil +} + +func (state *functionState) collectAliases(body *ast.BlockStmt) { + changed := true + for changed { + changed = false + ast.Inspect(body, func(node ast.Node) bool { + switch node := node.(type) { + case *ast.AssignStmt: + changed = state.collectAssignAliases(node) || isChanged(changed) + } + + return true + }) + } +} + +func (state *functionState) collectAssignAliases(stmt *ast.AssignStmt) bool { + changed := false + for i, lhs := range stmt.Lhs { + if i >= len(stmt.Rhs) { + continue + } + changed = state.collectOneAlias(lhs, stmt.Rhs[i]) || isChanged(changed) + } + + return changed +} + +func (state *functionState) collectOneAlias(lhs ast.Expr, rhs ast.Expr) bool { + ident, ok := lhs.(*ast.Ident) + if !ok { + return false + } + + obj := state.objectOf(ident) + if obj == nil { + return false + } + if state.trackParamAlias(obj, rhs) { + return true + } + return state.trackGlobalAlias(obj, rhs) +} + +func (state *functionState) exprRefersToCallerVisibleMemory(expr ast.Expr) (origin, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + orig, ok := state.originOf(expr) + if !ok { + return orig, false + } + + return orig, isReferenceLike(state.typeOf(expr)) + case *ast.SelectorExpr: + if orig, ok := state.exprRefersToCallerVisibleMemory(expr.X); ok { + return orig, true + } + + orig, ok := state.rootOrigin(expr.X) + if !ok { + return orig, false + } + + return orig, isReferenceLike(state.typeOf(expr)) + case *ast.IndexExpr: + orig, ok := state.rootOrigin(expr.X) + if !ok { + return orig, false + } + + return orig, isIndexReferenceLike(state.typeOf(expr.X)) + case *ast.StarExpr: + return state.exprRefersToCallerVisibleMemory(expr.X) + } + + return origin{}, false +} + +func (state *functionState) funObject(expr ast.Expr) types.Object { + fun := unparen(expr) + switch fun := fun.(type) { + case *ast.SelectorExpr: + return state.selectorObject(fun) + case *ast.IndexExpr: + return state.callObjectIndex(fun) + case *ast.IndexListExpr: + return state.funObject(fun.X) + case *ast.Ident: + return state.pass.TypesInfo.Uses[fun] + } + + return nil +} + +func (state *functionState) globalOf(expr ast.Expr) (string, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + obj := state.objectOf(expr) + if obj == nil { + return "", false + } + if isPackageVar(obj) { + return obj.Name(), true + } + name, ok := state.globals[obj] + return name, ok + case *ast.SelectorExpr: + obj := state.pass.TypesInfo.Uses[expr.Sel] + if obj == nil { + return "", false + } + if isPackageVar(obj) { + return obj.Name(), true + } + case *ast.UnaryExpr: + if isAddrOp(expr) { + return state.globalOf(expr.X) + } + } + + return "", false +} + +func (state *functionState) message(orig origin, isDelete bool) string { + if isDelete { + return report.PureDeletesFromMapParameter(orig.name) + } + if orig.isReceiver { + return report.PureMutatesReceiver(orig.name) + } + if _, ok := dereference(orig.typ).(*types.Pointer); ok { + return report.PureMutatesPointerParameter(orig.name) + } + + return report.PureMutatesParameter(orig.name) +} + +func (state *functionState) objectOf(ident *ast.Ident) types.Object { + if obj := state.pass.TypesInfo.Defs[ident]; obj != nil { + return obj + } + + return state.pass.TypesInfo.Uses[ident] +} + +func (state *functionState) originOf(expr ast.Expr) (origin, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + obj := state.objectOf(expr) + orig, ok := state.origins[obj] + return orig, ok + } + + return origin{}, false +} + +func (state *functionState) reportAssigns(stmt *ast.AssignStmt) { + for _, lhs := range stmt.Lhs { + state.reportMutationTarget(lhs) + } +} + +func (state *functionState) reportCall(call *ast.CallExpr) { + state.reportDelete(call) + state.reportImpureCall(call) +} + +func (state *functionState) reportDelete(call *ast.CallExpr) { + ident, ok := call.Fun.(*ast.Ident) + if !succeeded(ok) || !isDeleteName(ident.Name) || hasNoArgs(call) { + return + } + + if orig, ok := state.exprRefersToCallerVisibleMemory(call.Args[0]); ok { + state.pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + Message: state.message(orig, true), + }) + } + if name, ok := state.globalOf(call.Args[0]); ok { + state.pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + Message: report.PureWritesGlobal(name), + }) + } +} + +func (state *functionState) reportEffects(body *ast.BlockStmt) { + lefts := collectLeftSelectors(body) + ast.Inspect(body, func(node ast.Node) bool { + switch node := node.(type) { + case *ast.AssignStmt: + state.reportAssigns(node) + case *ast.IncDecStmt: + state.reportMutationTarget(node.X) + case *ast.CallExpr: + state.reportCall(node) + case *ast.GoStmt: + state.pass.Report(analysis.Diagnostic{ + Pos: node.Pos(), + Message: report.PureSpawnsGoroutine(), + }) + case *ast.SelectorExpr: + state.reportSelectorRead(node, lefts) + } + + return true + }) +} + +func (state *functionState) reportImpureCall(call *ast.CallExpr) { + obj := state.callObject(call) + if obj == nil { + return + } + if !isImpureObject(obj) { + return + } + state.pass.Report(analysis.Diagnostic{ + Pos: call.Pos(), + Message: report.PureCalls(qualifiedName(obj)), + }) +} + +func (state *functionState) reportMutationTarget(expr ast.Expr) { + if orig, ok := state.visibleMutationOrigin(expr); ok { + state.pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + Message: state.message(orig, false), + }) + } + if name, ok := state.visibleGlobalName(expr); ok { + state.pass.Report(analysis.Diagnostic{ + Pos: expr.Pos(), + Message: report.PureWritesGlobal(name), + }) + } +} + +func (state *functionState) reportSelectorRead(sel *ast.SelectorExpr, lefts map[*ast.SelectorExpr]bool) { + if lefts[sel] { + return + } + obj := state.pass.TypesInfo.Uses[sel.Sel] + if obj == nil { + return + } + if isAmbientVar(obj) { + state.pass.Report(analysis.Diagnostic{ + Pos: sel.Pos(), + Message: report.PureReadsVar(ambientName(obj)), + }) + } +} + +func (state *functionState) rootGlobalName(expr ast.Expr) (string, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + obj := state.objectOf(expr) + if obj == nil { + return "", false + } + if isPackageVar(obj) { + return obj.Name(), true + } + name, ok := state.globals[obj] + return name, ok + case *ast.SelectorExpr: + obj := state.pass.TypesInfo.Uses[expr.Sel] + if obj == nil { + return state.rootGlobalName(expr.X) + } + if isPackageVar(obj) { + return obj.Name(), true + } + return state.rootGlobalName(expr.X) + case *ast.StarExpr: + return state.rootGlobalName(expr.X) + case *ast.IndexExpr: + return state.rootGlobalName(expr.X) + } + + return "", false +} + +func (state *functionState) rootOrigin(expr ast.Expr) (origin, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + return state.originOf(expr) + case *ast.StarExpr: + return state.rootOrigin(expr.X) + case *ast.SelectorExpr: + return state.rootOrigin(expr.X) + case *ast.IndexExpr: + return state.rootOrigin(expr.X) + } + + return origin{}, false +} + +func (state *functionState) selectorObject(sel *ast.SelectorExpr) types.Object { + if selObj := state.pass.TypesInfo.Selections[sel]; selObj != nil { + return selObj.Obj() + } + return state.pass.TypesInfo.Uses[sel.Sel] +} + +func (state *functionState) trackGlobalAlias(obj types.Object, rhs ast.Expr) bool { + if _, exists := state.globals[obj]; exists { + return false + } + + name, ok := state.globalOf(rhs) + if !succeeded(ok) { + return false + } + + state.globals[obj] = name + return true +} + +func (state *functionState) trackParamAlias(obj types.Object, rhs ast.Expr) bool { + if _, exists := state.origins[obj]; exists { + return false + } + + orig, ok := state.originOf(rhs) + if !succeeded(ok) { + return false + } + + state.origins[obj] = orig + return true +} + +func (state *functionState) typeOf(expr ast.Expr) types.Type { + return state.pass.TypesInfo.TypeOf(expr) +} + +func (state *functionState) visibleGlobalName(expr ast.Expr) (string, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + obj := state.objectOf(expr) + if obj == nil { + return "", false + } + if isPackageVar(obj) { + return obj.Name(), true + } + return "", false + case *ast.SelectorExpr: + obj := state.pass.TypesInfo.Uses[expr.Sel] + if obj == nil { + return state.rootGlobalName(expr.X) + } + if isPackageVar(obj) { + return obj.Name(), true + } + return state.rootGlobalName(expr.X) + case *ast.StarExpr: + return state.rootGlobalName(expr.X) + case *ast.IndexExpr: + return state.rootGlobalName(expr.X) + } + + return "", false +} + +func (state *functionState) visibleMutationOrigin(expr ast.Expr) (origin, bool) { + expr = unparen(expr) + switch expr := expr.(type) { + case *ast.Ident: + return origin{}, false + case *ast.StarExpr: + return state.exprRefersToCallerVisibleMemory(expr.X) + case *ast.SelectorExpr: + return state.exprRefersToCallerVisibleMemory(expr.X) + case *ast.IndexExpr: + return state.exprRefersToCallerVisibleMemory(expr.X) + } + + return origin{}, false +} diff --git a/internal/analysis/pure/pure_test.go b/internal/analysis/pure/pure_test.go new file mode 100644 index 0000000..b5448e8 --- /dev/null +++ b/internal/analysis/pure/pure_test.go @@ -0,0 +1,17 @@ +package pure_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/nnutter/constable/internal/analysis/pure" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), pure.Analyzer, "p") +} + +func TestAnalyzerSkipsGeneratedFiles(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), pure.Analyzer, "g") +} diff --git a/internal/analysis/pure/testdata/src/g/g.go b/internal/analysis/pure/testdata/src/g/g.go new file mode 100644 index 0000000..0ef94d2 --- /dev/null +++ b/internal/analysis/pure/testdata/src/g/g.go @@ -0,0 +1,13 @@ +// Code generated by hand for the test. DO NOT EDIT. + +package g + +import "time" + +// GeneratedCallsClock is a violation, but a person cannot fix a generated +// file, so the analyzer ignores it. +// +//constable:pure +func GeneratedCallsClock() time.Time { + return time.Now() +} diff --git a/internal/analysis/pure/testdata/src/p/p.go b/internal/analysis/pure/testdata/src/p/p.go new file mode 100644 index 0000000..a6ffb49 --- /dev/null +++ b/internal/analysis/pure/testdata/src/p/p.go @@ -0,0 +1,292 @@ +package p + +import ( + "fmt" + "log" + "math/rand" + "os" + "strings" + "time" +) + +var counter int + +var globalSlice []int + +var globalMap = map[string]int{} + +var globalPtr *int + +var globalStruct struct{ n int } + +func UnannotatedGlobalWrite() { + counter = 1 +} + +//constable:pure +func ReadsPointer(p *int) int { + return *p +} + +//constable:pure +func MutatesPointer(p *int) { + *p = 1 // want "//constable:pure function mutates pointer parameter p" +} + +//constable:pure +func MutatesSlice(s []int) { + s[0] = 1 // want "//constable:pure function mutates parameter s" +} + +//constable:pure +func MutatesMap(m map[string]int) { + m["x"] = 1 // want "//constable:pure function mutates parameter m" +} + +//constable:pure +func DeletesMapEntry(m map[string]int) { + delete(m, "x") // want "//constable:pure function deletes from map parameter m" +} + +type Counter struct{ n int } + +//constable:pure +func (c *Counter) Inc() { + c.n++ // want "//constable:pure method mutates receiver c" +} + +type Count struct{ n int } + +//constable:pure +func (c Count) Increment() Count { + c.n++ + return c +} + +//constable:pure +func WritesGlobalAssign() { + counter = 1 // want "//constable:pure function writes global counter" +} + +//constable:pure +func WritesGlobalInc() { + counter++ // want "//constable:pure function writes global counter" +} + +//constable:pure +func WritesGlobalSliceElem() { + globalSlice[0] = 1 // want "//constable:pure function writes global globalSlice" +} + +//constable:pure +func WritesGlobalMapElem() { + globalMap["x"] = 1 // want "//constable:pure function writes global globalMap" +} + +//constable:pure +func DeletesGlobalMap() { + delete(globalMap, "x") // want "//constable:pure function writes global globalMap" +} + +//constable:pure +func WritesGlobalField() { + globalStruct.n = 1 // want "//constable:pure function writes global globalStruct" +} + +//constable:pure +func WritesGlobalDeref() { + *globalPtr = 1 // want "//constable:pure function writes global globalPtr" +} + +//constable:pure +func WritesGlobalAlias() { + g := &counter + *g = 1 // want "//constable:pure function writes global counter" +} + +//constable:pure +func ReadsGlobal() int { + return counter +} + +//constable:pure +func MutatesLocal() int { + x := 0 + x++ + return x +} + +//constable:pure +func SpawnsGoroutine() { + go func() {}() // want "//constable:pure function spawns goroutine" +} + +//constable:pure +func SpawnsGoroutineInClosure() { + f := func() { + go func() {}() // want "//constable:pure function spawns goroutine" + } + f() +} + +//constable:pure +func ReadsClock() time.Time { + return time.Now() // want "//constable:pure function calls time.Now" +} + +//constable:pure +func SleepsClock() { + time.Sleep(time.Second) // want "//constable:pure function calls time.Sleep" +} + +//constable:pure +func MakesClockDate() time.Time { + return time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) +} + +//constable:pure +func ReadsRandom() int { + return rand.Intn(100) // #nosec G404 -- the fixture is the point: random reads are banned. // want "//constable:pure function calls rand.Intn" +} + +//constable:pure +func ReadsEnv() string { + return os.Getenv("HOME") // want "//constable:pure function calls os.Getenv" +} + +//constable:pure +func ReadsProcess() int { + return os.Getpid() // want "//constable:pure function calls os.Getpid" +} + +//constable:pure +func ChecksErrorPredicate(err error) bool { + return os.IsNotExist(err) +} + +//constable:pure +func WritesFile() { + _ = os.WriteFile("example.txt", []byte("x"), 0o600) // want "//constable:pure function calls os.WriteFile" +} + +//constable:pure +func PrintsFmt() { + fmt.Println("x") // want "//constable:pure function calls fmt.Println" +} + +//constable:pure +func FormatsFmt() string { + return fmt.Sprintf("%d", 1) +} + +//constable:pure +func PrintsLog() { + log.Println("x") // want "//constable:pure function calls log.Println" +} + +//constable:pure +func PrintsBuiltin() { + println("x") // want "//constable:pure function calls println" +} + +//constable:pure +func ReadsArgs() []string { + return os.Args // want "//constable:pure function reads os.Args" +} + +//constable:pure +func ReadsLocalTime() *time.Location { + return time.Local // want "//constable:pure function reads time.Local" +} + +//constable:pure +func CallsPureHelpers() string { + return strings.ToUpper("x") +} + +//constable:pure +func MutatesLocalSlice() { + s := make([]int, 1) + s[0] = 1 + _ = s +} + +//constable:pure +func DeletesLocalMap() { + m := map[string]int{} + delete(m, "x") +} + +//constable:pure +func PureWithClosureCallsClock() time.Duration { + f := func() time.Duration { + return time.Since(time.Now().Add(-time.Hour)) // want "//constable:pure function calls time.Since" "//constable:pure function calls time.Now" + } + return f() +} + +//constable:pure +func PanicsOnly() { + panic("boom") +} + +type Holder struct{ p *int } + +//constable:pure +func MutatesFieldThroughPointer(h Holder) { + *h.p = 1 // want "//constable:pure function mutates parameter h" +} + +type Scalar struct{ n int } + +//constable:pure +func MutatesScalarFieldCopy(s Scalar) Scalar { + s.n++ + return s +} + +//constable:pure +func MutatesPointerAliasPure(p *int) { + q := p + *q = 1 // want "//constable:pure function mutates pointer parameter p" +} + +type Element struct{ n int } + +//constable:pure +func MutatesSliceElementField(elements []Element) { + elements[0].n = 1 // want "//constable:pure function mutates parameter elements" +} + +//constable:pure +func MutatesNestedSlicePure(ss [][]int) { + ss[0][0] = 1 // want "//constable:pure function mutates parameter ss" +} + +type Inner struct{ data []byte } + +type Outer struct{ inner Inner } + +//constable:pure +func MutatesNestedFieldPure(o Outer) { + o.inner.data[0] = 0 // want "//constable:pure function mutates parameter o" +} + +func Identity[T any](x T) T { return x } + +//constable:pure +func CallsGenericPure() int { + return Identity[int](1) +} + +//constable:pure +func ReadsNonAmbientVars() *time.Location { + _ = time.UTC + _ = os.ErrNotExist + return time.UTC +} + +//constable:pure +func ReadsStdoutVar() *os.File { + return os.Stdout // want "//constable:pure function reads os.Stdout" +} diff --git a/internal/directive/directive.go b/internal/directive/directive.go index cded827..d100a34 100644 --- a/internal/directive/directive.go +++ b/internal/directive/directive.go @@ -31,7 +31,7 @@ func Parse(comment string) ([]Check, error) { var checks []Check for _, key := range keys(tag) { switch enum.ParseCheckType(key).(type) { - case enum.Nonmutating: + case enum.Nonmutating, enum.Pure: checks = append(checks, Check{Name: key, Config: tag.Get(key)}) continue } @@ -56,6 +56,21 @@ func HasNonmutating(comment string) bool { return false } +func HasPure(comment string) bool { + checks, err := Parse(comment) + if err != nil { + return false + } + + for _, check := range checks { + if check.Name == enum.PureName { + return true + } + } + + return false +} + func keys(tag reflect.StructTag) []string { keyValuePairsPattern := regexp.MustCompile(`(?P\w+)(?::"[^"]*")?`) keyValuePairs := keyValuePairsPattern.FindAllStringSubmatch(string(tag), -1) diff --git a/internal/directive/directive_test.go b/internal/directive/directive_test.go index f53b072..b1ea760 100644 --- a/internal/directive/directive_test.go +++ b/internal/directive/directive_test.go @@ -22,6 +22,13 @@ func TestParse(t *testing.T) { {Name: "nonmutating"}, }, }, + { + name: "pure tag", + comment: "//constable:pure", + want: []directive.Check{ + {Name: "pure"}, + }, + }, { name: "not a constable tag", comment: "//go:noinline", @@ -60,3 +67,9 @@ func TestHasNonmutating(t *testing.T) { assert.True(t, directive.HasNonmutating("//constable:nonmutating")) assert.False(t, directive.HasNonmutating("//go:noinline")) } + +func TestHasPure(t *testing.T) { + assert.True(t, directive.HasPure("//constable:pure")) + assert.False(t, directive.HasPure("//constable:nonmutating")) + assert.False(t, directive.HasPure("//go:noinline")) +} diff --git a/internal/directive/enum/check_type.go b/internal/directive/enum/check_type.go index 7bcac2f..3bed787 100644 --- a/internal/directive/enum/check_type.go +++ b/internal/directive/enum/check_type.go @@ -9,6 +9,8 @@ func ParseCheckType(s string) CheckType { switch s { case NonmutatingName: return Nonmutating{} + case PureName: + return Pure{} default: return nil } @@ -19,3 +21,9 @@ const NonmutatingName = "nonmutating" type Nonmutating struct{} func (Nonmutating) isCheckType() {} + +const PureName = "pure" + +type Pure struct{} + +func (Pure) isCheckType() {} diff --git a/internal/report/report.go b/internal/report/report.go index e0f9231..e392349 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -41,3 +41,51 @@ func ComplexityExceedsLimit(name string, complexity, limit int) string { func UsePredicateInstead(expr string) string { return fmt.Sprintf("use predicate function instead of %q in compound conditional", expr) } + +func PureMutatesPointerParameter(name string) string { + return fmt.Sprintf("//constable:pure function mutates pointer parameter %s", name) +} + +func PureMutatesReceiver(name string) string { + return fmt.Sprintf("//constable:pure method mutates receiver %s", name) +} + +func PureMutatesParameter(name string) string { + return fmt.Sprintf("//constable:pure function mutates parameter %s", name) +} + +func PureDeletesFromMapParameter(name string) string { + return fmt.Sprintf("//constable:pure function deletes from map parameter %s", name) +} + +func PureSpawnsGoroutine() string { + return "//constable:pure function spawns goroutine" +} + +func PureWritesGlobal(name string) string { + return fmt.Sprintf("//constable:pure function writes global %s", name) +} + +func PureCalls(qualified string) string { + return fmt.Sprintf("//constable:pure function calls %s", qualified) +} + +func PureReadsVar(qualified string) string { + return fmt.Sprintf("//constable:pure function reads %s", qualified) +} + +func UnusedFunction(name string) string { + return fmt.Sprintf("unused unexported function %s is never used", name) +} + +func UnusedType(name string) string { + return fmt.Sprintf("unused unexported type %s is never used", name) +} + +func UnusedVariable(name string) string { + return fmt.Sprintf("unused unexported variable %s is never used", name) +} + +func UnusedConstant(name string) string { + return fmt.Sprintf("unused unexported constant %s is never used", name) +} diff --git a/mise.lock b/mise.lock index bb62fe4..85a7046 100644 --- a/mise.lock +++ b/mise.lock @@ -768,43 +768,36 @@ specifiers = ["latest"] [tools."github:alecthomas/go-check-sumtype"."platforms.linux-arm64"] checksum = "sha256:834a7831e9a11365d719d8d9bce03812bbde126357d009bd93617f7282bdf3a6" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-linux-arm64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663818" [tools."github:alecthomas/go-check-sumtype"."platforms.linux-arm64-musl"] checksum = "sha256:834a7831e9a11365d719d8d9bce03812bbde126357d009bd93617f7282bdf3a6" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-linux-arm64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663818" [tools."github:alecthomas/go-check-sumtype"."platforms.linux-x64"] checksum = "sha256:84c803f5d242d10983d7f16136fc034d6c5aa57f70d4560ca1a7f8eae16d0aab" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-linux-amd64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663817" [tools."github:alecthomas/go-check-sumtype"."platforms.linux-x64-musl"] checksum = "sha256:84c803f5d242d10983d7f16136fc034d6c5aa57f70d4560ca1a7f8eae16d0aab" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-linux-amd64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663817" [tools."github:alecthomas/go-check-sumtype"."platforms.macos-arm64"] checksum = "sha256:013587221143de7a9809591ef31761db50ceb10cbf73f1fba4f71a3bece6969b" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-darwin-arm64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663819" [tools."github:alecthomas/go-check-sumtype"."platforms.macos-x64"] checksum = "sha256:c04403ebbde16a18ba132474577f7c1f1ca5fe83000d18638d2c8cca2610309c" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-darwin-amd64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663830" [tools."github:alecthomas/go-check-sumtype"."platforms.windows-x64"] checksum = "sha256:7bdf4bcaecadbc6ccf97d7d60dd0d164c2b4a076f27b360a8ebf9e2633384d4d" -checksum_source = "mise-versions" url = "https://github.com/alecthomas/go-check-sumtype/releases/download/v0.5.0/go-check-sumtype-0.5.0-windows-amd64.tar.gz" url_api = "https://api.github.com/repos/alecthomas/go-check-sumtype/releases/assets/533663836" @@ -950,43 +943,36 @@ specifiers = ["latest"] [tools.gitleaks."platforms.linux-arm64"] checksum = "sha256:e4a487ee7ccd7d3a7f7ec08657610aa3606637dab924210b3aee62570fb4b080" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_arm64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332057" [tools.gitleaks."platforms.linux-arm64-musl"] checksum = "sha256:e4a487ee7ccd7d3a7f7ec08657610aa3606637dab924210b3aee62570fb4b080" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_arm64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332057" [tools.gitleaks."platforms.linux-x64"] checksum = "sha256:551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332058" [tools.gitleaks."platforms.linux-x64-musl"] checksum = "sha256:551f6fc83ea457d62a0d98237cbad105af8d557003051f41f3e7ca7b3f2470eb" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_linux_x64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332058" [tools.gitleaks."platforms.macos-arm64"] checksum = "sha256:b40ab0ae55c505963e365f271a8d3846efbc170aa17f2607f13df610a9aeb6a5" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_darwin_arm64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332059" [tools.gitleaks."platforms.macos-x64"] checksum = "sha256:dfe101a4db2255fc85120ac7f3d25e4342c3c20cf749f2c20a18081af1952709" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_darwin_x64.tar.gz" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378332053" [tools.gitleaks."platforms.windows-x64"] checksum = "sha256:d29144deff3a68aa93ced33dddf84b7fdc26070add4aa0f4513094c8332afc4e" -checksum_source = "mise-versions" url = "https://github.com/gitleaks/gitleaks/releases/download/v8.30.1/gitleaks_8.30.1_windows_x64.zip" url_api = "https://api.github.com/repos/gitleaks/gitleaks/releases/assets/378333178" @@ -1060,43 +1046,36 @@ specifiers = ["latest"] [tools.gofumpt."platforms.linux-arm64"] checksum = "sha256:4322f081241ab0618356aa5921a8c110e4c7acfc17175cc15dc57b2f4a4b652d" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_linux_arm64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473501" [tools.gofumpt."platforms.linux-arm64-musl"] checksum = "sha256:4322f081241ab0618356aa5921a8c110e4c7acfc17175cc15dc57b2f4a4b652d" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_linux_arm64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473501" [tools.gofumpt."platforms.linux-x64"] checksum = "sha256:3bc4fcad497439918f5f56d4987de4709a99da0a355e6bec13a7278532b01d38" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_linux_amd64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473521" [tools.gofumpt."platforms.linux-x64-musl"] checksum = "sha256:3bc4fcad497439918f5f56d4987de4709a99da0a355e6bec13a7278532b01d38" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_linux_amd64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473521" [tools.gofumpt."platforms.macos-arm64"] checksum = "sha256:1a3e325b956ed12c21031d9e13382bd51a4fabb57de8d40428819b9583c3cfb9" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_darwin_arm64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473536" [tools.gofumpt."platforms.macos-x64"] checksum = "sha256:1d3626892a321aca96a891696eaecb8f68e69e90582c2b55672d5ca210fd5fa8" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_darwin_amd64" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473545" [tools.gofumpt."platforms.windows-x64"] checksum = "sha256:66bd679908b64864fc85417e7ea01148774b477364b9a02ab1c194333a55c997" -checksum_source = "mise-versions" url = "https://github.com/mvdan/gofumpt/releases/download/v0.12.0/gofumpt_v0.12.0_windows_amd64.exe" url_api = "https://api.github.com/repos/mvdan/gofumpt/releases/assets/549473475" @@ -1195,43 +1174,36 @@ specifiers = ["latest"] [tools.pkl."platforms.linux-arm64"] checksum = "sha256:e60744e17d4ddc0257a35260998b5f98387b5d6a5e868317cccfbebe8d962ae3" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-linux-aarch64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850794" [tools.pkl."platforms.linux-arm64-musl"] checksum = "sha256:e60744e17d4ddc0257a35260998b5f98387b5d6a5e868317cccfbebe8d962ae3" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-linux-aarch64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850794" [tools.pkl."platforms.linux-x64"] checksum = "sha256:874c75484c79add77fb2346ea4ab2e8875eea35fb707894bf5457a2df5d03def" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-linux-amd64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850808" [tools.pkl."platforms.linux-x64-musl"] checksum = "sha256:874c75484c79add77fb2346ea4ab2e8875eea35fb707894bf5457a2df5d03def" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-linux-amd64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850808" [tools.pkl."platforms.macos-arm64"] checksum = "sha256:466952f0b30c93dfa36393a5b3695159917ce4291e3c936a18a0a240cfbe98e9" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-macos-aarch64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850972" [tools.pkl."platforms.macos-x64"] checksum = "sha256:3c4c914a75210d3f3292740b4a8d447f9f832e81b60720bec57221e884717eee" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-macos-amd64" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663850987" [tools.pkl."platforms.windows-x64"] checksum = "sha256:ac3c55c2e43c20964dc5faf51e47bf73a90150dfe1874aa37753651753edf201" -checksum_source = "mise-versions" url = "https://github.com/apple/pkl/releases/download/0.32.2/pkl-windows-amd64.exe" url_api = "https://api.github.com/repos/apple/pkl/releases/assets/663854370" @@ -1288,43 +1260,36 @@ specifiers = ["latest"] [tools.staticcheck."platforms.linux-arm64"] checksum = "sha256:ccbdaaf81c37eb246a6063f811ee3b89e02ff042cb068cd1ca73a7377e2f7045" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_linux_arm64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633961454" [tools.staticcheck."platforms.linux-arm64-musl"] checksum = "sha256:ccbdaaf81c37eb246a6063f811ee3b89e02ff042cb068cd1ca73a7377e2f7045" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_linux_arm64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633961454" [tools.staticcheck."platforms.linux-x64"] checksum = "sha256:14c081e0f0d05b5a4d4c53fd3d357d4a5af9d20501465fefc9024ee828d3aa5c" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_linux_amd64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633961213" [tools.staticcheck."platforms.linux-x64-musl"] checksum = "sha256:14c081e0f0d05b5a4d4c53fd3d357d4a5af9d20501465fefc9024ee828d3aa5c" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_linux_amd64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633961213" [tools.staticcheck."platforms.macos-arm64"] checksum = "sha256:c054bf7f561f5668175d31ec8770f6104b89ccb41f0f0b4ce92d423c34365417" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_darwin_arm64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633960392" [tools.staticcheck."platforms.macos-x64"] checksum = "sha256:400ce48d6069432c632cc32e2e54d63000f6037413024310fdc20c930f1df836" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_darwin_amd64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633960199" [tools.staticcheck."platforms.windows-x64"] checksum = "sha256:5f4f9491b41301e6a9d4de6f7542bd2efc1bfaf39e7cc52802d4db9e4a54e677" -checksum_source = "mise-versions" url = "https://github.com/dominikh/go-tools/releases/download/2026.2.2/staticcheck_windows_amd64.tar.gz" url_api = "https://api.github.com/repos/dominikh/go-tools/releases/assets/633962362" @@ -1429,42 +1394,35 @@ specifiers = ["latest"] [tools.yamlfmt."platforms.linux-arm64"] checksum = "sha256:5b2689c963b177271330c5ce8ca7396751107e5a826be46f03d2cb9b6f0c7784" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Linux_arm64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650208" [tools.yamlfmt."platforms.linux-arm64-musl"] checksum = "sha256:5b2689c963b177271330c5ce8ca7396751107e5a826be46f03d2cb9b6f0c7784" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Linux_arm64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650208" [tools.yamlfmt."platforms.linux-x64"] checksum = "sha256:1f300d9257b232bb3b541d7fb1b0e6b3c121bcbab381c86cd38cb8722be8a566" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Linux_x86_64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650203" [tools.yamlfmt."platforms.linux-x64-musl"] checksum = "sha256:1f300d9257b232bb3b541d7fb1b0e6b3c121bcbab381c86cd38cb8722be8a566" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Linux_x86_64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650203" [tools.yamlfmt."platforms.macos-arm64"] checksum = "sha256:4b417ecb94339d57e4c122ecc948c1a00fe328b5853266de9806e652a92858fa" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Darwin_arm64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650209" [tools.yamlfmt."platforms.macos-x64"] checksum = "sha256:060e943bcb8583c456810eb1ff4721b4f46c4a0c1a4432449d5dc3bbfe29a22b" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Darwin_x86_64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650207" [tools.yamlfmt."platforms.windows-x64"] checksum = "sha256:07f80ce5d741eb4b0a9380ac78a19c7cb5bd44e2a9a47a5a04839e3ba54dd463" -checksum_source = "mise-versions" url = "https://github.com/google/yamlfmt/releases/download/v0.21.0/yamlfmt_0.21.0_Windows_x86_64.tar.gz" url_api = "https://api.github.com/repos/google/yamlfmt/releases/assets/336650206" diff --git a/skills/constable/SKILL.md b/skills/constable/SKILL.md index 56e9ed8..8b1a44e 100644 --- a/skills/constable/SKILL.md +++ b/skills/constable/SKILL.md @@ -60,6 +60,49 @@ Repair it in this order: Do not hide a diagnostic by assigning a new value to the parameter. Prefer the copy or the new value. +### pure + +```text +//constable:pure function mutates pointer parameter p +//constable:pure function writes global counter +//constable:pure function spawns goroutine +//constable:pure function calls time.Now +//constable:pure function calls fmt.Println +//constable:pure function reads os.Args +``` + +`//constable:pure` is the non-mutating contract plus determinism. +It forbids every write the `nonmutating` check forbids, and also forbids package-global writes, `go` spawns, and calls that read the clock, random, environment, or perform I/O. +Repair it in this order: + +1. Move the effect to the caller. + Pass the clock, random source, or environment value in as a parameter, or return the value instead of printing it. + Write through the parameter value the caller passed, not through a global. +1. Delete the directive when the function must have an observable effect, because the contract is wrong for that function. + +The pure contract section below lists the effects that count. + +### lean + +```text +unused unexported function helper is never used +unused unexported type buffer is never used +unused unexported variable counter is never used +unused unexported constant limit is never used +``` + +Run the fix, + +```sh +constable -fix -diff ./... # read the change +constable -fix ./... # write the change +``` + +The fix deletes the reported declaration, including its doc comment, and removes imports that only the deleted declaration used. + +Delete the declaration by hand when the fix skips it. +The fix skips a generated file, a file with build constraints or Go directives, a file that uses cgo, a file whose import block holds comments, a file with comments that no declaration owns, a declaration that does not start its own line, and a single constant from a grouped block. + ### methodical ```text @@ -201,6 +244,49 @@ func (c Count) Increment() Count { } ``` +## The pure contract + +`//constable:pure` states that the function or method has no observable effect beyond its return values. +It includes the non-mutating contract and adds four more rules. + +Put the exact directive immediately before the declaration, with no space between `//` and `constable:`. + +```go +//constable:pure +func Add(a, b int) int { + return a + b +} +``` + +The directive applies to functions and methods only. + +### Reported effects + +- Every write the non-mutating check reports, such as `*p = 1` through a pointer parameter. +- A write to a package-level variable, such as `counter++`, `globalMap["x"] = 1`, or `delete(globalMap, "x")`. +- A `go` statement that spawns a goroutine. +- A call that reads the clock: `time.Now`, `time.Since`, `time.Until`, `time.Sleep`, `time.After`, `time.AfterFunc`, `time.Tick`, `time.NewTicker`, `time.NewTimer`. +- Any call from `math/rand` or `math/rand/v2`. +- A call that reads the environment or performs I/O through `os`, except the error predicates `os.IsNotExist`, `os.IsExist`, `os.IsPermission`, `os.IsTimeout`, and the pure accessor `File.Name`. +- A call that prints through `fmt` (`Print`, `Printf`, `Println`, `Fprint`, `Fprintf`, `Fprintln`) or through `log`, or the builtins `print` and `println`. +- A read of `os.Args`, `os.Stdout`, `os.Stderr`, `os.Stdin`, or `time.Local`. + +```go +var counter int + +//constable:pure +func Bump() { + counter++ // writes global counter +} +``` + +### Allowed code + +- Reading a global, such as `return counter`, because a read by itself does not change state. +- Pure construction and pure queries, such as `time.Date`, `fmt.Sprintf`, `strings.ToUpper`, and `os.IsNotExist`. +- A write to a local variable or to memory the function allocates. +- A `panic` and its `recover`, because the panic is part of the function's own control flow. + ## Workflow 1. Identify the functions and methods whose contract must not change caller-owned data. @@ -217,3 +303,18 @@ It is not a general immutability checker, and it does not by itself guarantee th The analyzer recognizes only the `nonmutating` directive. When a run is clean, inspect the implementation before you claim that it is fully non-mutating. + +The pure check finds the effects in the pure contract in one function body. +It does not model a call to a helper function, so a pure function that calls an impure helper passes until the helper is also checked or the transitive effect is made explicit. +It does not check `net`, `io.Writer` implementations, reflection, `unsafe`, `cgo`, or assembly, and it does not prove that a function never blocks. +The analyzer recognizes only the `pure` directive. + +When a run is clean, inspect the implementation before you claim that it is fully pure. + +The lean check finds package-local unused declarations only: unexported functions, types, variables, and constants. +It does not report methods, struct fields, unused parameters, exported symbols, or constants in a group that uses `iota` or hides a value. +Unused parameters may exist to satisfy an interface, and exported symbols may be used in another package, so package-local evidence cannot prove either one dead. +A symbol referenced in any file of the package, including via `var _ = x`, counts as used. +Entry points `init` and `main` are never reported. + +When a run is clean, the package still holds exported surface that only a whole-program search can judge.