Skip to content

fix(typescript): recognise CommonJS exports, and widen the labels gate - #73

Merged
Rinkia merged 1 commit into
masterfrom
fix/ts-commonjs-exports
Jul 23, 2026
Merged

Rinkia merged 1 commit into
masterfrom
fix/ts-commonjs-exports

Conversation

@Rinkia

@Rinkia Rinkia commented Jul 23, 2026

Copy link
Copy Markdown
Owner

The bug

Most of npm is still CommonJS, where nothing carries the ESM export keyword. The front-end set is_public only from export, so every CommonJS file reported zero public symbols — declarations parsed, then discarded as private, contributing no seams at all.

Found while starting #63 (TS moddability benchmark): pointing detect --language typescript at commander produced a ranking made entirely of type declarations from typings/index.d.ts, while lib/ — the whole implementation, including the documented Command and Help classes — yielded nothing.

The fix

exports.X = X, module.exports.X = X, module.exports = X and module.exports = { X, Y } now mark the named declarations public. A class that is never exported stays private.

After the fix, commander's lib/ surfaces Command, Help, Option, Argument as it should.

The self-check earned its keep immediately — it caught that my first version missed module.exports.Help = Help, a three-level member expression where the object is itself module.exports.

Also: widening the benchmark-labels gate

The gate refused a PR touching both the ground truth and detector.py/graph.py. But a language front-end decides what even becomes a candidate, so a front-end change can flatter a score exactly as a weight change can. It now covers modscan/languages/ too.

This PR is the case in point: it changes a front-end, so it deliberately carries no label change. The commander labels land separately, on their own evidence.

Known, not fixed here

With lib/ now visible, commander's top 8 are still TS interfaces (ParseOptions, HelpContext, OutputConfiguration) scoring 0.70 as "meant to be subclassed", above the actually-documented Command/Help at 0.10. interface → abstract is a deliberate choice in the front-end and is a ranking question — so it gets measured against a benchmark, not asserted. That's the next PR's job.

Test plan

  • tests/test_typescript.py: new CommonJS case — exports.X, module.exports.X recognised; never-exported class stays private; exported/private functions distinguished. Skips cleanly without tree-sitter.
  • Suite 25/25; Python benchmark 9/20 unchanged (no Python path touched).
  • ci.yml parses.

🤖 Generated with Claude Code

Most of npm is still CommonJS, where nothing carries the ESM `export` keyword.
The front-end set is_public only from `export`, so every CommonJS file reported
zero public symbols: the declarations were parsed, then discarded as private,
and the file contributed no seams at all.

Found by pointing `detect --language typescript` at commander, where lib/ — the
whole implementation, including the documented Command and Help classes —
produced nothing, and the ranking was made up entirely of type declarations from
typings/index.d.ts.

Now `exports.X = X`, `module.exports.X = X`, `module.exports = X` and
`module.exports = { X, Y }` mark the named declarations public. A class that is
never exported stays private.

The self-check earned its keep immediately: it caught that the first version of
this fix missed `module.exports.Help = Help`, a three-level member expression
where the object is itself `module.exports`.

Also widens the benchmark-labels gate. It refused a PR touching both the ground
truth and detector.py/graph.py, but a language front-end decides what even
becomes a candidate — so a front-end change can flatter a score exactly as a
weight change can. The gate now covers modscan/languages/ too. This commit is
itself the case in point: it changes a front-end, so it deliberately carries no
label change.

Python benchmark unchanged at 9/20 (this touches no Python path); suite 25/25.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

MODScan attack-surface diff — modscan

No new execution sinks

New attack surface to review — NOT a vulnerability verdict. This compares enumerated execution sinks between two snapshots. It does not trace whether input reaches them (no taint analysis), match CVEs, or detect secrets. An empty diff means no new catalogued sinks — not that the change is safe.

The two snapshots enumerate the same execution sinks.

@github-actions

Copy link
Copy Markdown

MODScan extension-point diff

No breaking changes to extension points

@Rinkia
Rinkia merged commit 251a4bd into master Jul 23, 2026
7 checks passed
@Rinkia
Rinkia deleted the fix/ts-commonjs-exports branch July 23, 2026 14:47
@Rinkia Rinkia mentioned this pull request Jul 23, 2026
3 of 5 tasks
Rinkia added a commit that referenced this pull request Jul 23, 2026
Bump to 0.1.7 and move the [Unreleased] entries under [0.1.7]. Ships the Java
front-end, the security lens over MCP and for TypeScript/JavaScript, the process
sinks the Bandit cross-check exposed, and two TypeScript front-end fixes.

Also adds the two CHANGELOG entries that were missing: recognising CommonJS
exports (#73) and no longer scanning .d.ts declaration files (#76). Both change
what `--language typescript` returns, so both belong in a user-facing changelog.
The cross-check harnesses (#70, #72) and benchmark targets (#74, #78) stay out:
they are repository artifacts, not shipped in the wheel.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant