Context
extractSymbolsFromPatch in src/review/impact-symbols.ts extracts exported top-level symbol names touched by a PR diff, feeding the deterministic impact-map computation (#2183). The matcher is EXPORTED_DECLARATION_RE (defined at line 37):
const EXPORTED_DECLARATION_RE =
/^export\s+(?:default\s+)?(?:async\s+)?(?:function\*?\s+([\w$]+)|class\s+([\w$]+)|(?:const|let|var)\s+([\w$]+)|interface\s+([\w$]+)|type\s+([\w$]+)|enum\s+([\w$]+))/;
Two concrete, verified regex-variant gaps:
export const enum Direction {} — since the (?:const|let|var)\s+([\w$]+) alternative is tried before the enum alternative, the regex matches const and captures the literal word enum itself as group 3. boundaryKindForMatch (line 40) and extractSymbolsFromPatch then emit a symbol literally named "enum" instead of the real symbol name "Direction". Confirmed via direct regex test against the exact pattern.
export abstract class Foo {} — there is no abstract alternative anywhere in the pattern, so this line doesn't match at all. A changed/added abstract base class silently contributes zero symbols to the impact map.
Both are unguarded regex-variant gaps in a pure, side-effect-free extractor. test/unit/impact-symbols.test.ts only exercises function/class/const =>/interface/type/enum — grepping that file confirms no const enum or abstract class case exists, so both branches are currently untested (and, for the const enum case, silently wrong today rather than merely uncovered).
Requirements
- Fix
EXPORTED_DECLARATION_RE so export const enum Foo {} is recognized as an enum declaration named Foo (not a const declaration named "enum") — the enum alternative (optionally preceded by const) must be tried before, or take precedence over, the bare const|let|var alternative for this specific const enum shape.
- Add an
abstract alternative so export abstract class Foo {} (and export default abstract class Foo {}, mirroring the existing default-prefix handling) is recognized as a class declaration named Foo.
boundaryKindForMatch must still correctly classify the new capture groups (an abstract class still reports kind: "class"; a const enum still reports kind: "export", matching how a plain enum is classified today — verify this against the existing group-index logic at lines 40-44 and adjust the capture-group count/positions consistently if the regex's group ordering changes).
- No change to
extractSymbolsFromPatch's or extractChangedSymbols's public signatures, or to the fail-safe behavior for unparseable/empty patches.
Deliverables
Test Coverage Requirements
Aim for 99%+ Codecov patch coverage (100% including both new branches) on the touched lines in src/review/impact-symbols.ts. Both new cases need dedicated regression tests — the const enum case is a correctness fix (wrong symbol name today, not just missing), so its test must assert the correct captured name, not merely that the regex matches.
Expected Outcome
A diff adding, modifying, or removing export const enum ... or export abstract class ... declarations is now correctly reflected in the deterministic impact-map's changed-symbol extraction — const enum no longer surfaces a bogus symbol literally named "enum", and abstract class declarations are no longer silently dropped from impact-map input.
Links & Resources
src/review/impact-symbols.ts (EXPORTED_DECLARATION_RE, line 37; boundaryKindForMatch, lines 40-44; extractSymbolsFromPatch, lines 54-72)
test/unit/impact-symbols.test.ts (existing test suite to extend)
- Related:
#2182 (original deterministic changed-symbol extraction), #2183 (impact-map computation that consumes this module's output)
Context
extractSymbolsFromPatchinsrc/review/impact-symbols.tsextracts exported top-level symbol names touched by a PR diff, feeding the deterministic impact-map computation (#2183). The matcher isEXPORTED_DECLARATION_RE(defined at line 37):Two concrete, verified regex-variant gaps:
export const enum Direction {}— since the(?:const|let|var)\s+([\w$]+)alternative is tried before theenumalternative, the regex matchesconstand captures the literal wordenumitself as group 3.boundaryKindForMatch(line 40) andextractSymbolsFromPatchthen emit a symbol literally named"enum"instead of the real symbol name"Direction". Confirmed via direct regex test against the exact pattern.export abstract class Foo {}— there is noabstractalternative anywhere in the pattern, so this line doesn't match at all. A changed/added abstract base class silently contributes zero symbols to the impact map.Both are unguarded regex-variant gaps in a pure, side-effect-free extractor.
test/unit/impact-symbols.test.tsonly exercisesfunction/class/const =>/interface/type/enum— grepping that file confirms noconst enumorabstract classcase exists, so both branches are currently untested (and, for theconst enumcase, silently wrong today rather than merely uncovered).Requirements
EXPORTED_DECLARATION_REsoexport const enum Foo {}is recognized as anenumdeclaration namedFoo(not aconstdeclaration named"enum") — theenumalternative (optionally preceded byconst) must be tried before, or take precedence over, the bareconst|let|varalternative for this specificconst enumshape.abstractalternative soexport abstract class Foo {}(andexport default abstract class Foo {}, mirroring the existingdefault-prefix handling) is recognized as aclassdeclaration namedFoo.boundaryKindForMatchmust still correctly classify the new capture groups (an abstract class still reportskind: "class"; aconst enumstill reportskind: "export", matching how a plainenumis classified today — verify this against the existing group-index logic at lines 40-44 and adjust the capture-group count/positions consistently if the regex's group ordering changes).extractSymbolsFromPatch's orextractChangedSymbols's public signatures, or to the fail-safe behavior for unparseable/empty patches.Deliverables
EXPORTED_DECLARATION_RE(andboundaryKindForMatchif its group-index mapping changes) updated to correctly handleexport const enum Foo {}andexport abstract class Foo {}(plus theexport default abstract class Foo {}variant).test/unit/impact-symbols.test.tsfor both cases:export const enum Direction {}extracts the symbol"Direction"withkind: "export"(not a symbol literally named"enum");export abstract class Foo {}extracts the symbol"Foo"withkind: "class".enum,class,function,const/let/var,interface, andtypedeclarations continue to pass with identical output.Test Coverage Requirements
Aim for 99%+ Codecov patch coverage (100% including both new branches) on the touched lines in
src/review/impact-symbols.ts. Both new cases need dedicated regression tests — theconst enumcase is a correctness fix (wrong symbol name today, not just missing), so its test must assert the correct captured name, not merely that the regex matches.Expected Outcome
A diff adding, modifying, or removing
export const enum ...orexport abstract class ...declarations is now correctly reflected in the deterministic impact-map's changed-symbol extraction —const enumno longer surfaces a bogus symbol literally named"enum", andabstract classdeclarations are no longer silently dropped from impact-map input.Links & Resources
src/review/impact-symbols.ts(EXPORTED_DECLARATION_RE, line 37;boundaryKindForMatch, lines 40-44;extractSymbolsFromPatch, lines 54-72)test/unit/impact-symbols.test.ts(existing test suite to extend)#2182(original deterministic changed-symbol extraction),#2183(impact-map computation that consumes this module's output)