Generic/IncrementDecrementSpacing: fix post-decrement error message #292
Conversation
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
@rodrigoprimo Approval on the fix.
However, even though the error message itself is not tested by the test suite as-is, this is a situation which was not covered by the pre-existing tests, so a test should be added to cover this.
Having a test in the test case file for this situation also makes it easier to validate the issue and review the change.
rodrigoprimo
force-pushed
the
fix-increment-decrement-spacing-error-message
branch
from
4d0f580 to
6ba6a91
Compare
|
Thanks for the review, @jrfnl. I added a commit with four new tests. They use a syntax that was not used in previous tests, and one of them has a newline between the object property accessed through a method call and the post-incrementor to test the fix added in this PR. |
While I can see the usefulness of the extra tests, they don't belong in this PR/commit - save for the one with the new line. |
If you'd move the test with the new line to the first commit and leave the other tests in the second commit, I can accept the PR. |
rodrigoprimo
force-pushed
the
fix-increment-decrement-spacing-error-message
branch
from
6ba6a91 to
781d9c2
Compare
Done |
This commit fixes the post-decrement error message when there is one or more newlines between the variable and the post-decrement operator. The wrong variable was being used to check whether the post-decrement and the variable were not on the same line, resulting in an incorrect error message. The error message should explicitly say that a newline was found. Before this commit the error message was: ``` Expected no spaces between $i and the decrement operator; 0 found ``` Now it is: ``` Expected no spaces between the decrement operator and $i; newline found ``` (`newline` instead of `0`) This commit also includes a test that exercises the modified line.
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
Thanks for making that tweak @rodrigoprimo !
I'll rebase without changes & merge the PR once the build has gone through.
Adds tests using pre/post-incrementors with an object property accessed through a method call.
jrfnl
force-pushed
the
fix-increment-decrement-spacing-error-message
branch
from
781d9c2 to
4607a26
Compare
2 participants
Description
This PR fixes the post-decrement error message when there is one or more newlines between the variable and the post-decrement operator.
The wrong variable was used to check whether the post-decrement and the variable were not on the same line, resulting in an incorrect error message. The error message should explicitly say that a newline was found.
Before this commit, the error message was:
Now it is:
(
newlineinstead of0)Suggested changelog entry
Generic/IncrementDecrementSpacing: fix error message when one or more newlines are found between the variable and the post-decrement operator
Related issues/external references
Fixes #288
Types of changes
PR checklist