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

Style/AccessorGrouping autocorrect leaves behind empty lines #8406

Closed
tas50 opened this issue Jul 27, 2020 · 3 comments
Closed

Style/AccessorGrouping autocorrect leaves behind empty lines #8406

tas50 opened this issue Jul 27, 2020 · 3 comments

Comments

@tas50
Copy link
Contributor

@tas50 tas50 commented Jul 27, 2020

Expected behavior

Left whitespace would be included in the node being removed

Actual behavior

Empty lines are left because the attr_reader entries are indented

Steps to reproduce the problem

Autocorrect this:

Class Chef
  Class HTTP
       attr_reader :url
       attr_reader :sign_on_redirect
       attr_reader :redirect_limit
       attr_reader :options
       attr_reader :middlewares 
  end
end

Results in:

Class Chef
  Class HTTP
    attr_reader :url, :sign_on_redirect, :redirect_limit, :options, :middlewares
    
    

    

    
  end
end

RuboCop version

master
@fatkodima
Copy link
Contributor

@fatkodima fatkodima commented Jul 27, 2020

This is intended behavior to avoid complicating the cop. Those empty lines will be further corrected by Layout/EmptyLines cop.

class Chef
  class HTTP
       attr_reader :url
       attr_reader :sign_on_redirect
       attr_reader :redirect_limit
       attr_reader :options
       attr_reader :middlewares
  end
end
$ rubocop -a
class Chef
  class HTTP
    attr_reader :url, :sign_on_redirect, :redirect_limit, :options, :middlewares
  end
end
@tas50
Copy link
Contributor Author

@tas50 tas50 commented Jul 27, 2020

I wouldn't assume that all users have every cop enabled and aren't just autocorrecting their codebases one rule at a time. When using a corrector.remove you should really make sure that you remove everything and not just the trailing text on the line. It's not a significant amount of complexity to add that. Here's an example where we're making sure we cleanup our whitespace in Cookstyle:

https://github.com/chef/cookstyle/blob/69037bfed2b37bfaf750721675c70abcb3ad822d/lib/rubocop/cop/chef/redundant/resource_with_nothing_action.rb#L48

koic added a commit to koic/rubocop that referenced this issue Jul 28, 2020
## Summary

Fixes rubocop-hq#8406.

This PR improves `Style/AccessorGrouping`'s auto-correction to remove
redundant blank lines.

## Other Information

This PR doesn't update to `Cop::Base` to make it clear the difference
between the changes. Will be updated with other `Style` cops in different PR.
koic added a commit to koic/rubocop that referenced this issue Jul 28, 2020
## Summary

Fixes rubocop-hq#8406.

This PR improves `Style/AccessorGrouping`'s auto-correction to remove
redundant blank lines.

## Other Information

This PR doesn't update to `Cop::Base` to make it clear the difference
between the changes. Will be updated with other `Style` cops in different PR.
@koic
Copy link
Member

@koic koic commented Jul 28, 2020

If it introduces implementation complexity that is not essential for cop purposes, it may be possible to delegate partial auto-corrections to other cop. I opened the PR #8410 due to the implementation added is simple at this time. Thank you.

@koic koic closed this in #8410 Jul 29, 2020
koic added a commit that referenced this issue Jul 29, 2020
…ssor_grouping

[Fix #8406] Improve `Style/AccessorGrouping`'s auto-correction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.