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

Fix regex in CodeQL TextMate grammar that was silently failing #3903

Merged
merged 2 commits into from
Jan 23, 2025

Conversation

slevithan
Copy link
Contributor

@slevithan slevithan commented Jan 19, 2025

The TM grammar for CodeQL includes an invalid regex. Specifically, in Oniguruma, lookahead is invalid when used within lookbehind. This leads to the regex failing silently in vscode-textmate.

Additionally, this makes the CodeQL grammar fail entirely when used with Shiki's JS engine (see CodeQL listed as unsupported in Shiki's JS engine compatibility report) because Shiki's JS engine doesn't fail silently for invalid Oniguruma regexes.

Because the lookahead appears at the end of the lookbehind, moving it outside of the lookbehind does not change the behavior of the regex at all (if the previous version had worked in the first place, like it would in some other regex flavors).

In this PR, I updated extensions/ql-vscode/syntaxes/ql.tmLanguage.yml and then ran the syntaxes/updateSyntax script to generate the JSON version of the grammar. I've also updated the changelog.

@slevithan slevithan requested a review from a team as a code owner January 19, 2025 19:45
@slevithan
Copy link
Contributor Author

@aeisenberg any concerns with landing this?

@aeisenberg
Copy link
Contributor

Looks fine to me. Thanks for your contribution!

@aeisenberg aeisenberg enabled auto-merge January 23, 2025 18:54
@aeisenberg aeisenberg merged commit 588441d into github:main Jan 23, 2025
16 checks passed
@slevithan slevithan deleted the fix-regex branch January 23, 2025 19:10
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