feat: use the new reserved rules api #263
Conversation
|
Awesome! Looks interesting, I'll play with it in the next couple of days. I'm fairly certain named arguments could benefit from this as well |
There was a problem hiding this comment.
For reference, here is the list of reserved PHP keywords:
- https://www.php.net/manual/en/reserved.keywords.php
- https://www.php.net/manual/en/reserved.other-reserved-words.php
The following words cannot be used as constants, class names, or function names. They are, however, allowed as property, constant, and method names of classes, interfaces and traits, except that
classmay not be used as constant name.
readonlymay be used as function name.
Generally, it seems like the reserved keywords are only reserved in the context of class, constant, function, trait, enum, interface, and (potentially namespace?) names
| // 'var' is allowed in variable names | ||
| variables: _ => BASE_RESERVED_KEYWORD_SET.concat([keyword('static', false)]), |
There was a problem hiding this comment.
This should be empty like properties, all of these keywords are allowed as variables
| $object-> | ||
| class Test { // 'class' should not be treated as a property | ||
| } | ||
|
|
||
| class Example { | ||
| public function test() { | ||
| $this-> // incomplete | ||
| function process() {} // function should be recognized |
There was a problem hiding this comment.
both class and function can technically be properties here so not sure if this will work
| @@ -1214,7 +1242,7 @@ module.exports = function defineGrammar(dialect) { | |||
| ), | |||
| ), | |||
|
|
|||
| _argument_name: $ => seq( | |||
| _argument_name: $ => reserved('properties', seq( | |||
There was a problem hiding this comment.
it would be great if this choices list can be cleaned up here---this was needed because the grammar was having trouble parsing named arguments that could also be casts or other types / functions:
e.g.,:
$this->foo(array: $bar); // named argument
$this->foo((array) $bar); // array cast
$this->foo(array(...)); // array function (long form of [...])However, not sure if this new feature would clean this up or if this is merely an exclusion list
There was a problem hiding this comment.
yeah, the PR is currently adding support by default for non-reserved keywords to be usable as the word token if they overlap (the pattern for the word token matches the keyword in its entirety). So obviously operators wouldn't match, but any keywords like these would :) Really excited for that since a lot of grammars will no longer need this "_reserved_identifier" hack they currently have.
@calebdw you probably know PHP better than me, it'd be nice if you can audit this, but so far from what I can tell everything in the corpus parses as it should, and we have much better error recovery in a variety of cases, I just put 2 in a test (compare with error recovery w/o this PR to see a staggering difference :) ) (you will need to build tree-sitter from the branch in the linked PR to play w/ this)