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

Lint/UselessAccessModifier not recognizing delegated methods #2731

Closed
eljojo opened this issue Jan 28, 2016 · 7 comments
Closed

Lint/UselessAccessModifier not recognizing delegated methods #2731

eljojo opened this issue Jan 28, 2016 · 7 comments

Comments

@eljojo
Copy link

eljojo commented Jan 28, 2016

Hi,
The Lint/UselessAccessModifier cop doesn't recognize delegated method's (using ActiveSupport).

It considers the following private useless.

  private
  delegate :logger, to: :Rails
@alexdowad
Copy link
Contributor

@bbatsov didn't agree to merge a fix for this issue, as he doesn't want Rails-specific stuff built into RuboCop. Our goal is to get all Rails-related code out of RC core and into some kind of plugin.

@eljojo
Copy link
Author

eljojo commented Feb 9, 2016

I see. Thanks a lot for taking the time to review this :)

@eljojo eljojo closed this as completed Feb 9, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 9, 2016

To be more specific - I'm fine with making this particular cop configurable, but I'm not OK with hardcoding Rails specific stuff in generic Ruby cops.

@eljojo
Copy link
Author

eljojo commented Feb 11, 2016

@bbatsov should I re-open this?

@bbatsov bbatsov reopened this Feb 11, 2016
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 11, 2016

I've reopened it. If you're up for a bit of hacking you can make the cop configurable yourself. It's pretty easy, but unfortunately I'm extremely short on time.

@Drenmi
Copy link
Collaborator

Drenmi commented Jun 26, 2016

I've been thinking a bit about this one, and can off the top of my head see two routes we could take.

  1. Add a SafeMode which accepts it as long as any DSL/class method is called in the scope of the modifier. (Simpler for the user, but less flexible, and more prone to false negatives.)
  2. Add a configurable list of acceptable DSL/class methods. (More complex for user, but allows to fully express intent.)

I'm leaning towards option 2. What do you think @alexdowad, @bbatsov?

@alexdowad
Copy link
Contributor

Option 2 is better.

pat added a commit to pat/rubocop that referenced this issue Jan 9, 2017
This addresses the issue raised in rubocop#2731 where methods (often from outside of Ruby itself) may be called within the context of a module that in turn define methods. The `Lint/UselessAccessModifier` wasn’t taking these into account.

Much like the existing approach taken with blocks/context-creating methods, there’s now a configuration option `MethodCreatingMethods` for the cop. This setting is an array of methods which, when called, are known to create other methods in the module’s current access context.
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

No branches or pull requests

4 participants