From 11809e101b07522a1eadbde7d501d49f92ae065e Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Sat, 19 Nov 2016 10:43:55 +0100 Subject: [PATCH] [Fix #3568] Detect style while handling offense in VariableNumber 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. --- CHANGELOG.md | 1 + .../cop/mixin/configurable_numbering.rb | 15 ++-- .../rubocop/cop/style/variable_number_spec.rb | 70 +++++++++++-------- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b0ab2dbf14d..a11513486fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/mixin/configurable_numbering.rb b/lib/rubocop/cop/mixin/configurable_numbering.rb index e86e5d851be3..5770d1a24af8 100644 --- a/lib/rubocop/cop/mixin/configurable_numbering.rb +++ b/lib/rubocop/cop/mixin/configurable_numbering.rb @@ -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 diff --git a/spec/rubocop/cop/style/variable_number_spec.rb b/spec/rubocop/cop/style/variable_number_spec.rb index d369dfff6cd5..0b6fce091851 100644 --- a/spec/rubocop/cop/style/variable_number_spec.rb +++ b/spec/rubocop/cop/style/variable_number_spec.rb @@ -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 @@ -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' @@ -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_' @@ -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'