Use typesinfo to compare packages (#6)

This avoids a false positive in shadow.go, and enables detecting
dot-imported.

Fixes #5

Co-authored-by: Choko <[email protected]>

Author: MichaelUrman

internal/analyzer/analyzer.go

@@ -3,20 +3,22 @@ package analyzer
 import (
 	"fmt"
 	"go/ast"
+	"go/types"
 	"regexp"
-	"strconv"
-	"strings"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
 )
 
 const FlagPattern = "pattern"
 
 func New() *analysis.Analyzer {
 	a := &analysis.Analyzer{
-		Name: "reassign",
-		Doc:  "Checks that package variables are not reassigned",
-		Run:  run,
+		Name:     "reassign",
+		Doc:      "Checks that package variables are not reassigned",
+		Requires: []*analysis.Analyzer{inspect.Analyzer},
+		Run:      run,
 	}
 	a.Flags.String(FlagPattern, `^(Err.*|EOF)$`, "Pattern to match package variables against to prevent reassignment")
 	return a
@@ -27,60 +29,56 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	if err != nil {
 		return nil, fmt.Errorf("invalid pattern: %w", err)
 	}
-	for _, f := range pass.Files {
-		state := &fileState{imports: make(map[string]struct{})}
-		ast.Inspect(f, func(node ast.Node) bool {
-			return inspect(pass, node, checkRE, state)
-		})
-	}
+
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	inspect.Preorder([]ast.Node{(*ast.AssignStmt)(nil), (*ast.UnaryExpr)(nil)}, func(node ast.Node) {
+		switch node := node.(type) {
+		case *ast.AssignStmt:
+			for _, lhs := range node.Lhs {
+				reportImported(pass, lhs, checkRE, "reassigning")
+			}
+		default:
+			// TODO(chokoswitch): Consider handling operations other than assignment on globals, for example
+			// taking their address.
+		}
+	})
 	return nil, nil
 }
 
-type fileState struct {
-	imports map[string]struct{}
-}
+func reportImported(pass *analysis.Pass, expr ast.Expr, checkRE *regexp.Regexp, prefix string) {
+	switch x := expr.(type) {
+	case *ast.SelectorExpr:
+		if !checkRE.MatchString(x.Sel.Name) {
+			return
+		}
 
-func inspect(pass *analysis.Pass, node ast.Node, checkRE *regexp.Regexp, state *fileState) bool {
-	if importSpec, ok := node.(*ast.ImportSpec); ok {
-		if importSpec.Name != nil {
-			state.imports[importSpec.Name.Name] = struct{}{}
-		} else {
-			n, err := strconv.Unquote(importSpec.Path.Value)
-			if err != nil {
-				return true
-			}
-			if idx := strings.LastIndexByte(n, '/'); idx != -1 {
-				n = n[idx+1:]
+		selectIdent, ok := x.X.(*ast.Ident)
+		if !ok {
+			return
+		}
+
+		if selectObj, ok := pass.TypesInfo.Uses[selectIdent]; ok {
+			if pkg, ok := selectObj.(*types.PkgName); !ok || pkg.Imported() == pass.Pkg {
+				return
 			}
-			state.imports[n] = struct{}{}
 		}
-		return true
-	}
 
-	assignStmt, ok := node.(*ast.AssignStmt)
-	if !ok {
-		return true
-	}
+		pass.Reportf(expr.Pos(), "%s variable %s in other package %s", prefix, x.Sel.Name, selectIdent.Name)
 
-	for _, lhs := range assignStmt.Lhs {
-		selector, ok := lhs.(*ast.SelectorExpr)
+	case *ast.Ident:
+		use, ok := pass.TypesInfo.Uses[x].(*types.Var)
 		if !ok {
-			return true
+			return
 		}
 
-		if !checkRE.MatchString(selector.Sel.Name) {
-			return true
+		if use.Pkg() == pass.Pkg {
+			return
 		}
 
-		selectIdent, ok := selector.X.(*ast.Ident)
-		if !ok {
-			return true
+		if !checkRE.MatchString(x.Name) {
+			return
 		}
 
-		if _, ok := state.imports[selectIdent.Name]; ok {
-			pass.Reportf(node.Pos(), "reassigning variable %s in other package %s", selector.Sel.Name, selectIdent.Name)
-		}
+		pass.Reportf(expr.Pos(), "%s variable %s from other package %s", prefix, x.Name, use.Pkg().Path())
 	}
-
-	return true
 }

internal/analyzer/analyzer_test.go

@@ -40,7 +40,7 @@ func TestAnalyzer(t *testing.T) {
 			if tt.pattern != "" {
 				_ = a.Flags.Set("pattern", tt.pattern)
 			}
-			analysistest.Run(t, td, a, "a")
+			analysistest.Run(t, td, a, "./...")
 		})
 	}
 }

testdata/defaults/src/dotimport/dotimport.go

@@ -0,0 +1,7 @@
+package dotimport
+
+import . "io"
+
+func direct() {
+	EOF = nil // want "reassigning variable"
+}

testdata/defaults/src/shadow/shadow.go

@@ -0,0 +1,13 @@
+package shadow
+
+import "fmt"
+
+func use() {
+	fmt.Println("hi")
+}
+
+func shadow() any {
+	fmt := struct{ EOF int }{}
+	fmt.EOF = 5
+	return fmt
+}