[ntdiary]

Description

Replace string concatenation with placeholders to follow the best practice.

Suggested changelog entry

Changed:

• PSR2.Classes.ClassDeclaration: improvements to error messages
  • ExtendsLine and ImplementsLine error messages now expose 3 data values (previously 1).
  • SpaceBeforeExtends and SpaceBeforeImplements error messages now expose 2 data values (previously 1).

Related issues/external references

Related to #1240

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
• Documentation improvement

PR checklist

• I have checked there is no other PR open for the same change.
• I have read the Contribution Guidelines.
• I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
• I have added tests to cover my changes.
• I have verified that the code complies with the projects coding standards.
• [Required for new sniffs] I have added XML documentation for the sniff.
• I have opened a sister-PR in the documentation repository to update the Wiki.

Changes are already covered by PSR2/Tests/Classes/ClassDeclarationUnitTest.inc

[jrfnl]

Thanks for this @ntdiary ! Sorry for the delay. Merging now.

[jrfnl]

For the record: these changes also affect overruled error messages for the PSR12.Classes.AnonClassDeclaration + Squiz.Classes.ClassDeclaration sniffs (by extension) and this should be mentioned in the changelog.

[ntdiary]

Thanks for this @ntdiary ! Sorry for the delay. Merging now.

No worries at all — please take your time. I've also been involved in other open-source projects for a few years, so I completely understand. :D

For the record: these changes also affect overruled error messages for the PSR12.Classes.AnonClassDeclaration + Squiz.Classes.ClassDeclaration sniffs (by extension) and this should be mentioned in the changelog.

Will update them later. I wasn't aware of the override behavior when I submitted these PRs. I did notice it while looking into issue #1282, but that issue grew into a somewhat large one for me, and I didn't realize this also affected these PRs. 😂

[jrfnl]

For the record: these changes also affect overruled error messages for the PSR12.Classes.AnonClassDeclaration + Squiz.Classes.ClassDeclaration sniffs (by extension) and this should be mentioned in the changelog.

Will update them later. I wasn't aware of the override behavior when I submitted these PRs.

Sorry, my comment apparently was unclear. There is no need to update any code for this.

What I meant to say is that the above mentioned sniffs inherit these error messages from this PSR2 sniff. In effect that means that if someone would be overruling the messages for the affected error codes in the PSR12 or Squiz sniff via a custom ruleset, they should also review the overruled message text to add the (additional) placeholders.

In other words: this is something which only needs to be handled via a mention in the changelog.

No worries at all — please take your time. I've also been involved in other open-source projects for a few years, so I completely understand. :D

Thank you for your understanding and patience!

[ntdiary]

Will update them later. I wasn't aware of the override behavior when I submitted these PRs.

In other words: this is something which only needs to be handled via a mention in the changelog.

That's what I understood as well. It was just my poor English wording. I originally wrote "Will update the changelog in the PR description", but since there are also other PRs whose changelogs need updating, I changed it to "them", which apparently made it ambiguous. 😅

BTW, I'm carefully checking some of the refactoring logic in Neovim, so it may take me another two or three days to work through your other comments. 😁

[jrfnl]

That's what I understood as well. .... I originally wrote "Will update the changelog", ...

Ah! No worries about that. I use the changelog entries in the PRs as a starting point and then craft the changelog manually anyway, so I'll handle that.

BTW, I'm carefully checking some of the refactoring logic in Neovim, so it may take me another two or three days to work through your other comments. 😁

All good. Take your time. Shall I merge those two PRs now and then you can submit separate PR(s) to remove the duplicate code after ?

[ntdiary]

Shall I merge those two PRs now and then you can submit separate PR(s) to remove the duplicate code after ?

Sounds good, I'll make a note of it. :D