Skip to content

File::getDeclarationName(): stop accepting tokens for non-named structures #1007

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

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 15, 2025

Description

Tests: rename some test case files

... in anticipation of additional test case files for parse error being added.

Various sniffs: add extra tests with live coding/parse errors

Includes fixing up some inline comments for sniffs which don't search for closures, but could encounter live coding.

Includes one minor bug fix/defensive coding fix in the Squiz.Commenting.ClosingDeclarationComment sniff.

File::getDeclarationName(): stop accepting tokens for non-named structures

The File::getDeclarationName() method - for historic reasons - accepted the T_CLOSURE and T_ANON_CLASS tokens, even though these structures will never have a name, and returned null for those tokens.

This commit changes the File::getDeclarationName() method to no longer accept those tokens and throw an exception if they are passed to the method instead.

As a secondary change, when the name of a valid structure cannot be determined, the method will now no longer return null, but will return an empty string.
This normalizes the return type of the method to always return a string (or throw an exception).

Includes updated unit tests to match.

Related to squizlabs/PHP_CodeSniffer#3766


All PHPCS native sniffs using the File::getDeclarationName() method have been reviewed and where necessary fixed to allow for this change.

Also note that in some cases, the sniff already contained sufficient protection and in other case, the fact that the method will no longer return null, meant that the sniff code could be simplified.

List of sniffs reviewed:

  • AbstractScopeSniff
  • Generic.Classes.DuplicateClassName
  • Generic.CodeAnalysis.UnusedFunctionParameter
  • Generic.CodeAnalysis.UselessOverridingMethod
  • Generic.NamingConventions.AbstractClassNamePrefix
  • Generic.NamingConventions.CamelCapsFunctionName
  • Generic.NamingConventions.ConstructorName
  • Generic.NamingConventions.InterfaceNameSuffix
  • Generic.NamingConventions.TraitNameSuffix
  • PEAR.Commenting.ClassComment
  • PEAR.Commenting.FunctionComment
  • PEAR.Functions.FunctionDeclaration
  • PEAR.NamingConventions.ValidFunctionName
  • PSR1.Methods.CamelCapsMethodName
  • PSR2.Methods.MethodDeclaration
  • Squiz.Classes.ClassFileName
  • Squiz.Classes.SelfMemberReference
  • Squiz.Commenting.ClassComment
  • Squiz.Commenting.ClosingDeclarationComment
  • Squiz.Commenting.FunctionComment
  • Squiz.Functions.GlobalFunction
  • Squiz.NamingConventions.ValidFunctionName
  • Squiz.Scope.MethodScope

👉 The changes to the PEAR/FunctionDeclaration sniff will be easier to review while ignoring whitespace.

Suggested changelog entry

Changed:

  • The File::getDeclarationName() method will no longer accept T_ANON_CLASS or T_CLOSURE tokens.
    • A RuntimeException will be thrown if these tokens are passed.
  • The File::getDeclarationName() method will now always return a string (or throw an Exception).
    • Previously, the method would return null if the name could not be determined, like during live coding.
      Now it will return an empty string in those situations.

jrfnl added 3 commits April 15, 2025 17:36
... in anticipation of additional test case files for parse error being added.
Includes fixing up some inline comments for sniffs which don't search for closures, but could encounter live coding.

Includes one minor bug fix/defensive coding fix in the `Squiz.Commenting.ClosingDeclarationComment` sniff.
…tures

The `File::getDeclarationName()` method - for historic reasons - accepted the `T_CLOSURE` and `T_ANON_CLASS` tokens, even though these structures will never have a name, and returned `null` for those tokens.

This commit changes the `File::getDeclarationName()` method to no longer accept those tokens and throw an exception if they are passed to the method instead.

As a secondary change, when the name of a valid structure cannot be determined, the method will now no longer return `null`, but will return an empty string.
This normalizes the return type of the method to always return a string (or throw an exception).

Includes updated unit tests to match.

Related to squizlabs/PHP_CodeSniffer 3766

---

All PHPCS native sniffs using the `File::getDeclarationName()` method have been reviewed and where necessary fixed to allow for this change.

Also note that in some cases, the sniff already contained sufficient protection and in other case, the fact that the method will no longer return `null`, meant that the sniff code could be simplified.

List of sniffs reviewed:
- `AbstractScopeSniff`
- `Generic.Classes.DuplicateClassName`
- `Generic.CodeAnalysis.UnusedFunctionParameter`
- `Generic.CodeAnalysis.UselessOverridingMethod`
- `Generic.NamingConventions.AbstractClassNamePrefix`
- `Generic.NamingConventions.CamelCapsFunctionName`
- `Generic.NamingConventions.ConstructorName`
- `Generic.NamingConventions.InterfaceNameSuffix`
- `Generic.NamingConventions.TraitNameSuffix`
- `PEAR.Commenting.ClassComment`
- `PEAR.Commenting.FunctionComment`
- `PEAR.Functions.FunctionDeclaration`
- `PEAR.NamingConventions.ValidFunctionName`
- `PSR1.Methods.CamelCapsMethodName`
- `PSR2.Methods.MethodDeclaration`
- `Squiz.Classes.ClassFileName`
- `Squiz.Classes.SelfMemberReference`
- `Squiz.Commenting.ClassComment`
- `Squiz.Commenting.ClosingDeclarationComment`
- `Squiz.Commenting.FunctionComment`
- `Squiz.Functions.GlobalFunction`
- `Squiz.NamingConventions.ValidFunctionName`
- `Squiz.Scope.MethodScope`

:point_right: The changes to the PEAR/FunctionDeclaration sniff will be easier to review while ignoring whitespace.
@jrfnl jrfnl merged commit 27a2700 into 4.x Apr 15, 2025
54 checks passed
@jrfnl jrfnl deleted the phpcs-4.0/feature/file-getdeclarationname-stop-handling-non-named branch April 15, 2025 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants