Reference Brackets Assigned by Reference Brackets Do Not Enforce Spaces. #5806
Labels
Comments
|
I have forked, branched and added two failing tests, one for |
|
I forked and fixed this issue in Pull Request #5807. |
joshuapinter
added a commit
to joshuapinter/rubocop
that referenced
this issue
Apr 25, 2018
Fixes Issue rubocop#5806. Create robust tests to catch both leading and trailing whitespace when assigning a reference bracket from another reference bracket for both `EnforcedStyle: no_space` and `EnforcedStyle: space` When assigning a reference bracket to another reference bracket, the `Layout/SpaceInsideReferenceBracket` Cop fails to pickup on the _assignee_ reference bracket correctly. For example: ```ruby a[ "foo"] = b["something"] ``` The above example would not register an offense when `EnforcedStyle: no_space` is set. The issue is due to how the `left_ref_bracket` method searches from the end of the node to the beginning of the node to find the _first_ left reference bracket that it encounters. This works fine for all other test cases, especially when there are multiple reference brackets, however, when a reference bracket is being assigned to another reference bracket, this mistakenly picks up the _assignor_ left reference bracket (i.e. `b[`) instead of the _assignee_ left reference bracket (i.e. `a[`). To handle this, we simple modify the `left_ref_bracket` method to check to see if the node is using the `[]=` method. If it is, look for the first left reference bracket from the _start_ of the node. Also added tests to ensure right hand side offenses still work.
bbatsov
pushed a commit
that referenced
this issue
Apr 29, 2018
Fixes Issue #5806. Create robust tests to catch both leading and trailing whitespace when assigning a reference bracket from another reference bracket for both `EnforcedStyle: no_space` and `EnforcedStyle: space` When assigning a reference bracket to another reference bracket, the `Layout/SpaceInsideReferenceBracket` Cop fails to pickup on the _assignee_ reference bracket correctly. For example: ```ruby a[ "foo"] = b["something"] ``` The above example would not register an offense when `EnforcedStyle: no_space` is set. The issue is due to how the `left_ref_bracket` method searches from the end of the node to the beginning of the node to find the _first_ left reference bracket that it encounters. This works fine for all other test cases, especially when there are multiple reference brackets, however, when a reference bracket is being assigned to another reference bracket, this mistakenly picks up the _assignor_ left reference bracket (i.e. `b[`) instead of the _assignee_ left reference bracket (i.e. `a[`). To handle this, we simple modify the `left_ref_bracket` method to check to see if the node is using the `[]=` method. If it is, look for the first left reference bracket from the _start_ of the node. Also added tests to ensure right hand side offenses still work.
|
Resolved in #5807 . This issue can/should be closed. |
8 tasks
|
@garettarrowood Agreed! Must not have put "Fixes #5806" in the PR. Closing now... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
To be clear, whether you have
EnforcedStyle: no_spaceorEnforcedStyle: space, the first Reference Bracket is completely ignored when assigning from another Reference Bracket.Expected behavior
Rubocop to register an offence when there is no space in the reference bracket being assigned:
EnforcedStyle: no_spaceEnforcedStyle: spaceActual behavior
No offence was registered in either test case.
Steps to reproduce the problem
Test the above lines with Rubocop, either enforcing spaces or not enforcing spaces.
RuboCop version
$ bundle exec rubocop -V 0.55.0 (using Parser 2.5.1.0, running on ruby 2.2.0 x86_64-darwin16)The text was updated successfully, but these errors were encountered: