Conversation
Instead of issuing an error for `module X {}`, the parser now always
treats it as `namespace X {}`. Only `declare module "foo"` and
`namespace` syntax are distinguished.
Changes:
- Parser: Always use KindNamespaceKeyword for identifier-named module declarations
- Checker: Remove suggestion diagnostic for module keyword usage
- Checker: Remove unused getNonModifierTokenRangeOfNode utility
- Printer tests: Update expected output for module->namespace conversion
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
|
@copilot the intent is that module is illegal and produces a parse error |
- Parser emits parse error on each identifier name when `module`
keyword is used for namespace declarations (matching TS behavior)
- Dotted names like `module A.B {}` get errors on both `A` and `B`
- AST still uses KindNamespaceKeyword for error recovery
- Baselines now match TypeScript exactly (diff files deleted)
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Updated in e50c1d7. The parser now emits a parse error (TS1540) on each identifier name when |
|
I guess this is fine, though I think I was hoping to hard remove it and not even have this diagnostic, as though |
There was a problem hiding this comment.
Pull request overview
Removes support for module-keyword namespace declarations by turning them into parser errors, aligning behavior and baselines with upstream TypeScript.
Changes:
- Parser now errors on
module X {}(and per-component on dotted names) and usesKindNamespaceKeywordfor recovery. - Checker cleanup: removes now-unreachable suggestion diagnostics and a related utility.
- Updates/removes printer tests and baseline diff artifacts to match TypeScript outputs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/parser/parser.go | Emits parse errors for module-keyword namespace declarations; normalizes AST keyword to namespace for recovery. |
| internal/checker/checker.go | Removes unreachable suggestion diagnostic for module-keyword namespace declarations. |
| internal/checker/utilities.go | Removes unused helper previously used for the suggestion diagnostic span. |
| internal/printer/printer_test.go | Drops printer cases that now fail to parse (module a{}, module a.b{}). |
| testdata/baselines/reference/submodule/compiler/moduleKeywordDeprecated.errors.txt.diff | Deletes baseline diff artifact. |
| testdata/baselines/reference/submodule/compiler/moduleKeywordDeprecated.errors.txt | Adds/updates baseline error output in the non-diff format. |
| testdata/baselines/reference/submodule/compiler/escapedIdentifiers.errors.txt.diff | Deletes baseline diff artifact. |
| testdata/baselines/reference/submodule/compiler/escapedIdentifiers.errors.txt | Updates baseline to include the newly parser-emitted TS1540 error. |
It's better to just gracefully parse it for as long as we can and issue a parse error. |
jakebailey
changed the title
There was a problem hiding this comment.
It's very silly that we report this twice, but probably fine.
There was a problem hiding this comment.
This could be fixable by tracking when we are nesting and skipping errors. This PR already adds a bit of plumbing.
@copilot Try it.
There was a problem hiding this comment.
Fixed in 878236d. Now only the first component of dotted names gets the error — added !nested to the error condition since parseModuleOrNamespaceDeclaration already tracks nesting via the nested parameter.
There was a problem hiding this comment.
Well, that worked, but is different than Strada. Probably for the best
Only report the module keyword error on the first component of dotted
names (e.g., `module A.B {}` now only errors on `A`, not both `A` and `B`).
The `nested` parameter already tracks this, so just add `!nested` to
the error condition.
Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
|
@copilot this is failing tests, you didn't run the whole suite |
The local test case at testdata/tests/cases/compiler/reachabilityChecks11.ts used `module` keyword for namespace declarations, which our parser now errors on. Updated to use `namespace` keyword to match the submodule version (which was already updated by TypeScript PR #62876). Co-authored-by: jakebailey <5341706+jakebailey@users.noreply.github.com>
Fixed in 8088408. The local test file |
modulekeyword is used for namespace declarationsmodule A.B {}→ error onAonly)KindNamespaceKeywordfor error recoverygetNonModifierTokenRangeOfNodeutilitymodule a{}andmodule a.b{}tests (now produce parse errors)reachabilityChecks11.tsto usenamespaceinstead ofmodule💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.