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

False Positive and Incorrect Error Message for Style/AccessModifierDeclarations #7740

Closed
mintyfresh opened this issue Feb 21, 2020 · 1 comment · Fixed by #7762
Closed

False Positive and Incorrect Error Message for Style/AccessModifierDeclarations #7740

mintyfresh opened this issue Feb 21, 2020 · 1 comment · Fixed by #7762

Comments

@mintyfresh
Copy link

mintyfresh commented Feb 21, 2020

RuboCop 0.80.0 is reporting offences for marking existing methods as private, even when the method is not defined within the scope it is marked private in.
Furthermore, the message produced by the offence is misleading.


Expected behavior

I expect to be able to mark methods introduced by reflection or otherwise defined outside of the current scope as protected or private.

Actual behavior

Consider a hypothetical example using a Rails model:

ApplicationRecord.connection.create_table(:articles) do |t|
  t.string :name
  t.string :slug
end

class Article < ApplicationRecord
  def name=(value)
    super
    self.slug = name&.parameterize
  end

  private :slug=
end

The rubocop offence is reported on the private :slug= line:

private should not be inlined in method definitions. (convention:Style/AccessModifierDeclarations)

The reported offence is incorrect since:

  • The line is not a method definition, hence the error message is incorrect and misleading.
  • The line affects a method that is not defined by application code. It is created by Rails based on the connected database schema, and its natural access modifier is outside of application control.

Steps to reproduce the problem

Minimum case to reproduce:

module Bar
  def bar
    nil
  end
end

class Foo
  include Bar
  private :bar
end

The error message is produced:

private should not be inlined in method definitions. (convention:Style/AccessModifierDeclarations)

The line neither defines a method, nor does class Foo itself declare the method bar.

RuboCop version

0.80.0 (using Parser 2.7.0.2, running on ruby 2.7.0 x86_64-linux)
@Drenmi
Copy link
Collaborator

Drenmi commented Feb 21, 2020

I think that we can skip registering an offense when the argument is a symbol literal, e.g.:

private :foo

This is even hinted in the current message:

private should not be inlined in method definitions.

tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Feb 26, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Feb 29, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 4, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 20, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 20, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 20, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 21, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 22, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 22, 2020
tejasbubane added a commit to tejasbubane/rubocop that referenced this issue Mar 22, 2020
bbatsov pushed a commit that referenced this issue Mar 22, 2020
Neodelf pushed a commit to Neodelf/rubocop that referenced this issue Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants