Refactoring mirror so we can use analysis.SuggestedFixes (#20)

* refactor: initial refactoring

- Simplified checker (instead of separate checekrs)
- Violations redone (& test generation)
- Fixed few mistakes in violations

* chores: update task & make

* feature: analysis.SuggestedFix

Makefile

@@ -9,6 +9,10 @@ build:
 	@ go build -trimpath -ldflags="-w -s" \
 		-o bin/mirror ./cmd/mirror/
 
+build-race:
+	@ go build -race -trimpath -ldflags="-w -s" \
+		-o bin/mirror ./cmd/mirror/
+
 tests:
 	go test -v -count=1 -race \
 		-failfast \
@@ -25,7 +29,7 @@ tests-summary:
 		-covermode=atomic \
 		-coverpkg=$(GOPKGS) -coverprofile=coverage.cov --json ./... | tparse -all
 
-generate:
+test-generate:
 	go run ./cmd/internal/generate-tests/ "$(PWD)/testdata"
 
 lints:

Taskfile.yml

@@ -4,46 +4,18 @@ tasks:
   default:
     sources:
     - "./**/*.go"
-    method: checksum
+    method: timestamp
     cmds:
     - clear
-    - make tests
-    - make lints
     - make build
-    - ls -lah bin/
-
-  testcase: go test -v -failfast -count=1 -run "TestAll/{{ .Case }}" ./...
-
-  regen:
-    sources:
-    - ./cmd/internal/generate-tests/*.go
-    - ./checkers_*.go
-    - ./cmd/internal/generate-tests/templates/*.tmpl
-    method: timestamp
-    cmds:
-    - go build -o bin/genrate-tests ./cmd/internal/generate-tests/
-    - ./bin/genrate-tests ./testdata
-    - make tests
-
-  tdd:
-    dir: ./testdata
-    sources:
-    - ./**/*.go
-    method: timestamp
-    cmds:
-    - cmd: clear
-    - cmd: date
-    - task: build
-    - cmd: go run ../cmd/mirror --with-tests --with-debug ./...
-      ignore_error: true
+    - make build-race
     - task: lints
-      ignore_error: true
+    - make test-generate
     - task: tests
+    - cmd: go run ./cmd/mirror/ --with-tests --with-debug ./demo
       ignore_error: true
-    - echo "done"
 
-  install: make install
-  build: make build
+  testcase: go test -v -failfast -count=1 -run "TestAll/{{ .Case }}" ./...
 
   tests:
     cmds:

analyzer.go

@@ -14,8 +14,6 @@ import (
 )
 
 type analyzer struct {
-	checkers map[string]*checker.Checker // Available checkers.
-
 	withTests bool
 	withDebug bool
 
@@ -25,18 +23,7 @@ type analyzer struct {
 func NewAnalyzer() *analysis.Analyzer {
 	flags := flags()
 
-	a := analyzer{
-		checkers: make(map[string]*checker.Checker),
-	}
-
-	a.register(newRegexpChecker())
-	a.register(newStringsChecker())
-	a.register(newBytesChecker())
-	a.register(newMaphashChecker())
-	a.register(newBufioChecker())
-	a.register(newOsChecker())
-	a.register(newUTF8Checker())
-	a.register(newHTTPTestChecker())
+	a := analyzer{}
 
 	return &analysis.Analyzer{
 		Name: "mirror",
@@ -50,23 +37,26 @@ func NewAnalyzer() *analysis.Analyzer {
 }
 
 func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
-	issues := []*analysis.Diagnostic{}
-	// --- Read the flags --------------------------------------------------------
-	a.once.Do(a.setup(pass.Analyzer.Flags))
+	// --- Setup -----------------------------------------------------------------
+
+	check := checker.New(
+		BytesFunctions, BytesBufferMethods,
+		RegexpFunctions, RegexpRegexpMethods,
+		StringFunctions, StringsBuilderMethods,
+		BufioMethods, HTTPTestMethods,
+		OsFileMethods, MaphashMethods,
+		UTF8Functions,
+	)
 
-	ins, _ := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	check.Type = checker.WrapType(pass.TypesInfo)
+	check.Print = checker.WrapPrint(pass.Fset)
 
-	// fmt.Println(pass.Pkg.Path())
-	var (
-		imports      = checker.Load(pass.Fset, ins)
-		fileCheckers = make(map[string][]*checker.Checker)
-		debugFn      = debugNoOp
-		fileImports  []checker.Import
-	)
+	violations := []*checker.Violation{}
 
-	if a.withDebug {
-		debugFn = debug(pass.Fset)
-	}
+	a.once.Do(a.setup(pass.Analyzer.Flags)) // loading flags info
+
+	ins, _ := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	imports := checker.Load(pass.Fset, ins)
 
 	// --- Preorder Checker ------------------------------------------------------
 	ins.Preorder([]ast.Node{(*ast.CallExpr)(nil)}, func(n ast.Node) {
@@ -77,51 +67,69 @@ func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
 			return
 		}
 
-		fileImports = imports.Lookup(fileName)
-		if _, ok := fileCheckers[fileName]; !ok {
-			fileCheckers[fileName] = a.filter(fileImports)
-		}
+		// -------------------------------------------------------------------------
+		switch expr := callExpr.Fun.(type) {
+		// NOTE(butuzov): Regular calls (`*ast.SelectorExpr`) like strings.HasPrefix
+		//                or re.Match are handled by this check
+		case *ast.SelectorExpr:
 
-		for i := range fileCheckers[fileName] {
-			c := fileCheckers[fileName][i].With(pass, fileImports, debugFn)
-			if violation := c.Check(callExpr); violation != nil {
-				issues = append(issues, violation.Diagnostic(n.Pos(), n.End()))
+			x, ok := expr.X.(*ast.Ident)
+			if !ok {
 				return
 			}
-		}
-	})
 
-	// --- Reporting issues ------------------------------------------------------
-	for _, issue := range issues {
-		pass.Report(*issue)
-	}
-
-	return nil, nil
-}
+			// TODO(butuzov): Add check for the ast.ParenExpr in e.Fun so we can
+			//                target the constructions like this (and other calls)
+			// -----------------------------------------------------------------------
+			// Example:
+			//       (&maphash.Hash{}).Write([]byte("foobar"))
+			// -----------------------------------------------------------------------
+
+			// Case 1: Is this is a function call?
+			pkgName, name := x.Name, expr.Sel.Name
+			if pkg, ok := imports.Lookup(fileName, pkgName); ok {
+				if v := check.Match(pkg, name); v != nil {
+					if args, found := check.Handle(v, callExpr); found {
+						violations = append(violations, v.With(check.Print(expr.X), callExpr, args))
+					}
+					return
+				}
+			}
 
-func (a *analyzer) register(c *checker.Checker) {
-	a.checkers[c.Package] = c
-}
+			// Case 2: Is this is a method call?
+			tv := pass.TypesInfo.Types[expr.X]
+			if !tv.IsValue() || tv.Type == nil {
+				return
+			}
 
-func (a *analyzer) filter(imports []checker.Import) []*checker.Checker {
-	out := make([]*checker.Checker, 0, len(a.checkers))
-	seen := make(map[string]bool, len(imports))
+			pkgStruct, name := cleanAsterisk(tv.Type.String()), expr.Sel.Name
+			if v := check.Match(pkgStruct, name); v != nil {
+				if args, found := check.Handle(v, callExpr); found {
+					violations = append(violations, v.With(check.Print(expr.X), callExpr, args))
+				}
+				return
+			}
 
-	var key string
-	for i := range imports {
-		key = imports[i].Pkg
+		case *ast.Ident:
+			// NOTE(butuzov): Special case of "." imported packages, only functions.
 
-		if _, ok := seen[key]; ok {
-			continue
+			if pkg, ok := imports.Lookup(fileName, "."); ok {
+				if v := check.Match(pkg, expr.Name); v != nil {
+					if args, found := check.Handle(v, callExpr); found {
+						violations = append(violations, v.With(nil, callExpr, args))
+					}
+					return
+				}
+			}
 		}
-		seen[key] = true
+	})
 
-		if _, ok := a.checkers[key]; ok {
-			out = append(out, a.checkers[key])
-		}
+	// --- Reporting violations via issues ---------------------------------------
+	for _, violation := range violations {
+		pass.Report(violation.Issue(pass.Fset))
 	}
 
-	return out
+	return nil, nil
 }
 
 func (a *analyzer) setup(f flag.FlagSet) func() {
@@ -137,3 +145,11 @@ func flags() flag.FlagSet {
 	set.Bool("with-debug", false, "debug linter run (development only)")
 	return *set
 }
+
+func cleanAsterisk(s string) string {
+	if strings.HasPrefix(s, "*") {
+		return s[1:]
+	}
+
+	return s
+}

checkers_bufio.go

@@ -2,36 +2,31 @@ package mirror
 
 import "github.com/butuzov/mirror/internal/checker"
 
-func newBufioChecker() *checker.Checker {
-	c := checker.New("bufio")
-	c.Methods["bufio.Writer"] = BufioMethods
+var BufioMethods = []checker.Violation{
+	{ // (*bufio.Writer).Write
+		Targets:   checker.Bytes,
+		Type:      checker.Method,
+		Package:   "bufio",
+		Struct:    "Writer",
+		Caller:    "Write",
+		Args:      []int{0},
+		AltCaller: "WriteString",
 
-	return c
-}
-
-var BufioMethods = map[string]checker.Violation{
-	"Write": {
-		Type:           checker.Method,
-		Message:        "avoid allocations with (*bufio.Writer).WriteString",
-		Args:           []int{0},
-		StringTargeted: false,
-		Alternative: checker.Alternative{
-			Method: "WriteString",
-		},
 		Generate: &checker.Generate{
 			PreCondition: `b := bufio.Writer{}`,
 			Pattern:      `Write($0)`,
 			Returns:      2,
 		},
 	},
-	"WriteString": {
-		Type:           checker.Method,
-		Message:        "avoid allocations with (*bufio.Writer).Write",
-		Args:           []int{0},
-		StringTargeted: true,
-		Alternative: checker.Alternative{
-			Method: "Write",
-		},
+	{ // (*bufio.Writer).WriteString
+		Type:      checker.Method,
+		Targets:   checker.Strings,
+		Package:   "bufio",
+		Struct:    "Writer",
+		Caller:    "WriteString",
+		Args:      []int{0},
+		AltCaller: "Write",
+
 		Generate: &checker.Generate{
 			PreCondition: `b := bufio.Writer{}`,
 			Pattern:      `WriteString($0)`,