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

[tracker] Do not flag issues in code comments - attributes pre-8.0 #1687

Open
fredden opened this issue Mar 15, 2024 · 2 comments
Open

[tracker] Do not flag issues in code comments - attributes pre-8.0 #1687

fredden opened this issue Mar 15, 2024 · 2 comments
Milestone

Comments

@fredden
Copy link
Contributor

fredden commented Mar 15, 2024

Background

During a routine call with @jrfnl, we discussed #1683. This is a class of bug / possible improvement that should be applied to all affected sniffs.

What needs to be done?

Each sniff in this repository (all of which are listed in the table below) should be reviewed. Most sniffs will not require any changes. The sniffs that need closer review are those that identify issues in attributes - such as new syntax introduced after PHP 8.0. As attributes are treated as comments before PHP 8.0, any new syntax being used in these "comments" should not be flagged as problematic if the code is not expected to run on PHP versions 8.0+.

Tracker

This issue is intended to be a tracker. Individual pull requests should be opened for each sniff requiring changes.

If you would like to claim a sniff, please update the "status" column for the relevant sniff; if you are not able to edit this issue description directly, please add a comment to this issue and someone with sufficient permissions will update the table accordingly.

Sniff Status
Attributes/NewAttributes N/A
Classes/ForbiddenExtendingFinalPHPClass N/A
Classes/NewAnonymousClasses N/A
Classes/NewClasses
Classes/NewConstructorPropertyPromotion N/A
Classes/NewConstVisibility N/A
Classes/NewFinalConstants N/A
Classes/NewLateStaticBinding
Classes/NewReadonlyClasses N/A
Classes/NewReadonlyProperties N/A
Classes/NewTypedProperties N/A
Classes/RemovedClasses N/A (removed - see [3] below)
Classes/RemovedOrphanedParent N/A (removed - see [3] below)
Constants/NewConstantsInTraits N/A
Constants/NewConstants
Constants/NewMagicClassConstant Fixed in #1683
Constants/RemovedConstants N/A (removed - see [3] below)
ControlStructures/DiscouragedSwitchContinue N/A
ControlStructures/ForbiddenBreakContinueOutsideLoop N/A
ControlStructures/ForbiddenBreakContinueVariableArguments N/A
ControlStructures/ForbiddenSwitchWithMultipleDefaultBlocks N/A
ControlStructures/NewExecutionDirectives N/A
ControlStructures/NewForeachExpressionReferencing N/A
ControlStructures/NewListInForeach N/A
ControlStructures/NewMultiCatch N/A
ControlStructures/NewNonCapturingCatch N/A
Extensions/RemovedExtensions N/A (sniff will be removed)
FunctionDeclarations/AbstractPrivateMethods N/A
FunctionDeclarations/ForbiddenFinalPrivateMethods N/A
FunctionDeclarations/ForbiddenParameterShadowSuperGlobals N/A
FunctionDeclarations/ForbiddenParametersWithSameName N/A
FunctionDeclarations/ForbiddenToStringParameters N/A
FunctionDeclarations/ForbiddenVariableNamesInClosureUse N/A
FunctionDeclarations/NewClosure N/A
FunctionDeclarations/NewExceptionsFromToString N/A
FunctionDeclarations/NewNullableTypes N/A
FunctionDeclarations/NewParamTypeDeclarations N/A
FunctionDeclarations/NewReturnTypeDeclarations N/A
FunctionDeclarations/NewTrailingComma N/A
FunctionDeclarations/NonStaticMagicMethods N/A
FunctionDeclarations/RemovedCallingDestructAfterConstructorExit N/A
FunctionDeclarations/RemovedOptionalBeforeRequiredParam N/A
FunctionDeclarations/RemovedReturnByReferenceFromVoid N/A
FunctionNameRestrictions/NewMagicMethods N/A
FunctionNameRestrictions/RemovedMagicAutoload N/A
FunctionNameRestrictions/RemovedNamespacedAssert N/A
FunctionNameRestrictions/RemovedPHP4StyleConstructors N/A
FunctionNameRestrictions/ReservedFunctionNames N/A
FunctionUse/ArgumentFunctionsReportCurrentValue N/A
FunctionUse/ArgumentFunctionsUsage N/A
FunctionUse/NewFunctionParameters N/A
FunctionUse/NewFunctions N/A
FunctionUse/NewNamedParameters (see [2] below)
FunctionUse/OptionalToRequiredFunctionParameters N/A
FunctionUse/RemovedFunctionParameters N/A
FunctionUse/RemovedFunctions N/A
FunctionUse/RequiredToOptionalFunctionParameters N/A
Generators/NewGeneratorReturn N/A
IniDirectives/NewIniDirectives N/A
IniDirectives/RemovedIniDirectives N/A
InitialValue/NewConstantArraysUsingConst N/A
InitialValue/NewConstantArraysUsingDefine N/A
InitialValue/NewConstantScalarExpressions
InitialValue/NewHeredoc
InitialValue/NewNewInDefine N/A
InitialValue/NewNewInInitializers N/A (8.1 feature)
Interfaces/InternalInterfaces N/A
Interfaces/NewInterfaces N/A
Interfaces/RemovedSerializable N/A
Keywords/CaseSensitiveKeywords
Keywords/ForbiddenNames N/A
Keywords/NewKeywords
LanguageConstructs/NewEmptyNonVariable N/A
LanguageConstructs/NewLanguageConstructs
Lists/AssignmentOrder N/A
Lists/ForbiddenEmptyListAssignment N/A
Lists/NewKeyedList N/A
Lists/NewListReferenceAssignment N/A
Lists/NewShortList N/A
MethodUse/ForbiddenToStringParameters N/A
MethodUse/NewDirectCallsToClone N/A
Miscellaneous/NewPHPOpenTagEOF N/A
Miscellaneous/RemovedAlternativePHPTags N/A
Namespaces/ReservedNames N/A
Numbers/NewExplicitOctalNotation (see [2] below)
Numbers/NewNumericLiteralSeparator
Numbers/RemovedHexadecimalNumericStrings N/A (removed - see [3] below)
Numbers/ValidIntegers
Operators/ChangedConcatOperatorPrecedence
Operators/ForbiddenNegativeBitshift
Operators/NewOperators
Operators/NewShortTernary
Operators/RemovedTernaryAssociativity N/A (removed - see [3] below)
ParameterValues/ChangedIntToBoolParamType N/A
ParameterValues/ChangedObStartEraseFlags N/A
ParameterValues/ForbiddenGetClassNoArgsOutsideOO N/A
ParameterValues/ForbiddenGetClassNull N/A
ParameterValues/ForbiddenSessionModuleNameUser N/A
ParameterValues/ForbiddenStripTagsSelfClosingXHTML N/A
ParameterValues/NewArrayMergeRecursiveWithGlobalsVar N/A
ParameterValues/NewArrayReduceInitialType N/A
ParameterValues/NewAssertCustomException N/A
ParameterValues/NewFopenModes N/A
ParameterValues/NewHashAlgorithms N/A
ParameterValues/NewHTMLEntitiesEncodingDefault N/A
ParameterValues/NewHTMLEntitiesFlagsDefault N/A
ParameterValues/NewIconvMbstringCharsetDefault N/A
ParameterValues/NewIDNVariantDefault N/A
ParameterValues/NewNegativeStringOffset N/A
ParameterValues/NewNumberFormatMultibyteSeparators N/A
ParameterValues/NewPackFormat N/A
ParameterValues/NewPasswordAlgoConstantValues N/A
ParameterValues/NewPCREModifiers N/A
ParameterValues/NewProcOpenCmdArray N/A
ParameterValues/NewStripTagsAllowableTagsArray N/A
ParameterValues/RemovedAssertStringAssertion N/A
ParameterValues/RemovedGetClassNoArgs N/A
ParameterValues/RemovedGetDefinedFunctionsExcludeDisabledFalse N/A
ParameterValues/RemovedHashAlgorithms N/A
ParameterValues/RemovedIconvEncoding N/A
ParameterValues/RemovedImplodeFlexibleParamOrder N/A
ParameterValues/RemovedLdapConnectSignatures N/A
ParameterValues/RemovedMbCheckEncodingNoArgs N/A
ParameterValues/RemovedMbStrimWidthNegativeWidth N/A
ParameterValues/RemovedMbstringModifiers N/A
ParameterValues/RemovedMbStrrposEncodingThirdParam N/A
ParameterValues/RemovedNonCryptoHash N/A
ParameterValues/RemovedPCREModifiers N/A
ParameterValues/RemovedSetlocaleString N/A
ParameterValues/RemovedSplAutoloadRegisterThrowFalse N/A
ParameterValues/RemovedVersionCompareOperators N/A
Syntax/ForbiddenCallTimePassByReference
Syntax/NewArrayStringDereferencing
Syntax/NewArrayUnpacking Fixed in #1688
Syntax/NewClassMemberAccess
Syntax/NewDynamicAccessToStatic
Syntax/NewFirstClassCallables (see [2] below)
Syntax/NewFlexibleHeredocNowdoc
Syntax/NewFunctionArrayDereferencing
Syntax/NewFunctionCallTrailingComma
Syntax/NewInterpolatedStringDereferencing (see [2] below)
Syntax/NewMagicConstantDereferencing (see [2] below)
Syntax/NewNestedStaticAccess
Syntax/NewShortArray
Syntax/RemovedCurlyBraceArrayAccess N/A (removed - see [3] below)
Syntax/RemovedNewReference N/A (removed - see [3] below)
TextStrings/NewUnicodeEscapeSequence
TextStrings/RemovedDollarBraceStringEmbeds N/A
TypeCasts/NewTypeCasts
TypeCasts/RemovedTypeCasts N/A (removed - see [3] below)
Upgrade/LowPHP N/A
UseDeclarations/NewGroupUseDeclarations N/A
UseDeclarations/NewUseConstFunction N/A
Variables/ForbiddenGlobalVariableVariable N/A
Variables/ForbiddenThisUseContexts N/A
Variables/NewUniformVariableSyntax N/A
Variables/RemovedIndirectModificationOfGlobals N/A
Variables/RemovedPredefinedGlobalVariables N/A
@jrfnl
Copy link
Member

jrfnl commented Mar 15, 2024

@fredden Thanks for opening the issue. I've taken the liberty to edit the table and to add a status to a lot of sniffs for which this is irrelevant (as the syntax cannot be used in attributes). Hoping that will allow people to focus on the sniffs which do need to be reviewed.

@jrfnl
Copy link
Member

jrfnl commented Mar 16, 2024

The sniffs that need closer review are those that identify issues in attributes - such as new syntax introduced after PHP 8.0. As attributes are treated as comments before PHP 8.0, any new syntax being used in these "comments" should not be flagged as problematic if the code is not expected to run on PHP versions 8.0+.

If the code is not expected to run on PHP 8.0, why would the code contain an attribute ?

I think this is slightly more nuanced.

Change introduced in PHP < 8.0 Change introduced in PHP 8.0+
Sniffs detecting new features sniff needs update if syntax can be used in attributes [1] No update needed [2]
Sniffs detecting deprecated/removed features No update needed [3] No update needed [3]

[1] When a "new" feature was introduced before PHP 8.0 and the feature is used in an attribute, it should not be flagged, as the attribute will be ignored as a comment in PHP < 8.0, so the use of the "new" feature will not cause problems if the code is run on PHP < 8.0 and when running the code on PHP 8.0+, the new syntax is supported anyway.
* Note: this applies to single-line attributes, but individual sniffs should not have to worry about whether the attribute itself is compatible with PHP < 8.0, that's for the NewAttributes sniff to handle.

[2] When a "new" feature was introduced in or after PHP 8.0 and the feature is used in an attribute, it should be flagged if the code under scan needs to support PHP 8.0 < PHP introduction version, but it doesn't need to be flagged if the code under scan does not need to support PHP 8.0+
Having said that, why would code which doesn't need to run on PHP 8.0+ contain attributes ?
So, yes, these sniffs could be updated, but in my opinion don't really need to be as the use of attributes alone means the code within the attribute should be able to run and if it can't be, it should be flagged.

[3] When a PHP feature was deprecated/removed, the sniff detecting this does not need an update as whether it is used in an attribute or not isn't really relevant, the feature should still be flagged as deprecated/removed when the codebase under scan needs to support the PHP version in which the feature was deprecated/removed.

@fredden Does that help ?

@jrfnl jrfnl added this to the 10.0.0 milestone Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants