diff --git a/CHANGELOG.md b/CHANGELOG.md index 5198c04..0b1a325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,11 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### New Features -- Impact and blast-radius analysis for TypeScript/JavaScript now understands the readers of a constant. When you change a file-scope `const`/`var` — a config object, a lookup table, a shared constant — the other symbols in that file that read it now show up as affected, where before they were invisible (impact only followed calls, imports, and inheritance, so a constant's consumers looked like "nothing depends on this"). This makes `codegraph impact`, and the impact trail in `codegraph_explore`/`codegraph_node`, catch the "change this table, break its readers" class of change. It's on by default for TS/JS and adds no nodes to your graph; bundled/minified files and ambiguously-shadowed names are skipped to keep results precise. Set `CODEGRAPH_VALUE_REFS=0` to turn it off. +- Impact and blast-radius analysis for TypeScript, JavaScript, Go, Python, Rust, Ruby, C, Java, C#, PHP, Scala, Kotlin, Swift, Dart, and Pascal/Delphi now understands the readers of a constant. When you change a file-scope, package-level, module-level, or class-level constant — a config object, a lookup table, a shared constant — the other symbols in that file that read it now show up as affected, where before they were invisible (impact only followed calls, imports, and inheritance, so a constant's consumers looked like "nothing depends on this"). This makes `codegraph impact`, and the impact trail in `codegraph_explore`/`codegraph_node`, catch the "change this table, break its readers" class of change. It's on by default and adds no nodes to your graph; bundled/minified files and ambiguously-shadowed names are skipped to keep results precise. Set `CODEGRAPH_VALUE_REFS=0` to turn it off. +- C file-scope constants and globals — `static const` scalars, pointer/array lookup tables, and shared mutable globals — are now recognized as symbols in their own right. They previously weren't extracted at all, so they never appeared in search or carried any dependents; now they show up in `codegraph search` and participate in impact analysis (see above), so changing a C lookup table surfaces the same-file functions that read it. +- Java `static final` constants, C# `const` / `static readonly` constants, Scala `object` vals, and Kotlin top-level / `object` / `companion object` `val`s are now classified as constants rather than generic fields, so they participate in the constant-reader impact analysis above — change a `public static final` table, a `const string`, a Scala `object Config { val Timeout = … }`, or a Kotlin `companion object { const val … }` and the methods that read it now show up as affected. (Per-object Java `final` / C# `readonly` / Scala & Kotlin `class` instance properties are unchanged.) Kotlin constants were previously not indexed as their own symbols at all, so they now also appear in `codegraph search`. +- Swift top-level `let`s and `static let` constants (including those namespaced in an `enum`/`struct`, the common Swift pattern) are now indexed as constants and participate in the constant-reader impact analysis above — change a `static let defaultRetryLimit` or an `enum Constants { static let … }` and the same-file code that reads it shows up as affected. Computed properties and per-instance `let`s are not treated as constants. +- Dart top-level `const`/`final` and class `static const`/`static final` constants are now indexed as constants and participate in the constant-reader impact analysis above. Instance fields, `var`s, and locals are not treated as constants. (Generated Dart code with the standard `.g.dart`/`.freezed.dart`/`.pb.dart` suffixes is already skipped.) ### Fixes diff --git a/__tests__/value-reference-edges.test.ts b/__tests__/value-reference-edges.test.ts index 3c2b65e..485a00b 100644 --- a/__tests__/value-reference-edges.test.ts +++ b/__tests__/value-reference-edges.test.ts @@ -12,13 +12,20 @@ import * as os from 'os'; import CodeGraph from '../src'; function valueRefReaders(cg: CodeGraph, constName: string): string[] { - const target = cg.searchNodes(constName).map((r) => r.node).find((n) => n.name === constName); - if (!target) return []; - return cg - .getIncomingEdges(target.id) - .filter((e) => e.kind === 'references' && (e.metadata as { valueRef?: boolean } | undefined)?.valueRef) - .map((e) => cg.getNode(e.source)?.name) - .filter((n): n is string => Boolean(n)); + // Aggregate across ALL nodes of this name — a conditionally-defined module + // const (`try: X=…; except: X=…`) has more than one, and the edge targets + // whichever one ended up in the target map. + const targets = cg.searchNodes(constName).map((r) => r.node).filter((n) => n.name === constName); + const readers = new Set(); + for (const t of targets) { + for (const e of cg.getIncomingEdges(t.id)) { + if (e.kind === 'references' && (e.metadata as { valueRef?: boolean } | undefined)?.valueRef) { + const r = cg.getNode(e.source)?.name; + if (r) readers.add(r); + } + } + } + return [...readers]; } describe('value-reference edges', () => { @@ -98,6 +105,606 @@ describe('value-reference edges', () => { expect(valueRefReaders(cg, 'Module')).toEqual([]); }); + it('edges readers that use the const only inside JSX (.tsx)', async () => { + // The tsx-specific path: the const is read ONLY inside JSX expressions, so + // the reader-scan must descend into the JSX subtree to find it. + fs.writeFileSync( + path.join(dir, 'widget.tsx'), + [ + 'export const THEME_TOKENS = { color: "red", size: 12 };', + 'export function Label() {', + ' return hi;', + '}', + 'export const Box = () =>
;', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'THEME_TOKENS')).toEqual(expect.arrayContaining(['Label', 'Box'])); + }); + + it('edges same-file readers to a module-level const/static (Rust)', async () => { + fs.writeFileSync( + path.join(dir, 'lib.rs'), + [ + 'const MAX_RETRIES: u32 = 3;', + 'static DEFAULT_LABEL: &str = "prod";', + '', + 'fn retry() -> u32 { MAX_RETRIES }', + "fn label() -> &'static str { DEFAULT_LABEL }", + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_RETRIES')).toEqual(expect.arrayContaining(['retry'])); + expect(valueRefReaders(cg, 'DEFAULT_LABEL')).toEqual(expect.arrayContaining(['label'])); + }); + + it('does NOT edge a Rust const shadowed by a local let of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'shadow.rs'), + [ + 'const TIMEOUT: u32 = 30;', + '', + 'fn uses_const() -> u32 { TIMEOUT }', + 'fn shadows() -> u32 {', + ' let TIMEOUT = 5;', + ' TIMEOUT', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges same-file readers to a package-level const/var (Go)', async () => { + fs.writeFileSync( + path.join(dir, 'main.go'), + [ + 'package main', + '', + 'const MaxRetries = 3', + 'var DefaultLabels = map[string]string{"env": "prod"}', + '', + 'func retry() int { return MaxRetries }', + 'func labels() map[string]string { return DefaultLabels }', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MaxRetries')).toEqual(expect.arrayContaining(['retry'])); + expect(valueRefReaders(cg, 'DefaultLabels')).toEqual(expect.arrayContaining(['labels'])); + }); + + it('does NOT edge a Go package const shadowed by a local := of the same name', async () => { + // `Timeout` is a package const AND a local `:=` (short_var_declaration) in + // shadows(). The local read resolves to the inner binding, so a file-scope + // edge would be a false positive — the shadow prune drops the whole target. + fs.writeFileSync( + path.join(dir, 'shadow.go'), + [ + 'package main', + '', + 'const Timeout = 30', + '', + 'func usesConst() int { return Timeout }', + 'func shadows() int {', + '\tTimeout := 5', + '\treturn Timeout', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'Timeout')).toEqual([]); + }); + + it('keeps a conditionally-defined module const (try/except), not a shadow (Python)', async () => { + // `HAS_SSL` is defined twice but BOTH at module scope (a conditional def, a + // very common Python idiom). It is one logical const, not a shadow, so its + // reader must stay edged — and the two halves must not edge each other. + fs.writeFileSync( + path.join(dir, 'cond.py'), + [ + 'try:', + '\tHAS_SSL = True', + 'except ImportError:', + '\tHAS_SSL = False', + '', + 'def uses_ssl():', + '\treturn HAS_SSL', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'HAS_SSL')).toEqual(['uses_ssl']); + }); + + it('edges readers to a top-level AND a class-internal constant (Ruby)', async () => { + // Ruby keeps almost all constants inside a class/module. Both the top-level + // `MAX_RETRIES` and the class-internal `Config::TIMEOUT` must be targets, and + // their same-file readers edged (TIMEOUT is read by two methods of Config). + fs.writeFileSync( + path.join(dir, 'app.rb'), + [ + 'MAX_RETRIES = 3', + '', + 'def retry_count', + ' MAX_RETRIES', + 'end', + '', + 'class Config', + ' TIMEOUT = 30', + ' def self.get_timeout', + ' TIMEOUT', + ' end', + ' def describe', + ' "timeout=#{TIMEOUT}"', + ' end', + 'end', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_RETRIES')).toEqual(expect.arrayContaining(['retry_count'])); + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual(expect.arrayContaining(['get_timeout', 'describe'])); + }); + + it('edges same-file readers to a file-scope const/table (C)', async () => { + // C keeps shareable values at file scope as `static const` — scalars and, + // very commonly, pointer/array lookup tables. Both must be extracted as + // nodes (the generic fallback misses C's nested init_declarator name) and + // their same-file readers edged. + fs.writeFileSync( + path.join(dir, 'config.c'), + [ + 'static const int MAX_ITEMS = 100;', + 'static const char *const STATUS_NAMES[] = { "ok", "fail", "pending" };', + '', + 'int capped(int n) { return n > MAX_ITEMS ? MAX_ITEMS : n; }', + 'const char *label(int i) { return STATUS_NAMES[i]; }', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_ITEMS')).toEqual(expect.arrayContaining(['capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + }); + + it('does NOT edge a C file const shadowed by a function-local of the same name', async () => { + // `TIMEOUT` is a file const AND a local `int TIMEOUT = 5` (init_declarator) + // in shadows(). The local read resolves to the inner binding, so a + // file-scope edge would be a false positive — the shadow prune drops it. + fs.writeFileSync( + path.join(dir, 'shadow.c'), + [ + 'static const int TIMEOUT = 30;', + '', + 'int uses_const(void) { return TIMEOUT; }', + 'int shadows(void) {', + ' int TIMEOUT = 5;', + ' return TIMEOUT;', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('does NOT mint a value target from a macro-prefixed C prototype (return-type misparse)', async () => { + // A prototype led by an unknown macro (`CURL_EXTERN CURLcode fn(args);`) + // makes tree-sitter-c misparse it as a declaration whose "variable" is the + // bare return-type identifier — which would mint a spurious `CURLcode` + // value target read by every function of that type. The bare-identifier + // skip prevents it, while real file-scope consts still edge their readers. + fs.writeFileSync( + path.join(dir, 'api.c'), + [ + 'typedef enum { CURLE_OK, CURLE_FAIL } CURLcode;', + 'CURL_EXTERN CURLcode curl_easy_init(int x);', + 'CURL_EXTERN CURLcode curl_easy_setopt(int y);', + '', + 'static const int REAL_LIMIT = 42;', + 'int use_real(void) { return REAL_LIMIT; }', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + // The return-type name is never extracted as a const/var, so it is not a + // value-ref target at all. + const curlcodeValues = cg + .searchNodes('CURLcode') + .map((r) => r.node) + .filter((n) => n.name === 'CURLcode' && (n.kind === 'constant' || n.kind === 'variable')); + expect(curlcodeValues).toEqual([]); + // Real file-scope consts alongside the misparse-prone prototypes still work. + expect(valueRefReaders(cg, 'REAL_LIMIT')).toEqual(expect.arrayContaining(['use_real'])); + }); + + it('edges same-file methods to a class-scope static final constant (Java)', async () => { + // Java keeps constants as `static final` fields inside a class. They extract + // as `constant` kind (not `field`) so the value-ref gate targets them; a + // plain instance `final` field is NOT a constant and must not be a target. + fs.writeFileSync( + path.join(dir, 'Limits.java'), + [ + 'class Limits {', + ' public static final int MAX_ITEMS = 100;', + ' static final String[] STATUS_NAMES = { "ok", "fail" };', + ' final int instanceId = 1;', + ' int capped(int n) { return n > MAX_ITEMS ? MAX_ITEMS : n; }', + ' String label(int i) { return STATUS_NAMES[i]; }', + ' int id() { return instanceId; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_ITEMS')).toEqual(expect.arrayContaining(['capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + // An instance `final` field is mutable per-object state, not a shared + // constant — it stays `field` kind and is never a value-ref target. + expect(valueRefReaders(cg, 'instanceId')).toEqual([]); + }); + + it('does NOT edge a Java class const shadowed by a method-local of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'Shadow.java'), + [ + 'class Shadow {', + ' static final int TIMEOUT = 30;', + ' int usesConst() { return TIMEOUT; }', + ' int shadows() { int TIMEOUT = 5; return TIMEOUT; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges same-file methods to a class const / static readonly (C#)', async () => { + // C# constants are `const` (compile-time) or `static readonly` (runtime); + // both extract as `constant`. An instance `readonly` field is per-object and + // stays `field`. + fs.writeFileSync( + path.join(dir, 'Limits.cs'), + [ + 'class Limits {', + ' const int MAX_ITEMS = 100;', + ' static readonly string[] STATUS_NAMES = { "ok", "fail" };', + ' readonly int instanceId = 1;', + ' int Capped(int n) { return n > MAX_ITEMS ? MAX_ITEMS : n; }', + ' string Label(int i) { return STATUS_NAMES[i]; }', + ' int Id() { return instanceId; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_ITEMS')).toEqual(expect.arrayContaining(['Capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['Label'])); + expect(valueRefReaders(cg, 'instanceId')).toEqual([]); + }); + + it('does NOT edge a C# class const shadowed by a method-local of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'Shadow.cs'), + [ + 'class Shadow {', + ' const int TIMEOUT = 30;', + ' int UsesConst() { return TIMEOUT; }', + ' int Shadows() { int TIMEOUT = 5; return TIMEOUT; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges same-file readers to a top-level and class const, incl. self:: / Class:: (PHP)', async () => { + // PHP keeps constants at file scope (`const X`) and inside classes (`const + // X`), both extracted as `constant`. A constant *reference* is a `name` node + // (bare `X`, or the const half of `self::X` / `Foo::X`), so the reader-scan + // must match `name`. A `$var` local is a different namespace and can never + // shadow a bare constant — so there is nothing to prune. + fs.writeFileSync( + path.join(dir, 'Config.php'), + [ + ' self::MAX_ITEMS ? self::MAX_ITEMS : $n; }', + ' function label($i) { return Config::STATUS_NAMES[$i]; }', + ' function version() { return APP_VERSION; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_ITEMS')).toEqual(expect.arrayContaining(['capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + expect(valueRefReaders(cg, 'APP_VERSION')).toEqual(expect.arrayContaining(['version'])); + // A static property is mutable class state, not a constant — never a target. + expect(valueRefReaders(cg, 'counter')).toEqual([]); + }); + + it('edges readers to a top-level and object-scope val, not a class instance val (Scala)', async () => { + // Scala has no `static`: an `object` is a singleton, so its `val`s are the + // shared-constant idiom (extracted as `constant`, like a top-level val). A + // `class` val is a per-instance immutable field (`field`, never a target). + fs.writeFileSync( + path.join(dir, 'Demo.scala'), + [ + 'val AppVersion = "1.0"', + 'object Config {', + ' val TIMEOUT_MS = 30', + ' val STATUS_NAMES = List("ok", "fail")', + ' def capped(n: Int): Int = if (n > TIMEOUT_MS) TIMEOUT_MS else n', + ' def label(i: Int): String = STATUS_NAMES(i)', + '}', + 'class Widget {', + ' val MaxItems = 100', + ' def within(n: Int): Int = if (n < MaxItems) n else MaxItems', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT_MS')).toEqual(expect.arrayContaining(['capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + // A class instance `val` is per-object state (kind `field`), not a shared + // constant — never a value-ref target even though `within` reads it. + expect(valueRefReaders(cg, 'MaxItems')).toEqual([]); + }); + + it('does NOT edge a Scala object val shadowed by a method-local val of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'Shadow.scala'), + [ + 'object Config {', + ' val TIMEOUT = 30', + ' def usesConst(): Int = TIMEOUT', + ' def shadows(): Int = { val TIMEOUT = 5; TIMEOUT }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges readers to top-level, object, and companion-object constants, not a class val (Kotlin)', async () => { + // Kotlin has no `static`: a top-level property, an `object` (singleton), and a + // class's `companion object` all hold shared constants (`val`→constant). A + // class instance `val` is per-object state (`field`, never a target). The + // property name nests as variable_declaration→simple_identifier, and a const + // reference is a `simple_identifier`. + fs.writeFileSync( + path.join(dir, 'Demo.kt'), + [ + 'const val TOP_LEVEL_MAX = 100', + 'object Config {', + ' const val TIMEOUT_MS = 30', + ' val STATUS_NAMES = listOf("ok", "fail")', + ' fun capped(n: Int): Int = if (n > TIMEOUT_MS) TIMEOUT_MS else n', + ' fun label(i: Int): String = STATUS_NAMES[i]', + '}', + 'class Widget {', + ' companion object { const val MAX_RETRIES = 3 }', + ' val instanceField = 1', + ' fun retries(): Int = MAX_RETRIES', + ' fun within(n: Int): Int = if (n < TOP_LEVEL_MAX) n else TOP_LEVEL_MAX', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + expect(valueRefReaders(cg, 'MAX_RETRIES')).toEqual(expect.arrayContaining(['retries'])); + expect(valueRefReaders(cg, 'TOP_LEVEL_MAX')).toEqual(expect.arrayContaining(['within'])); + // A class instance `val` is per-object state (kind `field`), never a target. + expect(valueRefReaders(cg, 'instanceField')).toEqual([]); + }); + + it('does NOT edge a Kotlin object const shadowed by a method-local val of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'Shadow.kt'), + [ + 'object Config {', + ' const val TIMEOUT = 30', + ' fun usesConst(): Int = TIMEOUT', + ' fun shadows(): Int { val TIMEOUT = 5; return TIMEOUT }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges readers to a top-level let and static let in enum/struct, not an instance let (Swift)', async () => { + // Swift has no `static` keyword for globals; the shared-constant idiom is a + // top-level `let` or a `static let` inside a type — Swift namespaces these in + // `enum`/`struct`. Those extract as `constant`; an instance stored `let` is + // per-object (`field`, never a target); a *computed* property is skipped. + fs.writeFileSync( + path.join(dir, 'Demo.swift'), + [ + 'let topLevelMax = 100', + 'enum Constants {', + ' static let TIMEOUT_MS = 30', + ' static let STATUS_NAMES = ["ok", "fail"]', + '}', + 'struct Widget {', + ' static let MAX_RETRIES = 3', + ' let instanceField = 1', + ' func retries() -> Int { return Widget.MAX_RETRIES }', + ' func within(_ n: Int) -> Int { return n < topLevelMax ? n : topLevelMax }', + '}', + 'func labels(_ i: Int) -> String { return Constants.STATUS_NAMES[i] }', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['labels'])); + expect(valueRefReaders(cg, 'MAX_RETRIES')).toEqual(expect.arrayContaining(['retries'])); + expect(valueRefReaders(cg, 'topLevelMax')).toEqual(expect.arrayContaining(['within'])); + // An instance `let` is per-object state (kind `field`), never a target. + expect(valueRefReaders(cg, 'instanceField')).toEqual([]); + }); + + it('does NOT edge a Swift static const shadowed by a function-local let of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'Shadow.swift'), + [ + 'enum Config {', + ' static let TIMEOUT = 30', + ' static func usesConst() -> Int { return TIMEOUT }', + ' static func shadows() -> Int { let TIMEOUT = 5; return TIMEOUT }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges readers to a top-level const and a class static const/final (Dart)', async () => { + // Dart's grammar uses `static_final_declaration` for exactly the top-level + // `const`/`final` and class `static const`/`static final` — the shared + // constants — so those extract as `constant`. Instance fields and `var` + // (`initialized_identifier`) and locals (`initialized_variable_definition`) + // are NOT this node, so they never become targets. Dart attaches a method + // body as a sibling of the signature, so the reader-scan pulls that in. + fs.writeFileSync( + path.join(dir, 'demo.dart'), + [ + 'const TOP_LEVEL_MAX = 100;', + 'class Config {', + ' static const TIMEOUT_MS = 30;', + ' static final STATUS_NAMES = ["ok", "fail"];', + ' final int instanceField = 1;', + ' int capped(int n) => n > TIMEOUT_MS ? TIMEOUT_MS : n;', + ' String label(int i) { return STATUS_NAMES[i]; }', + ' int withinLimit(int n) => n < TOP_LEVEL_MAX ? n : TOP_LEVEL_MAX;', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT_MS')).toEqual(expect.arrayContaining(['capped'])); + expect(valueRefReaders(cg, 'STATUS_NAMES')).toEqual(expect.arrayContaining(['label'])); + expect(valueRefReaders(cg, 'TOP_LEVEL_MAX')).toEqual(expect.arrayContaining(['withinLimit'])); + // An instance field is per-object state, never a value-ref target. + expect(valueRefReaders(cg, 'instanceField')).toEqual([]); + }); + + it('does NOT edge a Dart const shadowed by a method-local const of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'shadow.dart'), + [ + 'const TIMEOUT = 30;', + 'class C {', + ' int usesConst() => TIMEOUT;', + ' int shadows() { const TIMEOUT = 5; return TIMEOUT; }', + '}', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + + it('edges same-file functions to a unit-scope const (Pascal)', async () => { + // Pascal keeps shareable constants in a `const` section at unit (file) scope + // (and class scope). They already extract as `constant`. A const reference is + // an `identifier`; the catch is that Pascal attaches a proc body (`block`) as + // a sibling of the proc header (`declProc`, the reader scope), so the + // reader-scan pulls in that sibling. + fs.writeFileSync( + path.join(dir, 'demo.pas'), + [ + 'unit Demo;', + 'interface', + 'const', + ' MAX_ITEMS = 100;', + " APP_NAME = 'MyApp';", + 'implementation', + 'function Capped(n: Integer): Integer;', + 'begin', + ' if n > MAX_ITEMS then Capped := MAX_ITEMS else Capped := n;', + 'end;', + 'function AppLabel: string;', + 'begin', + ' AppLabel := APP_NAME;', + 'end;', + 'end.', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'MAX_ITEMS')).toEqual(expect.arrayContaining(['Capped'])); + expect(valueRefReaders(cg, 'APP_NAME')).toEqual(expect.arrayContaining(['AppLabel'])); + }); + + it('does NOT edge a Pascal unit const shadowed by a function-local const of the same name', async () => { + fs.writeFileSync( + path.join(dir, 'shadow.pas'), + [ + 'unit Shadow;', + 'interface', + 'const', + ' TIMEOUT = 30;', + 'implementation', + 'function UsesConst: Integer;', + 'begin', + ' UsesConst := TIMEOUT;', + 'end;', + 'function Shadows: Integer;', + 'const TIMEOUT = 5;', + 'begin', + ' Shadows := TIMEOUT;', + 'end;', + 'end.', + ].join('\n'), + ); + cg = index(); + await cg.indexAll(); + + expect(valueRefReaders(cg, 'TIMEOUT')).toEqual([]); + }); + it('emits nothing when CODEGRAPH_VALUE_REFS=0', async () => { const prev = process.env.CODEGRAPH_VALUE_REFS; process.env.CODEGRAPH_VALUE_REFS = '0'; diff --git a/docs/design/value-reference-edges-playbook.md b/docs/design/value-reference-edges-playbook.md new file mode 100644 index 0000000..17bb78b --- /dev/null +++ b/docs/design/value-reference-edges-playbook.md @@ -0,0 +1,544 @@ +# Playbook: extend value-reference edges to a new language + +**Purpose.** This is the operational runbook for adding + validating value-reference-edge +coverage for one more language. Point a fresh session at this file and say **"Start on +language X"** — it has everything: how the feature works, where the code is, the exact +validation recipe (with scripts), the per-language checklist, and the traps already hit. + +Design rationale + the validation matrix already done live in the companion doc: +[`value-reference-edges.md`](./value-reference-edges.md). This file is the *how-to*. + +--- + +## 0. "Start on language X" — do this in order + +1. Read §1 (how it works) and §2 (current state) so you know the mechanism and what's done. +2. Do the **per-language wiring check** (§5 step A–C) — this is where languages differ and + where most of the real work/decisions are. Do NOT skip: a wrong declarator node type or a + class-scope-vs-file-scope mismatch makes the feature silently emit nothing (or wrong edges). +3. Run the **validation sweep** (§4) on small/medium/large **public OSS** repos for that + language. Hunt FPs. **Fix FP clusters; record singletons.** (See §3 for what a real FP + looks like vs an acceptable one.) +4. Add a **row to the matrix** in `value-reference-edges.md` and a **test case** in + `__tests__/value-reference-edges.test.ts`. +5. Commit on a branch, open a PR. (§6 has the git workflow + how the prior PRs were done.) + +Scope rule (hard): **never eval on the maintainer's own repos** — clone a real public OSS +repo for the language. (Memory: `agent-eval-targets-public-oss-only`.) + +--- + +## 1. How value-reference edges work + +**What:** a `references` edge with `metadata: { valueRef: true }` from a *reader symbol* to +the **file-scope `const`/`var` it reads**, same-file only. It exists so impact analysis +catches "change this constant / config object / lookup table → affect its readers" — a class +of change calls/imports/inheritance edges never captured (a const's consumers used to look +like "nothing depends on this"). + +**Where it flows:** straight into `getImpactRadius` → `codegraph impact` and the impact trail +in `codegraph_explore` / `codegraph_node`. No agent-behaviour change required. **The win is +impact-radius correctness** (a const 90 symbols read going from "1 affected" to "90"), *not* +agent read-reduction (see §4.3). + +**Code — all in `src/extraction/tree-sitter.ts`:** + +| Symbol | Role | +|---|---| +| `VALUE_REF_LANGS` (static Set) | languages the feature runs for. Currently `typescript`, `javascript`, `tsx`, `go`, `python`, `rust`, `ruby`, `c`, `java`, `csharp`, `php`, `scala`, `kotlin`, `swift`, `dart`, `pascal`. **Add the new language here.** | +| `valueRefsEnabled` | `process.env.CODEGRAPH_VALUE_REFS !== '0'` — default ON, env opts out. | +| `MAX_VALUE_REF_NODES` (20_000) | per-scope traversal cap (and the shadow-scan cap). | +| `captureValueRefScope(kind, name, id, node)` | called from `createNode` on every node. Records **targets** (file-scope `const`/`var`) and **reader scopes** (`function`/`method`/`const`/`var`). | +| `flushValueRefs()` | called once at end of `extract()`. Prunes shadowed targets, then for each reader scope walks its subtree for identifiers matching a target name and emits the edges. | + +**The two gates inside `captureValueRefScope`** (what you may need to adjust per language): + +- **Target gate:** `kind ∈ {constant, variable}` **and** `name.length >= 3` **and** + `/[A-Z_]/.test(name)` (distinctive name — dodges single-letter / all-lowercase shadowing) + **and** the node's parent id starts with `file:`, `class:`, or `module:` (file/class/module scope). +- **Reader gate:** `kind ∈ {function, method, constant, variable}`. + +**The emit loop in `flushValueRefs`:** same-file only (targets + scopes are per-file, reset +each flush); deduped per `(reader, target)`; skips `isGeneratedFile(path)`; **prunes shadowed +targets** (see §3). + +--- + +## 2. Current state (what's shipped + validated) + +- **Default ON** for TS/JS/tsx + Go + Python + Rust + Ruby + C + Java + C# (`CODEGRAPH_VALUE_REFS=0` disables). Shipped in **PR #895** + (flip-on + the shadow prune); Go added in a later PR (the shadow-prune declarator switch + + `VALUE_REF_LANGS`); C added later still (extractor change to emit the nodes + the bare-identifier + misparse guard); Java + C# after that (field→constant kind switch for the const subset). +- **Validated S/M/L** in **TS, JS, tsx, Go, Python, Rust, Ruby, C, Java, and C#** — see the matrix in the + design doc. All clean: node count identical on/off, precision guards held, impact win + reproduced. Go required extending the shadow prune (per-grammar declarators) — the worked + example of "step B is load-bearing." **C required the Ruby treatment** (the extractor didn't emit + C file-scope const/var nodes at all) **plus** a C-specific FP guard (a macro-prefixed-prototype + misparse mints a bare-identifier "variable" named after the return type — skip bare-`identifier` + declarators). It was the worked example of "the §2b coverage table's *easy-path* guess can be + wrong — always do §5 step C (confirm the nodes exist) before trusting it." +- **Java + C# were the cleanest class-scope ("Ruby treatment") languages.** The constants already + extract — but as `field` kind, which the gate rejects. The whole change was emitting the const + *subset* as `constant`: an `isConst` predicate on each extractor (Java `static final`; C# `const` + / `static readonly`) + a kind switch in `extractField`. **No new shadow-prune wiring** (method + locals are `variable_declarator`, already in the switch) and **no FP guards** (UPPER_SNAKE / + PascalCase fit the distinctive-name gate). Instance `final`/`readonly` fields correctly stay + `field`. Validated S/M/L: gson/commons-lang/guava, automapper/newtonsoft/efcore — 0 leaks, node + parity, big impact wins (`INDEX_NOT_FOUND` 4→165, `_resourceManager` 22→1664). +- **PHP was the cleanest of all — one reader-scan line.** Constants already extract as `constant` + (top-level + class), so the only change was teaching the reader-scan that a PHP constant + *reference* is a `name` node (bare `X`, or the const half of `self::X` / `Foo::X`). **No extractor + change, no prune wiring** (a `$var` local can't shadow a bare constant — different namespace). + Validated S/M/L (guzzle/monolog/laravel), all clean, 0 class/const collisions. The honest caveat: + **lower yield** — PHP reads constants cross-file far more than same-file (laravel 2,956 files → 86 + edges), and value-refs is same-file only; still correct, just a smaller contribution. +- **Scala — an `object` is the constant scope.** Scala has no `static`; a singleton `object`'s `val`s + are the shared-constant idiom (`object Config { val Timeout = 30 }`). Top-level `val` already + extracted as `constant`, but object/class vals both came out as `field`. The fix: in the Scala + `val_definition` handler, walk to the enclosing definition — `object_definition` (or top-level) → + `constant`/`variable`; `class`/`trait`/`enum` → `field` (per-instance, like Java instance `final`). + Added `val_definition`/`var_definition` to the shadow prune (method-local `val` shadows). Reader-scan + needed nothing (refs are `identifier`). Minor known limitation: Scala uses `val`/`def` + interchangeably for members, so a camelCase val can share a name with a method — same-file name + matching can't tell them apart (bounded, like Ruby's sibling-class; sweep showed flagged collisions + were mostly real object vals read by siblings). Validated S/M/L (upickle/cats/pekko). +- **C++ was attempted and reverted — DON'T retry without solving parse fidelity first.** tree-sitter-cpp + mis-parses real template/macro-heavy C++ (and `.h` files route to the C grammar): class members and + parameters leak to file scope as bogus constants/variables. Two guards (skip `ERROR`-ancestor and + `compound_statement`-ancestor declarations) removed ~83% of gross leaks, but the residual pervades + even well-structured library source (template-class member leaks, amalgamated mega-headers, + `.h`-as-C++). It did not reach the precision bar of the other languages. See the C++ section below. +- **Kotlin = C + Scala + PHP techniques combined (and clean).** Nothing extracted before (property name + nests `property_declaration → variable_declaration → simple_identifier` — the C problem). Fix: + handle `property_declaration` in the Kotlin `visitNode` hook — pull the nested name, walk to the + enclosing definition for the kind (`object`/`companion object`/top-level → `constant`/`variable`; + `class` → `field` — the Scala rule; skip locals under a `function_body`/`init`/lambda), add + `simple_identifier` to the reader-scan (the PHP-`name` move), and `property_declaration` to the + shadow prune. Clean parse fidelity (the one `fun interface` misparse is already handled), so no + C++-style tail. One of the cleanest yields — companion-object bit-masks/state consts are a heavy + same-file-read idiom. Validated S/M/L (okio/coroutines/ktor); only the bounded val/def-or-class and + sibling-companion name overlaps remain (shared with Scala/Ruby). +- **Swift reused Kotlin + two Swift-specific touches.** Top-level `let` + `static let` in a type are + the shared constants (`enum`/`struct` namespace them); instance `let` stays `field`. Nested name + (`property_declaration → pattern → simple_identifier`); reader-scan already covered + (`simple_identifier`, from Kotlin). Two new things: **(1) the target gate was widened to `struct:`/ + `enum:` parents** — Swift namespaces constants there (`enum Constants { static let X }`), and every + other language's targets are `file:`/`class:`/`module:`; **(2) computed properties are skipped** (a + `var x:Int{ … }` getter has no stored value — detect the `computed_property` child). Node creation + slots into the *existing* Swift `property_declaration` handler (property-wrapper/type deps), leaving + that untouched. Clean parse, no tail. Validated S/M/L (Alamofire/swift-argument-parser/swift-nio). +- **Dart — clean grammar separation, but a sibling-body reader-scan fix.** Dart's grammar already + splits the cases: **`static_final_declaration`** is *exactly* a top-level/`static` `const`/`final` + (the shared-constant idiom), while instance fields/`var` use `initialized_identifier` and locals use + `initialized_variable_definition` — so extracting `static_final_declaration` → `constant` (in a + `visitNode` hook) has **no instance/local leaks to guard**. Reader-scan free (Dart refs are + `identifier`). The catch was the **reader-scan**: Dart attaches a method/function `body` as a *next + sibling* of the signature node (the stored scope), not a child, so the scan saw only the signature + and **found nothing** until it was taught to pull in a `function_body` next-sibling (Dart-only among + the value-ref set). Shadow prune needed `static_final_declaration` + `initialized_identifier` + + `initialized_variable_definition` (a local `const X` shadowing a file `const X`). Validated S/M/L + (http/flame/flutter-packages). **Caveat:** generated Dart files inflate the sibling-class ambiguity + (a JNIGEN `_bindings.dart` with hundreds of `static final _class` collapses to the file-wide target). + The common codegen suffixes (`.g.dart`/`.freezed.dart`/`.pb.dart`) are already filtered by + `isGeneratedFile`; header-only-marked generators (JNIGEN) are not, so real source is clean but + generated FFI/JNI bindings are noisy. +- **Pascal — the genuine easy path + the Dart sibling-body fix again.** Unit/class `const` *already* + extracted as `constant` (`variableTypes: ['declConst', …]`), so it was add-to-`VALUE_REF_LANGS` + + the shadow prune (`declConst`/`declVar`; a local `const X` shadows a unit `const X`). The catch was + the *same* reader-scan bug as Dart: Pascal's proc body is a **`block` sibling** of the `declProc` + header (the reader scope), both under a `defProc` — so the same sibling-pull fix was extended to + `block`. Reader-scan node type already covered (refs are `identifier`). **Low yield** — Pascal reads + constants cross-unit more than same-file (horse: 4 edges). **Caveat:** Pascal is case-insensitive, + but the reader-scan matches exact text, so a differently-cased reference is missed (no FP, just a + miss); not worth normalizing. +- **Tests:** `__tests__/value-reference-edges.test.ts` — same-file readers edged; surfaced in + impact radius; shadowed const NOT edged (verified to fail without the guard); JSX-only read + edged (tsx); `CODEGRAPH_VALUE_REFS=0` emits nothing. +- **Memory:** `value-reference-edges-default-on` (the A/B finding + shadow guard rationale). + +--- + +## 2b. Coverage vs the README (languages + frameworks) + +Tracked against the README's **Supported Languages** table (24 rows) and **Framework-aware +Routes** list. Value-refs is **language-level**, so frameworks are *not* a separate axis (see +the bottom of this section). + +**✅ Done — validated S/M/L (15 + 3 inherited):** + +| Language | How | +|---|---| +| TypeScript, JavaScript, tsx | file-scope `const`/`var`; the original languages | +| Python | module-level `NAME =` | +| Go | package `const`/`var` | +| Rust | module + impl `const`/`static` | +| Ruby | class/module `CONST` (the class-scope extension) | +| C | file-scope `static const` scalars + pointer/array lookup tables + mutable globals. **Needed an extractor change** (nodes weren't emitted) + a bare-identifier misparse guard — NOT the easy path the table below first guessed | +| Java | class `static final` fields. Nodes existed as `field` kind; emitted the const subset as `constant` (`isConst` + `extractField` kind switch). No new prune wiring, no FP guards | +| C# | class `const` / `static readonly`. Identical to Java — same `field`→`constant` change | +| PHP | top-level `const` + class `const` (both already `constant` kind). **Only** change was the reader-scan: a PHP const *reference* is a `name` node. No extractor change, no prune wiring (a `$var` local can't shadow a bare constant). Lower yield — PHP reads consts cross-file more than same-file | +| Scala | top-level `val` (already `constant`) + **`object` val** (the singleton-constant idiom; re-kinded from `field` by walking to the enclosing `object_definition`). `class`/`trait`/`enum` vals stay `field`. `val_definition`/`var_definition` added to the shadow prune. Minor val/def name-collision limit | +| Kotlin | top-level / `object` / `companion object` `val` (re-kinded from nothing — properties weren't extracted at all). Handled in `visitNode`: nested name (`variable_declaration → simple_identifier`, the C move) + scope-walk for kind (Scala move) + `simple_identifier` in the reader-scan (PHP move) + prune. `class` instance vals stay `field`. Clean — one of the best yields (companion bit-masks) | +| Swift | top-level `let` + `static let` in `struct`/`enum`/`class`. Reused Kotlin (nested name + `simple_identifier` reader-scan). Two Swift touches: **gate widened to `struct:`/`enum:` parents** (Swift namespaces consts there), and **computed properties skipped**. `class`/instance stored props stay `field`. Slots into the existing Swift property-wrapper handler | +| Dart | top-level `const`/`final` + class `static const`/`static final` — all the **`static_final_declaration`** node, cleanly separated by the grammar from instance/`var`/local (so no leak guard). `visitNode` → `constant`. Needed a reader-scan fix: Dart's method **body is a next sibling** of the signature, so the scan pulls in a `function_body` sibling. Generated-FFI noise (JNIGEN `_bindings.dart`) is the one caveat | +| Pascal / Delphi | unit/class `const` (already extracted as `constant`). Add-to-`VALUE_REF_LANGS` + shadow prune (`declConst`/`declVar`) + the **same Dart sibling-body fix** (Pascal's proc body is a `block` sibling of the `declProc` header). Low yield (cross-unit reads); case-insensitive (exact-text scan misses re-cased refs) | +| **Svelte, Vue, Astro** | **inherited for free** — their extractors re-parse the `