fix(es/parser): emit TS2371 for object defaults in declare signatures - #12095
fix(es/parser): emit TS2371 for object defaults in declare signatures#12095Felix-Ayush (Ayush7614) wants to merge 4 commits into
Conversation
Ambient/declare parameter lists already rejected AssignPat defaults, but object shorthand defaults use AssignPatProp and skipped the InDeclare check. Emit TS2371 for those defaults and deeply walk declare function/constructor parameters for nested initializers.
🦋 Changeset detectedLatest commit: 732b90e The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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: c76a9eae77
ℹ️ 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 (@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 (@codex) address that feedback".
| if self.ctx().contains(Context::InDeclare) { | ||
| self.emit_err(self.span(start), SyntaxError::TS2371); | ||
| } |
There was a problem hiding this comment.
This check runs for every object binding pattern parsed under Context::InDeclare, not just function/constructor parameters. For example, declare const { a = 1 }: { a?: number }; reaches this branch while parsing the variable declarator, and TypeScript accepts that ambient destructuring binding, but SWC will now emit the parameter-only TS2371 diagnostic. Narrow the eager check to parameter/signature parsing (or rely on the post-parse parameter traversal) so declare variable bindings are not rejected.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Object shorthand defaults under InDeclare also appear in ambient
variable bindings such as `declare const { a = 1 }`, which TypeScript
accepts. Drop the eager object-pattern check and limit binding-element
TS2371 to parameter lists; declare signatures still use the post-parse
walk.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a15012a7d8
ℹ️ 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 (@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 (@codex) address that feedback".
| if self.ctx().contains(Context::InDeclare) { | ||
| // Only reject defaults in declare *parameter* lists. Ambient | ||
| // destructuring bindings such as `declare const { a: b = 1 }` are valid. | ||
| if self.ctx().contains(Context::InDeclare) && self.ctx().contains(Context::InParameters) |
There was a problem hiding this comment.
This guard fixes ambient variable destructuring, but it also disables the existing TS2371 check for .d.ts function/type signatures parsed through parse_ts_binding_list_for_signature, because that path has Context::InDeclare set but never enters Context::InParameters. As a result, inputs like type T = ({ a: b = 1 }: { a?: number }) => void; are now accepted by SWC even though tsc --noEmit reports TS2371, and the new recursive checker only runs for function/constructor nodes with body.is_none(), not TsFnParam signatures.
Useful? React with 👍 / 👎.
| pub(crate) fn emit_ts2371_for_param_initializers(&mut self, pat: &Pat) { | ||
| match pat { | ||
| Pat::Assign(a) => { | ||
| self.emit_err(a.span(), SyntaxError::TS2371); |
There was a problem hiding this comment.
For declare function/method signatures parsed under Context::InParameters, nested Pat::Assign defaults have already reported TS2371 from parse_binding_element before this post-parse walk runs. This line reports the same span a second time for cases such as declare function f([x = 1]): void; or declare function f({ a: b = 1 }): void;, so consumers and the newly added stderr fixtures will see duplicate TS2371 diagnostics unless the walker skips already-checked assign patterns in that context or the eager check is centralized here.
Useful? React with 👍 / 👎.
Drop eager InDeclare emits that double-fired with the post-list walker for declare functions. Recover AssignPat in signature bindings as TS2371 and walk nested object/array defaults in type/call/method signatures.
Drop eager InDeclare emits that double-fired with the post-list walker for declare functions. Recover AssignPat in signature bindings as TS2371 and walk nested object/array defaults in type/call/method signatures.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 732b90e090
ℹ️ 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 (@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 (@codex) address that feedback".
| }) | ||
| } | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
This new #[test] adds parser coverage outside the fixture suites even though this commit already adds tests/typescript-errors/ts2371/mixed, whose stderr snapshot would catch duplicate TS2371 diagnostics. Keeping the ad-hoc inline test bypasses the parser fixture workflow and violates the repo's fixture-test convention; please move the duplicate-diagnostic assertion into the fixture suite or rely on the existing stderr fixture.
AGENTS.md reference: AGENTS.md:L52-L55
Useful? React with 👍 / 👎.
1 participant
Description:
TypeScript's TS2371 forbids parameter initializers in ambient / declare signatures. SWC already rejected top-level
AssignPatdefaults (and array defaults likedeclare function bar([x = 1])), but object shorthand defaults ({ a = 1 }) useAssignPatPropand skipped theInDeclarecheck. There was also aTODO: Search deeply for assignment patternon declare function/constructor validation.This change:
InDeclareVerified against
tscand with fixtures undertests/typescript-errors/ts2371/**.Related issue (if exists):
N/A (direct PR; no existing tracked issue)