Skip to content

Conversation

SteveL-MSFT
Copy link
Member

PR Summary

The grammar didn't allow for functions true() and false() and only expected boolean arguments true and false so this resulted in a parsing error. Fix is to update the grammar with precedence for functions and booleans (in that order) so that true() and false() will be accepted as function names and not treated as booleans and thus invalidating the rest of the expression.

Some of the invalid tests needed to be updated, but all still result in an error as expected.

PR Context

Fix #1136

@SteveL-MSFT SteveL-MSFT requested a review from tgauth September 27, 2025 05:03
Copy link
Contributor

@Gijsreyn Gijsreyn left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@andyleejordan andyleejordan left a comment

Choose a reason for hiding this comment

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

I think this LGTM though I'm not a tree-sitter expert and can't explain exactly how this automatically resolves true() and false() to true and false. Per the tests, it does. My guess is that it's a recursive evaluation of the $._booleanLiteral when it's the functionName.

@SteveL-MSFT
Copy link
Member Author

I think this LGTM though I'm not a tree-sitter expert and can't explain exactly how this automatically resolves true() and false() to true and false. Per the tests, it does. My guess is that it's a recursive evaluation of the $._booleanLiteral when it's the functionName.

The main change in the grammar to fix this is to ensure that true and false are both booleans as parameters to functions, but also as true() and false() functions. Without this change (even if you set precedence), it would match to the boolean first because it's an exact match compared to the regex and thus never match as a function. In this case, we have explicitly added true and false as possible function names (in addition to the regex) so that it can be matched and the precedence (higher number is higher precedence) means it'll be evaluated first as a potential function name before identifying it as a boolean value. This behavior is specific to tree-sitter generated parser. Thanks for the extra set of eyes!

@SteveL-MSFT SteveL-MSFT merged commit 2c970df into PowerShell:main Sep 30, 2025
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the boolean-functions branch September 30, 2025 00:48
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.

Parser: Found error node parsing function while using user-defined functions

3 participants