Skip to content

Comments

feat(parser): improve error message for missing namespace/module names#15720

Closed
camc314 wants to merge 1 commit intomainfrom
c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names
Closed

feat(parser): improve error message for missing namespace/module names#15720
camc314 wants to merge 1 commit intomainfrom
c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names

Conversation

@camc314
Copy link
Contributor

@camc314 camc314 commented Nov 15, 2025

BEFORE:
When parsing declare namespace { or module {, oxc reported:

  "Expected a semicolon or an implicit semicolon after a statement, but found none"

This was confusing because the actual issue is a missing name, not a semicolon.

AFTER:
Now reports TypeScript-compatible error TS(1437):

  "Namespace must be given a name."
  "Module must be given a name."

@camc314 camc314 marked this pull request as ready for review November 15, 2025 13:55
@github-actions github-actions bot added A-parser Area - Parser C-enhancement Category - New feature or request labels Nov 15, 2025
Copy link
Contributor Author

camc314 commented Nov 15, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 15, 2025

CodSpeed Performance Report

Merging #15720 will not alter performance

Comparing c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names (6d0b1b5) with main (f7ef032)

Summary

✅ 42 untouched
⏩ 3 skipped1

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel changed the base branch from 11-06-fix_ast_add_tsglobaldeclaration_type to graphite-base/15720 November 15, 2025 15:20
@overlookmotel
Copy link
Member

@camc314 Just in case you're not aware, tests failing. I'm not sure if the error in the diff is correct or not.

@camc314 camc314 force-pushed the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch from fa9fd0f to 9729540 Compare November 18, 2025 09:16
@github-actions github-actions bot added A-linter Area - Linter A-semantic Area - Semantic A-cli Area - CLI A-minifier Area - Minifier A-ast Area - AST A-transformer Area - Transformer / Transpiler A-codegen Area - Code Generation A-cfg Area - Control Flow Graph A-isolated-declarations Isolated Declarations A-ast-tools Area - AST tools A-editor Area - Editor and Language Server A-formatter Area - Formatter A-linter-plugins Area - Linter JS plugins labels Nov 18, 2025
@camc314 camc314 force-pushed the graphite-base/15720 branch from 3f85660 to 646d020 Compare November 18, 2025 10:03
@camc314 camc314 force-pushed the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch from 9729540 to 704b561 Compare November 18, 2025 10:03
@camc314 camc314 changed the base branch from graphite-base/15720 to main November 18, 2025 10:03
@camc314 camc314 changed the base branch from main to graphite-base/15720 November 18, 2025 10:03
@camc314 camc314 force-pushed the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch from 704b561 to d230bf8 Compare November 18, 2025 10:03
@camc314 camc314 changed the base branch from graphite-base/15720 to c/11-18-test_linter_prefer-namespace-keyword_remove_test_case_with_invalid_syntax November 18, 2025 10:04
@graphite-app graphite-app bot changed the base branch from c/11-18-test_linter_prefer-namespace-keyword_remove_test_case_with_invalid_syntax to graphite-base/15720 November 18, 2025 10:05
@graphite-app graphite-app bot force-pushed the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch from d230bf8 to 53015ea Compare November 18, 2025 10:10
@graphite-app graphite-app bot force-pushed the graphite-base/15720 branch from 1ea734c to f7ef032 Compare November 18, 2025 10:10
@graphite-app graphite-app bot changed the base branch from graphite-base/15720 to main November 18, 2025 10:10
**BEFORE:**
When parsing `declare namespace {` or `module {`, oxc reported:
```
  "Expected a semicolon or an implicit semicolon after a statement, but found none"
```
This was confusing because the actual issue is a missing name, not a semicolon.

**AFTER:**
Now reports TypeScript-compatible error TS(1437):
```
  "Namespace must be given a name."
  "Module must be given a name."
```
@graphite-app graphite-app bot force-pushed the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch from 53015ea to 6d0b1b5 Compare November 18, 2025 10:11
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the parse_binding_identifier_with_error function, will take a look later.

@Boshen Boshen marked this pull request as draft November 19, 2025 13:55
@overlookmotel overlookmotel removed the A-linter-plugins Area - Linter JS plugins label Dec 1, 2025
@camc314 camc314 closed this Dec 10, 2025
@overlookmotel overlookmotel deleted the c/11-15-feat_parser_improve_error_message_for_missing_namespace_module_names branch December 10, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast Area - AST A-ast-tools Area - AST tools A-cfg Area - Control Flow Graph A-cli Area - CLI A-codegen Area - Code Generation A-editor Area - Editor and Language Server A-formatter Area - Formatter A-isolated-declarations Isolated Declarations A-linter Area - Linter A-minifier Area - Minifier A-parser Area - Parser A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants