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

Builds broken: Tokenizer change for return types in PHPCS 3.3.0 #639

Closed
jrfnl opened this issue Apr 17, 2018 · 2 comments · Fixed by vfalies/php7compatibility#6 or drevops/scaffold#168
Assignees
Milestone

Comments

@jrfnl
Copy link
Member

jrfnl commented Apr 17, 2018

PHPCS 3.3.0 will change the way class based return type declarations are tokenized.

Previously, a class based return type was tokenized as:

  • ClassName => T_STRING
  • NSName\NSSubName\ClassName => T_STRING + T_NS_SEPARATOR + T_STRING + T_NS_SEPARATOR + T_STRING

Now they will be tokenized as one T_RETURN_TYPE token.

Also, the getMethodProperties() method will now return information on the return type.

Based on the existing unit tests, this following sniffs will need adjusting:

  • NewNullableTypes (test 10 / line 77)
  • ForbiddenClosureUseVariableNames (now throws an undefined offset on line 125 of the sniff)

Until those adjustments have been made, the builds will be broken.

Other than that, I think it would be prudent to review the following sniffs as I would have expected something to break there too, so those sniffs may need improvements anyway to also look at return type declarations or they may already have been set up in such a way that all is well:

  • ForbiddenNames (all three)
  • InternalInterfaces
  • NewClasses
  • NewInterfaces
  • NewLanguageConstructs (ns separator detection in type declarations)
  • NewReturnTypeDeclarations

If no improvements are needed for these sniffs, at the very least we need to make sure that return type declarations are covered in the unit tests for these sniffs.

Refs:

@jrfnl jrfnl added this to the 8.2.0 milestone Apr 17, 2018
@jrfnl
Copy link
Member Author

jrfnl commented Apr 19, 2018

Since I posted this, this change has been reverted and the T_RETURN_TYPE and T_ARRAY_HINT have been completely deprecated (to be removed in v 4.0).

So, while the actual change is now different, the principle of the issue still stands: we need to check whether any adjustments need to be made in PHPCompatibility to deal with these tokenizer changes.

More info in the upstream issue (referenced above).

Note to self: review the methods in the PHPCSHelper class to see if any of them may also need adjusting/updating.

@jrfnl
Copy link
Member Author

jrfnl commented May 10, 2018

PR #642 should fix this. PR #643 and #645 are loosely related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment