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

TrivialAccessors breaks methods with ? #2763

Closed
ptarjan opened this issue Feb 3, 2016 · 11 comments
Closed

TrivialAccessors breaks methods with ? #2763

ptarjan opened this issue Feb 3, 2016 · 11 comments

Comments

@ptarjan
Copy link
Contributor

ptarjan commented Feb 3, 2016

class Bar
  def self.foo?
    @foo
  end
end

$ rubocop /tmp/a.rb -a

gives

class Bar
  class << self
    attr_reader :foo?
  end
end

which is invalid:

$ ruby /tmp/a.rb
/tmp/a.rb:3:in `attr_reader': invalid attribute name `foo?' (NameError)
        from /tmp/a.rb:3:in `singleton class'
        from /tmp/a.rb:2:in `<class:Bar>'
        from /tmp/a.rb:1:in `<main>'
@Drenmi
Copy link
Collaborator

Drenmi commented Feb 3, 2016

I just tried this out, and can confirm it only happens with class methods. The following is marked as an offense, but not auto-corrected:

class Foo
  def bar?
    @bar
  end
end

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 3, 2016

@Drenmi why is that an offense?

@Drenmi
Copy link
Collaborator

Drenmi commented Feb 4, 2016

@ptarjan: The style guide favours the use of attr_reader. Of course you can't do attr_reader :bar?, because it will try to create an instance variable called @bar?, and question marks are not valid in variable names. It looks like the cop doesn't fully take this into account.

An example of a working way would be:

class Foo
  attr_reader :bar
  alias_method :bar?, :bar
end

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 4, 2016

I thought alias_method as frowned upon? Well whatever works, I just want the rubocop -a to create code that doesn't have syntax errors and is semantically equivalent to the original.

@alexdowad
Copy link
Contributor

Just configure Style/TrivialAccessors with AllowPredicates: true.

(Maintainers, what would you think about making AllowPredicates: true the default? Personally, I prefer it.)

This issue can be closed.

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 5, 2016

Thanks @alexdowad. Where is that option documented? Can you add a little blurb to https://github.com/bbatsov/rubocop/blob/master/config/default.yml

@alexdowad
Copy link
Contributor

Where is that option documented?

RuboCop is, for the most part, not documented. It's a WIP, and a lot of help is needed to finish it.

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 5, 2016

@alexdowad This is similarly broken for writers that don't match the exact pattern.

    def self.email_from(email)
      @email_from = email
    end

Can't be written with attr_writter because it doesn't have the =

@alexdowad
Copy link
Contributor

Let's close this issue. It is resolved using an existing config option.

@ptarjan
Copy link
Contributor Author

ptarjan commented Feb 9, 2016

@alexdowad what existing config option? If I set AllowPredicates: true it still complains about #2763 (comment)

@alexdowad
Copy link
Contributor

This is similarly broken for writers that don't match the exact pattern.

The cop wants you to write methods like this using a =.

The original issue here was: "TrivialAccessors breaks methods with ?". That issue is resolved using AllowPredicates.

@ptarjan ptarjan closed this as completed Feb 10, 2016
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

3 participants