Conversation
This input file contained a destructuring rest parameter
8fc4e8d to
7356bf4
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The removed type guard can cause an uncaught ClassCastException for parenthesized async rest parameters.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Simplifies Jcorn’s destructuring parsing and enables array/object destructuring in JavaScript rest parameters.
Changes:
- Removes delayed pattern-specific syntax diagnostics.
- Allows destructuring rest parameters and permissive trailing commas.
- Adds parser, extraction, and data-flow coverage.
| File | Description |
|---|---|
javascript/ql/test/library-tests/TripleDot/tst.js |
Tests flow through destructured rest parameters. |
javascript/extractor/tests/es2015/output/trap/restparms2.js.trap |
Updates generated extraction expectations. |
javascript/extractor/test/com/semmle/js/extractor/test/RobustnessTests.java |
Tests permissive destructuring parsing. |
javascript/extractor/src/com/semmle/jcorn/Parser.java |
Simplifies parsing and permits destructured rest parameters. |
javascript/extractor/src/com/semmle/jcorn/jsx/JSXParser.java |
Adapts JSX parser signatures. |
javascript/extractor/src/com/semmle/jcorn/flow/FlowParser.java |
Adapts Flow parser signatures. |
javascript/extractor/src/com/semmle/jcorn/ESNextParser.java |
Adapts ESNext parser signatures. |
javascript/extractor/src/com/semmle/jcorn/CustomParser.java |
Adapts custom parser signatures. |
javascript/extractor/src/com/semmle/jcorn/AngularExpressionParser.java |
Adapts Angular parser signatures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new pattern check rejects valid member-expression targets in destructuring assignments.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
Resolved since last review (1)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The parser simplification is consistently propagated across subclasses and covered by extraction, robustness, and data-flow tests.
Review effort: Balanced
Findings: None
Simpler version of #22629 where we stop trying to do rigorous syntax-checking for pattern-specific errors. This means we now accept invalid-but-harmless things like a trailing comma after a rest parameter.
With that change in place, supporting destructuring rest parameters is just a matter removing a check that is no longer needed. It's actually ends up being a simplification.