PHPCSStandards / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
906 stars 55 forks source link

Generic/InlineControlStructure: fix two bugs and improve code coverage #482

Closed rodrigoprimo closed 4 months ago

rodrigoprimo commented 5 months ago

Description

This PR makes a few changes to the Generic.ControlStructures.InlineControlStructure sniff:

While working on this PR, I noticed that the sniff does not handle well a few syntax errors related to the do-while control structure. I will open a separate issue describing what I found.

Suggested changelog entry

Generic.ControlStructures.InlineControlStructure: properly handle else if statements where there is a comment between else and if Generic.ControlStructures.InlineControlStructure: fix false positives when handling elseif, if, and foreach without a body

Related issues/external references

Part of https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/146

Types of changes

PR checklist

rodrigoprimo commented 4 months ago

Closing this PR. As discussed via chat, I will open smaller PRs following @jrfnl suggestion:

PR 1 - commit 1 PR 2 - commit 2 PR 3 - commit 6 + 7 PR 4 - commit 3 + 4 + 5 + 8

jrfnl commented 2 months ago

@rodrigoprimo The first follow-up PR, cherrypicked out of this PR was pulled and merged a while ago. What is the status of the rest ?

Note: it's fine if you want to divert this for later, but in that case, let's open a reminder ticket to make sure this is not overlooked as this PR is closed.

rodrigoprimo commented 2 months ago

@jrfnl, I started working on the issues related to commit 2, but I paused because I wasn't sure about the changes that I was making to the tokenizer tests. I plan to discuss this with you in our pairing session tomorrow. I can create an issue (or a few issues) for the items that are still pending after our conversation tomorrow.

jrfnl commented 2 months ago

I can create an issue (or a few issues) for the items that are still pending after our conversation tomorrow.

@rodrigoprimo That sounds like a good idea as it will allow us to track what still needs doing more easily (at least more easily than to remember to look at a closed issue)