search

slackhq / tree-sitter-hack

Hack grammar for tree-sitter
MIT License
33 stars 15 forks source link

Remove unused tuple keyword #34

Closed frankeld closed 2 years ago

frankeld commented 3 years ago

I believe tupe is a typo of tuple. So, I could remove it or fix the typo. However, since we don't have errors in this area even with the typo, it seems to be an unnecessary keyword. As such, I removed it.

aosq commented 3 years ago

Oh interesting. I think I misunderstood what was preventing keywords in positions like test/cases/declarations/const-keyword.hack. Even weirder, const tuple = 1; works without tuple in the $._keyword, but I just tested this with $var->tuple() and that fails without tuple in $._keyword but if you fix the typo, it works.

We can merge your change as is, and create an issue to add support for tuple in $.selection_expression including tests or if you want to make the update here, that works too.

I've been meaning to look into what other expressions support keywords. Hack source has these tests for keywords (interestingly no tests for member selection expressions).

hphp/hack/test/full_fidelity/cases/keyword_as_class_allowed.php hphp/hack/test/full_fidelity/cases/keyword_as_class_reserved.php hphp/hack/test/full_fidelity/cases/keyword_as_class_reserved_php.php hphp/hack/test/full_fidelity/cases/keyword_as_class_unexpected.php hphp/hack/test/full_fidelity/cases/keyword_as_const.php hphp/hack/test/full_fidelity/cases/keyword_as_const_w_ty_spec.php hphp/hack/test/full_fidelity/cases/keyword_as_enum.php hphp/hack/test/full_fidelity/cases/keyword_as_function_allowed.php hphp/hack/test/full_fidelity/cases/keyword_as_function_unexpected.php hphp/hack/test/full_fidelity/cases/keyword_as_member.php hphp/hack/test/full_fidelity/cases/keyword_as_method.php hphp/hack/test/full_fidelity/cases/keyword_as_static_method.php

frankeld commented 2 years ago

I decided to just update in this PR. I looked at those Hack test files and added some of the tests in. The tests in https://github.com/facebook/hhvm/blob/master/hphp/hack/test/full_fidelity/cases/keyword_as_function_allowed.php/ and https://github.com/facebook/hhvm/blob/d7dc631ce/hphp/hack/test/full_fidelity/cases/keyword_as_member.php and https://github.com/facebook/hhvm/blob/d7dc631ce/hphp/hack/test/full_fidelity/cases/keyword_as_method.php still have some errors, but they also cause HHVM errors so I don't think it's a priority. https://github.com/facebook/hhvm/blob/master/hphp/hack/test/full_fidelity/cases/keyword_as_function_unexpected.php also causes failures, but that's as it's supposed to.