From c21ced8d5ad603d342547ccf6e7d4db0433313e5 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 15 Jul 2021 02:55:31 +0900 Subject: [PATCH 1/8] [Fix #9928] Fix a false auto-correct for `Layout/EndAlignment` Fixes #9928. This PR fixes an infinite loop error and a false auto-correction behavior for `Layout/EndAlignment` when using operator methods and `EnforcedStyleAlignWith: variable`. This infinite loop error is due to an offense being registered but not auto-corrected. `EnforcedStyleAlignWith: variable` enforces alignment to variable, it should be auto-corrected to be aligned to variable even when operator method is used. --- ...autocorrection_for_layout_end_alignment.md | 1 + lib/rubocop/cop/layout/end_alignment.rb | 9 ++++- spec/rubocop/cop/layout/end_alignment_spec.rb | 38 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_false_autocorrection_for_layout_end_alignment.md diff --git a/changelog/fix_false_autocorrection_for_layout_end_alignment.md b/changelog/fix_false_autocorrection_for_layout_end_alignment.md new file mode 100644 index 000000000000..630a73469fc4 --- /dev/null +++ b/changelog/fix_false_autocorrection_for_layout_end_alignment.md @@ -0,0 +1 @@ +* [#9928](https://github.com/rubocop/rubocop/issues/9928): Fix an infinite loop error and a false auto-correction behavior for `Layout/EndAlignment` when using operator methods and `EnforcedStyleAlignWith: variable`. ([@koic][]) diff --git a/lib/rubocop/cop/layout/end_alignment.rb b/lib/rubocop/cop/layout/end_alignment.rb index f5d12d12543a..07c58af1df4e 100644 --- a/lib/rubocop/cop/layout/end_alignment.rb +++ b/lib/rubocop/cop/layout/end_alignment.rb @@ -167,7 +167,8 @@ def alignment_node(node) def alignment_node_for_variable_style(node) return node.parent if node.case_type? && node.argument? - assignment = node.ancestors.find(&:assignment_or_similar?) + assignment = assignment_or_operator_method(node) + if assignment && !line_break_before_keyword?(assignment.source_range, node) assignment else @@ -177,6 +178,12 @@ def alignment_node_for_variable_style(node) node end end + + def assignment_or_operator_method(node) + node.ancestors.find do |ancestor| + ancestor.assignment_or_similar? || ancestor.send_type? && ancestor.operator_method? + end + end end end end diff --git a/spec/rubocop/cop/layout/end_alignment_spec.rb b/spec/rubocop/cop/layout/end_alignment_spec.rb index b6a5e716a5e5..a7d3d5f82850 100644 --- a/spec/rubocop/cop/layout/end_alignment_spec.rb +++ b/spec/rubocop/cop/layout/end_alignment_spec.rb @@ -301,6 +301,44 @@ module Test include_examples 'aligned', 'puts 1; while', 'Test', ' end' include_examples 'aligned', 'puts 1; until', 'Test', ' end' include_examples 'aligned', 'puts 1; case', 'a when b', ' end' + + it 'register an offense when using `+` operator method and `end` is not aligned' do + expect_offense(<<~RUBY) + variable + if condition + foo + else + bar + end + ^^^ `end` at 5, 11 is not aligned with `variable + if` at 1, 0. + RUBY + + expect_correction(<<~RUBY) + variable + if condition + foo + else + bar + end + RUBY + end + + it 'register an offense when using `-` operator method and `end` is not aligned' do + expect_offense(<<~RUBY) + variable - if condition + foo + else + bar + end + ^^^ `end` at 5, 11 is not aligned with `variable - if` at 1, 0. + RUBY + + expect_correction(<<~RUBY) + variable - if condition + foo + else + bar + end + RUBY + end end context 'correct + opposite' do From 903a4f9f30591d273dedce2d75b5ed11d9c1c9e7 Mon Sep 17 00:00:00 2001 From: ojab Date: Thu, 15 Jul 2021 20:45:45 +0000 Subject: [PATCH 2/8] Fixup GitHub Actions formatter when run in non-default directory In some cases (specifically when running inside the container with non-default `working-directory`) GitHub cannot link absolute paths produced by formatter to the code, while relative paths works. Output relative paths if they're inside current directory, so this usecase is covered. --- changelog/fix_github_actions_in_non_default_directory.md | 1 + lib/rubocop/formatter/git_hub_actions_formatter.rb | 2 +- spec/rubocop/formatter/git_hub_actions_formatter_spec.rb | 9 +++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_github_actions_in_non_default_directory.md diff --git a/changelog/fix_github_actions_in_non_default_directory.md b/changelog/fix_github_actions_in_non_default_directory.md new file mode 100644 index 000000000000..4fe559c24371 --- /dev/null +++ b/changelog/fix_github_actions_in_non_default_directory.md @@ -0,0 +1 @@ +* [#9933](https://github.com/rubocop/rubocop/pull/9933): Fix GitHub Actions formatter when running in non-default directory. ([@ojab][]) diff --git a/lib/rubocop/formatter/git_hub_actions_formatter.rb b/lib/rubocop/formatter/git_hub_actions_formatter.rb index e39fff05bc4d..0628778463b0 100644 --- a/lib/rubocop/formatter/git_hub_actions_formatter.rb +++ b/lib/rubocop/formatter/git_hub_actions_formatter.rb @@ -33,7 +33,7 @@ def report_offense(file, offense) output.printf( "\n::%s file=%s,line=%d,col=%d::%s\n", severity: github_severity(offense), - file: file, + file: PathUtil.smart_path(file), line: offense.line, column: offense.real_column, message: github_escape(offense.message) diff --git a/spec/rubocop/formatter/git_hub_actions_formatter_spec.rb b/spec/rubocop/formatter/git_hub_actions_formatter_spec.rb index 21ce0cf96340..fe7562164adf 100644 --- a/spec/rubocop/formatter/git_hub_actions_formatter_spec.rb +++ b/spec/rubocop/formatter/git_hub_actions_formatter_spec.rb @@ -30,6 +30,15 @@ end end + context 'when file is relative to the current directory' do + let(:file) { "#{Dir.pwd}/path/to/file" } + + it 'reports offenses as error with the relative path' do + expect(output.string) + .to include('::error file=path/to/file,line=1,col=1::This is a message.') + end + end + context 'when no offenses are detected' do let(:offenses) { [] } From 49b3999e39d01da608a5a1dcaf12375306679a92 Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Fri, 16 Jul 2021 17:37:17 +0200 Subject: [PATCH 3/8] [Fix #9922] Replace extra disable directives with comma Just removing cop names from disable directives is not a good solution. It's bound to cause infinite loop errors when those cops try to add the disable directives back. The natural solution is to put in commas instead. --- .../fix_autocorrect_in_Style_DoubleCopDisableDirective.md | 1 + lib/rubocop/cop/style/double_cop_disable_directive.rb | 8 +------- spec/rubocop/cli/disable_uncorrectable_spec.rb | 2 +- .../cop/style/double_cop_disable_directive_spec.rb | 4 ++-- 4 files changed, 5 insertions(+), 10 deletions(-) create mode 100644 changelog/fix_autocorrect_in_Style_DoubleCopDisableDirective.md diff --git a/changelog/fix_autocorrect_in_Style_DoubleCopDisableDirective.md b/changelog/fix_autocorrect_in_Style_DoubleCopDisableDirective.md new file mode 100644 index 000000000000..8c59c68d692a --- /dev/null +++ b/changelog/fix_autocorrect_in_Style_DoubleCopDisableDirective.md @@ -0,0 +1 @@ +* [#9922](https://github.com/rubocop/rubocop/issues/9922): Make better auto-corrections in `Style/DoubleCopDisableDirective`. ([@jonas054][]) diff --git a/lib/rubocop/cop/style/double_cop_disable_directive.rb b/lib/rubocop/cop/style/double_cop_disable_directive.rb index 49fd0f20b453..32bce9c279e3 100644 --- a/lib/rubocop/cop/style/double_cop_disable_directive.rb +++ b/lib/rubocop/cop/style/double_cop_disable_directive.rb @@ -36,13 +36,7 @@ def on_new_investigation next unless comment.text.scan(/# rubocop:(?:disable|todo)/).size > 1 add_offense(comment) do |corrector| - prefix = if comment.text.start_with?('# rubocop:disable') - '# rubocop:disable' - else - '# rubocop:todo' - end - - corrector.replace(comment, comment.text[/#{prefix} \S+/]) + corrector.replace(comment, comment.text.gsub(%r{ # rubocop:(disable|todo)}, ',')) end end end diff --git a/spec/rubocop/cli/disable_uncorrectable_spec.rb b/spec/rubocop/cli/disable_uncorrectable_spec.rb index a1a9a735cd50..b5ce90ad0bcb 100644 --- a/spec/rubocop/cli/disable_uncorrectable_spec.rb +++ b/spec/rubocop/cli/disable_uncorrectable_spec.rb @@ -177,7 +177,7 @@ def choose_move(who_to_move) class Chess # rubocop:todo Metrics/MethodLength # rubocop:todo Metrics/AbcSize - def choose_move(who_to_move) # rubocop:todo Metrics/CyclomaticComplexity + def choose_move(who_to_move) # rubocop:todo Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength legal_moves = all_legal_moves_that_dont_put_me_in_check(who_to_move) return nil if legal_moves.empty? diff --git a/spec/rubocop/cop/style/double_cop_disable_directive_spec.rb b/spec/rubocop/cop/style/double_cop_disable_directive_spec.rb index 58f6c6c0ba47..0e30ac4bd739 100644 --- a/spec/rubocop/cop/style/double_cop_disable_directive_spec.rb +++ b/spec/rubocop/cop/style/double_cop_disable_directive_spec.rb @@ -9,7 +9,7 @@ def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity # ru RUBY expect_correction(<<~RUBY) - def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity + def choose_move(who_to_move) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength end RUBY end @@ -22,7 +22,7 @@ def choose_move(who_to_move) # rubocop:todo Metrics/CyclomaticComplexity # ruboc RUBY expect_correction(<<~RUBY) - def choose_move(who_to_move) # rubocop:todo Metrics/CyclomaticComplexity + def choose_move(who_to_move) # rubocop:todo Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength end RUBY end From f2c5f0ea0c79479ca6afa027739ac0a6d54bc48c Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 17 Jul 2021 01:45:43 +0900 Subject: [PATCH 4/8] Use `offense` instead of `offence` Follow up to https://github.com/rubocop/rubocop/commit/c7ecb857. --- .rubocop.yml | 4 +++ docs/modules/ROOT/pages/cops_style.adoc | 2 +- lib/rubocop/cop/layout/hash_alignment.rb | 30 +++++++++---------- lib/rubocop/cop/layout/indentation_style.rb | 4 +-- lib/rubocop/cop/style/eval_with_location.rb | 2 +- spec/rubocop/cli/options_spec.rb | 2 +- .../lint/mixed_regexp_capture_types_spec.rb | 12 ++++---- .../cop/lint/out_of_range_regexp_ref_spec.rb | 2 +- .../cop/style/block_delimiters_spec.rb | 2 +- .../cop/style/format_string_token_spec.rb | 2 +- spec/rubocop/result_cache_spec.rb | 10 +++---- 11 files changed, 38 insertions(+), 34 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e4af88e2dbcb..026dd222722e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -99,6 +99,10 @@ Metrics/ModuleLength: - 'spec/**/*.rb' Naming/InclusiveLanguage: + FlaggedTerms: + offence: + Suggestions: + - offense Exclude: - lib/rubocop/cop/naming/inclusive_language.rb diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index 3848a3fdea29..591a92927372 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -3330,7 +3330,7 @@ will not attempt to automatically add a binding, or add filename and line values. This cop works only when a string literal is given as a code string. -No offence is reported if a string variable is given as below: +No offense is reported if a string variable is given as below: === Examples diff --git a/lib/rubocop/cop/layout/hash_alignment.rb b/lib/rubocop/cop/layout/hash_alignment.rb index d94b5291fac9..ff6c6b0daa91 100644 --- a/lib/rubocop/cop/layout/hash_alignment.rb +++ b/lib/rubocop/cop/layout/hash_alignment.rb @@ -213,7 +213,7 @@ def on_hash(node) check_pairs(node) end - attr_accessor :offences_by, :column_deltas + attr_accessor :offenses_by, :column_deltas private @@ -224,7 +224,7 @@ def autocorrect_incompatible_with_other_cops?(node) end def reset! - self.offences_by = {} + self.offenses_by = {} self.column_deltas = Hash.new { |hash, key| hash[key] = {} } end @@ -248,33 +248,33 @@ def check_pairs(node) end end - add_offences + add_offenses end - def add_offences - kwsplat_offences = offences_by.delete(KeywordSplatAlignment) - register_offences_with_format(kwsplat_offences, KeywordSplatAlignment) + def add_offenses + kwsplat_offenses = offenses_by.delete(KeywordSplatAlignment) + register_offenses_with_format(kwsplat_offenses, KeywordSplatAlignment) - format, offences = offences_by.min_by { |_, v| v.length } - register_offences_with_format(offences, format) + format, offenses = offenses_by.min_by { |_, v| v.length } + register_offenses_with_format(offenses, format) end - def register_offences_with_format(offences, format) - (offences || []).each do |offence| - add_offense(offence, message: MESSAGES[format]) do |corrector| - delta = column_deltas[alignment_for(offence).first.class][offence] + def register_offenses_with_format(offenses, format) + (offenses || []).each do |offense| + add_offense(offense, message: MESSAGES[format]) do |corrector| + delta = column_deltas[alignment_for(offense).first.class][offense] - correct_node(corrector, offence, delta) unless delta.nil? + correct_node(corrector, offense, delta) unless delta.nil? end end end def check_delta(delta, node:, alignment:) - offences_by[alignment.class] ||= [] + offenses_by[alignment.class] ||= [] return if good_alignment? delta column_deltas[alignment.class][node] = delta - offences_by[alignment.class].push(node) + offenses_by[alignment.class].push(node) end def ignore_hash_argument?(node) diff --git a/lib/rubocop/cop/layout/indentation_style.rb b/lib/rubocop/cop/layout/indentation_style.rb index 130c2da0db02..acf21a925fbc 100644 --- a/lib/rubocop/cop/layout/indentation_style.rb +++ b/lib/rubocop/cop/layout/indentation_style.rb @@ -43,7 +43,7 @@ def on_new_investigation str_ranges = string_literal_ranges(processed_source.ast) processed_source.lines.each.with_index(1) do |line, lineno| - next unless (range = find_offence(line, lineno)) + next unless (range = find_offense(line, lineno)) next if in_string_literal?(str_ranges, range) add_offense(range) { |corrector| autocorrect(corrector, range) } @@ -60,7 +60,7 @@ def autocorrect(corrector, range) end end - def find_offence(line, lineno) + def find_offense(line, lineno) match = if style == :spaces line.match(/\A\s*\t+/) else diff --git a/lib/rubocop/cop/style/eval_with_location.rb b/lib/rubocop/cop/style/eval_with_location.rb index 47fd11336d6f..2f9ff67762b8 100644 --- a/lib/rubocop/cop/style/eval_with_location.rb +++ b/lib/rubocop/cop/style/eval_with_location.rb @@ -43,7 +43,7 @@ module Style # RUBY # # This cop works only when a string literal is given as a code string. - # No offence is reported if a string variable is given as below: + # No offense is reported if a string variable is given as below: # # @example # # not checked diff --git a/spec/rubocop/cli/options_spec.rb b/spec/rubocop/cli/options_spec.rb index f1ed8476c7f9..bed23fbe71d8 100644 --- a/spec/rubocop/cli/options_spec.rb +++ b/spec/rubocop/cli/options_spec.rb @@ -1806,7 +1806,7 @@ def f $stdin = STDIN end - it 'prints offence reports to stderr and corrected code to stdout if --auto-correct-all and --stderr are used' do + it 'prints offense reports to stderr and corrected code to stdout if --auto-correct-all and --stderr are used' do $stdin = StringIO.new('p $/') argv = ['--auto-correct-all', '--only=Style/SpecialGlobalVars', diff --git a/spec/rubocop/cop/lint/mixed_regexp_capture_types_spec.rb b/spec/rubocop/cop/lint/mixed_regexp_capture_types_spec.rb index 2b80d385d530..e19a9466bd43 100644 --- a/spec/rubocop/cop/lint/mixed_regexp_capture_types_spec.rb +++ b/spec/rubocop/cop/lint/mixed_regexp_capture_types_spec.rb @@ -37,38 +37,38 @@ # For example, `/(?#{var}*)` is interpreted as `/(?*)`. # So it does not offense when variables are used in regexp literals. context 'when containing a non-regexp literal' do - it 'does not register an offence when containing a lvar' do + it 'does not register an offense when containing a lvar' do expect_no_offenses(<<~'RUBY') var = '(\d+)' /(?#{var}*)/ RUBY end - it 'does not register an offence when containing a ivar' do + it 'does not register an offense when containing a ivar' do expect_no_offenses(<<~'RUBY') /(?#{@var}*)/ RUBY end - it 'does not register an offence when containing a cvar' do + it 'does not register an offense when containing a cvar' do expect_no_offenses(<<~'RUBY') /(?#{@@var}*)/ RUBY end - it 'does not register an offence when containing a gvar' do + it 'does not register an offense when containing a gvar' do expect_no_offenses(<<~'RUBY') /(?#{$var}*)/ RUBY end - it 'does not register an offence when containing a method' do + it 'does not register an offense when containing a method' do expect_no_offenses(<<~'RUBY') /(?#{do_something}*)/ RUBY end - it 'does not register an offence when containing a constant' do + it 'does not register an offense when containing a constant' do expect_no_offenses(<<~'RUBY') /(?#{CONST}*)/ RUBY diff --git a/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb index 8023862e57f8..f075eeecef35 100644 --- a/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb +++ b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb @@ -66,7 +66,7 @@ # RuboCop does not know a value of variables that it will contain in the regexp literal. # For example, `/(?#{var}*)` is interpreted as `/(?*)`. # So it does not offense when variables are used in regexp literals. - it 'does not register an offence regexp containing non literal' do + it 'does not register an offense regexp containing non literal' do expect_no_offenses(<<~'RUBY') var = '(\d+)' /(?#{var}*)/ =~ "12" diff --git a/spec/rubocop/cop/style/block_delimiters_spec.rb b/spec/rubocop/cop/style/block_delimiters_spec.rb index bc51589c65ed..51b9efe5281c 100644 --- a/spec/rubocop/cop/style/block_delimiters_spec.rb +++ b/spec/rubocop/cop/style/block_delimiters_spec.rb @@ -633,7 +633,7 @@ expect_no_offenses('each { |x| }') end - it 'registers an offence for a multi-line block with do-end' do + it 'registers an offense for a multi-line block with do-end' do expect_offense(<<~RUBY) each do |x| ^^ Prefer `{...}` over `do...end` for blocks. diff --git a/spec/rubocop/cop/style/format_string_token_spec.rb b/spec/rubocop/cop/style/format_string_token_spec.rb index be01da772053..99ce2c7ffa1e 100644 --- a/spec/rubocop/cop/style/format_string_token_spec.rb +++ b/spec/rubocop/cop/style/format_string_token_spec.rb @@ -21,7 +21,7 @@ expect_no_offenses("format('%#{token}', foo)") end - it 'registers offence for dual unannotated' do + it 'registers offense for dual unannotated' do expect_offense(<<~RUBY) format('%#{token} %s', foo, bar) ^^ Prefer [...] diff --git a/spec/rubocop/result_cache_spec.rb b/spec/rubocop/result_cache_spec.rb index 263012a4d4ed..474d5fb01236 100644 --- a/spec/rubocop/result_cache_spec.rb +++ b/spec/rubocop/result_cache_spec.rb @@ -53,7 +53,7 @@ def abs(path) # Fixes https://github.com/rubocop/rubocop/issues/6274 context 'when offenses are saved' do - context 'an offence with status corrected' do + context 'an offense with status corrected' do let(:offense) do RuboCop::Cop::Offense.new( :warning, location, 'unused var', 'Lint/UselessAssignment', :corrected @@ -66,7 +66,7 @@ def abs(path) end end - context 'an offence with status corrected_with_todo' do + context 'an offense with status corrected_with_todo' do let(:offense) do RuboCop::Cop::Offense.new( :warning, location, 'unused var', 'Lint/UselessAssignment', :corrected_with_todo @@ -79,7 +79,7 @@ def abs(path) end end - context 'an offence with status uncorrected' do + context 'an offense with status uncorrected' do let(:offense) do RuboCop::Cop::Offense.new( :warning, location, 'unused var', 'Lint/UselessAssignment', :uncorrected @@ -92,7 +92,7 @@ def abs(path) end end - context 'an offence with status unsupported' do + context 'an offense with status unsupported' do let(:offense) do RuboCop::Cop::Offense.new( :warning, location, 'unused var', 'Lint/UselessAssignment', :unsupported @@ -105,7 +105,7 @@ def abs(path) end end - context 'an offence with status new_status' do + context 'an offense with status new_status' do let(:offense) do RuboCop::Cop::Offense.new( :warning, location, 'unused var', 'Lint/UselessAssignment', :new_status From 0886479c39e31c06820696ca7c899d0542fad3b5 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sun, 18 Jul 2021 01:24:37 +0900 Subject: [PATCH 5/8] Fix an incorrect auto-correct for `Layout/LineLength` This PR fixes the following incorrect auto-correct for `Layout/LineLength` when a heredoc is used as the first element of an array. ```console % cat example.rb [<<~STRING, { key1: value1, key2: value2 }] STRING % ruby -c example.rb Syntax OK % cat .rubocop.yml Layout/LineLength: Max: 40 % bundle exec rubocop --only Layout/LineLength example.rb -a (snip) Inspecting 1 file C Offenses: example.rb:1:41: C: [Corrected] Layout/LineLength: Line is too long. [43/40] [<<~STRING, { key1: value1, key2: value2 }] ^^^ 1 file inspected, 1 offense detected, 1 offense corrected % cat example.rb [<<~STRING, { key1: value1, key2: value2 }] STRING % ruby -c example.rb example.rb:1: syntax error, unexpected end-of-input, expecting ']' ``` --- ...fix_incorrect_autocorrect_for_layout_line_length.md | 1 + lib/rubocop/cop/mixin/check_line_breakable.rb | 4 ++-- spec/rubocop/cop/layout/line_length_spec.rb | 10 ++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 changelog/fix_incorrect_autocorrect_for_layout_line_length.md diff --git a/changelog/fix_incorrect_autocorrect_for_layout_line_length.md b/changelog/fix_incorrect_autocorrect_for_layout_line_length.md new file mode 100644 index 000000000000..3ab5e086fe11 --- /dev/null +++ b/changelog/fix_incorrect_autocorrect_for_layout_line_length.md @@ -0,0 +1 @@ +* [#9938](https://github.com/rubocop/rubocop/pull/9938): Fix an incorrect auto-correct for `Layout/LineLength` when a heredoc is used as the first element of an array. ([@koic][]) diff --git a/lib/rubocop/cop/mixin/check_line_breakable.rb b/lib/rubocop/cop/mixin/check_line_breakable.rb index 7329fe6402ab..4297b9e2a564 100644 --- a/lib/rubocop/cop/mixin/check_line_breakable.rb +++ b/lib/rubocop/cop/mixin/check_line_breakable.rb @@ -97,9 +97,9 @@ def first_argument_is_heredoc?(node) # If a send node contains a heredoc argument, splitting cannot happen # after the heredoc or else it will cause a syntax error. def shift_elements_for_heredoc_arg(node, elements, index) - return index unless node.send_type? + return index unless node.send_type? || node.array_type? - heredoc_index = elements.index { |arg| (arg.str_type? || arg.dstr_type?) && arg.heredoc? } + heredoc_index = elements.index { |arg| arg.respond_to?(:heredoc?) && arg.heredoc? } return index unless heredoc_index return nil if heredoc_index.zero? diff --git a/spec/rubocop/cop/layout/line_length_spec.rb b/spec/rubocop/cop/layout/line_length_spec.rb index 5ad3d910af61..29eb59e098cd 100644 --- a/spec/rubocop/cop/layout/line_length_spec.rb +++ b/spec/rubocop/cop/layout/line_length_spec.rb @@ -779,6 +779,16 @@ def baz(bar) expect_no_corrections end + it 'does not break up the line when a heredoc is used as the first element of an array' do + expect_offense(<<~RUBY) + [<<~STRING, { key1: value1, key2: value2 }] + ^^^ Line is too long. [43/40] + STRING + RUBY + + expect_no_corrections + end + context 'and other arguments before the heredoc' do it 'can break up the line before the heredoc argument' do args = 'x' * 20 From dfb9c265fd4fd6164cb66e4faa7c19c8fa11dd6b Mon Sep 17 00:00:00 2001 From: Jonas Arvidsson Date: Fri, 16 Jul 2021 10:42:27 +0200 Subject: [PATCH 6/8] [Fix #9752] Improve error message for top level department When there are cop departments that seem to have a structure, like in the cookstyle gem where departments are named `Chef/Style`, `Chef/Correctness`, etc., it's easy to assume that there is a `Chef` department that can disable all departments under it. This is not the case, but the error message `Error: unrecognized cop Chef found in .cookstyle.yml` is not informative enough. If an unrecognized cop or department is the first part of existing departments, we list the existing departments. This should help the user realize what needs to be changed. Also, we change the wording of `unrecognized cop` to `unrecognized cop or department`, and extend the did-you-mean functionality to also look for similar department names. --- changelog/fix_error_message_for_dept.md | 1 + lib/rubocop/config_validator.rb | 23 ++++++++++++++++++----- spec/rubocop/cli_spec.rb | 12 ++++++++---- spec/rubocop/config_loader_spec.rb | 23 +++++++++++++++++------ spec/rubocop/config_spec.rb | 2 +- 5 files changed, 45 insertions(+), 16 deletions(-) create mode 100644 changelog/fix_error_message_for_dept.md diff --git a/changelog/fix_error_message_for_dept.md b/changelog/fix_error_message_for_dept.md new file mode 100644 index 000000000000..8c50df131a80 --- /dev/null +++ b/changelog/fix_error_message_for_dept.md @@ -0,0 +1 @@ +* [#9752](https://github.com/rubocop/rubocop/issues/9752): Improve error message for top level department used in configuration. ([@jonas054][]) diff --git a/lib/rubocop/config_validator.rb b/lib/rubocop/config_validator.rb index f8c7d3ecd1a9..87656273736b 100644 --- a/lib/rubocop/config_validator.rb +++ b/lib/rubocop/config_validator.rb @@ -104,12 +104,9 @@ def alert_about_unrecognized_cops(invalid_cop_names) # to do so than to pass the value around to various methods. next if name == 'inherit_mode' - suggestions = NameSimilarity.find_similar_names(name, Cop::Registry.global.map(&:cop_name)) - suggestion = "Did you mean `#{suggestions.join('`, `')}`?" if suggestions.any? - message = <<~MESSAGE.rstrip - unrecognized cop #{name} found in #{smart_loaded_path} - #{suggestion} + unrecognized cop or department #{name} found in #{smart_loaded_path} + #{suggestion(name)} MESSAGE unknown_cops << message @@ -117,6 +114,22 @@ def alert_about_unrecognized_cops(invalid_cop_names) raise ValidationError, unknown_cops.join("\n") if unknown_cops.any? end + def suggestion(name) + registry = Cop::Registry.global + departments = registry.departments.map(&:to_s) + suggestions = NameSimilarity.find_similar_names(name, departments + registry.map(&:cop_name)) + if suggestions.any? + "Did you mean `#{suggestions.join('`, `')}`?" + else + # Department names can contain slashes, e.g. Chef/Correctness, but there's no support for + # the concept of higher level departments in RuboCop. It's a flat structure. So if the user + # tries to configure a "top level department", we hint that it's the bottom level + # departments that should be configured. + suggestions = departments.select { |department| department.start_with?("#{name}/") } + "#{name} is not a department. Use `#{suggestions.join('`, `')}`." if suggestions.any? + end + end + def validate_syntax_cop syntax_config = @config['Lint/Syntax'] default_config = ConfigLoader.default_configuration['Lint/Syntax'] diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index c84bc7c5cb4c..75a4e43bfd90 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -1380,6 +1380,8 @@ def meow_at_4am? Layout/LyneLenth: Enabled: true Max: 100 + Linth: + Enabled: false Lint/LiteralInCondition: Enabled: true Style/AlignHash: @@ -1389,11 +1391,13 @@ def meow_at_4am? expect(cli.run(%w[--format simple example])).to eq(2) expect($stderr.string) .to eq(<<~OUTPUT) - Error: unrecognized cop Layout/LyneLenth found in example/.rubocop.yml + Error: unrecognized cop or department Layout/LyneLenth found in example/.rubocop.yml Did you mean `Layout/LineLength`? - unrecognized cop Lint/LiteralInCondition found in example/.rubocop.yml + unrecognized cop or department Linth found in example/.rubocop.yml + Did you mean `Lint`? + unrecognized cop or department Lint/LiteralInCondition found in example/.rubocop.yml Did you mean `Lint/LiteralAsCondition`? - unrecognized cop Style/AlignHash found in example/.rubocop.yml + unrecognized cop or department Style/AlignHash found in example/.rubocop.yml Did you mean `Style/Alias`, `Style/OptionHash`? OUTPUT end @@ -1688,7 +1692,7 @@ def method(foo, bar, qux, fred, arg5, f) end #{'#' * 85} YAML expect(cli.run(['example1.rb'])).to eq(2) expect($stderr.string.strip).to eq( - 'Error: unrecognized cop Syntax/Whatever found in .rubocop.yml' + 'Error: unrecognized cop or department Syntax/Whatever found in .rubocop.yml' ) end end diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index a9af310322b1..9f363d134f7c 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -540,11 +540,12 @@ context 'when a department is disabled', :restore_registry do let(:file_path) { '.rubocop.yml' } - shared_examples 'resolves enabled/disabled for all cops' do |enabled_by_default, disabled_by_default| + shared_examples 'resolves enabled/disabled for all ' \ + 'cops' do |enabled_by_default, disabled_by_default, custom_dept_to_disable| before { stub_cop_class('RuboCop::Cop::Foo::Bar::Baz') } it "handles EnabledByDefault: #{enabled_by_default}, " \ - "DisabledByDefault: #{disabled_by_default}" do + "DisabledByDefault: #{disabled_by_default} with disabled #{custom_dept_to_disable}" do create_file('grandparent_rubocop.yml', <<~YAML) Naming/FileName: Enabled: pending @@ -573,7 +574,7 @@ Naming: Enabled: false - Foo/Bar: + #{custom_dept_to_disable}: Enabled: false YAML create_file(file_path, <<~YAML) @@ -603,6 +604,15 @@ def enabled?(cop) configuration_from_file.for_cop(cop)['Enabled'] end + if custom_dept_to_disable == 'Foo' + message = <<~'OUTPUT'.chomp + unrecognized cop or department Foo found in parent_rubocop.yml + Foo is not a department. Use `Foo/Bar`. + OUTPUT + expect { enabled?('Foo/Bar/Baz') }.to raise_error(RuboCop::ValidationError, message) + next + end + # Department disabled in parent config, cop enabled in child. expect(enabled?('Metrics/MethodLength')).to be(true) @@ -641,9 +651,10 @@ def enabled?(cop) end end - include_examples 'resolves enabled/disabled for all cops', false, false - include_examples 'resolves enabled/disabled for all cops', false, true - include_examples 'resolves enabled/disabled for all cops', true, false + include_examples 'resolves enabled/disabled for all cops', false, false, 'Foo/Bar' + include_examples 'resolves enabled/disabled for all cops', false, true, 'Foo/Bar' + include_examples 'resolves enabled/disabled for all cops', true, false, 'Foo/Bar' + include_examples 'resolves enabled/disabled for all cops', false, false, 'Foo' end context 'when a third party require defines a new gem', :restore_registry do diff --git a/spec/rubocop/config_spec.rb b/spec/rubocop/config_spec.rb index c01b0ee886dd..f17a08d77eaa 100644 --- a/spec/rubocop/config_spec.rb +++ b/spec/rubocop/config_spec.rb @@ -31,7 +31,7 @@ it 'raises an validation error' do expect { configuration }.to raise_error( RuboCop::ValidationError, - 'unrecognized cop LyneLenth found in .rubocop.yml' + 'unrecognized cop or department LyneLenth found in .rubocop.yml' ) end end From 36029ff9499251bee76dfc43141f684b396a053d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Wed, 14 Jul 2021 10:06:10 +0900 Subject: [PATCH 7/8] Support Ruby 2.7's pattern matching for `Lint/DuplicateBranch` This PR supports Ruby 2.7's pattern matching for `Lint/DuplicateBranch`. And this PR uses rubocop/rubocop-ast#192 feature, so it requires RuboCop AST 1.8.0 or higher. --- ..._doc_and_test_for_lint_duplicate_branch.md | 1 + lib/rubocop/cop/lint/duplicate_branch.rb | 3 +- rubocop.gemspec | 2 +- .../rubocop/cop/lint/duplicate_branch_spec.rb | 32 ++++++++++++++++++- 4 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 changelog/new_add_pattern_matching_doc_and_test_for_lint_duplicate_branch.md diff --git a/changelog/new_add_pattern_matching_doc_and_test_for_lint_duplicate_branch.md b/changelog/new_add_pattern_matching_doc_and_test_for_lint_duplicate_branch.md new file mode 100644 index 000000000000..a6a6781ee091 --- /dev/null +++ b/changelog/new_add_pattern_matching_doc_and_test_for_lint_duplicate_branch.md @@ -0,0 +1 @@ +* [#9930](https://github.com/rubocop/rubocop/pull/9930): Support Ruby 2.7's pattern matching for `Lint/DuplicateBranch` cop. ([@koic][]) diff --git a/lib/rubocop/cop/lint/duplicate_branch.rb b/lib/rubocop/cop/lint/duplicate_branch.rb index 32d68d0daacc..587142b4b036 100644 --- a/lib/rubocop/cop/lint/duplicate_branch.rb +++ b/lib/rubocop/cop/lint/duplicate_branch.rb @@ -4,7 +4,7 @@ module RuboCop module Cop module Lint # This cop checks that there are no repeated bodies - # within `if/unless`, `case-when` and `rescue` constructs. + # within `if/unless`, `case-when`, `case-in` and `rescue` constructs. # # With `IgnoreLiteralBranches: true`, branches are not registered # as offenses if they return a basic literal value (string, symbol, @@ -97,6 +97,7 @@ def on_branching_statement(node) end alias on_if on_branching_statement alias on_case on_branching_statement + alias on_case_match on_branching_statement alias on_rescue on_branching_statement private diff --git a/rubocop.gemspec b/rubocop.gemspec index 032a9e4d571a..abb729e9e9ff 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -35,7 +35,7 @@ Gem::Specification.new do |s| s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0') s.add_runtime_dependency('regexp_parser', '>= 1.8', '< 3.0') s.add_runtime_dependency('rexml') - s.add_runtime_dependency('rubocop-ast', '>= 1.7.0', '< 2.0') + s.add_runtime_dependency('rubocop-ast', '>= 1.8.0', '< 2.0') s.add_runtime_dependency('ruby-progressbar', '~> 1.7') s.add_runtime_dependency('unicode-display_width', '>= 1.4.0', '< 3.0') diff --git a/spec/rubocop/cop/lint/duplicate_branch_spec.rb b/spec/rubocop/cop/lint/duplicate_branch_spec.rb index ccfbada61589..1137c04ad1b5 100644 --- a/spec/rubocop/cop/lint/duplicate_branch_spec.rb +++ b/spec/rubocop/cop/lint/duplicate_branch_spec.rb @@ -65,6 +65,36 @@ end end + shared_examples_for 'literal case-match allowed' do |type, value| + context "when returning a #{type} in multiple branches", :ruby27 do + it 'allows branches to be duplicated' do + expect_no_offenses(<<~RUBY) + case foo + in x then #{value} + in y then #{value} + else #{value} + end + RUBY + end + end + end + + shared_examples_for 'literal case-match disallowed' do |type, value| + context "when returning a #{type} in multiple branches", :ruby27 do + it 'registers an offense' do + expect_offense(<<~RUBY, value: value) + case foo + in x then #{value} + in y then #{value} + ^^^^^^^^^^^{value} Duplicate branch body detected. + else #{value} + ^^^^ Duplicate branch body detected. + end + RUBY + end + end + end + shared_examples_for 'literal rescue allowed' do |type, value| context "when returning a #{type} in multiple branches" do it 'allows branches to be duplicated' do @@ -400,7 +430,7 @@ context 'with IgnoreConstantBranches: true' do let(:cop_config) { { 'IgnoreConstantBranches' => true } } - %w[if case rescue].each do |keyword| + %w[if case case-match rescue].each do |keyword| context "with `#{keyword}`" do it_behaves_like "literal #{keyword} allowed", 'constant', 'MY_CONST' From 4874caa1fe45fe3fda85a9c66b4fdb577def0043 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Mon, 19 Jul 2021 19:18:26 +0900 Subject: [PATCH 8/8] [Fix #9940] Fix an incorrect auto-correct for `Style/HashTransformValues` Fixes #9940. This PR fixes an incorrect auto-correct for `Style/HashTransformValues` when value is a hash literal for `_.to_h{...}`. --- ...correct_for_style_hash_transform_values.md | 1 + .../cop/mixin/hash_transform_method.rb | 7 +++++- .../cop/style/hash_transform_values_spec.rb | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 changelog/fix_incorrect_autocorrect_for_style_hash_transform_values.md diff --git a/changelog/fix_incorrect_autocorrect_for_style_hash_transform_values.md b/changelog/fix_incorrect_autocorrect_for_style_hash_transform_values.md new file mode 100644 index 000000000000..217c35b0a0af --- /dev/null +++ b/changelog/fix_incorrect_autocorrect_for_style_hash_transform_values.md @@ -0,0 +1 @@ +* [#9940](https://github.com/rubocop/rubocop/issues/9940): Fix an incorrect auto-correct for `Style/HashTransformValues` when value is a hash literal for `_.to_h{...}`. ([@koic][]) diff --git a/lib/rubocop/cop/mixin/hash_transform_method.rb b/lib/rubocop/cop/mixin/hash_transform_method.rb index 099a0807044f..68df99b76baa 100644 --- a/lib/rubocop/cop/mixin/hash_transform_method.rb +++ b/lib/rubocop/cop/mixin/hash_transform_method.rb @@ -175,7 +175,12 @@ def set_new_arg_name(transformed_argname, corrector) end def set_new_body_expression(transforming_body_expr, corrector) - corrector.replace(block_node.body, transforming_body_expr.loc.expression.source) + body_source = transforming_body_expr.loc.expression.source + if transforming_body_expr.hash_type? && !transforming_body_expr.braces? + body_source = "{ #{body_source} }" + end + + corrector.replace(block_node.body, body_source) end end end diff --git a/spec/rubocop/cop/style/hash_transform_values_spec.rb b/spec/rubocop/cop/style/hash_transform_values_spec.rb index 30bee15271f9..0f3ec021226d 100644 --- a/spec/rubocop/cop/style/hash_transform_values_spec.rb +++ b/spec/rubocop/cop/style/hash_transform_values_spec.rb @@ -211,6 +211,28 @@ RUBY end + it 'register and corrects an offense _.to_h{...} when value is a hash literal and is enclosed in braces' do + expect_offense(<<~RUBY) + {a: 1, b: 2}.to_h { |key, val| [key, { value: val }] } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `to_h {...}`. + RUBY + + expect_correction(<<~RUBY) + {a: 1, b: 2}.transform_values { |val| { value: val } } + RUBY + end + + it 'register and corrects an offense _.to_h{...} when value is a hash literal and is not enclosed in braces' do + expect_offense(<<~RUBY) + {a: 1, b: 2}.to_h { |key, val| [key, value: val] } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `transform_values` over `to_h {...}`. + RUBY + + expect_correction(<<~RUBY) + {a: 1, b: 2}.transform_values { |val| { value: val } } + RUBY + end + it 'does not flag `_.to_h{...}` when both key & value are transformed' do expect_no_offenses(<<~RUBY) x.to_h { |k, v| [k.to_sym, foo(v)] }