search

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
880 stars 54 forks source link

Class bracing sniffs with PER 2.0 #566

Open rikvdh opened 1 month ago

rikvdh commented 1 month ago

As I'm working on the implementation of PER2.0 I'm facing some issues with some sniffs.

In the first introductory of section 4 of PER 2.0 it states:

If class contains no additional declarations (such as an exception that exists only to extend another exception with a new type), then the body of the class SHOULD be abbreviated as {} and placed on the same line as the previous symbol, separated by a space. For example:

class MyException extends \RuntimeException {}

For this this issue, specifically: PSR2.Classes.ClassDeclaration.OpenBraceNewLine and Squiz.WhiteSpace.ScopeClosingBrace.ContentBefore have an effect.

PSR2.Classes.ClassDeclaration.OpenBraceNewLine

This is for PER 2.0 'not always' valid anymore. When the closing brace is adjacent to the opening brace, it needs to not be on a newline. To check this in this sniff might be odd since it is not related to PSR2. But I would like some option to 'disable' specific cases for this error.

We could also make a new sniff where we inherit this one and extend it for PER2.0? We should discuss the desired solution for this.

Squiz.WhiteSpace.ScopeClosingBrace.ContentBefore

This is a difficult sniff for PER 2.0. Since closing braces do not need to be on a line by itself when the opening-brace is adjacent to the left AND for only functions, methods and classes. The easiest fix for PER2.0 might be to have an option to this sniff to disable this error when a opening brace is adjacent on the left. The openbrace sniffs might catch the cases where the 'closing brace is not allowed there' and then we omit this rule for these situations.

@jrfnl Combining 2 sniffs in one issue feels a bit odd I think, but it is related to one section of PER 2.0. Let me know if you rather like '1 sniff discussion' per issue.

jrfnl commented 1 month ago

@rikvdh Would you mind adding the specific rule in PER which you're trying to address ? That should make it more straight forward to figure out what the best solution direction will be.

rikvdh commented 1 month ago

Thank you. I've updated the issue. While thinking about this, having a configuration option for both sniffs to disable/enable this behavior is the most compatible way I think and is not very hard to implement.