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

Detect class_variable_set in Style/ClassVars #8211

Merged
merged 1 commit into from Jun 30, 2020

Conversation

biinari
Copy link
Contributor

@biinari biinari commented Jun 24, 2020

class_variable_set(:@@name, 2) has the same effect as @@name = 2

This variation can be within the class definition:

class A
  class_variable_set(:@@name, 2)
end

Or it could be called elsewhere, with the class as receiver:

class A; end

class B
  def change(name, value)
    A.class_variable_set(":@@#{name}", value)
  end
end

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.

@koic
Copy link
Member

koic commented Jun 25, 2020

Can you add this bad case to the example code?

@biinari biinari force-pushed the feature/class_var_set branch 2 times, most recently from fa3f156 to 4d03ccb Compare June 29, 2020 14:32
Copy link
Member

@koic koic left a comment

Choose a reason for hiding this comment

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

This looks good to me. Can you rebase with the latest master branch for resolve changelog entry?

@biinari
Copy link
Contributor Author

biinari commented Jun 30, 2020

Thanks for the review @koic. Rebased from master now. Looks like the Changelog has somehow ended up with two Changes sections in the unreleased part. Would you like me to merge those together?

@koic
Copy link
Member

koic commented Jun 30, 2020

Yeah, "Changes" section has been merged on the master branch.
https://github.com/rubocop-hq/rubocop/blob/master/CHANGELOG.md#master-unreleased

class_variable_set(:@@name, 2) has the same effect as @@name = 2

Add changelog entry for class_variable_set

Simplify class_variable_set check

Add examples for class_variable_set

Fix examples for class_variable_set

Give complete examples that work in isolation
@biinari
Copy link
Contributor Author

biinari commented Jun 30, 2020

Ah thanks. Now I see why it didn't get thrown up as a conflict. That changelog entry should be right now.

@koic koic merged commit a5b980d into rubocop:master Jun 30, 2020
@koic
Copy link
Member

koic commented Jun 30, 2020

Thanks 👍

@biinari biinari deleted the feature/class_var_set branch June 30, 2020 12:33
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.

None yet

2 participants