Forbid some empty separated lists in v2 #667

Xanewok · 2023-11-20T18:22:51Z

Part of #652

These are a bug-fixes along with regression tests added for each case, we use a definition from v0/v1 for these.

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.symbolAliases

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.usingDirective

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.tupleExpression Unfortunately, we use `Separated(Optional(...))` in both definitions, which wrongly accepts an empty list in this specific case.

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.overrideSpecifier

changeset-bot · 2023-11-20T18:22:56Z

⚠️ No Changeset found

Latest commit: 8914e23

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

crates/solidity/inputs/language/src/definition.rs

...testing/snapshots/cst_output/TupleDeconstructionStatement/empty/generated/0.4.11-success.yml

crates/solidity/testing/snapshots/cst_output/UsingDirective/destructure_empty/input.sol

OmarTawfik

Left a couple of questions

Xanewok added 4 commits November 20, 2023 18:30

fix: Mark destructuring import as non-empty in v2

388d477

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.symbolAliases

fix: Mark destructuring using directive as non-empty in v2

b2af449

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.usingDirective

fix: Mark tuple expressions as non-empty in DSL v2

d39373e

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.tupleExpression Unfortunately, we use `Separated(Optional(...))` in both definitions, which wrongly accepts an empty list in this specific case.

fix: Mark override paths list group as non-empty in DSL v2

276cbc0

See https://docs.soliditylang.org/en/latest/grammar.html#a4.SolidityParser.overrideSpecifier

Xanewok requested a review from a team as a code owner November 20, 2023 18:22

fix: Allow specifying nested empty rules in CST snapshot tests

9f5969c

Xanewok mentioned this pull request Nov 20, 2023

Unify language definitions #652

Closed

9 tasks

Xanewok enabled auto-merge November 20, 2023 22:48

OmarTawfik reviewed Nov 20, 2023

View reviewed changes

crates/solidity/inputs/language/src/definition.rs Outdated Show resolved Hide resolved

OmarTawfik reviewed Nov 20, 2023

View reviewed changes

crates/solidity/inputs/language/src/definition.rs Outdated Show resolved Hide resolved

OmarTawfik reviewed Nov 20, 2023

View reviewed changes

...testing/snapshots/cst_output/TupleDeconstructionStatement/empty/generated/0.4.11-success.yml Outdated Show resolved Hide resolved

OmarTawfik reviewed Nov 20, 2023

View reviewed changes

crates/solidity/testing/snapshots/cst_output/UsingDirective/destructure_empty/input.sol Show resolved Hide resolved

OmarTawfik requested changes Nov 20, 2023

View reviewed changes

Xanewok added 4 commits November 21, 2023 13:12

Add a comment for the CST test

e666e14

Fix the overriden spelling mistake

3ade584

Change the field name from overridden to paths

0ae2072

Resolve conflicts from upstream/main

861d926

Xanewok mentioned this pull request Nov 21, 2023

Specify Optional(NonEmpty(List)) in v2 where we do in v1 #668

Merged

Xanewok requested a review from OmarTawfik November 21, 2023 12:27

Link to an issue for the wrong empty tuple deconstruction parse

23f8804

OmarTawfik approved these changes Nov 21, 2023

View reviewed changes

Xanewok added 2 commits November 22, 2023 10:33

Merge remote-tracking branch 'upstream/main' into fix-lists-in-v2

6555c10

fixup: Use new UintKeyword in the new tests

8914e23

Xanewok added this pull request to the merge queue Nov 22, 2023

Merged via the queue into NomicFoundation:main with commit 54af80d Nov 22, 2023
1 check passed

Xanewok deleted the fix-lists-in-v2 branch November 22, 2023 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forbid some empty separated lists in v2 #667

Forbid some empty separated lists in v2 #667

Xanewok commented Nov 20, 2023

changeset-bot bot commented Nov 20, 2023 •

edited

Loading

OmarTawfik left a comment

Forbid some empty separated lists in v2 #667

Forbid some empty separated lists in v2 #667

Conversation

Xanewok commented Nov 20, 2023

changeset-bot bot commented Nov 20, 2023 • edited Loading

⚠️ No Changeset found

OmarTawfik left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Nov 20, 2023 •

edited

Loading