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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -22,6 +22,7 @@
* [#7868](https://github.com/rubocop-hq/rubocop/pull/7868): **(Breaking)** Extensive refactoring of internal classes `Team`, `Commissioner`, `Corrector`. `Cop::Cop#corrections` not completely compatible. See Upgrade Notes. ([@marcandre][])
* [#8156](https://github.com/rubocop-hq/rubocop/issues/8156): **(Breaking)** `rubocop -a / --autocorrect` no longer run unsafe corrections; `rubocop -A / --autocorrect-all` run both safe and unsafe corrections. Options `--safe-autocorrect` is deprecated. ([@marcandre][])
* [#8207](https://github.com/rubocop-hq/rubocop/pull/8207): **(Breaking)** Order for gems names now disregards underscores and dashes unless `ConsiderPunctuation` setting is set to `true`. ([@marcandre][])
* [#8211](https://github.com/rubocop-hq/rubocop/pull/8211): `Style/ClassVars` cop now detects `class_variable_set`. ([@biinari][])

## 0.86.0 (2020-06-22)

Expand Down
15 changes: 15 additions & 0 deletions docs/modules/ROOT/pages/cops_style.adoc
Expand Up @@ -951,6 +951,15 @@ class A
@@test = 10
end

class A
def self.test(name, value)
class_variable_set("@@#{name}", value)
end
end

class A; end
A.class_variable_set(:@@test, 10)

# good
class A
@test = 10
Expand All @@ -961,6 +970,12 @@ class A
@@test # you can access class variable without offense
end
end

class A
def self.test(name)
class_variable_get("@@#{name}") # you can access without offense
end
end
----

=== References
Expand Down
21 changes: 21 additions & 0 deletions lib/rubocop/cop/style/class_vars.rb
Expand Up @@ -19,6 +19,15 @@ module Style
# @@test = 10
# end
#
# class A
# def self.test(name, value)
# class_variable_set("@@#{name}", value)
# end
# end
#
# class A; end
# A.class_variable_set(:@@test, 10)
#
# # good
# class A
# @test = 10
Expand All @@ -30,6 +39,12 @@ module Style
# end
# end
#
# class A
# def self.test(name)
# class_variable_get("@@#{name}") # you can access without offense
# end
# end
#
class ClassVars < Cop
MSG = 'Replace class var %<class_var>s with a class ' \
'instance var.'
Expand All @@ -38,6 +53,12 @@ def on_cvasgn(node)
add_offense(node, location: :name)
end

def on_send(node)
return unless node.method?(:class_variable_set)

add_offense(node.first_argument)
end
biinari marked this conversation as resolved.
Show resolved Hide resolved

def message(node)
class_var, = *node
format(MSG, class_var: class_var)
Expand Down
17 changes: 17 additions & 0 deletions spec/rubocop/cop/style/class_vars_spec.rb
Expand Up @@ -10,6 +10,23 @@ class TestClass; @@test = 10; end
RUBY
end

it 'registers an offense for class variable set in class' do
expect_offense(<<~RUBY)
class TestClass
class_variable_set(:@@test, 2)
^^^^^^^ Replace class var @@test with a class instance var.
end
RUBY
end

it 'registers an offense for class variable set on class receiver' do
expect_offense(<<~RUBY)
class TestClass; end
TestClass.class_variable_set(:@@test, 42)
^^^^^^^ Replace class var @@test with a class instance var.
RUBY
end

it 'does not register an offense for class variable usage' do
expect_no_offenses('@@test.test(20)')
end
Expand Down