Repository navigation
Add lint rule to enforce PHP deprecated-function detection #4205
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
base: master
Are you sure you want to change the base?
Changes from all commits
eda637a
41a0b29
ede2439
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,33 @@ | |
| <rule ref="Squiz.WhiteSpace.SuperfluousWhitespace"/> | ||
| <rule ref="Generic.WhiteSpace.DisallowTabIndent"/> | ||
|
|
||
| <!-- | ||
| Enabled: recurring review-feedback theme, now mechanically enforced. | ||
|
|
||
| Over the trailing ~400 days of PR review activity, human reviewers | ||
| repeatedly had to flag calls to functions that are deprecated (or | ||
| outright unavailable) on some PHP version this repo still supports, | ||
| e.g.: | ||
| - "spl_object_hash is deprecated since PHP 8.6. You can simply | ||
| use spl_object_id." (Drupal integration) | ||
| - "Needs a PHP_VERSION_ID < 80000 check to avoid deprecations" | ||
| (Symfony integration) | ||
| - "this is php 7.4 syntax" (repo supports PHP 7.0+) | ||
| - "`const` was missing on the PHP 7.0 and 7.1 versions" (stub | ||
| signatures) | ||
| This class of feedback is exactly what Generic.PHP.DeprecatedFunctions | ||
| is built to catch mechanically: it walks the token stream for calls | ||
| to any function PHP itself reports as deprecated via | ||
| ReflectionFunction::isDeprecated() on the interpreter running phpcs | ||
| (the newest supported PHP in CI, see .gitlab/generate-common.php). | ||
| It does not parse the file with the runner's parser the way | ||
| Generic.PHP.Syntax does, so it carries none of that sniff's | ||
| cross-version syntax risk (see the note below). It was already | ||
| vetted as a zero-hit, low-risk candidate for src/ (see git history | ||
| of this file) — this change only turns it on. | ||
| --> | ||
|
Comment on lines
+44
to
+68
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a commit message, not a comment belonging into this file.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DataDog fix this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can only run on private repositories. |
||
| <rule ref="Generic.PHP.DeprecatedFunctions"/> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the lint image advances from PHP 8.5 to 8.6, this token-based sniff will report the two existing Useful? React with 👍 / 👎. |
||
|
|
||
| <!-- | ||
| Close, but not a mechanical fix. Uncomment after the TODOs | ||
| are resolved or dropped. | ||
|
|
@@ -59,11 +86,6 @@ | |
| runs php -l with the job's PHP. A file valid on 7.0 can fail | ||
| on 8.x (and the reverse) without any source change. | ||
|
|
||
| Generic.PHP.DeprecatedFunctions | ||
| current src/ has no hits, but the sniff list grows with | ||
| PHPCS releases and tracks language deprecations. | ||
|
|
||
| <rule ref="Generic.PHP.Syntax"/> | ||
| <rule ref="Generic.PHP.DeprecatedFunctions"/> | ||
| --> | ||
| </ruleset> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sniff only asks the PHP 8.5 lint process whether a function is deprecated; it has no information about when functions were introduced. For example, an unguarded
spl_object_id()orstr_contains()call passes this check even though those functions are unavailable on PHP 7.0 and PHP 7.4 respectively, both within the repository's supported range. Thus the rule does not catch the advertised version-incompatible-function cases; add a version-aware compatibility check or narrow this documentation to deprecated functions on the runner only.Useful? React with 👍 / 👎.