From 3f740200b0a03eff6e3f2177192155922aa18c64 Mon Sep 17 00:00:00 2001 From: Martin Najemi Date: Thu, 20 Aug 2026 06:37:55 +0200 Subject: [PATCH] fix: Side-effect taint crossing type-only edges Risk: low --- CHANGELOG.md | 6 +++++ VERSION | 2 +- internal/analyzer/analyzer.go | 45 +++++++++++++++++++++++++++++------ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60a1c80..3659638 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ 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.8] - 2026-08-20 + +### Fixed +- Import-time side-effect taint no longer propagates through **type-only** import/re-export edges. `import type { T } from "./x"` and `export type { T } from "./x"` are erased at compile time and never load `./x` at runtime, but the dependency graph didn't distinguish them from runtime edges — so a barrel whose only link to a side-effectful module was a type-only re-export was wholesale-tainted (all runtime symbols + the `__side-effect__` sentinel) and kept cascading to its own consumers, flagging targets for changes their code can never execute. Import edges now carry the statement's type-only-ness (tracked on `tsparse.Import` since 0.25.3) and synthetic re-export edges carry the export's; the side-effect transitivity blocks in both propagation paths fire only when at least one **runtime** edge connects the importer to the side-effectful module. A runtime re-export correctly *upgrades* a previously-recorded type-only edge to the same source (the deduped edge is flipped, so `export type { T } from "./x"` followed by `export { f } from "./x"` still counts as a runtime connection). Ordinary named-symbol taint through type-only edges is unchanged (relevant for `INCLUDE_TYPES`); only the side-effect wildcard/sentinel flow is gated. + ## [0.25.7] - 2026-08-11 ### Fixed @@ -428,6 +433,7 @@ Together these keep genuine import-time changes flagged while eliminating the la - Multi-stage Docker build - Automated vendor upgrade workflow +[0.25.8]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.7...v0.25.8 [0.25.7]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.6...v0.25.7 [0.25.6]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.5...v0.25.6 [0.25.5]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.25.4...v0.25.5 diff --git a/VERSION b/VERSION index ee1f11d..58510a1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.25.7 \ No newline at end of file +0.25.8 \ No newline at end of file diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index cd1fa7c..42dee1d 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -263,6 +263,11 @@ type importEdge struct { localNames []string origNames []string isSideEffect bool // true for unassigned imports like import "./foo" + // isTypeOnly marks edges that exist only at the type level (`import type` / + // `export type { … } from`). Such statements are erased at compile time and + // never load the source module at runtime, so import-time side effects must + // not propagate through them. + isTypeOnly bool } // AnalyzeLibraryPackage builds a full internal file dependency graph, @@ -337,6 +342,7 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge localNames: localNames, origNames: origNames, isSideEffect: len(imp.Names) == 0, + isTypeOnly: imp.IsTypeOnly, }) } @@ -352,11 +358,17 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge continue } // Check if we already have an import edge to this source - // (to avoid duplicating edges when a file both imports and re-exports) + // (to avoid duplicating edges when a file both imports and re-exports). + // A RUNTIME re-export upgrades an existing type-only edge: the file may + // first hit `import type`/`export type` from the source and only later a + // runtime re-export — the connection as a whole is then runtime. alreadyHasEdge := false - for _, edge := range importGraph[stem] { + for i, edge := range importGraph[stem] { if edge.fromStem == resolvedStem { alreadyHasEdge = true + if edge.isTypeOnly && !exp.IsTypeOnly { + importGraph[stem][i].isTypeOnly = false + } break } } @@ -377,6 +389,7 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge fromStem: resolvedStem, localNames: localNames, origNames: origNames, + isTypeOnly: exp.IsTypeOnly, }) } } @@ -747,11 +760,15 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge // Check for side-effect (unassigned) imports and named imports from the tainted source hasSideEffectImport := false + hasRuntimeEdge := false var taintedLocalNames []string for _, edge := range importGraph[importerStem] { if edge.fromStem != currentStem { continue } + if !edge.isTypeOnly { + hasRuntimeEdge = true + } if edge.isSideEffect { hasSideEffectImport = true continue @@ -792,8 +809,10 @@ func AnalyzeLibraryPackage(projectFolder string, entrypoints []Entrypoint, merge // TODO: make this precise using the "sideEffects" field in each package's // package.json — a module marked side-effect-free is tree-shaken and not // re-executed on import, so it should not propagate. Until then we assume - // the worst and propagate through every import/re-export edge. Follow-up. - if currentTainted[sideEffectTaint] { + // the worst and propagate through every RUNTIME import/re-export edge. + // Type-only edges (`import type` / `export type … from`) are erased at + // compile time and never load the module, so they don't count. Follow-up. + if currentTainted[sideEffectTaint] && hasRuntimeEdge { for _, sym := range importerAnalysis.Symbols { newlyTainted = append(newlyTainted, sym.Name) } @@ -1269,6 +1288,7 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m localNames: localNames, origNames: origNames, isSideEffect: len(imp.Names) == 0, + isTypeOnly: imp.IsTypeOnly, }) } // Re-exports as import edges @@ -1283,10 +1303,15 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m if _, ok := fileAnalyses[resolvedStem]; !ok { continue } + // A RUNTIME re-export upgrades an existing type-only edge — see the + // matching dedup in AnalyzeLibraryPackage's graph builder. alreadyHasEdge := false - for _, edge := range localImportGraph[stem] { + for i, edge := range localImportGraph[stem] { if edge.fromStem == resolvedStem { alreadyHasEdge = true + if edge.isTypeOnly && !exp.IsTypeOnly { + localImportGraph[stem][i].isTypeOnly = false + } break } } @@ -1305,6 +1330,7 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m fromStem: resolvedStem, localNames: localNames, origNames: origNames, + isTypeOnly: exp.IsTypeOnly, }) } } @@ -1621,11 +1647,15 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m } hasSideEffectImport := false + hasRuntimeEdge := false var taintedLocalNames []string for _, edge := range localImportGraph[importerStem] { if edge.fromStem != currentStem { continue } + if !edge.isTypeOnly { + hasRuntimeEdge = true + } if edge.isSideEffect { hasSideEffectImport = true continue @@ -1660,8 +1690,9 @@ func FindAffectedFiles(globPattern string, filterPattern string, upstreamTaint m // AnalyzeLibraryPackage. Importing a side-effectful module re-runs its // side effect here, so this file becomes side-effectful too and keeps // propagating it (assume-the-worst; refine later via package.json - // "sideEffects" — see that TODO). - if currentTainted[sideEffectTaint] { + // "sideEffects" — see that TODO). Runtime edges only: type-only + // imports/re-exports are erased and never load the module. + if currentTainted[sideEffectTaint] && hasRuntimeEdge { for _, sym := range importerAnalysis.Symbols { newlyTainted = append(newlyTainted, sym.Name) }