Repository navigation
SONARPHP-1957 Merge changes to S1068 with proper commits - #1838
Conversation
Detect dynamic class instantiation where the class is accessed from a private property. That counts as property use and S1068 shouldn't raise.
Recognize Doctrine ORM property mappings in attributes and docblock annotations, including imported aliases and qualified names. Keep reporting unrelated private members, even within mapped entities.
Before PHP 8.1 native enum support, a common way to define enums was to define private classes constants and use enum libraries that can discover these private constants through reflection and expose access functions, leaving no direct references for S1068 to find. User feedback shows this pattern with base classes beyond the popular `myclabs/php-enum`. Exempt private constants when the superclass's resolved short name is Enum and the subclass declares no properties. Keep reporting unused members in classes with properties or a different superclass.
A promoted property can be unused even when its constructor parameter is used. The existing "Remove this unused private field" message can make it sound as though the parameter should be removed too, which could change the constructor's behavior. Also, there is user feedback with comment "Property defined in construct" is another (weak) signal that the message is unclear. When the parameter is used but the property is not, explain that property promotion creates an unused field and should be removed.
hashicorp-vault-sonar-prod
Bot
changed the title
|
Code Review ✅ Approved🟡 Medium risk · S1068 changes field-use detection and suppresses findings for mapped or enum-like members Merges four S1068 rule improvements: fixes false positives for dynamic class instantiation, ORM attributes, and enum-like class constants, and clarifies the message for unused promoted properties. No issues found. Review coverage🧪 Functional validation No results 📋 Rules No rules evaluated Cross-repo coverage 4 repositories selected Cross-repo inspection is incomplete. Unread code may contain additional impacts. 🤖 Auto-approval Not enabled · Set up OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
felix-pauck-sonarsource
left a comment
felix-pauck-sonarsource
left a comment
There was a problem hiding this comment.
Was reviewed and merged before (see https://sonarsource.atlassian.net/browse/PREQ-9089). We are only fixing commit messages.
Context
The PRs in the PR stack #1823 were merged via rebase by mistake, causing intermediate commits and commits that don't follow project-convention to land on
master. To fix the mistake, the commits were removed from master (a backup branchbackup/master-before-reset-PREQ-9089that contains of state ofmasterpre-removal was created).In this PR I've created the proper commits, each commit corresponds to the commit I intended to land on master for the corresponding PR in the PR stack.
This PR should be merged via rebase.
See the full discussion on:
https://sonarsource.slack.com/archives/CFUMLJ26S/p1790759840845249
https://sonarsource.atlassian.net/browse/PREQ-9089
Validation
$ git diff origin/backup/master-before-reset-PREQ-9089 # The diff is empty, as intended.Part of AT-19