Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated the grammar rules to allow for optional trailing commas. #1510

Merged
merged 26 commits into from
Oct 28, 2024

Conversation

MahtabNorouzi
Copy link
Collaborator

  • Tests added for any new code
  • Documentation added for any new functionality
  • Entries added to the respective CHANGELOG.md for any new functionality
  • Feature table on README.md updated for any listed functionality

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

We should add a test for this, such as adding some usages of all new forms of trailing commas to SuperSpec.qnt - and then updating SuperSpec.json by running npm run update-fixtures

quint/src/generated/Quint.g4 Outdated Show resolved Hide resolved
@MahtabNorouzi MahtabNorouzi changed the title fix: enforce trailing comma in parameter list for grammar rules ### Summary of Changes - Updated the grammar rules to allow for optional trailing commas in function parameter definitions. - Modified the min function in superspec.map.json to demonstrate this change while keeping the add function unchanged for reference. - Added an import statement: import Proto(N = N,) as Inst1. ### Details - The min function now accepts parameters in the following formats: - (x: int, y: int,) - (x: int, y: int) - The import statement is included to utilize the Proto module with an optional trailing comma. ### Reason for Changes - Allowing an optional trailing comma makes the syntax more flexible and consistent with common programming practices. ### Test Cases - Added test cases for both scenarios in superspec.map.json to ensure correct parsing of functions with optional trailing commas. Sep 23, 2024
@MahtabNorouzi MahtabNorouzi changed the title ### Summary of Changes - Updated the grammar rules to allow for optional trailing commas in function parameter definitions. - Modified the min function in superspec.map.json to demonstrate this change while keeping the add function unchanged for reference. - Added an import statement: import Proto(N = N,) as Inst1. ### Details - The min function now accepts parameters in the following formats: - (x: int, y: int,) - (x: int, y: int) - The import statement is included to utilize the Proto module with an optional trailing comma. ### Reason for Changes - Allowing an optional trailing comma makes the syntax more flexible and consistent with common programming practices. ### Test Cases - Added test cases for both scenarios in superspec.map.json to ensure correct parsing of functions with optional trailing commas. fix: Updated the grammar rules to allow for optional trailing commas in function parameter definitions. Sep 23, 2024
@MahtabNorouzi MahtabNorouzi changed the title fix: Updated the grammar rules to allow for optional trailing commas in function parameter definitions. fix: Updated the grammar rules to allow for optional trailing commas. Sep 23, 2024
@MahtabNorouzi MahtabNorouzi changed the title fix: Updated the grammar rules to allow for optional trailing commas. Updated the grammar rules to allow for optional trailing commas. Sep 23, 2024
@MahtabNorouzi
Copy link
Collaborator Author

Summary of Changes

  • Updated the grammar rules to allow for optional trailing commas.
  • Modified the min function in superspec.map.json to demonstrate this change while keeping the add function unchanged for reference.
  • Added an import statement: import Proto(N = N,) as Inst1.

Details

  • The min function now accepts parameters in the following formats:
    • (x: int, y: int,)
    • (x: int, y: int)
  • The import statement is included to utilize the Proto module with an optional trailing comma.

Reason for Changes

  • Allowing an optional trailing comma makes the syntax more flexible and consistent with common programming practices.

Test Cases

  • Added test cases for both scenarios in superspec.map.json to ensure correct parsing of functions with optional trailing commas.

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

Looks great! Before merging it, can you add "Closes #1383" to the PR description, so the issue gets automatically closed when we merge this?

@bugarela
Copy link
Collaborator

Actually, I remembered something else: we should add an entry to CHANGELOG.md saying that we now allow trailing commas in operator definitions and constant initialization - which also makes me think we might have missed one case: operator call. We should be able to call min(1, 2,) just as well as we can define min(x: int, y: int,)

CHANGELOG.md Outdated

- Updated grammar rule to allow an optional trailing comma in parameter lists:

- Function calls
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Function calls
- Operator calls

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

Looks great! I've just merged another PR that touches the grammar, so you'll need to re-generate the files once merging main on this - sorry about that! Otherwise, we're good to go ✨

CHANGELOG.md Outdated Show resolved Hide resolved
@bugarela bugarela merged commit f3468b2 into main Oct 28, 2024
14 checks passed
@bugarela bugarela deleted the grammar-update branch October 28, 2024 14:06
@bugarela bugarela mentioned this pull request Oct 28, 2024
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.

2 participants