cowork: namespace & side-effect imports consume a module's whole export surface - #49
Conversation
…s Revenue Holdings / stale 2026 year); W-directed fleet-wide pass
…d-code scan
Named (`export { X } from './mod'`), renamed (`export { X as Y }`),
type (`export { type X }`), and star (`export * from './mod'`) re-exports
now mark the forwarded symbols as used, so barrel/index files no longer
produce false-positive 'unused_export' findings flagged removable=True
(which could delete live public API). Resolves `export *` specifiers to
scanned files (incl. directory index.*). Adds TestReexportForwarding
(8 cases) + removes a pre-existing F841 unused var. 113 tests pass, ruff clean.
… mixed default+named imports, and correct group-index reversal
- Rewrote _IMPORT_PATTERN regex to handle: import type {Foo}, import Default, {Named},
import {type Foo}, and import Foo as Bar forms
- Fixed _parse_imports group-number reversal (group 1 = named imports block, group 2 = default)
- Strips 'type ' prefix from named import entries in both named-block positions
- All 113 existing tests pass; ruff clean
…code # Conflicts: # CHANGELOG.md # src/deadcode/scanner.py # tests/test_scanner.py
…fect imports as whole-module consumption A namespace binding (import * as Utils from './utils') or a bare side-effect import (import './polyfill') consumes the target module's entire export surface. The scanner previously ignored both forms entirely, so exports used ONLY through them were falsely reported as unused with removable=True — live code queued for deletion by 'deadcode remove'. Both now resolve like barrel star-reexports: the resolved module's exports are treated as used. Bare package specifiers stay unresolvable and keep flagging. +5 regression tests (namespace, export * as ns, side-effect, bare-specifier, no-consumer control). Full suite: 121 passed, ruff clean.
The previous commit (2ef1848) was built from a stale temp index and accidentally recorded deletions of 34 unrelated tracked files. This commit restores the full tree of 30e09bb while keeping the intended scanner fix (namespace/side-effect imports as whole-module consumption) and its 5 regression tests. No force-push used.
…heckout-index efa7ce2 restored the tree but its checkout-index step reverted src/deadcode/scanner.py to the pre-fix version. This commit re-applies the scanner fix from 2ef1848: import * as NS / bare side-effect imports consume the target module's whole export surface (resolves like barrel star-reexports). Final tree vs master-base 30e09bb = exactly scanner.py fix + 5-test file.
🤖 Automated Code Review✅ Ruff Lint — No issues
|
…s whole export surface (previously invisible -> exports used only via lazy loading flagged removable=True); +3 regression tests
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9624324dc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # exports used only via a namespace are falsely reported as unused with | ||
| # removable=True — live code queued for deletion. | ||
| _NAMESPACE_IMPORT_PATTERN = re.compile( | ||
| r"import\s+\*\s+as\s+\w+\s+from\s*['\"]([^'\"]+)['\"]" |
There was a problem hiding this comment.
When a TypeScript consumer uses the valid import type * as Types from './types' form, the type token prevents this pattern from matching. Consequently, exports referenced as Types.Foo are still reported with removable=True, and a one-line type or interface can be deleted by deadcode remove; allow the optional type modifier before *.
Useful? React with 👍 / 👎.
| for m in _NAMESPACE_IMPORT_PATTERN.finditer(content): | ||
| # `import * as NS from './mod'` — whole-module consumption. | ||
| star_reexports.append((rel_path, m.group(1))) |
There was a problem hiding this comment.
When a source file contains a commented example such as // import * as Utils from './utils', this unanchored regex still matches because _parse_reexports scans the raw file contents. If the path resolves, every export from that module is then treated as used, hiding genuinely unused exports merely because an old import was commented out; import matches need to exclude comments and literals.
Useful? React with 👍 / 👎.
1 participant
Problem
The dead-code scanner ignored two common import forms entirely:
import * as Utils from './utils'(namespace import)import './polyfill'(bare side-effect import)Exports consumed only through these forms were falsely reported as
unused_exportwithremovable=True— meaningdeadcode removewould blank live code. Reproduced pre-fix: a module whose only consumer usedUtils.helper()was flagged removable.Fix
Both forms now resolve their module specifier like barrel star-reexports already do (
_resolve_relative_module): when the target resolves to a scanned file, its entire export surface is treated as used. Bare package specifiers (e.g.'lodash') stay unresolvable and keep flagging local modules correctly.Tests
New
tests/test_namespace_sideeffect_imports.py(5 cases):export * as ns from ...re-export dittoFull suite: 121 passed, ruff clean.