Skip to content

Commit

Permalink
[Fix rubocop#3568] Detect style while handling offense in VariableNumber
Browse files Browse the repository at this point in the history
When the variable name doesn't match the configured style, we should
detect which style is used so that the right information is
generated in .rubocop_todo.yml.
  • Loading branch information
jonas054 committed Nov 19, 2016
1 parent 9128852 commit 11809e1
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* [#3662](https://github.com/bbatsov/rubocop/issues/3662): Fix the auto-correction of `Lint/UnneededSplatExpansion` when the splat expansion is inside of another array. ([@rrosenblum][])
* [#3699](https://github.com/bbatsov/rubocop/issues/3699): Fix false positive in `Style/VariableNumber` on variable names ending with an underscore. ([@bquorning][])
* [#3687](https://github.com/bbatsov/rubocop/issues/3687): Fix the fact that `Style/TernaryParentheses` cop claims to correct uncorrected offenses. ([@Ana06][])
* [#3568](https://github.com/bbatsov/rubocop/issues/3568): Fix `--auto-gen-config` behavior for `Style/VariableNumber`. ([@jonas054][])

## 0.45.0 (2016-10-31)

Expand Down
15 changes: 11 additions & 4 deletions lib/rubocop/cop/mixin/configurable_numbering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,23 @@ module ConfigurableNumbering
def check_name(node, name, name_range)
return if operator?(name)

if valid_name?(node, name)
if valid_name?(node, name, style)
correct_style_detected
else
add_offense(node, name_range, message(style))
add_offense(node, name_range, message(style)) do
(supported_styles - [style]).each do |other_style|
if valid_name?(node, name, other_style)
unexpected_style_detected(other_style)
break
end
end
end
end
end

def valid_name?(node, name)
def valid_name?(node, name, given_style)
pattern =
case style
case given_style
when :snake_case
SNAKE_CASE
when :normalcase
Expand Down
70 changes: 40 additions & 30 deletions spec/rubocop/cop/style/variable_number_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,19 @@
describe RuboCop::Cop::Style::VariableNumber, :config do
subject(:cop) { described_class.new(config) }

shared_examples :offense do |style, variable|
it "registers an offense for #{variable} in #{style}" do
inspect_source(cop, "#{variable} = 1")
shared_examples :offense do |style, variable, style_to_allow_offenses|
it "registers an offense for #{Array(variable).first} in #{style}" do
inspect_source(cop, Array(variable).map { |v| "#{v} = 1" }.join("\n"))

expect(cop.messages).to eq(["Use #{style} for variable numbers."])
expect(cop.highlights).to eq([variable])
expect(cop.highlights).to eq(Array(variable)[0, 1])
config_to_allow_offenses =
if style_to_allow_offenses
{ 'EnforcedStyle' => style_to_allow_offenses.to_s }
else
{ 'Enabled' => false }
end
expect(cop.config_to_allow_offenses).to eq(config_to_allow_offenses)
end
end

Expand All @@ -25,13 +32,14 @@
context 'when configured for snake_case' do
let(:cop_config) { { 'EnforcedStyle' => 'snake_case' } }

it_behaves_like :offense, 'snake_case', 'local1'
it_behaves_like :offense, 'snake_case', '@local1'
it_behaves_like :offense, 'snake_case', '@@local1'
it_behaves_like :offense, 'snake_case', 'camelCase1'
it_behaves_like :offense, 'snake_case', '@camelCase1'
it_behaves_like :offense, 'snake_case', '_unused1'
it_behaves_like :offense, 'snake_case', 'aB1'
it_behaves_like :offense, 'snake_case', 'local1', :normalcase
it_behaves_like :offense, 'snake_case', '@local1', :normalcase
it_behaves_like :offense, 'snake_case', '@@local1', :normalcase
it_behaves_like :offense, 'snake_case', 'camelCase1', :normalcase
it_behaves_like :offense, 'snake_case', '@camelCase1', :normalcase
it_behaves_like :offense, 'snake_case', '_unused1', :normalcase
it_behaves_like :offense, 'snake_case', 'aB1', :normalcase
it_behaves_like :offense, 'snake_case', %w(a1 a_2), nil

it_behaves_like :accepts, 'snake_case', 'local_1'
it_behaves_like :accepts, 'snake_case', 'local_12'
Expand Down Expand Up @@ -62,15 +70,16 @@
context 'when configured for normal' do
let(:cop_config) { { 'EnforcedStyle' => 'normalcase' } }

it_behaves_like :offense, 'normalcase', 'local_1'
it_behaves_like :offense, 'normalcase', 'sha_256'
it_behaves_like :offense, 'normalcase', '@local_1'
it_behaves_like :offense, 'normalcase', '@@local_1'
it_behaves_like :offense, 'normalcase', 'myAttribute_1'
it_behaves_like :offense, 'normalcase', '@myAttribute_1'
it_behaves_like :offense, 'normalcase', '_myLocal_1'
it_behaves_like :offense, 'normalcase', 'localFOO_1'
it_behaves_like :offense, 'normalcase', 'local_FOO_1'
it_behaves_like :offense, 'normalcase', 'local_1', :snake_case
it_behaves_like :offense, 'normalcase', 'sha_256', :snake_case
it_behaves_like :offense, 'normalcase', '@local_1', :snake_case
it_behaves_like :offense, 'normalcase', '@@local_1', :snake_case
it_behaves_like :offense, 'normalcase', 'myAttribute_1', :snake_case
it_behaves_like :offense, 'normalcase', '@myAttribute_1', :snake_case
it_behaves_like :offense, 'normalcase', '_myLocal_1', :snake_case
it_behaves_like :offense, 'normalcase', 'localFOO_1', :snake_case
it_behaves_like :offense, 'normalcase', 'local_FOO_1', :snake_case
it_behaves_like :offense, 'normalcase', %w(a_1 a2), nil

it_behaves_like :accepts, 'normalcase', 'local1'
it_behaves_like :accepts, 'normalcase', 'local_'
Expand Down Expand Up @@ -103,16 +112,17 @@
context 'when configured for non integer' do
let(:cop_config) { { 'EnforcedStyle' => 'non_integer' } }

it_behaves_like :offense, 'non_integer', 'local_1'
it_behaves_like :offense, 'non_integer', 'local1'
it_behaves_like :offense, 'non_integer', '@local_1'
it_behaves_like :offense, 'non_integer', '@local1'
it_behaves_like :offense, 'non_integer', 'myAttribute_1'
it_behaves_like :offense, 'non_integer', 'myAttribute1'
it_behaves_like :offense, 'non_integer', '@myAttribute_1'
it_behaves_like :offense, 'non_integer', '@myAttribute1'
it_behaves_like :offense, 'non_integer', '_myLocal_1'
it_behaves_like :offense, 'non_integer', '_myLocal1'
it_behaves_like :offense, 'non_integer', 'local_1', :snake_case
it_behaves_like :offense, 'non_integer', 'local1', :normalcase
it_behaves_like :offense, 'non_integer', '@local_1', :snake_case
it_behaves_like :offense, 'non_integer', '@local1', :normalcase
it_behaves_like :offense, 'non_integer', 'myAttribute_1', :snake_case
it_behaves_like :offense, 'non_integer', 'myAttribute1', :normalcase
it_behaves_like :offense, 'non_integer', '@myAttribute_1', :snake_case
it_behaves_like :offense, 'non_integer', '@myAttribute1', :normalcase
it_behaves_like :offense, 'non_integer', '_myLocal_1', :snake_case
it_behaves_like :offense, 'non_integer', '_myLocal1', :normalcase
it_behaves_like :offense, 'non_integer', %w(a_1 aone), nil

it_behaves_like :accepts, 'non_integer', 'localone'
it_behaves_like :accepts, 'non_integer', 'local_one'
Expand Down

0 comments on commit 11809e1

Please sign in to comment.