[Fix #6305] Fix infinite loop for Layout/EmptyLinesAroundAccessModifier
#6307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ssModifier` Fixes rubocop#6305. This is a problem that occurs after rubocop#6236. This PR fixes infinite loop for `Layout/EmptyLinesAroundAccessModifier` and `Layout/EmptyLinesAroundClassBody` when specifying a superclass that breaks the line. ```ruby class AVerryLongClassNameDoYouLikeIt < ItEvenInheritsFromALongClassNameSOINeedToAddALineBrake protected def foobar end end ``` With this PR, `Layout/EmptyLinesAroundAccessModifier` cop makes aware of the above case that specifying a superclass that breaks the line. This is the same behavior as writing a class definition on a single line. As a result, `Layout/EmptyLinesAroundAccessModifier` and `Layout/EmptyLinesAroundClassBody` will not conflict with the handling of empty line under the class definition. This problem was caused by using the regular expression in `EmptyLinesAroundAccessModifier#class_def?` method. This PR will change it to use AST.
Ah, regular expressions bite us again. :-) Great work, Koichi! |
koic
deleted the
fix_infinite_loop_empty_lines_around_access_modifier
branch
|
Thank you, Bozhidar! |
8 tasks
koic
added a commit
to koic/rubocop
that referenced
this pull request
Oct 5, 2018
…of regexp Follow up of rubocop#6307 (comment). This PR refactors `Layout/EmptyLinesAroundAccessModifier` to use AST instread of regexp.
bbatsov
pushed a commit
that referenced
this pull request
Oct 6, 2018
…of regexp Follow up of #6307 (comment). This PR refactors `Layout/EmptyLinesAroundAccessModifier` to use AST instread of regexp.
8 tasks
koic
added a commit
to koic/rubocop
that referenced
this pull request
Dec 14, 2018
…ccessModifier` Fixes rubocop#6566. This PR fixes a false positive for `Layout/EmptyLinesAroundAccessModifier` when at the end of specifying a superclass is missing blank line. The following is originally no offense code. (It is probably a regression by rubocop#6307) ```console % cat example.rb class SomeController < SomeOtherController def index; end private end % rubocop example.rb --only Layout/EmptyLinesAroundAccessModifier -a Inspecting 1 file C Offenses: example.rb:4:3: C: [Corrected] Layout/EmptyLinesAroundAccessModifier: Keep a blank line before and after private. private ^^^^^^^ 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct adds a blank line after `private`. This is due to false positives. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private + end ``` This caused the infinite loop in `Layout/EmptyLinesAroundAccessModifier` and `Layout/EmptyLinesAroundClassBody`. ```console % rubocop example.rb --only Layout/EmptyLinesAroundClassBody -a Inspecting 1 file C Offenses: example.rb:5:1: C: [Corrected] Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end. 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct removes the blank line from after of `private`. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private - end ``` It loops to the first code. This PR fixes the infinite loop.
bbatsov
pushed a commit
that referenced
this pull request
Dec 14, 2018
…difier` Fixes #6566. This PR fixes a false positive for `Layout/EmptyLinesAroundAccessModifier` when at the end of specifying a superclass is missing blank line. The following is originally no offense code. (It is probably a regression by #6307) ```console % cat example.rb class SomeController < SomeOtherController def index; end private end % rubocop example.rb --only Layout/EmptyLinesAroundAccessModifier -a Inspecting 1 file C Offenses: example.rb:4:3: C: [Corrected] Layout/EmptyLinesAroundAccessModifier: Keep a blank line before and after private. private ^^^^^^^ 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct adds a blank line after `private`. This is due to false positives. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private + end ``` This caused the infinite loop in `Layout/EmptyLinesAroundAccessModifier` and `Layout/EmptyLinesAroundClassBody`. ```console % rubocop example.rb --only Layout/EmptyLinesAroundClassBody -a Inspecting 1 file C Offenses: example.rb:5:1: C: [Corrected] Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end. 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct removes the blank line from after of `private`. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private - end ``` It loops to the first code. This PR fixes the infinite loop.
Nyangawa
pushed a commit
to Nyangawa/rubocop
that referenced
this pull request
Jan 24, 2019
…ccessModifier` Fixes rubocop#6566. This PR fixes a false positive for `Layout/EmptyLinesAroundAccessModifier` when at the end of specifying a superclass is missing blank line. The following is originally no offense code. (It is probably a regression by rubocop#6307) ```console % cat example.rb class SomeController < SomeOtherController def index; end private end % rubocop example.rb --only Layout/EmptyLinesAroundAccessModifier -a Inspecting 1 file C Offenses: example.rb:4:3: C: [Corrected] Layout/EmptyLinesAroundAccessModifier: Keep a blank line before and after private. private ^^^^^^^ 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct adds a blank line after `private`. This is due to false positives. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private + end ``` This caused the infinite loop in `Layout/EmptyLinesAroundAccessModifier` and `Layout/EmptyLinesAroundClassBody`. ```console % rubocop example.rb --only Layout/EmptyLinesAroundClassBody -a Inspecting 1 file C Offenses: example.rb:5:1: C: [Corrected] Layout/EmptyLinesAroundClassBody: Extra empty line detected at class body end. 1 file inspected, 1 offense detected, 1 offense corrected ``` Auto-correct removes the blank line from after of `private`. ```diff % cat example.rb class SomeController < SomeOtherController def index; end private - end ``` It loops to the first code. This PR fixes the infinite loop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #6305.
This is a problem that occurs after #6236.
This PR fixes infinite loop for
Layout/EmptyLinesAroundAccessModifierandLayout/EmptyLinesAroundClassBodywhen specifying a superclass that breaks the line.With this PR,
Layout/EmptyLinesAroundAccessModifiercop makes aware of the above case that specifying a superclass that breaks the line. This is the same behavior as writing a class definition on a single line.As a result,
Layout/EmptyLinesAroundAccessModifierandLayout/EmptyLinesAroundClassBodywill not conflict with the handling of empty line under the class definition.This problem was caused by using the regular expression in
EmptyLinesAroundAccessModifier#class_def?method. This PR will change it to use AST.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake default. It executes all tests and RuboCop for itself, and generates the documentation.