Skip to content

Commit

Permalink
[Fix #5155] Fix a false negative for Naming/ConstantName cop (#5157)
Browse files Browse the repository at this point in the history
This PR fixes a false negative for `Naming/ConstantName` cop when
using frozen object assignment.

It also fixed the offenses that occurred in RuboCop itself.
  • Loading branch information
koic authored and bbatsov committed Dec 1, 2017
1 parent 991d492 commit e367cf6
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
* [#5099](https://github.com/bbatsov/rubocop/pull/5099): Prevent `Style/MinMax` from breaking on implicit receivers. ([@drenmi][])
* [#5079](https://github.com/bbatsov/rubocop/issues/5079): Fix false positive for `Style/SafeNavigation` when safe guarding comparisons. ([@tiagotex][])
* [#5075](https://github.com/bbatsov/rubocop/issues/5075): Fix auto-correct for `Style/RedundantParentheses` cop when unspaced ternary is present. ([@tiagotex][])
* [#5155](https://github.com/bbatsov/rubocop/issues/5155): Fix a false negative for `Naming/ConstantName` cop when using frozen object assignment. ([@koic][])

### Changes

Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/layout/indent_heredoc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class IndentHeredoc < Cop
'`%<current_indent_type>s`.'.freeze
LIBRARY_MSG = 'Use %<indentation_width>d spaces for indentation in a ' \
'heredoc by using %<method>s.'.freeze
StripMethods = {
STRIP_METHODS = {
unindent: 'unindent',
active_support: 'strip_heredoc',
powerpack: 'strip_indent'
Expand Down Expand Up @@ -98,7 +98,7 @@ def message(node)
method = "some library(e.g. ActiveSupport's `String#strip_heredoc`)"
library_message(indentation_width, method)
else
method = "`String##{StripMethods[style]}`"
method = "`String##{STRIP_METHODS[style]}`"
library_message(indentation_width, method)
end
end
Expand Down Expand Up @@ -152,7 +152,7 @@ def correct_by_squiggly(node)
def correct_by_library(node)
lambda do |corrector|
corrector.replace(node.loc.heredoc_body, indented_body(node))
corrected = ".#{StripMethods[style]}"
corrected = ".#{STRIP_METHODS[style]}"
corrector.insert_after(node.loc.expression, corrected)
end
end
Expand Down
22 changes: 20 additions & 2 deletions lib/rubocop/cop/naming/constant_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,35 @@ class ConstantName < Cop
MSG = 'Use SCREAMING_SNAKE_CASE for constants.'.freeze
SNAKE_CASE = /^[\dA-Z_]+$/

def_node_matcher :class_or_struct_return_method?, <<-PATTERN
(send
(const _ {:Class :Struct}) :new
...)
PATTERN

def on_casgn(node)
_scope, const_name, value = *node

# We cannot know the result of method calls like
# NewClass = something_that_returns_a_class
# It's also ok to assign a class constant another class constant
# It's also ok to assign a class constant another class constant,
# `Class.new(...)` or `Struct.new(...)`
# SomeClass = SomeOtherClass
return if value && %i[send block const casgn].include?(value.type)
# SomeClass = Class.new(...)
# SomeClass = Struct.new(...)
return if value && %i[block const casgn].include?(value.type) ||
allowed_method_call_on_rhs?(value) ||
class_or_struct_return_method?(value)

add_offense(node, location: :name) if const_name !~ SNAKE_CASE
end

private

def allowed_method_call_on_rhs?(node)
node && node.send_type? &&
(node.receiver.nil? || !node.receiver.literal?)
end
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/naming/constant_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@
RUBY
end

it 'registers an offense for camel case in const name' \
'when using frozen object assignment' do
expect_offense(<<-RUBY.strip_indent)
TopCase = 5.freeze
^^^^^^^ Use SCREAMING_SNAKE_CASE for constants.
RUBY
end

it 'registers offenses for camel case in multiple const assignment' do
expect_offense(<<-RUBY.strip_indent)
TopCase, Test2, TEST_3 = 5, 6, 7
Expand Down Expand Up @@ -44,6 +52,14 @@
expect_no_offenses('AnythingGoes = test')
end

it 'does not check names if rhs is a `Class.new`' do
expect_no_offenses('Invalid = Class.new(StandardError)')
end

it 'does not check names if rhs is a `Struct.new`' do
expect_no_offenses('Investigation = Struct.new(:offenses, :errors)')
end

it 'does not check names if rhs is a method call with block' do
expect_no_offenses(<<-RUBY.strip_indent)
AnythingGoes = test do
Expand Down

0 comments on commit e367cf6

Please sign in to comment.