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 #6861] Fix a false positive for Layout/IndentationWidth #6866

Merged

Conversation

koic
Copy link
Member

@koic koic commented Mar 31, 2019

Fixes #6861.

This is a regression by #6792.

This PR fixes infinite loop for Layout/IndentationWidth cop and Layout/AccessModifierIndentation cop.

# exmaple.rb
class Foo
private

  def do_something
  end
end
# .rubocop.yml
% cat .rubocop.yml
Layout/AccessModifierIndentation:
  EnforcedStyle: outdent

First, auto-corrected by Layout/IndentationWidth.

% rubocop -v
0.66.0
% rubocop -a --only Layout/IndentationWidth
Inspecting 1 file
C

Offenses:

example.rb:2:1: C: [Corrected] Layout/IndentationWidth: Use 2 (not 0)
spaces for indentation.
private

1 file inspected, 1 offense detected, 1 offense corrected
% git diff example.rb
diff --git a/example.rb b/example.rb
index 063e6a5..081b80a 100644
--- a/example.rb
+++ b/example.rb
@@ -1,5 +1,5 @@
 class Foo
-private
+  private

   def do_something
   end

Next, auto-corrected by Layout/Layout/AccessModifierIndentation (EnforcedStyle: outdent).

% rubocop -a --only Layout/AccessModifierIndentation
Inspecting 1 file
C

Offenses:

example.rb:2:3: C: [Corrected] Layout/AccessModifierIndentation: Outdent
access modifiers like private.
  private
  ^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected

This will return to the original code.

% git diff
diff --git a/example.rb b/example.rb
index 081b80a..063e6a5 100644
--- a/example.rb
+++ b/example.rb
@@ -1,5 +1,5 @@
 class Foo
-  private
+private

   def do_something
   end

That caused the infinite loop in Layout/IndentationWidth and Layout/AccessModifierIndentation (EnforcedStyle: outdent).

With this PR, Layout/indentationWidth cop makes aware of Layout/AccessModifierIndentation cop.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.


check_indentation(base, member)
end
check_members_for_non_rails_style(base, members)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝I extracted to the method because it was warned by Metrics cops.

@jonas054
Copy link
Collaborator

Hi @koic,
Great description of the problem and solution. I found one remaining problem. If I start with EnforcedStyle: outdent for Layout/AccessModifierIndentation and

# example.rb
class Foo
                 private

  def do_something
    # something
  end
end

Auto-correction changes it to

# example.rb
class Foo
private

                 def do_something
                   # something
                 end
end

@koic
Copy link
Member Author

koic commented Apr 1, 2019

@jonas054 Sure! I was not aware of this problem 💦 I'll look it up. Thanks for your checking.

@deivid-rodriguez
Copy link
Contributor

Hello @koic! Thanks for working on this, do you still plan to wrap this up?

@mvastola
Copy link

Wanted to second merging this PR once the remaining issues are fixed. If it helps, this only seems to be a problem if the access modifier is the first line of code in the class/module. So maybe the issue with autocorrect could be fixed by limiting changes to the first line of code in this case?

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 16, 2019

@koic What's the state here?

@koic koic force-pushed the fix_false_positive_for_layout_indentation_width branch from 26f8733 to 778c557 Compare July 18, 2019 08:56
@koic
Copy link
Member Author

koic commented Jul 18, 2019

Sorry I'm late. I updated this PR and added the following mentioned issue as a test.
#6866 (comment)

@koic koic force-pushed the fix_false_positive_for_layout_indentation_width branch 3 times, most recently from 0661cc6 to fe3ea26 Compare July 24, 2019 23:35
Fixes rubocop#6861.

This is a regression by rubocop#6792.

This PR fixes infinite loop for `Layout/IndentationWidth` cop and
`Layout/AccessModifierIndentation` cop.

```ruby
# exmaple.rb
class Foo
private

  def do_something
  end
end
```

```yaml
# .rubocop.yml
% cat .rubocop.yml
Layout/AccessModifierIndentation:
  EnforcedStyle: outdent
```

First, auto-corrected by `Layout/IndentationWidth`.

```console
% rubocop -v
0.66.0
% rubocop -a --only Layout/IndentationWidth
Inspecting 1 file
C

Offenses:

example.rb:2:1: C: [Corrected] Layout/IndentationWidth: Use 2 (not 0)
spaces for indentation.
private

1 file inspected, 1 offense detected, 1 offense corrected
```

```diff
% git diff example.rb
diff --git a/example.rb b/example.rb
index 063e6a5..081b80a 100644
--- a/example.rb
+++ b/example.rb
@@ -1,5 +1,5 @@
 class Foo
-private
+  private

   def do_something
   end
```

Next, auto-corrected by `Layout/Layout/AccessModifierIndentation`
(`EnforcedStyle: outdent`).

```console
% rubocop -a --only Layout/AccessModifierIndentation
Inspecting 1 file
C

Offenses:

example.rb:2:3: C: [Corrected] Layout/AccessModifierIndentation: Outdent
access modifiers like private.
  private
  ^^^^^^^

1 file inspected, 1 offense detected, 1 offense corrected
```

This will return to the original code.

```diff
% git diff
diff --git a/example.rb b/example.rb
index 081b80a..063e6a5 100644
--- a/example.rb
+++ b/example.rb
@@ -1,5 +1,5 @@
 class Foo
-  private
+private

   def do_something
   end
```

That caused the infinite loop in `Layout/IndentationWidth` and
`Layout/AccessModifierIndentation` (`EnforcedStyle: outdent`).

With this PR, `Layout/indentationWidth` cop makes aware of
`Layout/AccessModifierIndentation` cop.
@koic koic force-pushed the fix_false_positive_for_layout_indentation_width branch from fe3ea26 to 20d76ec Compare July 26, 2019 05:42
@deivid-rodriguez
Copy link
Contributor

Thanks for fixing this @koic!! ❤️

@jonas054 jonas054 merged commit 95dd998 into rubocop:master Jul 27, 2019
@koic koic deleted the fix_false_positive_for_layout_indentation_width branch July 27, 2019 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Layout/IndentationWidth conflicts with Layout/AccessModifierIndentation EnforcedStyle: outdent
5 participants