Add new sniff for checking Control structure brackets are on a new line

[photodude]

Add new sniff for checking Control structure brackets are on a new line Add new test for the new sniff checking Control structure brackets are on a new line

[photodude]

@gsherwood What Code standard should I use to format things here? I seem to have chosen the wrong one for the formatting.

[jrfnl]

@photodude PHPCS has its own ruleset you can use: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/phpcs.xml.dist :smile:

[photodude]

Thanks @jrfnl, hopefully I implemented the fixer right since I just ran it directly from the github files so I didn't have to mess with my PHPCS 2.9 install.

[jrfnl]

@photodude Want me to have a look/review ?

[photodude]

Do you mean to review the sniff? Yes, please. There are probably better was to do this sniff. The sniff was originally based off the PEAR.Classes.ClassDeclaration Sniff but modified to work with control structure tokens.

I think the code style is fixed, if I can get the one comment section to stop yelling at me that it wants more or less indenting, or things at the end.

[photodude]

code style should be all in the green now

[photodude]

@jrfnl @gsherwood This is now good to go for review consideration

[photodude]

Quick question, I'm getting a CS error on "Implicit true comparisons prohibited; use === TRUE instead" the error occurs for this comparison

if (array_key_exists('nested_parenthesis', $tokens[$stackPtr])) {

What is the correct solution for this?

[gsherwood]

@photodude

if (array_key_exists('nested_parenthesis', $tokens[$stackPtr]) === true) {

[photodude]

Thanks @gsherwood (and @jrfnl I saw your comment too for a moment)

I believe the last items here are

• [x] convert the unit test into fake code
• [ ] add unit test cases using // comment and the phpcs annotations like // phpcs:ignore -- for reasons

[gsherwood]

I've done another review of this but it's not ready to merge yet. It still needs those last two items completed (remove real code from tests + add tests for comment syntax) plus:

• I think this would be worth renaming to BraceNewLine to match other sniffs
• Please replace tabs with spaces in the unit tests files unless the tab is testing something

I've also put a few minor changes into a review for you.

[gsherwood]

I've done another review of this and it's not ready to merge:

• The unit test files don't match the sniff name, so they aren't actually running. The files need to be renamed so they are tested.
• If the files are renamed, the unit tests will fail because the list of line numbers to error counts is wrong.
• The sniff enforces indentation using tabs, which is not something other sniffs do. If the sniff wants to check indentation of the opening brace, it needs to do so using spaces. If you use tabs in your coding standard, the DisallowSpaceIndent sniff will go ahead and fix that for you, understanding where the proper tab stops are.
• The unit test files is indented using tabs, so this needs to be replaced with spaces.
• There is no need for an indent property in this sniff if you are just trying to align the opening brace under the IF statement. PHPCS provides 'column' indexes in the tokens array so you know where to align things. This will make the sniff possible to use for any indent type and length.

I'm going to remove this from the 3.5.0 release and there looks to be too much more to do here to get it ready.

[pimjansen]

@photodude any way that this will be finished? If you need some assistance let me know

[photodude]

@pimjansen I would love to get this finished. Assistance would be greatly appreciated and would greatly speed along the process. (I, unfortunately, have reduced availability to work on side projects like this at the moment)

• [ ] fix issue with closure combined with a chained function call
• [ ] fix issue with closure combined with switch case control structure
• [ ] The sniff enforces indentation using tabs, which is not something other sniffs do. If the sniff wants to check indentation of the opening brace, it needs to do so using spaces. If you use tabs in your coding standard, the DisallowSpaceIndent sniff will go ahead and fix that for you, understanding where the proper tab stops are.
• [ ] There is no need for an indent property in this sniff if you are just trying to align the opening brace under the IF statement. PHPCS provides 'column' indexes in the tokens array so you know where to align things. This will make the sniff possible to use for any indent type and length.
• [x] The unit test files are indented using tabs, replace tabs with spaces unless the tab is testing something (add comments to specify if the tabs are testing something specific)
• [x] The test file doesn't have any examples of comments being placed between the closing parenthesis and the opening brace of the control structures, so it's not obvious how those would be fixed. A few examples of using // comment and the phpcs annotations like // phpcs:ignore -- for reasons would be good, just to make sure the sniff copes with them on other people's code.

[pimjansen]

What is the easiest way to help here since this is open for way too long. What is still open and the fastest way to continue on this branch?

@photodude @gsherwood

[photodude]

What is the easiest way to help here since this is open for way too long. What is still open and the fastest way to continue on this branch?

@pimjansen If you can work on any of the open items in the to do checklist that would help complete this sniff.

[photodude]

@pimjansen I don't think I'm going to get back to working on this, as I'm now working in a different field. If you would like to take over and finish this PR all the necessary information on what needs to be completed is here.