Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 19, 2025

Fix incorrect parser diagnostic message for duplicate export modifier

  • Explore the repository structure
  • Understand the issue: Parser shows wrong diagnostic for duplicate export modifier
  • Locate the problematic code in crates/oxc_parser/src/modifiers.rs
  • Fix the diagnostic logic in eat_modifiers_before_declaration function
  • Add test case for duplicate export modifier
  • Build and test the fix
  • Verify the fix produces correct diagnostic message
  • Run coverage tests and entire test suite
  • Update snapshots

Changes Made

Modified the eat_modifiers_before_declaration function in crates/oxc_parser/src/modifiers.rs to:

  • Detect when the export modifier appears after an export declaration is already started
  • Emit the correct diagnostic: TS(1030): 'export' modifier already seen. instead of the incorrect TS(1029): 'export' modifier must precede 'declare' modifier.
  • Skip adding the duplicate export modifier to the modifiers list to prevent secondary validation errors
  • Handle the edge case where all modifiers are skipped (empty modifiers list)

Added test case: tasks/coverage/misc/fail/duplicate-export-modifier.ts

Updated snapshots:

  • tasks/coverage/snapshots/parser_misc.snap - Added new test case entry
  • tasks/coverage/snapshots/parser_typescript.snap - Updated diagnostic messages for duplicate export modifiers

Before

  x TS(1029): 'export' modifier must precede 'declare' modifier.
   ,-[:1:8]
 1 | export export function foo() {
   :        ^^^^^^
   `----

  x 'export' modifier cannot be used here.
   ,-[:1:8]
 1 | export export function foo() {
   :        ^^^^^^
   `----
  help: Allowed modifiers are: declare, async

After

  x TS(1030): 'export' modifier already seen.
   ,-[1:8]
 1 | export export function foo() {
   :        ^^^^^^
 2 | }
   `----
  help: Remove the duplicate modifier.

Testing

  • ✅ All parser unit tests pass
  • ✅ Manual verification confirms correct diagnostic message
  • ✅ Verified other duplicate modifier detection still works correctly (e.g., duplicate async)
  • ✅ Test case added to coverage suite
  • ✅ Coverage tests completed successfully
  • ✅ Entire test suite passes
  • ✅ Snapshots updated

Fixes #15870

Original prompt

This section details on the original issue you should resolve

<issue_title>Incorrect parser diagnostic message for duplicate export modifier</issue_title>
<issue_description>https://playground.oxc.rs/#eNqNVF1v0zAU/SuWX2CjqGOANAUhgbZVmjRY2Rh7Qaqc5Kbz5tjBvlk7qv53jpM4HdKEeIpzbs79OPc4G1nITNK6cZ7F8KhaW7B2VpQUyGtl9G86K8myrjT5r6qml40Le2Lz0wphiEVgBdpHMd0XnxaL+fXl6WIh9qdP+ddvDzvSh8iZTiOLbPlfHPFKvOt5nrj1tq8rBD82lIkXu85eTPqARYfZvxJfsR8SH+0NnG6E4YzGutMWRbdyIp3MNtK3Nj6Mtiwz9i1NZOV8rZjJy6xSJgBhr2yI8Ijo4IxiKk+oMMqrKGsYg6tbzRQaVdAI1couDV77CoWrG08BjP49FK4Zg+Gxzp0ZP62WQ5LtRDbKY9jYL62ZbEBVbJnDGtMoY9zqslPyouWgS5oN+06pYknyDzRXHtyEdrwfR2eWvUbGYtcUoWnWxan3zg8oeohKdT3g/ESpjWwDfVf5TgVW+Y0u+VZmhxMJ7S+qc20xpTQV2m1QjYf40QGq6SjQt9bxTrS7sL56Bv4V3+beNaglVXhtiUoqkRNr0mhveezqWsWYMUAxhx6HxSyrND/iK/UY8EnuVXFPfIWVgZ4+Tihs13c+1Hf5HRV841WDHEnUWKjr9TNDyLxFh+T/ouEWwqg1XK3MBY6KnZ+7oPsVRYUkJB2t1osK+y4JzpQUDg/evEcVyHxCFRLPnD82KoSZJlOOW3taBeZ0XZldtNY8ol+IVakYSg2rfd7UGxkwUQODkLcKxkxuLFxJS+quj41GGD17F0qH388gubWOu1wJMbRM33ZZ0l2Ijkr3BJO75pweaEx6T9TEf1QyWEeFaZ2ZwcCR8UA+dyEJHuexcVExNJ62gMtOvgiPp23Eo8xppu0fT/DWsw==

export export function foo() {
}

Reports:

  x TS(1029): 'export' modifier must precede 'declare' modifier.
   ,-[:1:8]
 1 | export export function foo() {
   :        ^^^^^^
   `----

  x 'export' modifier cannot be used here.
   ,-[:1:8]
 1 | export export function foo() {
   :        ^^^^^^
   `----
  help: Allowed modifiers are: declare, async

We should follow typescript (or better), and do somthing like just report 'export' modifier already seen.

https://www.typescriptlang.org/play/?#code/KYDwDg9gTgLgBKSs4DMCuA7AxjAlhDOAE2AGdgpcBDAG1wC9gBJEjPFXCgOSoFtgAFJFIBKOAG8AUHDg1g8UjCrIAvHAD0AKjgABAPp6ACgFUASgFEDcTeuJkK1Oo2MBmAExCIogNzSN6uXhgDCI4NS1dAxMLKxs7ckpaBmBXD2E4AGo4ABYRXxkoeTQoQikZGRgATzBgAC44AHIWYPZOKAaAGj8ZDD46jW19IzNLPWtbEgTHZIBlGChPUky4AA4RLvK4RWUYDfLgoj2AX18joA</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 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.

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 19, 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.

@camc314
Copy link
Contributor

camc314 commented Nov 19, 2025

@copilot continue

- Changed diagnostic from "export modifier must precede declare modifier" to "export modifier already seen"
- Skip adding duplicate export modifier to the modifiers list to avoid secondary validation error
- Add test case for duplicate export modifier
- Handle empty modifiers case when all modifiers are skipped

Co-authored-by: camc314 <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 19, 2025

@copilot continue

Completed the fix for the duplicate export modifier diagnostic message in commit 4ed6304.

The parser now correctly reports TS(1030): 'export' modifier already seen. instead of the incorrect TS(1029): 'export' modifier must precede 'declare' modifier. when encountering export export function foo().

The fix also eliminates the secondary validation error by skipping the duplicate export modifier from being added to the modifiers list.

Copilot AI changed the title [WIP] Fix incorrect parser diagnostic message for duplicate export modifier Fix incorrect parser diagnostic message for duplicate export modifier Nov 19, 2025
Copilot finished work on behalf of camc314 November 19, 2025 17:16
@camc314
Copy link
Contributor

camc314 commented Nov 19, 2025

@copilot run just converage and the entire test suite and update the snapshots

- Updated parser_misc.snap to include new test case
- Updated parser_typescript.snap to reflect correct diagnostic message

Co-authored-by: camc314 <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 19, 2025

@copilot run just converage and the entire test suite and update the snapshots

Completed in commit 61c94ea.

Coverage tests and entire test suite run successfully. Snapshots updated:

  • parser_misc.snap: Added new test case for duplicate export modifier
  • parser_typescript.snap: Updated 84 diagnostic messages from incorrect "must precede 'declare'" to correct "'export' modifier already seen"

All tests pass including parser_misc (119/119 negative tests), parser_test262 (99.98%), parser_babel (98.20%), and parser_typescript (99.88%).

Copilot finished work on behalf of camc314 November 19, 2025 17:58
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.

Incorrect parser diagnostic message for duplicate export modifier

2 participants