Generic/MultipleStatementAlignment: fix wrapped chained assignment

[morozov]

Description

Fixed Generic.Formatting.MultipleStatementAlignment fixer failure to fix a standard violation if a chained assignment is wrapped.

Change summary:

1. Instead of aligning a wrapped assignment operator against its operand, align it against the start of its own line when it follows another assignment. This stops the fixer from looping.
2. Measure its padding from the indentation before the operator, not the whitespace after the operand.

Suggested changelog entry

Fixed Generic.Formatting.MultipleStatementAlignment fixer failure to fix a standard violation if a chained assignment is wrapped.

Related issues/external references

Fixes #1435

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.

[jrfnl]

@morozov Thanks for this PR. Just wondering what happened to the PR template ?

[morozov]

Just wondering what happened to the PR template ?

Sorry, I prepared the PR description in a local editor and then mechanically inserted into the form.

[jrfnl]

Sorry, I prepared the PR description in a local editor and then mechanically inserted into the form.

Understood. Thanks for fixing that up. I'll try and have a look at this tomorrow when my brain is fresh again ;-)

[jrfnl]

@morozov Thanks for this PR! I'm in two minds about the principle of this patch.

The sniff name is MultipleStatementAlignment, but this patch now makes it so the sniff would also address alignment within multi-line single statements.
I believe this wasn't handled previously as this sniff is commonly used in combination with the (incorrectly named) DisallowMultipleStatements sniff.

I wonder whether this situation - "alignment within multi-line single statements" - should:

• Get its own sniff instead ? With this sniff bowing out, similar to this sniff already bowing out when multi-assignment in a single-line single statement is detected.
• If it will be addressed within this sniff (because a new sniff would need largely the same code) - whether it should get its own error code ?

What do you think ?

[jrfnl]

Would you mind adding some more tests ? I'm thinking the patch needs the following additional tests:

• A wrapped statement which is already correctly aligned.
• A similar set to the current tests, but then for indented statements, i.e. this kind of code, but found in a function/method declaration or a control structure.
• A set with the $maxPadding property set to verify and document how the sniff will handle that.
• A set with the $alignAtEnd property set to verify the code handles that correctly too.
• Code where there is a comment on the same line as the second assignment, i.e.:

$wrappedChain = $foo
 /* foo */   = $bar;

I believe the patch will largely, if not completely, handle these additional situations correctly already, but would like to safeguard that for the future.

[morozov]

Get its own sniff instead ? With this sniff bowing out, similar to this sniff already bowing out when multi-assignment in a single-line single statement is detected.

Let's start with bowing out. I put aside the version of the patch that actually fixes the multi-assignment alignments, we can consider it for a separate sniff separately.

[morozov]

@jrfnl are you still interested in this PR?

[jrfnl]

@jrfnl are you still interested in this PR?

@morozov Not to worry, I have not forgotten about this PR. I'm trying to work through my backlog at the moment. Life and such got in the way. Sorry 🤷🏻‍♀️

[morozov]

No problem. I will keep it open then.