diff --git a/README.md b/README.md index 35e56a3..0379a04 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,10 @@ It consists of several analyzers: 1. `unused`: Detects interfaces which are not used anywhere in the same package where they are defined. -2. `identical`: Detects interfaces within the same package that have identical methods or type constraints. -3. `opaque`: Detects functions that return an interface type, but only ever return a single concrete implementation. -4. `unexported`: Detects interfaces which are not exported but are used as parameters or return values in exported functions or methods. +2. `unusedmethod`: Detects interface methods that are never used anywhere in the same package where they are defined. +3. `identical`: Detects interfaces within the same package that have identical methods or type constraints. +4. `opaque`: Detects functions that return an interface type, but only ever return a single concrete implementation. +5. `unexported`: Detects interfaces which are not exported but are used as parameters or return values in exported functions or methods. ## Usage @@ -20,6 +21,7 @@ go install github.com/uudashr/iface/cmd/ifacecheck@latest To install individual linter, use the following command: ```sh go install github.com/uudashr/iface/unused/cmd/unusediface@latest +go install github.com/uudashr/iface/unusedmethod/cmd/unusedmethodiface@latest go install github.com/uudashr/iface/identical/cmd/identicaliface@latest go install github.com/uudashr/iface/opaque/cmd/opaqueiface@latest go install github.com/uudashr/iface/unexported/cmd/unexportediface@latest @@ -49,11 +51,12 @@ By default it only enables the `identical` analyzer, you need to enable them all ### Package exclusion -We encourage using the default behavior and making an effort to follow the rules. However, in some cases the rules may not apply. Due to this we can exclude specific package to be scanned by the analyzers. Use `-unused.exclude` flag and currently only `unused` has this feature. See help for more information: +We encourage using the default behavior and making an effort to follow the rules. However, in some cases the rules may not apply. Due to this we can exclude specific package to be scanned by the analyzers. Use `-unused.exclude` or `-unusedmethod.exclude` flag. See help for more information: Example usage: ```sh ifacecheck -unused.exclude=github.com/example/log ./... +ifacecheck -unusedmethod.exclude=github.com/example/log ./... ``` ### Ignore Directive diff --git a/cmd/ifacecheck/main.go b/cmd/ifacecheck/main.go index 3988fc6..156cdff 100644 --- a/cmd/ifacecheck/main.go +++ b/cmd/ifacecheck/main.go @@ -5,12 +5,14 @@ import ( "github.com/uudashr/iface/opaque" "github.com/uudashr/iface/unexported" "github.com/uudashr/iface/unused" + "github.com/uudashr/iface/unusedmethod" "golang.org/x/tools/go/analysis/multichecker" ) func main() { multichecker.Main( unused.Analyzer, + unusedmethod.Analyzer, identical.Analyzer, opaque.Analyzer, unexported.Analyzer, diff --git a/unusedmethod/cmd/unusedmethodiface/main.go b/unusedmethod/cmd/unusedmethodiface/main.go new file mode 100644 index 0000000..b9b6fb1 --- /dev/null +++ b/unusedmethod/cmd/unusedmethodiface/main.go @@ -0,0 +1,10 @@ +package main + +import ( + "github.com/uudashr/iface/unusedmethod" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(unusedmethod.Analyzer) +} diff --git a/unusedmethod/doc.go b/unusedmethod/doc.go new file mode 100644 index 0000000..cef63a7 --- /dev/null +++ b/unusedmethod/doc.go @@ -0,0 +1,3 @@ +// Package unusedmethod defines an Analyzer that detects interface methods which +// are never used anywhere in the same package where they are defined. +package unusedmethod diff --git a/unusedmethod/testdata/src/a/a.go b/unusedmethod/testdata/src/a/a.go new file mode 100644 index 0000000..dfb2cd7 --- /dev/null +++ b/unusedmethod/testdata/src/a/a.go @@ -0,0 +1,23 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error + MustSendMessage(username, msg string) // want "method 'MustSendMessage\\(\\)' is declared on interface 'MessageSender' but not used within the package" + + // Close the resource. + Close() error // want "method 'Close\\(\\)' is declared on interface 'MessageSender' but not used within the package" +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/a/a.go.golden b/unusedmethod/testdata/src/a/a.go.golden new file mode 100644 index 0000000..4bc4ac4 --- /dev/null +++ b/unusedmethod/testdata/src/a/a.go.golden @@ -0,0 +1,19 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/b/b.go b/unusedmethod/testdata/src/b/b.go new file mode 100644 index 0000000..a4fc189 --- /dev/null +++ b/unusedmethod/testdata/src/b/b.go @@ -0,0 +1,24 @@ +package comm + +//iface:ignore=unusedmethod +type MessageSender interface { + SendMessage(username, msg string) error + MustSendMessage(username, msg string) + + // Close the resource. + Close() error +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/c/c.go b/unusedmethod/testdata/src/c/c.go new file mode 100644 index 0000000..baef5fc --- /dev/null +++ b/unusedmethod/testdata/src/c/c.go @@ -0,0 +1,25 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error + + //iface:ignore=unusedmethod + MustSendMessage(username, msg string) + + // Close the resource. + Close() error // want "method 'Close\\(\\)' is declared on interface 'MessageSender' but not used within the package" +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/d/d.go b/unusedmethod/testdata/src/d/d.go new file mode 100644 index 0000000..ca9f7ac --- /dev/null +++ b/unusedmethod/testdata/src/d/d.go @@ -0,0 +1,23 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error + MustSendMessage(username, msg string) //iface:ignore=unusedmethod + + // Close the resource. + Close() error // want "method 'Close\\(\\)' is declared on interface 'MessageSender' but not used within the package" +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/e/e.go b/unusedmethod/testdata/src/e/e.go new file mode 100644 index 0000000..06845ff --- /dev/null +++ b/unusedmethod/testdata/src/e/e.go @@ -0,0 +1,13 @@ +package comm + +type Callback interface { + Call(msg string) +} + +type Service struct { + callbacks []func(msg string) +} + +func (s *Service) RegisterCallaback(cb Callback) { + s.callbacks = append(s.callbacks, cb.Call) +} diff --git a/unusedmethod/testdata/src/f/f.go b/unusedmethod/testdata/src/f/f.go new file mode 100644 index 0000000..7fae34d --- /dev/null +++ b/unusedmethod/testdata/src/f/f.go @@ -0,0 +1,20 @@ +package comm + +type Callback interface { + Call(msg string) +} + +type entry struct { + cb Callback + call func(cb Callback, msg string) +} +type Service struct { + callbacks []entry +} + +func (s *Service) RegisterCallaback(cb Callback) { + s.callbacks = append(s.callbacks, entry{ + cb: cb, + call: Callback.Call, + }) +} diff --git a/unusedmethod/testdata/src/g/a.go b/unusedmethod/testdata/src/g/a.go new file mode 100644 index 0000000..2e6b7f9 --- /dev/null +++ b/unusedmethod/testdata/src/g/a.go @@ -0,0 +1,23 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error + MustSendMessage(username, msg string) + + // Close the resource. + Close() error +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/testdata/src/g/a.go.golden b/unusedmethod/testdata/src/g/a.go.golden new file mode 100644 index 0000000..4bc4ac4 --- /dev/null +++ b/unusedmethod/testdata/src/g/a.go.golden @@ -0,0 +1,19 @@ +package comm + +type MessageSender interface { + SendMessage(username, msg string) error +} + +type Service struct { + sender MessageSender +} + +func NewService(sender MessageSender) *Service { + return &Service{ + sender: sender, + } +} + +func (s *Service) GreetMorning(username string) error { + return s.sender.SendMessage(username, "Good morning") +} diff --git a/unusedmethod/unusedmethod.go b/unusedmethod/unusedmethod.go new file mode 100644 index 0000000..2f67146 --- /dev/null +++ b/unusedmethod/unusedmethod.go @@ -0,0 +1,226 @@ +package unusedmethod + +import ( + "fmt" + "go/ast" + "go/token" + "go/types" + "os" + "slices" + "strings" + + "github.com/uudashr/iface/internal/directive" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +var Analyzer = newAnalyzer() + +func newAnalyzer() *analysis.Analyzer { + r := runner{} + + analyzer := &analysis.Analyzer{ + Name: "unusedmethod", + Doc: "Detects interface methods that are never used anywhere in the same package where they are defined.", + URL: "https://pkg.go.dev/github.com/uudashr/iface/unusedmethod", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: r.run, + } + + analyzer.Flags.BoolVar(&r.debug, "nerd", false, "enable nerd mode") + analyzer.Flags.StringVar(&r.exclude, "exclude", "", "comma-separated list of packages to exclude from the check") + + return analyzer +} + +type methodEntry struct { + ifaceName string + field *ast.Field +} + +type runner struct { + debug bool + exclude string +} + +func (r *runner) run(pass *analysis.Pass) (any, error) { + var excludes []string + + if r.exclude != "" { + for _, pkg := range strings.Split(r.exclude, ",") { + if p := strings.TrimSpace(pkg); p != "" { + excludes = append(excludes, p) + } + } + } + + if slices.Contains(excludes, pass.Pkg.Path()) { + return nil, nil + } + + inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + unusedMethods := make(map[*types.Func]methodEntry) + + nodeFilter := []ast.Node{ + (*ast.GenDecl)(nil), + } + + inspect.Preorder(nodeFilter, func(n ast.Node) { + decl, ok := n.(*ast.GenDecl) + if !ok { + return + } + + if r.debug { + fmt.Fprintf(os.Stderr, "GenDecl: %v specs=%d\n", decl.Tok, len(decl.Specs)) + } + + if decl.Tok != token.TYPE { + return + } + + if directive.ShouldIgnore(decl.Doc, pass.Analyzer.Name) { + return + } + + for i, spec := range decl.Specs { + r.debugf(" spec[%d]: %v %T\n", i, spec, spec) + + ts, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + + ifaceType, ok := ts.Type.(*ast.InterfaceType) + if !ok { + continue + } + + r.debugln(" -> Interface type declaration:", ts.Name.Name) + + if directive.ShouldIgnore(ts.Doc, pass.Analyzer.Name) { + continue + } + + for j, field := range ifaceType.Methods.List { + switch ft := field.Type.(type) { + case *ast.FuncType: + name := field.Names[0] + obj := pass.TypesInfo.Defs[name] + + if r.debug { + fmt.Fprintf(os.Stderr, " [%d] Field: func %s %T\n", j, name.Name, ft) + fmt.Fprintf(os.Stderr, " obj: %v %T %p\n", obj, obj, obj) + } + + if directive.ShouldIgnore(field.Doc, pass.Analyzer.Name) { + continue + } + + if directive.ShouldIgnore(field.Comment, pass.Analyzer.Name) { + continue + } + + if fn, ok := obj.(*types.Func); ok { + unusedMethods[fn] = methodEntry{ + ifaceName: ts.Name.Name, + field: field, + } + } + default: + r.debugf(" [%d] Field: unknown %v %T\n", j, ft, ft) + } + } + } + }) + + // Method is always SelectorExpr + nodeFilter = []ast.Node{ + (*ast.SelectorExpr)(nil), + } + + r.debugln("Method usage") + inspect.Preorder(nodeFilter, func(n ast.Node) { + r.debugf(" n %v %T\n", n, n) + + selExp, ok := n.(*ast.SelectorExpr) + if !ok { + return + } + + r.debugf(" selExp sel: %v %T, x: %v %T\n", selExp.Sel, selExp.Sel, selExp.X, selExp.X) + + sel, ok := pass.TypesInfo.Selections[selExp] + if !ok { + return + } + + if r.debug { + fmt.Fprintf(os.Stderr, " sel -> %v %T, kind: %v %T, obj: %v %T\n", sel, sel, sel.Kind(), sel.Kind(), sel.Obj(), sel.Obj()) + } + + fn, ok := sel.Obj().(*types.Func) + if !ok { + return + } + + delete(unusedMethods, fn) + }) + + if r.debug { + fmt.Fprintf(os.Stderr, "Unused methods %d\n", len(unusedMethods)) + } + + for fn, entry := range unusedMethods { + if r.debug { + fmt.Fprintf(os.Stderr, " %v %T %s\n", fn, fn, fn.Name()) + } + + msg := fmt.Sprintf("method '%s()' is declared on interface '%s' but not used within the package", fn.Name(), entry.ifaceName) + + field := entry.field + + pos := field.Pos() + if doc := field.Doc; doc != nil { + pos = doc.Pos() + } + + end := field.End() + if comment := field.Comment; comment != nil { + end = comment.End() + } + + pass.Report(analysis.Diagnostic{ + Pos: field.Pos(), + Message: msg, + SuggestedFixes: []analysis.SuggestedFix{ + { + Message: "Remove the unused method", + TextEdits: []analysis.TextEdit{ + { + Pos: pos, + End: end, + NewText: []byte{}, + }, + }, + }, + }, + }) + } + + return nil, nil +} + +func (r *runner) debugln(a ...any) { + if r.debug { + fmt.Fprintln(os.Stderr, a...) + } +} + +func (r *runner) debugf(format string, a ...any) { + if r.debug { + fmt.Fprintf(os.Stderr, format, a...) + } +} diff --git a/unusedmethod/unusedmethod_test.go b/unusedmethod/unusedmethod_test.go new file mode 100644 index 0000000..51bf5ff --- /dev/null +++ b/unusedmethod/unusedmethod_test.go @@ -0,0 +1,29 @@ +package unusedmethod_test + +import ( + "testing" + + "github.com/uudashr/iface/unusedmethod" + "golang.org/x/tools/go/analysis/analysistest" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, unusedmethod.Analyzer, "a") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "b") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "c") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "d") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "e") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "f") +} + +func TestExclusion(t *testing.T) { + err := unusedmethod.Analyzer.Flags.Set("exclude", "g") + if err != nil { + t.Fatal(err) + } + + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, unusedmethod.Analyzer, "a") + analysistest.Run(t, testdata, unusedmethod.Analyzer, "g") +}