rawloginlib: resolve log package identity via types to fix shadowing FP and alias FN (#39990)

Author: Copilot

docs/adr/39990-resolve-package-identity-via-types-in-linters.md

@@ -0,0 +1,38 @@
+# ADR-39990: Resolve Package Identity via Type Information in Custom Linters
+
+**Date**: 2026-06-18
+**Status**: Draft
+
+## Context
+
+The `rawloginlib` analyzer detects raw `log.Printf`/`log.Fatal`-style calls in library packages by matching the selector's receiver identifier name syntactically (`ident.Name == "log"`). This name-based heuristic is unaware of Go's actual binding semantics: it produced CI-blocking false positives when a local variable or parameter named `log` shadowed the package (e.g. a `*customLogger` receiver), and false negatives when the stdlib package was imported under an alias (`import applog "log"`). Custom linters in this repo (e.g. ADR-39133, ADR-39263) share the same family of selector-inspection logic, so the fix needs to be reusable rather than local to one analyzer.
+
+## Decision
+
+We will determine package identity for selector expressions using resolved type information (`go/types`) instead of matching identifier names. We added `astutil.IsPkgSelector(pass, sel, pkgPath)`, which resolves the selector's receiver identifier through `pass.TypesInfo.ObjectOf`, confirms it is a `*types.PkgName`, and compares the imported package's import path. `rawloginlib` now gates its diagnostic on `IsPkgSelector(pass, sel, "log")` rather than the receiver's spelling. The helper is centralized in the shared `astutil` package so other linters can reuse the same type-resolved identity check.
+
+## Alternatives Considered
+
+### Alternative 1: Keep name matching with extra heuristics
+We could have kept `ident.Name == "log"` and layered on ad-hoc handling for aliases and shadowing (e.g. scanning import specs to map alias names back to paths). This was rejected because it reconstructs, imperfectly, information the type checker already computes; it would remain fragile against any binding the heuristics did not anticipate.
+
+### Alternative 2: Inline type resolution per linter
+We could have inlined the `TypesInfo` / `*types.PkgName` resolution directly inside `rawloginlib` without a shared helper. This was rejected because the same logic already recurs across the repo's custom linters; duplicating it invites drift and repeated nil-safety bugs.
+
+## Consequences
+
+### Positive
+- Eliminates the false-positive and false-negative classes for shadowed receivers and aliased imports, removing spurious CI failures.
+- Provides a single, tested `astutil.IsPkgSelector` helper reusable by current and future linters.
+
+### Negative
+- Correctness now depends on `pass.TypesInfo` being populated; the helper degrades to `false` when type info is absent, so a misconfigured analysis pass silently under-reports rather than erroring.
+- Type-based resolution is marginally more complex and less obvious to a reader than a direct name comparison.
+
+### Neutral
+- Diagnostic message text and `//nolint` suppression behavior are unchanged.
+- Adds testdata cases (alias import, shadowed param/local) and `astutil` unit tests covering the new helper.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27739763043) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/linters/internal/astutil/astutil.go

@@ -72,6 +72,27 @@ func IsFmtErrorf(pass *analysis.Pass, call *ast.CallExpr) bool {
 	return pkgName.Imported().Path() == "fmt"
 }
 
+// IsPkgSelector reports whether sel is a selector on an imported package with
+// the given import path.
+func IsPkgSelector(pass *analysis.Pass, sel *ast.SelectorExpr, pkgPath string) bool {
+	if pass == nil || pass.TypesInfo == nil || sel == nil {
+		return false
+	}
+	pkgIdent, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return false
+	}
+	obj := pass.TypesInfo.ObjectOf(pkgIdent)
+	if obj == nil {
+		return false
+	}
+	pkgName, ok := obj.(*types.PkgName)
+	if !ok || pkgName.Imported() == nil {
+		return false
+	}
+	return pkgName.Imported().Path() == pkgPath
+}
+
 // Inspector extracts the *inspector.Inspector from pass.ResultOf.
 // It returns an error if the result has an unexpected type.
 func Inspector(pass *analysis.Pass) (*inspector.Inspector, error) {

pkg/linters/internal/astutil/astutil_test.go

@@ -3,7 +3,10 @@ package astutil
 import (
 	"go/ast"
 	"go/token"
+	"go/types"
 	"testing"
+
+	"golang.org/x/tools/go/analysis"
 )
 
 func TestRhsExprForIndex(t *testing.T) {
@@ -62,3 +65,87 @@ func TestNodeText(t *testing.T) {
 		t.Fatalf("NodeText = %q, want %q", got, "myVar")
 	}
 }
+
+func TestIsPkgSelector(t *testing.T) {
+	t.Parallel()
+
+	makePass := func(ident *ast.Ident, obj types.Object) *analysis.Pass {
+		return &analysis.Pass{
+			TypesInfo: &types.Info{
+				Uses: map[*ast.Ident]types.Object{
+					ident: obj,
+				},
+			},
+		}
+	}
+
+	logIdent := ast.NewIdent("log")
+	aliasIdent := ast.NewIdent("applog")
+	localIdent := ast.NewIdent("log")
+
+	logPkg := types.NewPackage("log", "log")
+	customType := types.NewNamed(
+		types.NewTypeName(token.NoPos, nil, "customLogger", nil),
+		types.NewStruct(nil, nil),
+		nil,
+	)
+
+	tests := []struct {
+		name    string
+		pass    *analysis.Pass
+		sel     *ast.SelectorExpr
+		pkgPath string
+		want    bool
+	}{
+		{
+			name: "direct import name",
+			pass: makePass(logIdent, types.NewPkgName(token.NoPos, nil, "log", logPkg)),
+			sel: &ast.SelectorExpr{
+				X:   logIdent,
+				Sel: ast.NewIdent("Printf"),
+			},
+			pkgPath: "log",
+			want:    true,
+		},
+		{
+			name: "aliased import name",
+			pass: makePass(aliasIdent, types.NewPkgName(token.NoPos, nil, "applog", logPkg)),
+			sel: &ast.SelectorExpr{
+				X:   aliasIdent,
+				Sel: ast.NewIdent("Fatal"),
+			},
+			pkgPath: "log",
+			want:    true,
+		},
+		{
+			name: "local shadowed identifier",
+			pass: makePass(localIdent, types.NewVar(token.NoPos, nil, "log", types.NewPointer(customType))),
+			sel: &ast.SelectorExpr{
+				X:   localIdent,
+				Sel: ast.NewIdent("Printf"),
+			},
+			pkgPath: "log",
+			want:    false,
+		},
+		{
+			name: "nil pass",
+			pass: nil,
+			sel: &ast.SelectorExpr{
+				X:   logIdent,
+				Sel: ast.NewIdent("Printf"),
+			},
+			pkgPath: "log",
+			want:    false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := IsPkgSelector(tt.pass, tt.sel, tt.pkgPath)
+			if got != tt.want {
+				t.Fatalf("IsPkgSelector() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/linters/rawloginlib/rawloginlib.go

@@ -55,17 +55,17 @@ func run(pass *analysis.Pass) (any, error) {
 		if !ok {
 			return
 		}
-		ident, ok := sel.X.(*ast.Ident)
-		if !ok {
+		if !astutil.IsPkgSelector(pass, sel, "log") {
+			return
+		}
+		if !rawLogFuncs[sel.Sel.Name] {
 			return
 		}
-		if ident.Name == "log" && rawLogFuncs[sel.Sel.Name] {
-			position := pass.Fset.PositionFor(call.Pos(), false)
-			if nolint.HasDirective(position, noLintLinesByFile) {
-				return
-			}
-			pass.ReportRangef(call, "log.%s called in library package %s; use pkg/logger instead", sel.Sel.Name, pkgPath)
+		position := pass.Fset.PositionFor(call.Pos(), false)
+		if nolint.HasDirective(position, noLintLinesByFile) {
+			return
 		}
+		pass.ReportRangef(call, "log.%s called in library package %s; use pkg/logger instead", sel.Sel.Name, pkgPath)
 	})
 
 	return nil, nil

pkg/linters/rawloginlib/testdata/src/rawloginlib/alias_import.go

@@ -0,0 +1,7 @@
+package rawloginlib
+
+import applog "log"
+
+func badAlias() {
+	applog.Fatal("boom") // want `log\.Fatal called in library package`
+}

pkg/linters/rawloginlib/testdata/src/rawloginlib/rawloginlib.go

@@ -2,6 +2,10 @@ package rawloginlib
 
 import "log"
 
+type customLogger struct{}
+
+func (*customLogger) Printf(string, ...any) {}
+
 func bad() {
 	log.Printf("hello %s", "world") // want `log\.Printf called in library package`
 	log.Println("oops")             // want `log\.Println called in library package`
@@ -17,3 +21,12 @@ func suppressed() {
 	log.Printf("suppressed previous line")
 	log.Println("suppressed same line") //nolint:rawloginlib
 }
+
+func shadowedParam(log *customLogger) {
+	log.Printf("shadowed parameter should not trigger")
+}
+
+func shadowedLocal() {
+	log := &customLogger{}
+	log.Printf("shadowed local should not trigger")
+}