fix: Whole-file taint on comment-only helper changes

CHANGELOG.md

@@ -5,9 +5,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.25.2] - 2026-08-07
+## [0.25.3] - 2026-08-07
 
 ### Fixed
+- Replaced the fine-grained path's whole-file `*` fallback with import-time-aware detection, so an empty symbol diff no longer floods every importer. Previously, when a changed file produced no symbol-level diff, `FindAffectedFiles` tainted the **entire file** with `*` — so a comment-only edit, a formatting change, a type-only change, or reordering imports in a widely-imported helper flagged all its consumers. That blanket is removed; a change now wildcards a file only when something that actually **runs at import time** changed:
+  - a top-level side-effect statement (`console.log(...)`, `describe(...)`, `test(...)`, etc.) — already detected via `hasSideEffectStmtChanges`;
+  - a bare side-effect import (`import "./x"`) added, removed, or re-pointed.
+- Named-import **re-pointing** is now handled precisely instead of via the blanket. When a binding keeps its name but resolves to a different module/export (`import { x } from "./a"` → `"./b"`, or `{ a as x }` → `{ b as x }`), its usages don't change textually and the symbol diff missed them; those usages are now tainted directly. Reordering imports and re-pointing a *type-only* import (`import type`, when `includeTypes` is off) correctly taint nothing. Import statement type-only-ness is now tracked on `tsparse.Import`.
+
+Together these keep genuine import-time changes flagged while eliminating the large false-positive class where a comment or dead-code edit to a shared file re-ran every dependent target.
 - Removing an unused export no longer floods every importer with taint. A deleted symbol was logged but never recorded as a change, so the per-file AST diff returned *no affected symbols* — and in the fine-grained path (`FindAffectedFiles`) an empty diff falls through to tainting the **whole file** with `*`. Deleting one unused export from a widely-imported helper (e.g. `ERROR_MESSAGE` from `gdc-ldm-modeler-e2e`'s `playwright/helpers/selectors.ts`) therefore tainted every file that imported it, flagging all ~48 dependent specs. Deleted symbols are now recorded as changed and propagate by name, so a removed export taints exactly the files that imported *that* symbol: an unused one taints nobody, while a removed *used* export still flags its importers (no false negative). The deleted names are appended after intra-file propagation (which walks only surviving symbols) and before the whole-file side-effect fallback, so a deletion-only change is carried precisely instead of being widened.
 
 ## [0.25.1] - 2026-08-07
@@ -402,6 +408,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Multi-stage Docker build
 - Automated vendor upgrade workflow
 
+[0.25.3]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.2...v0.25.3
 [0.25.2]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.1...v0.25.2
 [0.25.1]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.0...v0.25.1
 [0.25.0]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.24.13...v0.25.0

VERSION

@@ -1 +1 @@
-0.25.2
+0.25.3

internal/analyzer/analyzer.go

@@ -1327,25 +1327,26 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m
 		}
 		changedSymbols := findAffectedSymbolsByASTDiff(oldAnalysis, analysis, oldContent, includeTypes)
 		log.Debugf("  %s: affected symbols (AST diff): %v", stem, changedSymbols)
-		if tainted[stem] == nil {
-			tainted[stem] = make(map[string]bool)
-		}
 		if oldAnalysis == nil {
-			// New file: taint all symbols
+			// New file: taint all symbols (their behavior is entirely new).
 			log.Debugf("  %s: new file — tainting all symbols", stem)
+			tainted[stem] = make(map[string]bool)
 			for _, sym := range analysis.Symbols {
 				tainted[stem][sym.Name] = true
 			}
 			tainted[stem]["*"] = true
 		} else if len(changedSymbols) > 0 {
+			tainted[stem] = make(map[string]bool)
 			for _, s := range changedSymbols {
 				tainted[stem][s] = true
 			}
-		} else {
-			// File changed but no symbol-level diff detected (e.g. changes in
-			// test()/describe() blocks or other non-declaration code).
-			tainted[stem]["*"] = true
 		}
+		// else: the AST diff found nothing that affects consumers (comments,
+		// formatting, type-only, or reordered imports). Import-time side effects
+		// (top-level statements, bare `import "x"`) and re-pointed value imports are
+		// already surfaced as "*"/affected symbols by findAffectedSymbolsByASTDiff,
+		// so there is deliberately NO whole-file fallback here (the old blanket `*`
+		// over-tainted every importer on a comment-only or dead-code change).
 	}
 
 	// Seed from upstream workspace taint

internal/analyzer/astdiff.go

@@ -131,6 +131,16 @@ func findAffectedSymbolsByASTDiff(oldAnalysis *tsparse.FileAnalysis, newAnalysis
 		}
 	}
 
+	// Re-pointed value imports: a local binding keeps its name but now resolves to
+	// a different module/export (`import { x } from "./a"` → "./b", or
+	// `{ a as x }` → `{ b as x }`). The usages don't change textually, so the
+	// symbol diff above misses them — taint the symbols that use the re-pointed
+	// binding. (Type-only imports are excluded unless includeTypes.)
+	if repointed := repointedImportBindings(oldAnalysis, newAnalysis, includeTypes); len(repointed) > 0 {
+		log.Debugf("    re-pointed import bindings: %v", repointed)
+		affected = append(affected, findTaintedSymbolsByUsage(newAnalysis, repointed)...)
+	}
+
 	// Intra-file propagation: if symbol A changed and symbol B references A,
 	// then B is also affected. E.g. `UiPagedVirtualListNotWrapped` changed,
 	// `UiPagedVirtualList = memo(UiPagedVirtualListNotWrapped)` is also affected.
@@ -215,9 +225,13 @@ func findAffectedSymbolsByASTDiff(oldAnalysis *tsparse.FileAnalysis, newAnalysis
 		}
 		if normalizeWhitespace(oldText) != normalizeWhitespace(newText) {
 			// File changed but no symbol was affected — changes are outside symbols.
-			// Check if the changes include runtime side-effect statements.
-			if hasSideEffectStmtChanges(oldAnalysis.SourceFile, newAnalysis.SourceFile) {
-				log.Debugf("    file changed with RUNTIME side-effect statements — tainting all symbols")
+			// Wildcard only when something that RUNS at import time changed: a
+			// top-level side-effect statement, or a bare `import "x"` side-effect
+			// import. Comment / formatting / type-only / import-reordering changes
+			// fall through untainted.
+			if hasSideEffectStmtChanges(oldAnalysis.SourceFile, newAnalysis.SourceFile) ||
+				bareImportsChanged(oldAnalysis, newAnalysis) {
+				log.Debugf("    file changed with import-time side effects — tainting all symbols")
 				// Use "*" wildcard to mark all exports as affected.
 				// This handles barrel/entrypoint files that have no symbol declarations
 				// but whose runtime side effects affect all importers.
@@ -509,3 +523,75 @@ func isSideEffectStatement(stmt *ast.Node) bool {
 		return true
 	}
 }
+
+// importBindingOrigins maps each local binding introduced by a value import to a
+// stable "origin" key (source + source-side name). Bare side-effect imports (no
+// bindings) and — unless includeTypes — `import type` statements are excluded.
+func importBindingOrigins(a *tsparse.FileAnalysis, includeTypes bool) map[string]string {
+	origins := make(map[string]string)
+	if a == nil {
+		return origins
+	}
+	for _, imp := range a.Imports {
+		if imp.IsTypeOnly && !includeTypes {
+			continue
+		}
+		for i, local := range imp.LocalNames {
+			src := ""
+			if i < len(imp.Names) {
+				src = imp.Names[i]
+			}
+			origins[local] = imp.Source + "\x00" + src
+		}
+	}
+	return origins
+}
+
+// repointedImportBindings returns local binding names present both before and
+// after the change that now resolve to a different module/export.
+func repointedImportBindings(oldA, newA *tsparse.FileAnalysis, includeTypes bool) []string {
+	if oldA == nil || newA == nil {
+		return nil
+	}
+	oldOrigins := importBindingOrigins(oldA, includeTypes)
+	newOrigins := importBindingOrigins(newA, includeTypes)
+	var repointed []string
+	for local, newOrigin := range newOrigins {
+		if oldOrigin, ok := oldOrigins[local]; ok && oldOrigin != newOrigin {
+			repointed = append(repointed, local)
+		}
+	}
+	return repointed
+}
+
+// bareImportSources returns the set of module specifiers imported purely for
+// their side effects (`import "./x"` — no bindings). These execute at import time.
+func bareImportSources(a *tsparse.FileAnalysis) map[string]bool {
+	sources := make(map[string]bool)
+	if a == nil {
+		return sources
+	}
+	for _, imp := range a.Imports {
+		if len(imp.Names) == 0 && len(imp.LocalNames) == 0 {
+			sources[imp.Source] = true
+		}
+	}
+	return sources
+}
+
+// bareImportsChanged reports whether the set of side-effect imports differs
+// between old and new (added, removed, or re-pointed) — an import-time behavior
+// change that warrants whole-file taint.
+func bareImportsChanged(oldA, newA *tsparse.FileAnalysis) bool {
+	oldSet := bareImportSources(oldA)
+	newSet := bareImportSources(newA)
+	if len(oldSet) != len(newSet) {
+		return true
+	}
+	for s := range newSet {
+		if !oldSet[s] {
+			return true
+		}
+	}
+	return false
+}

internal/tsparse/tsparse.go

@@ -19,6 +19,7 @@ type Import struct {
 	// LocalNames[i] is "Y" (what this file references in its body).
 	LocalNames []string
 	Source     string // module specifier (e.g., "./Button/Button.js")
+	IsTypeOnly bool   // true for `import type { … }` / `import type X` (whole-statement type-only)
 }
 
 type Export struct {
@@ -135,8 +136,10 @@ func extractImports(stmt *ast.Node, analysis *FileAnalysis) {
 	source := strings.Trim(imp.ModuleSpecifier.Text(), "\"'`")
 
 	var names, localNames []string
+	typeOnly := false
 	if imp.ImportClause != nil {
 		clause := imp.ImportClause.AsImportClause()
+		typeOnly = clause.PhaseModifier == ast.KindTypeKeyword
 		if clause.Name() != nil {
 			n := clause.Name().Text()
 			names = append(names, n)
@@ -171,6 +174,7 @@ func extractImports(stmt *ast.Node, analysis *FileAnalysis) {
 		Names:      names,
 		LocalNames: localNames,
 		Source:     source,
+		IsTypeOnly: typeOnly,
 	})
 }