Skip to content

Conversation

@jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 29, 2019

Code:

$anonClass = new class($arg) {};

As anonymous classes have a distinct token, it makes sense to me to assign that token as the parenthesis owner, along the same lines as is done for anonymous functions.

The main difference is that for anonymous classes, the parenthesis are optional.

Assigning an owner for them can not be done from within the Tokenizer::createTokenMap() as at that point in time the PHP::processAdditional() method hasn't run yet, so the token is still a T_CLASS.

It can however be adjusted from within the PHP::processAdditional() method when the T_CLASS token is changed to a T_ANON_CLASS token.

This commit implements this.

This means that for the class token in the above code snippet will now have a parenthesis_owner, parenthesis_opener and parenthesis_closer in the $tokens array.
The parenthesis opener and closer will both now also have the parenthesis_owner key, in addition to the parenthesis_opener and parenthesis_closer keys which they already had.

Instead of testing this via existing sniffs, I have chosen to add a new set of Core tests for specific tokenizer issues, with the tests for this change being the first set of tests added.

Includes:

  • Adding the T_ANON_CLASS token to the Tokens::$parenthesisOpeners array.
  • Removing the T_ANON_CLASS from the "additional tokens indicating that parenthesis are not arbitrary" list in the Generic.WhiteSpace.ArbitraryParenthesesSpacing sniff.

…nthesis

Code:
```php
$anonClass = new class($arg) {};
```

As anonymous classes have a distinct token, it makes sense to me to assign that token as the parenthesis owner, along the same lines as is done for anonymous functions.

The main difference is that for anonymous classes, the parenthesis are optional.

Assigning an owner for them can not be done from within the `Tokenizer::createTokenMap()` as at that point in time the `PHP::processAdditional()` method hasn't run yet, so the token is still a `T_CLASS`.

It can however be adjusted from within the `PHP::processAdditional()` method when the `T_CLASS` token is changed to a `T_ANON_CLASS` token.

This commit implements this.

This means that for the `class` token in the above code snippet will now have a `parenthesis_owner`, `parenthesis_opener` and `parenthesis_closer` in the `$tokens` array.
The parenthesis opener and closer will both now also have the `parenthesis_owner` key, in addition to the `parenthesis_opener` and `parenthesis_closer` keys which they already had.

Instead of testing this via existing sniffs, I have chosen to add a new set of `Core` tests for specific tokenizer issues, with the tests for this change being the first set of tests added.

Includes:
* Adding the `T_ANON_CLASS` token to the `Tokens::$parenthesisOpeners` array.
* Removing the `T_ANON_CLASS` from the "additional tokens indicating that parenthesis are not arbitrary" list in the `Generic.WhiteSpace.ArbitraryParenthesesSpacing` sniff.
@gsherwood gsherwood added this to the 3.5.0 milestone Sep 5, 2019
gsherwood added a commit that referenced this pull request Sep 5, 2019
@gsherwood gsherwood merged commit baaf896 into squizlabs:master Sep 5, 2019
@gsherwood
Copy link
Member

Thanks a lot for this. I was just about to need it for the PSR-12 standard.

I ended up moving the unit tests to remove the PHP folder because I'm committed to removing the other tokenizers and wanted to leave the core PHP tests in the main folder. I'd probably add a sub-folder for Commenting if that remains as-is.

But thanks a lot for those first tokenizer tests as well.

gsherwood added a commit that referenced this pull request Sep 5, 2019
@jrfnl jrfnl deleted the feature/tokenizer-anonymous-class-parenthesis-owner branch September 5, 2019 11:58
@jrfnl
Copy link
Contributor Author

jrfnl commented Sep 5, 2019

There was a reason I made the change, just couldn't pull it earlier as it would conflict with and benefit from the change in #2295, so that needed to be merged first.

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