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

quint/grammar: add support for optional leading hashbang line #1522

Merged
merged 7 commits into from
Oct 3, 2024

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Oct 3, 2024

This updates Quint grammar to support a leading hashbang (#!) line in spec files.
It allows using a spec file as a self-running script on Unix systems, as shown in SuperSpec.qnt.

  • Tests added for any new code
  • Entries added to the respective CHANGELOG.md for any new functionality

Closes #1500

@lucab
Copy link
Contributor Author

lucab commented Oct 3, 2024

/cc @bugarela for review.

I decided to follow JS approach and only carve a niche in the grammar for the first line, see https://github.com/tc39/proposal-hashbang for details.

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.

Thanks so much for taking the time and linking that proposal!
I just left a comment on the grammar. Also, it would be good to add an integration tests that calls ./testFixture/SuperSpec.qnt to test the overall hashbang behavior. It should be simple to add that to cli-tests.md, and you can run the integration tests with npm run integration - lmk if you need assistance with that or would rather have me take on from here.

@@ -313,6 +314,7 @@ CAP_ID : ([A-Z][a-zA-Z0-9_]*|[_][a-zA-Z0-9_]+) ;
DOCCOMMENT : '///' .*? '\n';

// comments and whitespaces
HASHBANG_LINE : '#!' .*? '\n' -> skip ; // only used for the first line of a file
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using skip where tells ANTLR that we want to skip this token, wherever it appears, as it is the case with comments and whitespaces. However, in this case, we only want to allow this in the very first line, so we shouldn't skip it. I've tested this locally and confirmed that, in the current state, this will allow hasbangs in the middle of a file.

We can simply remove the -> skip here and move it closer to the other regular tokens (and away from the skipped ones).

Copy link
Contributor Author

@lucab lucab Oct 3, 2024

Choose a reason for hiding this comment

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

Apologies, I'm not overly familiar with ANTLR grammars so I didn't realize that skip had such a side-effect.
I did the changes you suggested and also added a few unit tests to ensure that the hashbang rule only applies to the first line of the file.

@lucab
Copy link
Contributor Author

lucab commented Oct 3, 2024

Fixed grammar rules and rebased to latest main. Also added an integration test at the bottom of cli-tests.md to run the typechecking on SuperSpec.qnt via the hashbang on Unix.

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! Thanks again

@bugarela bugarela merged commit dcd506f into informalsystems:main Oct 3, 2024
14 checks passed
@lucab lucab deleted the ups/grammar-hashbang branch October 3, 2024 17:20
@bugarela bugarela mentioned this pull request Oct 8, 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.

grammar: consider adding support for hashbang scripting
2 participants