Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indent for closing function parenthesis #3758

Closed

Conversation

fredden
Copy link
Contributor

@fredden fredden commented Feb 10, 2023

I'm having trouble writing a test case for this. This seems to mostly be because changing this line alone doesn't break any existing tests, and I can't find a test case that breaks like the live example does because it seems to be an interaction between PEAR.Functions.FunctionCallSignature and Generic.WhiteSpace.ScopeIndent / Squiz.WhiteSpace.ScopeClosingBrace which causes the issue.

Long explanation with a real example of the problem

Here's a code snippet that I'm working with:

 70|
 71|        // Use spaceship operator to force the array sort order on the pinned items
 72|        uasort($pinnedOptions, function ($item, $itemComparison) {
 73|            return $item[Option::KEY_SORT_ORDER] <=> $itemComparison[Option::KEY_SORT_ORDER];
 74|        });
 75|

The first pass through the fixer says:

	PEAR.Functions.FunctionCallSignature:412 replaced token 514 (T_OPEN_PARENTHESIS on line 72) "(" => "(\n············"
	PEAR.Functions.FunctionCallSignature:425 replaced token 551 (T_CLOSE_PARENTHESIS on line 74) ")" => "\n············)"
        => Fixing file: 2/2 violations remaining [made 1 pass]...               
	* fixed 2 violations, starting loop 2 *

Which gives us:

 70|
 71|        // Use spaceship operator to force the array sort order on the pinned items
 72|        uasort(
 73|            $pinnedOptions, function ($item, $itemComparison) {
 74|            return $item[Option::KEY_SORT_ORDER] <=> $itemComparison[Option::KEY_SORT_ORDER];
 75|        }
 76|            );
 77|

This is the step which I'm fixing in this pull request. The closing parenthesis of the function call is misaligned by one indentation step. (Found 12, expected 8.)

The second pass through the fixer says:

	Generic.WhiteSpace.ScopeIndent:1530 replaced token 531 (T_WHITESPACE on line 74) "············return" => "················return"
	Generic.WhiteSpace.ScopeIndent:1530 replaced token 551 (T_WHITESPACE on line 75) "········}" => "············}"
	=> Changeset started by PEAR.Functions.FunctionCallSignature:622
		Q: PEAR.Functions.FunctionCallSignature:628 replaced token 519 (T_WHITESPACE on line 73) "·function" => "function"
		Q: PEAR.Functions.FunctionCallSignature:631 replaced token 520 (T_CLOSURE on line 73) "function" => "\n············function"
		A: PEAR.Functions.FunctionCallSignature:635 replaced token 519 (T_WHITESPACE on line 73) "·function" => "function"
		A: PEAR.Functions.FunctionCallSignature:635 replaced token 520 (T_CLOSURE on line 73) "function" => "\n············function"
	=> Changeset ended: 2 changes applied
	Squiz.WhiteSpace.ScopeClosingBrace:104 replaced token 552 (T_CLOSE_CURLY_BRACKET on line 75) "}" => "····}"
        => Fixing file: 5/2 violations remaining [made 2 passes]...             
	* fixed 5 violations, starting loop 3 *

Which gives us:

 70|
 71|        // Use spaceship operator to force the array sort order on the pinned items
 72|        uasort(
 73|            $pinnedOptions,
 74|            function ($item, $itemComparison) {
 75|                return $item[Option::KEY_SORT_ORDER] <=> $itemComparison[Option::KEY_SORT_ORDER];
 76|                }
 77|            );
 78|

The third pass through the fixer says:

	Generic.WhiteSpace.ScopeIndent:1530 replaced token 552 (T_WHITESPACE on line 76) "················}" => "············}"
	* token 552 has already been modified, skipping *
        => Fixing file: 1/2 violations remaining [made 3 passes]...             
	* fixed 1 violations, starting loop 4 *

Which gives us:

 70|
 71|        // Use spaceship operator to force the array sort order on the pinned items
 72|        uasort(
 73|            $pinnedOptions,
 74|            function ($item, $itemComparison) {
 75|                return $item[Option::KEY_SORT_ORDER] <=> $itemComparison[Option::KEY_SORT_ORDER];
 76|            }
 77|            );
 78|

And then the fixer declared victory. However, it's left the indentation of the closing function call vertically lined up differently from what I expect.

I'm expecting:

 70|
 71|        // Use spaceship operator to force the array sort order on the pinned items
 72|        uasort(
 73|            $pinnedOptions,
 74|            function ($item, $itemComparison) {
 75|                return $item[Option::KEY_SORT_ORDER] <=> $itemComparison[Option::KEY_SORT_ORDER];
 76|            }
 77|        );
 78|

Hopefully the change in this pull request fixes the problem, but I'm having trouble verifying that it actually does. Please can I have some help?

@fredden
Copy link
Contributor Author

fredden commented May 31, 2023

I can confirm that this change is working well with phpcbf in our automated pipeline. The closing ) is lined up as expected.

@jrfnl
Copy link
Contributor

jrfnl commented Jun 7, 2023

On hold until a viable test scenario has been identified and added to the PR.

@fredden
Copy link
Contributor Author

fredden commented Sep 6, 2023

I've managed to track this down to 6d1b209 (which used to be part of #3631 before dc4c2d9 was added).
Without 6d1b209, I'm unable to reproduce this bug.
With the current version of #3631, I'm also unable to reproduce this bug.

@fredden fredden closed this Sep 6, 2023
@fredden fredden deleted the fix-closing-function-paren-indent branch September 6, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants