diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 5f169b781fcf..378a85b81b02 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -39,7 +39,7 @@ output by `rubocop -V`, include them as well. Here's an example: ``` $ [bundle exec] rubocop -V -1.75.5 (using Parser 3.3.5.0, rubocop-ast 1.32.3, analyzing as Ruby 3.3, running on ruby 3.3.5) [x86_64-linux] +1.75.7 (using Parser 3.3.5.0, rubocop-ast 1.32.3, analyzing as Ruby 3.3, running on ruby 3.3.5) [x86_64-linux] - rubocop-performance 1.22.1 - rubocop-rspec 3.1.0 ``` diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index ca700d5628b2..dd5c60e4a9bb 100644 --- a/.github/workflows/rubocop.yml +++ b/.github/workflows/rubocop.yml @@ -65,7 +65,9 @@ jobs: fail-fast: false matrix: ruby: - - '2.7' # Oldest supported version + # FIXME: When the MSYS2 issue https://github.com/ruby/setup-msys2-gcc/pull/26#issuecomment-2874639475 is resolved, + # please uncomment the following oldest supported version. + # - '2.7' # Oldest supported version - 'ruby' # Latest stable CRuby version steps: @@ -110,7 +112,9 @@ jobs: matrix: os: [ubuntu, windows] ruby: - - '2.7' # Oldest supported version + # FIXME: When the MSYS2 issue https://github.com/ruby/setup-msys2-gcc/pull/26#issuecomment-2874639475 is resolved, + # please uncomment the following oldest supported version. + # - '2.7' # Oldest supported version - 'ruby' # Latest stable CRuby version steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e7c428363a7..736711dcac8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,41 @@ ## master (unreleased) +## 1.75.7 (2025-05-21) + +### Bug fixes + +* [#14185](https://github.com/rubocop/rubocop/pull/14185): Fix an error for `Style/IfUnlessModifierOfIfUnless` when using nested modifier. ([@koic][]) +* [#14192](https://github.com/rubocop/rubocop/pull/14192): Fix negatives for `Layout/SpaceBeforeBrackets` when using space between method argument parentheses and left bracket. ([@koic][]) +* [#14189](https://github.com/rubocop/rubocop/issues/14189): Fix incorrect autocorrect for `Layout/SpaceBeforeBrackets` when using space between receiver and left brackets, and a space inside left bracket. ([@koic][]) +* [#14170](https://github.com/rubocop/rubocop/pull/14170): Fix `Style/AccessModifierDeclarations` cop error on semicolon after modifier. ([@viralpraxis][]) +* [#14195](https://github.com/rubocop/rubocop/pull/14195): Fix `Style/AccessModifierDeclarations` cop error on symbol modifier without surrounding scope. ([@viralpraxis][]) +* [#14172](https://github.com/rubocop/rubocop/pull/14172): Fix `Style/AccessModifierDeclarations` cop false positives when there are no method definitions and style is `inline`. ([@viralpraxis][]) +* [#14193](https://github.com/rubocop/rubocop/issues/14193): Fix `Lint/UselessAssignment` cop error when using nested assignment with splat. ([@earlopain][]) + +### Changes + +* [#14188](https://github.com/rubocop/rubocop/pull/14188): Enhance `Gemspec/DuplicatedAssignment` cop to detect duplicated indexed assignment. ([@viralpraxis][]) +* [#14183](https://github.com/rubocop/rubocop/pull/14183): Recognize `prefix` argument for `delegate` method in `Lint/DuplicateMethods`. ([@lovro-bikic][]) + +## 1.75.6 (2025-05-15) + +### Bug fixes + +* [#14176](https://github.com/rubocop/rubocop/pull/14176): Fix an error for `Style/MultilineIfModifier` when using nested modifier. ([@koic][]) +* [#14077](https://github.com/rubocop/rubocop/issues/14077): Change `nil` representation in todo file comments. ([@jonas054][]) +* [#14164](https://github.com/rubocop/rubocop/pull/14164): Fix an error for `Lint/UselessAssignment` when variables are assigned using unary operator in chained assignment and remain unreferenced. ([@koic][]) +* [#14173](https://github.com/rubocop/rubocop/pull/14173): Fix an error for `Style/StringConcatenation` when using implicit concatenation with string interpolation. ([@koic][]) +* [#14177](https://github.com/rubocop/rubocop/issues/14177): Fix false positives for `Style/SoleNestedConditional` when using nested `if` and `not` in condition. ([@koic][]) +* [#14152](https://github.com/rubocop/rubocop/pull/14152): Fix `Layout/SpaceInsideArrayLiteralBrackets` cop error on array pattern without brackets. ([@viralpraxis][]) +* [#14153](https://github.com/rubocop/rubocop/pull/14153): Fix `Style/PercentQLiterals` cop error on Unicode escape sequence. ([@viralpraxis][]) + +### Changes + +* [#14082](https://github.com/rubocop/rubocop/issues/14082): Mark `Style/ComparableBetween` as unsafe. ([@earlopain][]) +* [#14181](https://github.com/rubocop/rubocop/issues/14181): Make `Lint/DuplicateMethods` aware of Active Support's `delegate` method. ([@lovro-bikic][]) +* [#14156](https://github.com/rubocop/rubocop/issues/14156): Make `Style/IfUnlessModifier` allow endless method definition in the `if` body. ([@koic][]) + ## 1.75.5 (2025-05-05) ### Bug fixes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e17bb73b23bd..174e8bb60508 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,7 +17,7 @@ do so. ```console $ rubocop -V -1.75.5 (using Parser 3.3.5.0, rubocop-ast 1.32.3, analyzing as Ruby 3.3, running on ruby 3.3.5) [x86_64-linux] +1.75.7 (using Parser 3.3.5.0, rubocop-ast 1.32.3, analyzing as Ruby 3.3, running on ruby 3.3.5) [x86_64-linux] - rubocop-performance 1.22.1 - rubocop-rspec 3.1.0 ``` diff --git a/config/default.yml b/config/default.yml index 51fb93e89828..5e797e776547 100644 --- a/config/default.yml +++ b/config/default.yml @@ -3702,7 +3702,9 @@ Style/CommentedKeyword: Style/ComparableBetween: Description: 'Enforces the use of `Comparable#between?` instead of logical comparison.' Enabled: pending + Safe: false VersionAdded: '1.74' + VersionChanged: '1.75' StyleGuide: '#ranges-or-between' Style/ComparableClamp: diff --git a/docs/modules/ROOT/pages/cops_gemspec.adoc b/docs/modules/ROOT/pages/cops_gemspec.adoc index 9a829cd2020d..2ea734cced54 100644 --- a/docs/modules/ROOT/pages/cops_gemspec.adoc +++ b/docs/modules/ROOT/pages/cops_gemspec.adoc @@ -329,10 +329,11 @@ gem "bar" An attribute assignment method calls should be listed only once in a gemspec. -Assigning to an attribute with the same name using `spec.foo =` will be -an unintended usage. On the other hand, duplication of methods such -as `spec.requirements`, `spec.add_runtime_dependency`, and others are -permitted because it is the intended use of appending values. +Assigning to an attribute with the same name using `spec.foo =` or +`spec.attribute#[]=` will be an unintended usage. On the other hand, +duplication of methods such # as `spec.requirements`, +`spec.add_runtime_dependency`, and others are permitted because it is +the intended use of appending values. [#examples-gemspecduplicatedassignment] === Examples @@ -361,6 +362,17 @@ Gem::Specification.new do |spec| spec.add_dependency('parallel', '~> 1.10') spec.add_dependency('parser', '>= 2.3.3.1', '< 3.0') end + +# bad +Gem::Specification.new do |spec| + spec.metadata["key"] = "value" + spec.metadata["key"] = "value" +end + +# good +Gem::Specification.new do |spec| + spec.metadata["key"] = "value" +end ---- [#configurable-attributes-gemspecduplicatedassignment] diff --git a/docs/modules/ROOT/pages/cops_layout.adoc b/docs/modules/ROOT/pages/cops_layout.adoc index 1ca6cc4474a6..c35345c170df 100644 --- a/docs/modules/ROOT/pages/cops_layout.adoc +++ b/docs/modules/ROOT/pages/cops_layout.adoc @@ -833,7 +833,7 @@ end | Name | Default value | Configurable values | Categories -| `{"module_inclusion"=>["include", "prepend", "extend"]}` +| `{"module_inclusion" => ["include", "prepend", "extend"]}` | | ExpectedOrder diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index f60bd420c1b3..0c053e0d2548 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -1119,11 +1119,11 @@ require 'my_debugger/start' | Name | Default value | Configurable values | DebuggerMethods -| `{"Kernel"=>["binding.irb", "Kernel.binding.irb"], "Byebug"=>["byebug", "remote_byebug", "Kernel.byebug", "Kernel.remote_byebug"], "Capybara"=>["page.save_and_open_page", "page.save_and_open_screenshot", "page.save_page", "page.save_screenshot", "save_and_open_page", "save_and_open_screenshot", "save_page", "save_screenshot"], "debug.rb"=>["binding.b", "binding.break", "Kernel.binding.b", "Kernel.binding.break"], "Pry"=>["binding.pry", "binding.remote_pry", "binding.pry_remote", "Kernel.binding.pry", "Kernel.binding.remote_pry", "Kernel.binding.pry_remote", "Pry.rescue", "pry"], "Rails"=>["debugger", "Kernel.debugger"], "RubyJard"=>["jard"], "WebConsole"=>["binding.console"]}` +| `{"Kernel" => ["binding.irb", "Kernel.binding.irb"], "Byebug" => ["byebug", "remote_byebug", "Kernel.byebug", "Kernel.remote_byebug"], "Capybara" => ["page.save_and_open_page", "page.save_and_open_screenshot", "page.save_page", "page.save_screenshot", "save_and_open_page", "save_and_open_screenshot", "save_page", "save_screenshot"], "debug.rb" => ["binding.b", "binding.break", "Kernel.binding.b", "Kernel.binding.break"], "Pry" => ["binding.pry", "binding.remote_pry", "binding.pry_remote", "Kernel.binding.pry", "Kernel.binding.remote_pry", "Kernel.binding.pry_remote", "Pry.rescue", "pry"], "Rails" => ["debugger", "Kernel.debugger"], "RubyJard" => ["jard"], "WebConsole" => ["binding.console"]}` | | DebuggerRequires -| `{"debug.rb"=>["debug/open", "debug/start"]}` +| `{"debug.rb" => ["debug/open", "debug/start"]}` | |=== @@ -1230,7 +1230,7 @@ Etc::Passwd | Name | Default value | Configurable values | DeprecatedConstants -| `{"NIL"=>{"Alternative"=>"nil", "DeprecatedVersion"=>"2.4"}, "TRUE"=>{"Alternative"=>"true", "DeprecatedVersion"=>"2.4"}, "FALSE"=>{"Alternative"=>"false", "DeprecatedVersion"=>"2.4"}, "Net::HTTPServerException"=>{"Alternative"=>"Net::HTTPClientException", "DeprecatedVersion"=>"2.6"}, "Random::DEFAULT"=>{"Alternative"=>"Random.new", "DeprecatedVersion"=>"3.0"}, "Struct::Group"=>{"Alternative"=>"Etc::Group", "DeprecatedVersion"=>"3.0"}, "Struct::Passwd"=>{"Alternative"=>"Etc::Passwd", "DeprecatedVersion"=>"3.0"}}` +| `{"NIL" => {"Alternative" => "nil", "DeprecatedVersion" => "2.4"}, "TRUE" => {"Alternative" => "true", "DeprecatedVersion" => "2.4"}, "FALSE" => {"Alternative" => "false", "DeprecatedVersion" => "2.4"}, "Net::HTTPServerException" => {"Alternative" => "Net::HTTPClientException", "DeprecatedVersion" => "2.6"}, "Random::DEFAULT" => {"Alternative" => "Random.new", "DeprecatedVersion" => "3.0"}, "Struct::Group" => {"Alternative" => "Etc::Group", "DeprecatedVersion" => "3.0"}, "Struct::Passwd" => {"Alternative" => "Etc::Passwd", "DeprecatedVersion" => "3.0"}}` | |=== @@ -1782,6 +1782,55 @@ end alias bar foo ---- +[#allcops_activesupportextensionsenabled_-false-_default_-lintduplicatemethods] +==== AllCops:ActiveSupportExtensionsEnabled: false (default) + +[source,ruby] +---- +# good +def foo + 1 +end + +delegate :foo, to: :bar +---- + +[#allcops_activesupportextensionsenabled_-true-lintduplicatemethods] +==== AllCops:ActiveSupportExtensionsEnabled: true + +[source,ruby] +---- +# bad +def foo + 1 +end + +delegate :foo, to: :bar + +# good +def foo + 1 +end + +delegate :baz, to: :bar + +# good - delegate with splat arguments is ignored +def foo + 1 +end + +delegate :foo, **options + +# good - delegate inside a condition is ignored +def foo + 1 +end + +if cond + delegate :foo, to: :bar +end +---- + [#lintduplicateregexpcharacterclasselement] == Lint/DuplicateRegexpCharacterClassElement @@ -7285,7 +7334,7 @@ values.sort { |*x| x[0] <=> x[1] } | Name | Default value | Configurable values | Methods -| `{"chunk_while"=>2, "each_with_index"=>2, "each_with_object"=>2, "inject"=>2, "max"=>2, "min"=>2, "minmax"=>2, "reduce"=>2, "slice_when"=>2, "sort"=>2}` +| `{"chunk_while" => 2, "each_with_index" => 2, "each_with_object" => 2, "inject" => 2, "max" => 2, "min" => 2, "minmax" => 2, "reduce" => 2, "slice_when" => 2, "sort" => 2}` | |=== diff --git a/docs/modules/ROOT/pages/cops_naming.adoc b/docs/modules/ROOT/pages/cops_naming.adoc index 72ec61cc43af..cef09a31f43f 100644 --- a/docs/modules/ROOT/pages/cops_naming.adoc +++ b/docs/modules/ROOT/pages/cops_naming.adoc @@ -806,7 +806,7 @@ TeslaVehicle | Boolean | FlaggedTerms -| `{"whitelist"=>{"Regex"=>/white[-_\s]?list/, "Suggestions"=>["allowlist", "permit"]}, "blacklist"=>{"Regex"=>/black[-_\s]?list/, "Suggestions"=>["denylist", "block"]}, "slave"=>{"WholeWord"=>true, "Suggestions"=>["replica", "secondary", "follower"]}}` +| `{"whitelist" => {"Regex" => /white[-_\s]?list/, "Suggestions" => ["allowlist", "permit"]}, "blacklist" => {"Regex" => /black[-_\s]?list/, "Suggestions" => ["denylist", "block"]}, "slave" => {"WholeWord" => true, "Suggestions" => ["replica", "secondary", "follower"]}}` | |=== diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index ee8f58adde8a..fa5db7ddb9b1 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -2350,7 +2350,7 @@ items.include? | Name | Default value | Configurable values | PreferredMethods -| `{"collect"=>"map", "collect!"=>"map!", "collect_concat"=>"flat_map", "inject"=>"reduce", "detect"=>"find", "find_all"=>"select", "member?"=>"include?"}` +| `{"collect" => "map", "collect!" => "map!", "collect_concat" => "flat_map", "inject" => "reduce", "detect" => "find", "find_all" => "select", "member?" => "include?"}` | | MethodsAcceptingSymbol @@ -2853,10 +2853,10 @@ end | Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed | Pending -| Yes -| Always +| No +| Always (Unsafe) | 1.74 -| - +| 1.75 |=== Checks for logical comparison which can be replaced with `Comparable#between?`. @@ -2865,6 +2865,11 @@ NOTE: `Comparable#between?` is on average slightly slower than logical compariso although the difference generally isn't observable. If you require maximum performance, consider using logical comparison. +[#safety-stylecomparablebetween] +=== Safety + +This cop is unsafe because the receiver may not respond to `between?`. + [#examples-stylecomparablebetween] === Examples @@ -3248,6 +3253,7 @@ NOTE: Requires Ruby version 3.2 |=== Checks for inheritance from `Data.define` to avoid creating the anonymous parent class. +Inheriting from `Data.define` adds a superfluous level in inheritance tree. [#safety-styledatainheritance] === Safety @@ -3267,12 +3273,18 @@ class Person < Data.define(:first_name, :last_name) end end +Person.ancestors +# => [Person, #, Data, (...)] + # good Person = Data.define(:first_name, :last_name) do def age 42 end end + +Person.ancestors +# => [Person, Data, (...)] ---- [#references-styledatainheritance] @@ -7499,6 +7511,17 @@ if [42] in [x] end ---- +The code `def method_name = body if condition` is considered a bad case by +`Style/AmbiguousEndlessMethodDefinition` cop. So, to respect the user's intention to use +an endless method definition in the `if` body, the following code is allowed: + +[source,ruby] +---- +if condition + def method_name = body +end +---- + NOTE: It is allowed when `defined?` argument has an undefined value, because using the modifier form causes the following incompatibility: @@ -7917,11 +7940,11 @@ end | Name | Default value | Configurable values | InverseMethods -| `{:any?=>:none?, :even?=>:odd?, :===>:!=, :=~=>:!~, :<=>:>=, :>=>:<=}` +| `{any?: :none?, even?: :odd?, "==": :!=, "=~": :!~, "<": :>=, ">": :<=}` | | InverseBlocks -| `{:select=>:reject, :select!=>:reject!}` +| `{select: :reject, select!: :reject!}` | |=== @@ -7999,7 +8022,7 @@ foo if !condition | Name | Default value | Configurable values | InverseMethods -| `{:!==>:==, :>=>:<=, :<==>:>, :<=>:>=, :>==>:<, :!~=>:=~, :zero?=>:nonzero?, :nonzero?=>:zero?, :any?=>:none?, :none?=>:any?, :even?=>:odd?, :odd?=>:even?}` +| `{"!=": :==, ">": :<=, "<=": :>, "<": :>=, ">=": :<, "!~": :=~, zero?: :nonzero?, nonzero?: :zero?, any?: :none?, none?: :any?, even?: :odd?, odd?: :even?}` | |=== @@ -12427,7 +12450,7 @@ default. | Name | Default value | Configurable values | PreferredDelimiters -| `{"default"=>"()", "%i"=>"[]", "%I"=>"[]", "%r"=>"{}", "%w"=>"[]", "%W"=>"[]"}` +| `{"default" => "()", "%i" => "[]", "%I" => "[]", "%r" => "{}", "%w" => "[]", "%W" => "[]"}` | |=== @@ -12913,7 +12936,7 @@ A.foo | Name | Default value | Configurable values | Methods -| `{"join"=>"", "sum"=>0, "exit"=>true, "exit!"=>false, "split"=>" ", "chomp"=>"\n", "chomp!"=>"\n"}` +| `{"join" => "", "sum" => 0, "exit" => true, "exit!" => false, "split" => " ", "chomp" => "\n", "chomp!" => "\n"}` | |=== @@ -16072,7 +16095,7 @@ end | Name | Default value | Configurable values | Methods -| `{"reduce"=>["acc", "elem"]}`, `{"inject"=>["acc", "elem"]}` +| `{"reduce" => ["acc", "elem"]}`, `{"inject" => ["acc", "elem"]}` | Array |=== @@ -16988,7 +17011,7 @@ from the `String` class. | Name | Default value | Configurable values | PreferredMethods -| `{"intern"=>"to_sym"}` +| `{"intern" => "to_sym"}` | |=== @@ -17034,7 +17057,8 @@ Identifies places where `lstrip.rstrip` can be replaced by | 1.20 |=== -Checks for inheritance from Struct.new. +Checks for inheritance from `Struct.new`. Inheriting from `Struct.new` +adds a superfluous level in inheritance tree. [#safety-stylestructinheritance] === Safety @@ -17054,12 +17078,18 @@ class Person < Struct.new(:first_name, :last_name) end end +Person.ancestors +# => [Person, #, Struct, (...)] + # good Person = Struct.new(:first_name, :last_name) do def age 42 end end + +Person.ancestors +# => [Person, Struct, (...)] ---- [#references-stylestructinheritance] diff --git a/docs/modules/ROOT/pages/extensions.adoc b/docs/modules/ROOT/pages/extensions.adoc index 265833209b63..07955d0c8391 100644 --- a/docs/modules/ROOT/pages/extensions.adoc +++ b/docs/modules/ROOT/pages/extensions.adoc @@ -300,6 +300,6 @@ On the extension side, the code would be something like this: RuboCop::Runner.ruby_extractors.unshift(ruby_extractor) ---- -`RuboCop::Runners.ruby_extractors` is processed from the beginning and ends when one of them returns a non-nil value. By default, there is a Ruby extractor that returns the given Ruby source code with offset 0, so you can unshift any Ruby extractor before it. +`RuboCop::Runner.ruby_extractors` is processed from the beginning and ends when one of them returns a non-nil value. By default, there is a Ruby extractor that returns the given Ruby source code with offset 0, so you can unshift any Ruby extractor before it. NOTE: This is still an experimental feature and may change in the future. diff --git a/docs/modules/ROOT/pages/integration_with_other_tools.adoc b/docs/modules/ROOT/pages/integration_with_other_tools.adoc index edfe0e074f16..733df33e1dca 100644 --- a/docs/modules/ROOT/pages/integration_with_other_tools.adoc +++ b/docs/modules/ROOT/pages/integration_with_other_tools.adoc @@ -125,7 +125,7 @@ following to your `.pre-commit-config.yaml` file: [source,yaml] ---- - repo: https://github.com/rubocop/rubocop - rev: v1.75.5 + rev: v1.75.7 hooks: - id: rubocop ---- @@ -136,7 +136,7 @@ entries in `additional_dependencies`: [source,yaml] ---- - repo: https://github.com/rubocop/rubocop - rev: v1.75.5 + rev: v1.75.7 hooks: - id: rubocop additional_dependencies: diff --git a/lib/rubocop/cop/gemspec/duplicated_assignment.rb b/lib/rubocop/cop/gemspec/duplicated_assignment.rb index c9db77142d1e..b39869384a40 100644 --- a/lib/rubocop/cop/gemspec/duplicated_assignment.rb +++ b/lib/rubocop/cop/gemspec/duplicated_assignment.rb @@ -6,10 +6,11 @@ module Gemspec # An attribute assignment method calls should be listed only once # in a gemspec. # - # Assigning to an attribute with the same name using `spec.foo =` will be - # an unintended usage. On the other hand, duplication of methods such - # as `spec.requirements`, `spec.add_runtime_dependency`, and others are - # permitted because it is the intended use of appending values. + # Assigning to an attribute with the same name using `spec.foo =` or + # `spec.attribute#[]=` will be an unintended usage. On the other hand, + # duplication of methods such # as `spec.requirements`, + # `spec.add_runtime_dependency`, and others are permitted because it is + # the intended use of appending values. # # @example # # bad @@ -34,6 +35,18 @@ module Gemspec # spec.add_dependency('parallel', '~> 1.10') # spec.add_dependency('parser', '>= 2.3.3.1', '< 3.0') # end + # + # # bad + # Gem::Specification.new do |spec| + # spec.metadata["key"] = "value" + # spec.metadata["key"] = "value" + # end + # + # # good + # Gem::Specification.new do |spec| + # spec.metadata["key"] = "value" + # end + # class DuplicatedAssignment < Base include RangeHelp include GemspecHelp @@ -47,9 +60,26 @@ class DuplicatedAssignment < Base (lvar #match_block_variable_name?) _ ...) PATTERN + # @!method indexed_assignment_method_declarations(node) + def_node_search :indexed_assignment_method_declarations, <<~PATTERN + (send + (send (lvar #match_block_variable_name?) _) + :[]= + literal? + _ + ) + PATTERN + def on_new_investigation return if processed_source.blank? + process_assignment_method_nodes + process_indexed_assignment_method_nodes + end + + private + + def process_assignment_method_nodes duplicated_assignment_method_nodes.each do |nodes| nodes[1..].each do |node| register_offense(node, node.method_name, nodes.first.first_line) @@ -57,7 +87,14 @@ def on_new_investigation end end - private + def process_indexed_assignment_method_nodes + duplicated_indexed_assignment_method_nodes.each do |nodes| + nodes[1..].each do |node| + assignment = "#{node.children.first.method_name}[#{node.first_argument.source}]=" + register_offense(node, assignment, nodes.first.first_line) + end + end + end def match_block_variable_name?(receiver_name) gem_specification(processed_source.ast) do |block_variable_name| @@ -73,6 +110,13 @@ def duplicated_assignment_method_nodes .select { |nodes| nodes.size > 1 } end + def duplicated_indexed_assignment_method_nodes + indexed_assignment_method_declarations(processed_source.ast) + .group_by { |node| [node.children.first.method_name, node.first_argument] } + .values + .select { |nodes| nodes.size > 1 } + end + def register_offense(node, assignment, line_of_first_occurrence) line_range = node.loc.column...node.loc.last_column offense_location = source_range(processed_source.buffer, node.first_line, line_range) diff --git a/lib/rubocop/cop/layout/space_before_brackets.rb b/lib/rubocop/cop/layout/space_before_brackets.rb index 1e8a206a1a23..669761629004 100644 --- a/lib/rubocop/cop/layout/space_before_brackets.rb +++ b/lib/rubocop/cop/layout/space_before_brackets.rb @@ -22,51 +22,25 @@ class SpaceBeforeBrackets < Base RESTRICT_ON_SEND = %i[[] []=].freeze def on_send(node) - return unless (first_argument = node.first_argument) - - begin_pos = first_argument.source_range.begin_pos - return unless (range = offense_range(node, begin_pos)) - - register_offense(range) - end - - private - - def offense_range(node, begin_pos) receiver_end_pos = node.receiver.source_range.end_pos selector_begin_pos = node.loc.selector.begin_pos return if receiver_end_pos >= selector_begin_pos return if dot_before_brackets?(node, receiver_end_pos, selector_begin_pos) - if reference_variable_with_brackets?(node) - range_between(receiver_end_pos, selector_begin_pos) - elsif node.method?(:[]=) - offense_range_for_assignment(node, begin_pos) + range = range_between(receiver_end_pos, selector_begin_pos) + + add_offense(range) do |corrector| + corrector.remove(range) end end + private + def dot_before_brackets?(node, receiver_end_pos, selector_begin_pos) return false unless node.loc.respond_to?(:dot) && (dot = node.loc.dot) dot.begin_pos == receiver_end_pos && dot.end_pos == selector_begin_pos end - - def offense_range_for_assignment(node, begin_pos) - end_pos = node.receiver.source_range.end_pos - - return if begin_pos - end_pos == 1 || - (range = range_between(end_pos, begin_pos - 1)).source.start_with?('[') - - range - end - - def register_offense(range) - add_offense(range) { |corrector| corrector.remove(range) } - end - - def reference_variable_with_brackets?(node) - node.receiver&.variable? && node.method?(:[]) && node.arguments.size == 1 - end end end end diff --git a/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb b/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb index 8b33dacadf76..e1c10d500597 100644 --- a/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb +++ b/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb @@ -87,6 +87,7 @@ def on_array(node) return if node.array_type? && !node.square_brackets? tokens, left, right = array_brackets(node) + return unless left && right if empty_brackets?(left, right, tokens: tokens) return empty_offenses(node, left, right, EMPTY_MSG) diff --git a/lib/rubocop/cop/lint/deprecated_class_methods.rb b/lib/rubocop/cop/lint/deprecated_class_methods.rb index cf4ae4b86732..df998d94906e 100644 --- a/lib/rubocop/cop/lint/deprecated_class_methods.rb +++ b/lib/rubocop/cop/lint/deprecated_class_methods.rb @@ -43,7 +43,7 @@ class DeprecatedClassMethods < Base dup: 'to_h', exists?: 'exist?', gethostbyaddr: 'Addrinfo#getnameinfo', - gethostbyname: 'Addrinfo#getaddrinfo', + gethostbyname: 'Addrinfo.getaddrinfo', iterator?: 'block_given?' }.freeze diff --git a/lib/rubocop/cop/lint/duplicate_methods.rb b/lib/rubocop/cop/lint/duplicate_methods.rb index ddbc681499ad..d35393a481c9 100644 --- a/lib/rubocop/cop/lint/duplicate_methods.rb +++ b/lib/rubocop/cop/lint/duplicate_methods.rb @@ -39,9 +39,52 @@ module Lint # end # # alias bar foo + # + # @example AllCops:ActiveSupportExtensionsEnabled: false (default) + # + # # good + # def foo + # 1 + # end + # + # delegate :foo, to: :bar + # + # @example AllCops:ActiveSupportExtensionsEnabled: true + # + # # bad + # def foo + # 1 + # end + # + # delegate :foo, to: :bar + # + # # good + # def foo + # 1 + # end + # + # delegate :baz, to: :bar + # + # # good - delegate with splat arguments is ignored + # def foo + # 1 + # end + # + # delegate :foo, **options + # + # # good - delegate inside a condition is ignored + # def foo + # 1 + # end + # + # if cond + # delegate :foo, to: :bar + # end + # class DuplicateMethods < Base MSG = 'Method `%s` is defined at both %s and %s.' - RESTRICT_ON_SEND = %i[alias_method attr_reader attr_writer attr_accessor attr].freeze + RESTRICT_ON_SEND = %i[alias_method attr_reader attr_writer attr_accessor attr + delegate].freeze def initialize(config = nil, options = nil) super @@ -85,15 +128,28 @@ def on_alias(node) (send nil? :alias_method (sym $_name) _) PATTERN + # @!method delegate_method?(node) + def_node_matcher :delegate_method?, <<~PATTERN + (send nil? :delegate + ({sym str} $_)+ + (hash <(pair (sym :to) _) ...>) + ) + PATTERN + # @!method sym_name(node) def_node_matcher :sym_name, '(sym $_name)' - def on_send(node) + + def on_send(node) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity if (name = alias_method?(node)) return if node.ancestors.any?(&:if_type?) found_instance_method(node, name) elsif (attr = node.attribute_accessor?) on_attr(node, *attr) + elsif active_support_extensions_enabled? && (names = delegate_method?(node)) + return if node.ancestors.any?(&:if_type?) + + on_delegate(node, names) end end @@ -118,6 +174,32 @@ def message_for_dup(node, method_name, key) current: source_location(node)) end + def on_delegate(node, method_names) + name_prefix = delegate_prefix(node) + + method_names.each do |name| + name = "#{name_prefix}_#{name}" if name_prefix + + found_instance_method(node, name) + end + end + + def delegate_prefix(node) + kwargs_node = node.last_argument + + return unless (prefix = hash_value(kwargs_node, :prefix)) + + if prefix.true_type? + hash_value(kwargs_node, :to).value + elsif prefix.type?(:sym, :str) + prefix.value + end + end + + def hash_value(node, key) + node.pairs.find { |pair| pair.key.value == key }&.value + end + def found_instance_method(node, name) return found_sclass_method(node, name) unless (scope = node.parent_module_name) diff --git a/lib/rubocop/cop/lint/useless_assignment.rb b/lib/rubocop/cop/lint/useless_assignment.rb index 9d76452a436b..6ef10022b311 100644 --- a/lib/rubocop/cop/lint/useless_assignment.rb +++ b/lib/rubocop/cop/lint/useless_assignment.rb @@ -104,6 +104,8 @@ def sequential_assignment?(node) end def chained_assignment?(node) + return true if node.lvasgn_type? && node.expression&.send_type? + node.respond_to?(:expression) && node.expression&.lvasgn_type? end diff --git a/lib/rubocop/cop/metrics/abc_size.rb b/lib/rubocop/cop/metrics/abc_size.rb index 970aff5f9fb8..5ed1ab110b83 100644 --- a/lib/rubocop/cop/metrics/abc_size.rb +++ b/lib/rubocop/cop/metrics/abc_size.rb @@ -39,7 +39,7 @@ module Metrics class AbcSize < Base include MethodComplexity - MSG = 'Assignment Branch Condition size for %s is too high. ' \ + MSG = 'Assignment Branch Condition size for `%s` is too high. ' \ '[%s %.4g/%.4g]' private diff --git a/lib/rubocop/cop/style/access_modifier_declarations.rb b/lib/rubocop/cop/style/access_modifier_declarations.rb index 079fdd865ecf..35b097bc4e2d 100644 --- a/lib/rubocop/cop/style/access_modifier_declarations.rb +++ b/lib/rubocop/cop/style/access_modifier_declarations.rb @@ -195,15 +195,27 @@ def allowed?(node) def autocorrect(corrector, node) case style when :group - def_nodes = find_corresponding_def_nodes(node) - return unless def_nodes.any? - - replace_defs(corrector, node, def_nodes) + autocorrect_group_style(corrector, node) when :inline + autocorrect_inline_style(corrector, node) + end + end + + def autocorrect_group_style(corrector, node) + def_nodes = find_corresponding_def_nodes(node) + return unless def_nodes.any? + + replace_defs(corrector, node, def_nodes) + end + + def autocorrect_inline_style(corrector, node) + if node.parent&.begin_type? + remove_modifier_node_within_begin(corrector, node, node.parent) + else remove_nodes(corrector, node) - select_grouped_def_nodes(node).each do |grouped_def_node| - insert_inline_modifier(corrector, grouped_def_node, node.method_name) - end + end + select_grouped_def_nodes(node).each do |grouped_def_node| + insert_inline_modifier(corrector, grouped_def_node, node.method_name) end end @@ -224,9 +236,13 @@ def allow_modifiers_on_alias_method?(node) end def offense?(node) - (group_style? && access_modifier_is_inlined?(node) && - !node.parent&.if_type? && !right_siblings_same_inline_method?(node)) || - (inline_style? && access_modifier_is_not_inlined?(node)) + if group_style? + return false if node.parent ? node.parent.if_type? : access_modifier_with_symbol?(node) + + access_modifier_is_inlined?(node) && !right_siblings_same_inline_method?(node) + else + access_modifier_is_not_inlined?(node) && select_grouped_def_nodes(node).any? + end end def correctable_group_offense?(node) @@ -331,6 +347,12 @@ def remove_nodes(corrector, *nodes) end end + def remove_modifier_node_within_begin(corrector, modifier_node, begin_node) + def_node = begin_node.children[1] + range = modifier_node.source_range.begin.join(def_node.source_range.begin) + corrector.remove(range) + end + def def_source(node, def_nodes) [ *processed_source.ast_with_comments[node].map(&:text), diff --git a/lib/rubocop/cop/style/command_literal.rb b/lib/rubocop/cop/style/command_literal.rb index dfba98e1aa37..56ff49bd6a74 100644 --- a/lib/rubocop/cop/style/command_literal.rb +++ b/lib/rubocop/cop/style/command_literal.rb @@ -173,7 +173,7 @@ def default_delimiter end def preferred_delimiters_config - config.for_cop('Style/PercentLiteralDelimiters') ['PreferredDelimiters'] + config.for_cop('Style/PercentLiteralDelimiters')['PreferredDelimiters'] end end end diff --git a/lib/rubocop/cop/style/comparable_between.rb b/lib/rubocop/cop/style/comparable_between.rb index 4e6c956593c4..17fdf07919a3 100644 --- a/lib/rubocop/cop/style/comparable_between.rb +++ b/lib/rubocop/cop/style/comparable_between.rb @@ -9,6 +9,9 @@ module Style # although the difference generally isn't observable. If you require maximum # performance, consider using logical comparison. # + # @safety + # This cop is unsafe because the receiver may not respond to `between?`. + # # @example # # # bad diff --git a/lib/rubocop/cop/style/data_inheritance.rb b/lib/rubocop/cop/style/data_inheritance.rb index 4dc8fab97669..a6a612ccdb54 100644 --- a/lib/rubocop/cop/style/data_inheritance.rb +++ b/lib/rubocop/cop/style/data_inheritance.rb @@ -4,6 +4,7 @@ module RuboCop module Cop module Style # Checks for inheritance from `Data.define` to avoid creating the anonymous parent class. + # Inheriting from `Data.define` adds a superfluous level in inheritance tree. # # @safety # Autocorrection is unsafe because it will change the inheritance @@ -17,12 +18,18 @@ module Style # end # end # + # Person.ancestors + # # => [Person, #, Data, (...)] + # # # good # Person = Data.define(:first_name, :last_name) do # def age # 42 # end # end + # + # Person.ancestors + # # => [Person, Data, (...)] class DataInheritance < Base include RangeHelp extend AutoCorrector diff --git a/lib/rubocop/cop/style/if_unless_modifier.rb b/lib/rubocop/cop/style/if_unless_modifier.rb index 448039eba494..50a8bba78edb 100644 --- a/lib/rubocop/cop/style/if_unless_modifier.rb +++ b/lib/rubocop/cop/style/if_unless_modifier.rb @@ -23,6 +23,17 @@ module Style # end # ---- # + # The code `def method_name = body if condition` is considered a bad case by + # `Style/AmbiguousEndlessMethodDefinition` cop. So, to respect the user's intention to use + # an endless method definition in the `if` body, the following code is allowed: + # + # [source,ruby] + # ---- + # if condition + # def method_name = body + # end + # ---- + # # NOTE: It is allowed when `defined?` argument has an undefined value, # because using the modifier form causes the following incompatibility: # @@ -77,10 +88,14 @@ def self.autocorrect_incompatible_with [Style::SoleNestedConditional] end + # rubocop:disable Metrics/AbcSize def on_if(node) + return if endless_method?(node.body) + condition = node.condition return if defined_nodes(condition).any? { |n| defined_argument_is_undefined?(node, n) } || pattern_matching_nodes(condition).any? + return unless (msg = message(node)) add_offense(node.loc.keyword, message: format(msg, keyword: node.keyword)) do |corrector| @@ -90,9 +105,14 @@ def on_if(node) ignore_node(node) end end + # rubocop:enable Metrics/AbcSize private + def endless_method?(body) + body&.any_def_type? && body.endless? + end + def defined_nodes(condition) if condition.defined_type? [condition] diff --git a/lib/rubocop/cop/style/if_unless_modifier_of_if_unless.rb b/lib/rubocop/cop/style/if_unless_modifier_of_if_unless.rb index 2c97198163b7..ad274bdb2edc 100644 --- a/lib/rubocop/cop/style/if_unless_modifier_of_if_unless.rb +++ b/lib/rubocop/cop/style/if_unless_modifier_of_if_unless.rb @@ -28,19 +28,16 @@ class IfUnlessModifierOfIfUnless < Base MSG = 'Avoid modifier `%s` after another conditional.' + # rubocop:disable Metrics/AbcSize def on_if(node) return unless node.modifier_form? && node.body.if_type? add_offense(node.loc.keyword, message: format(MSG, keyword: node.keyword)) do |corrector| - keyword = node.if? ? 'if' : 'unless' - - corrector.replace(node, <<~RUBY.chop) - #{keyword} #{node.condition.source} - #{node.if_branch.source} - end - RUBY + corrector.wrap(node.if_branch, "#{node.keyword} #{node.condition.source}\n", "\nend") + corrector.remove(node.if_branch.source_range.end.join(node.condition.source_range.end)) end end + # rubocop:enable Metrics/AbcSize end end end diff --git a/lib/rubocop/cop/style/multiline_if_modifier.rb b/lib/rubocop/cop/style/multiline_if_modifier.rb index 1a2e4cb54784..77fc1baaa978 100644 --- a/lib/rubocop/cop/style/multiline_if_modifier.rb +++ b/lib/rubocop/cop/style/multiline_if_modifier.rb @@ -23,11 +23,13 @@ class MultilineIfModifier < Base 'clause in a multiline statement.' def on_if(node) + return if part_of_ignored_node?(node) return unless node.modifier_form? && node.body.multiline? add_offense(node, message: format(MSG, keyword: node.keyword)) do |corrector| corrector.replace(node, to_normal_if(node)) end + ignore_node(node) end private diff --git a/lib/rubocop/cop/style/percent_q_literals.rb b/lib/rubocop/cop/style/percent_q_literals.rb index 3cbe5fd9d013..f3052dc4acdb 100644 --- a/lib/rubocop/cop/style/percent_q_literals.rb +++ b/lib/rubocop/cop/style/percent_q_literals.rb @@ -45,7 +45,7 @@ def on_percent_literal(node) # Report offense only if changing case doesn't change semantics, # i.e., if the string would become dynamic or has special characters. ast = parse(corrected(node.source)).ast - return if node.children != ast.children + return if node.children != ast&.children add_offense(node.loc.begin) do |corrector| corrector.replace(node, corrected(node.source)) diff --git a/lib/rubocop/cop/style/regexp_literal.rb b/lib/rubocop/cop/style/regexp_literal.rb index cb8139d9d6ff..d9890c4f8010 100644 --- a/lib/rubocop/cop/style/regexp_literal.rb +++ b/lib/rubocop/cop/style/regexp_literal.rb @@ -155,7 +155,7 @@ def slash_literal?(node) end def preferred_delimiters - config.for_cop('Style/PercentLiteralDelimiters') ['PreferredDelimiters']['%r'].chars + config.for_cop('Style/PercentLiteralDelimiters')['PreferredDelimiters']['%r'].chars end def allowed_omit_parentheses_with_percent_r_literal?(node) diff --git a/lib/rubocop/cop/style/sole_nested_conditional.rb b/lib/rubocop/cop/style/sole_nested_conditional.rb index 6b698c433fcb..a5a20738fde4 100644 --- a/lib/rubocop/cop/style/sole_nested_conditional.rb +++ b/lib/rubocop/cop/style/sole_nested_conditional.rb @@ -185,8 +185,10 @@ def parenthesize_method?(node) end def add_parentheses?(node) - node.assignment? || (node.operator_keyword? && !node.and_type?) || - (node.call_type? && node.arguments.any? && !node.parenthesized?) + return true if node.assignment? || (node.operator_keyword? && !node.and_type?) + return false unless node.call_type? + + (node.arguments.any? && !node.parenthesized?) || node.prefix_not? end def parenthesized_method_arguments(node) diff --git a/lib/rubocop/cop/style/string_concatenation.rb b/lib/rubocop/cop/style/string_concatenation.rb index 1e742472f154..918a28810f90 100644 --- a/lib/rubocop/cop/style/string_concatenation.rb +++ b/lib/rubocop/cop/style/string_concatenation.rb @@ -51,7 +51,6 @@ module Style # Pathname.new('/') + 'test' # class StringConcatenation < Base - include RangeHelp extend AutoCorrector MSG = 'Prefer string interpolation to string concatenation.' @@ -147,7 +146,7 @@ def replacement(parts) when :str adjust_str(part) when :dstr - part.children.all?(&:str_type?) ? adjust_str(part) : contents_range(part).source + part.children.all?(&:str_type?) ? adjust_str(part) : part.value else "\#{#{part.source}}" end diff --git a/lib/rubocop/cop/style/struct_inheritance.rb b/lib/rubocop/cop/style/struct_inheritance.rb index cc06abad1f3e..073c09d64cf5 100644 --- a/lib/rubocop/cop/style/struct_inheritance.rb +++ b/lib/rubocop/cop/style/struct_inheritance.rb @@ -3,7 +3,8 @@ module RuboCop module Cop module Style - # Checks for inheritance from Struct.new. + # Checks for inheritance from `Struct.new`. Inheriting from `Struct.new` + # adds a superfluous level in inheritance tree. # # @safety # Autocorrection is unsafe because it will change the inheritance @@ -17,12 +18,18 @@ module Style # end # end # + # Person.ancestors + # # => [Person, #, Struct, (...)] + # # # good # Person = Struct.new(:first_name, :last_name) do # def age # 42 # end # end + # + # Person.ancestors + # # => [Person, Struct, (...)] class StructInheritance < Base include RangeHelp extend AutoCorrector diff --git a/lib/rubocop/cop/variable_force/assignment.rb b/lib/rubocop/cop/variable_force/assignment.rb index 85667a527a0f..6795bc24d5d9 100644 --- a/lib/rubocop/cop/variable_force/assignment.rb +++ b/lib/rubocop/cop/variable_force/assignment.rb @@ -110,8 +110,13 @@ def operator_assignment_node end def multiple_assignment_node - return nil unless node.parent&.mlhs_type? - return nil unless (grandparent_node = node.parent&.parent) + return nil unless (candidate_mlhs_node = node.parent) + + # In `(foo, bar), *baz`, the splat node must be traversed as well. + candidate_mlhs_node = candidate_mlhs_node.parent if candidate_mlhs_node.splat_type? + + return nil unless candidate_mlhs_node.mlhs_type? + return nil unless (grandparent_node = node.parent.parent) if (node = find_multiple_assignment_node(grandparent_node)) return node end @@ -139,7 +144,6 @@ def for_assignment_node def find_multiple_assignment_node(grandparent_node) return unless grandparent_node.type == MULTIPLE_LEFT_HAND_SIDE_TYPE - return if grandparent_node.children.any?(&:splat_type?) parent = grandparent_node.parent return parent if parent.type == MULTIPLE_ASSIGNMENT_TYPE diff --git a/lib/rubocop/formatter/disabled_config_formatter.rb b/lib/rubocop/formatter/disabled_config_formatter.rb index 7f0668bafc7a..6b74eb45138e 100644 --- a/lib/rubocop/formatter/disabled_config_formatter.rb +++ b/lib/rubocop/formatter/disabled_config_formatter.rb @@ -178,6 +178,7 @@ def output_cop_param_comments(output_buffer, params, default_cfg) next unless value.is_a?(Array) next if value.empty? + value.map! { |v| v.nil? ? '~' : v } # Change nil back to ~ as in the YAML file. output_buffer.puts "# #{param}: #{value.uniq.join(', ')}" end end diff --git a/lib/rubocop/formatter/html_formatter.rb b/lib/rubocop/formatter/html_formatter.rb index bd62db6e8b2f..fa1eab1d0202 100644 --- a/lib/rubocop/formatter/html_formatter.rb +++ b/lib/rubocop/formatter/html_formatter.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'cgi' +require 'cgi/escape' require 'erb' module RuboCop diff --git a/lib/rubocop/version.rb b/lib/rubocop/version.rb index d033eee1f667..c158538d6c1e 100644 --- a/lib/rubocop/version.rb +++ b/lib/rubocop/version.rb @@ -3,7 +3,7 @@ module RuboCop # This module holds the RuboCop version information. module Version - STRING = '1.75.5' + STRING = '1.75.7' MSG = '%s (using %s, ' \ 'rubocop-ast %s, ' \ diff --git a/relnotes/v1.75.6.md b/relnotes/v1.75.6.md new file mode 100644 index 000000000000..514a912ab98a --- /dev/null +++ b/relnotes/v1.75.6.md @@ -0,0 +1,21 @@ +### Bug fixes + +* [#14176](https://github.com/rubocop/rubocop/pull/14176): Fix an error for `Style/MultilineIfModifier` when using nested modifier. ([@koic][]) +* [#14077](https://github.com/rubocop/rubocop/issues/14077): Change `nil` representation in todo file comments. ([@jonas054][]) +* [#14164](https://github.com/rubocop/rubocop/pull/14164): Fix an error for `Lint/UselessAssignment` when variables are assigned using unary operator in chained assignment and remain unreferenced. ([@koic][]) +* [#14173](https://github.com/rubocop/rubocop/pull/14173): Fix an error for `Style/StringConcatenation` when using implicit concatenation with string interpolation. ([@koic][]) +* [#14177](https://github.com/rubocop/rubocop/issues/14177): Fix false positives for `Style/SoleNestedConditional` when using nested `if` and `not` in condition. ([@koic][]) +* [#14152](https://github.com/rubocop/rubocop/pull/14152): Fix `Layout/SpaceInsideArrayLiteralBrackets` cop error on array pattern without brackets. ([@viralpraxis][]) +* [#14153](https://github.com/rubocop/rubocop/pull/14153): Fix `Style/PercentQLiterals` cop error on Unicode escape sequence. ([@viralpraxis][]) + +### Changes + +* [#14082](https://github.com/rubocop/rubocop/issues/14082): Mark `Style/ComparableBetween` as unsafe. ([@earlopain][]) +* [#14181](https://github.com/rubocop/rubocop/issues/14181): Make `Lint/DuplicateMethods` aware of Active Support's `delegate` method. ([@lovro-bikic][]) +* [#14156](https://github.com/rubocop/rubocop/issues/14156): Make `Style/IfUnlessModifier` allow endless method definition in the `if` body. ([@koic][]) + +[@koic]: https://github.com/koic +[@jonas054]: https://github.com/jonas054 +[@viralpraxis]: https://github.com/viralpraxis +[@earlopain]: https://github.com/earlopain +[@lovro-bikic]: https://github.com/lovro-bikic diff --git a/relnotes/v1.75.7.md b/relnotes/v1.75.7.md new file mode 100644 index 000000000000..20377c50308e --- /dev/null +++ b/relnotes/v1.75.7.md @@ -0,0 +1,19 @@ +### Bug fixes + +* [#14185](https://github.com/rubocop/rubocop/pull/14185): Fix an error for `Style/IfUnlessModifierOfIfUnless` when using nested modifier. ([@koic][]) +* [#14192](https://github.com/rubocop/rubocop/pull/14192): Fix negatives for `Layout/SpaceBeforeBrackets` when using space between method argument parentheses and left bracket. ([@koic][]) +* [#14189](https://github.com/rubocop/rubocop/issues/14189): Fix incorrect autocorrect for `Layout/SpaceBeforeBrackets` when using space between receiver and left brackets, and a space inside left bracket. ([@koic][]) +* [#14170](https://github.com/rubocop/rubocop/pull/14170): Fix `Style/AccessModifierDeclarations` cop error on semicolon after modifier. ([@viralpraxis][]) +* [#14195](https://github.com/rubocop/rubocop/pull/14195): Fix `Style/AccessModifierDeclarations` cop error on symbol modifier without surrounding scope. ([@viralpraxis][]) +* [#14172](https://github.com/rubocop/rubocop/pull/14172): Fix `Style/AccessModifierDeclarations` cop false positives when there are no method definitions and style is `inline`. ([@viralpraxis][]) +* [#14193](https://github.com/rubocop/rubocop/issues/14193): Fix `Lint/UselessAssignment` cop error when using nested assignment with splat. ([@earlopain][]) + +### Changes + +* [#14188](https://github.com/rubocop/rubocop/pull/14188): Enhance `Gemspec/DuplicatedAssignment` cop to detect duplicated indexed assignment. ([@viralpraxis][]) +* [#14183](https://github.com/rubocop/rubocop/pull/14183): Recognize `prefix` argument for `delegate` method in `Lint/DuplicateMethods`. ([@lovro-bikic][]) + +[@koic]: https://github.com/koic +[@viralpraxis]: https://github.com/viralpraxis +[@earlopain]: https://github.com/earlopain +[@lovro-bikic]: https://github.com/lovro-bikic diff --git a/spec/rubocop/cli/auto_gen_config_spec.rb b/spec/rubocop/cli/auto_gen_config_spec.rb index 8b828e751b3f..b44fd0a9aed7 100644 --- a/spec/rubocop/cli/auto_gen_config_spec.rb +++ b/spec/rubocop/cli/auto_gen_config_spec.rb @@ -300,6 +300,17 @@ def f '#' * 125, 'y ', 'puts x']) + create_file('example2.rb', <<~RUBY) + # frozen_string_literal: true + + module M1::M2 + class C # :nodoc: + def m + puts '!' + end + end + end + RUBY create_file('.rubocop_todo.yml', <<~YAML) Layout/LineLength: Enabled: false @@ -326,6 +337,17 @@ def f " - 'example1.rb'", '', '# Offense count: 1', + '# This cop supports unsafe autocorrection (--autocorrect-all).', + '# Configuration parameters: EnforcedStyle, EnforcedStyleForClasses, ' \ + 'EnforcedStyleForModules.', + '# SupportedStyles: nested, compact', + '# SupportedStylesForClasses: ~, nested, compact', + '# SupportedStylesForModules: ~, nested, compact', + 'Style/ClassAndModuleChildren:', + ' Exclude:', + " - 'example2.rb'", + '', + '# Offense count: 1', '# This cop supports safe autocorrection (--autocorrect).', '# Configuration parameters: AllowHeredoc, ' \ 'AllowURI, URISchemes, IgnoreCopDirectives, ' \ diff --git a/spec/rubocop/cop/gemspec/duplicated_assignment_spec.rb b/spec/rubocop/cop/gemspec/duplicated_assignment_spec.rb index c9d84eb9a1b1..6bd15e384c69 100644 --- a/spec/rubocop/cop/gemspec/duplicated_assignment_spec.rb +++ b/spec/rubocop/cop/gemspec/duplicated_assignment_spec.rb @@ -43,6 +43,19 @@ RUBY end + it 'registers an offense when using `metadata#[]=` with same key twice' do + expect_offense(<<~RUBY) + ::Gem::Specification.new do |spec| + spec.metadata['key'] = 1 + spec.metadata[:key] = 2 + spec.metadata['key'] = 2 + ^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['key']=` method calls already given on line 2 of the gemspec. + spec.metadata['key'] = 3 + ^^^^^^^^^^^^^^^^^^^^^^^^ `metadata['key']=` method calls already given on line 2 of the gemspec. + end + RUBY + end + it 'does not register an offense when using `<<` twice' do expect_no_offenses(<<~RUBY) Gem::Specification.new do |spec| @@ -70,4 +83,60 @@ end RUBY end + + it 'does not register an offense when using `#[]=` with different keys' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata[:foo] = 1 + spec.metadata[:bar] = 2 + end + RUBY + end + + it 'does not register an offense when using `#[]=` with same keys and different receivers' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.misc[:foo] = 1 + spec.metadata[:foo] = 2 + end + RUBY + end + + it 'does not register an offense when using both `metadata#[]=` and `metadata=`' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata = { foo: 1 } + spec.metadata[:foo] = 1 + end + RUBY + end + + it 'does not register an offense when using `metadata#[]=` with same key twice which are not literals' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata[foo()] = 1 + spec.metadata[foo()] = 2 + end + RUBY + end + + context 'with non-standard `[]=` method arity' do + it 'does not register an offense with single argument' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata.[]=(1) + spec.metadata.[]=(2) + end + RUBY + end + + it 'does not register an offense with more than two arguments' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.metadata[1, 2] = 3 + spec.metadata[1, 2] = 3 + end + RUBY + end + end end diff --git a/spec/rubocop/cop/layout/space_before_brackets_spec.rb b/spec/rubocop/cop/layout/space_before_brackets_spec.rb index 1432b1e38652..d7bcd9f018af 100644 --- a/spec/rubocop/cop/layout/space_before_brackets_spec.rb +++ b/spec/rubocop/cop/layout/space_before_brackets_spec.rb @@ -48,6 +48,17 @@ RUBY end + it 'registers an offense and corrects when using space between method argument parentheses and left bracket' do + expect_offense(<<~RUBY) + collection.call(arg) [index_or_key] + ^ Remove the space before the opening brackets. + RUBY + + expect_correction(<<~RUBY) + collection.call(arg)[index_or_key] + RUBY + end + it 'does not register an offense when using space between method call and left brackets' do expect_no_offenses(<<~RUBY) do_something [item_of_array_literal] @@ -128,6 +139,17 @@ RUBY end + it 'registers an offense and corrects when using space between receiver and left brackets, and a space inside left bracket' do + expect_offense(<<~RUBY) + @correction [ index_or_key] = :value + ^ Remove the space before the opening brackets. + RUBY + + expect_correction(<<~RUBY) + @correction[ index_or_key] = :value + RUBY + end + it 'does not register an offense when not using space between receiver and left brackets' do expect_no_offenses(<<~RUBY) @correction[index_or_key] = :value diff --git a/spec/rubocop/cop/layout/space_inside_array_literal_brackets_spec.rb b/spec/rubocop/cop/layout/space_inside_array_literal_brackets_spec.rb index 4fa3d614af1f..a454731a8b40 100644 --- a/spec/rubocop/cop/layout/space_inside_array_literal_brackets_spec.rb +++ b/spec/rubocop/cop/layout/space_inside_array_literal_brackets_spec.rb @@ -514,10 +514,21 @@ def Vector.[](*array) end end + shared_examples 'array pattern without brackets' do + it 'does not register an offense' do + expect_no_offenses(<<~RUBY) + case ary + in a, b, c, d + end + RUBY + end + end + context 'when EnforcedStyle is space' do let(:cop_config) { { 'EnforcedStyle' => 'space' } } it_behaves_like 'space inside arrays' + it_behaves_like 'array pattern without brackets' it 'does not register offense for valid 2-dimensional array' do expect_no_offenses(<<~RUBY) @@ -596,6 +607,7 @@ def Vector.[](*array) let(:cop_config) { { 'EnforcedStyle' => 'compact' } } it_behaves_like 'space inside arrays' + it_behaves_like 'array pattern without brackets' it 'does not register offense for valid 2-dimensional array' do expect_no_offenses(<<~RUBY) diff --git a/spec/rubocop/cop/lint/deprecated_class_methods_spec.rb b/spec/rubocop/cop/lint/deprecated_class_methods_spec.rb index 5d0d766c79d5..1ff2cfa9eef8 100644 --- a/spec/rubocop/cop/lint/deprecated_class_methods_spec.rb +++ b/spec/rubocop/cop/lint/deprecated_class_methods_spec.rb @@ -230,11 +230,11 @@ end end - context 'prefer `Addrinfo#getaddrinfo` over `Socket.gethostbyname`' do + context 'prefer `Addrinfo.getaddrinfo` over `Socket.gethostbyname`' do it 'registers an offense for Socket.gethostbyname' do expect_offense(<<~RUBY) Socket.gethostbyname("hal") - ^^^^^^^^^^^^^^^^^^^^ `Socket.gethostbyname` is deprecated in favor of `Addrinfo#getaddrinfo`. + ^^^^^^^^^^^^^^^^^^^^ `Socket.gethostbyname` is deprecated in favor of `Addrinfo.getaddrinfo`. RUBY expect_no_corrections @@ -243,7 +243,7 @@ it 'registers an offense for ::Socket.gethostbyname' do expect_offense(<<~RUBY) ::Socket.gethostbyname("hal") - ^^^^^^^^^^^^^^^^^^^^^^ `::Socket.gethostbyname` is deprecated in favor of `Addrinfo#getaddrinfo`. + ^^^^^^^^^^^^^^^^^^^^^^ `::Socket.gethostbyname` is deprecated in favor of `Addrinfo.getaddrinfo`. RUBY expect_no_corrections diff --git a/spec/rubocop/cop/lint/duplicate_methods_spec.rb b/spec/rubocop/cop/lint/duplicate_methods_spec.rb index 27d4b81c3563..6b6f5e745c6f 100644 --- a/spec/rubocop/cop/lint/duplicate_methods_spec.rb +++ b/spec/rubocop/cop/lint/duplicate_methods_spec.rb @@ -479,6 +479,185 @@ def some_method end RUBY end + + context 'when `AllCops/ActiveSupportExtensionsEnabled: true`' do + let(:config) do + RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => true }) + end + + it "registers an offense for duplicate delegate with symbol method in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :some_method, to: :foo + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense for duplicate delegate with string method in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate 'some_method', to: :foo + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense for duplicate delegate with string `to` argument in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :some_method, to: 'foo' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense for duplicate delegate with symbol prefix in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :method, to: :foo, prefix: :some + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense for duplicate delegate with string prefix in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :method, to: :foo, prefix: 'some' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense for duplicate delegate with prefix true in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :method, to: :some, prefix: true + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "registers an offense with multiple delegates in #{type}" do + expect_offense(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :other_method, :some_method, to: :foo + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both example.rb:2 and example.rb:5. + end + RUBY + end + + it "does not register an offense for non-duplicate delegate with prefix false in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :method, prefix: false, to: :some + end + RUBY + end + + it "does not register an offense for non-duplicate delegate in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :other_method, to: :foo + end + RUBY + end + + it "does not register an offense for duplicate delegate inside a condition in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + + if cond + delegate :some_method, to: :foo + end + end + RUBY + end + + it "does not register an offense for duplicate delegate with splat keyword arguments in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + + delegate :some_method, **options + end + RUBY + end + + it "does not register an offense for duplicate delegate without keyword arguments in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + + delegate :some_method + end + RUBY + end + + it 'does not register an offense for duplicate delegate without `to` argument' do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + + delegate :some_method, prefix: true + end + RUBY + end + end + + context 'when `AllCops/ActiveSupportExtensionsEnabled: false`' do + let(:config) do + RuboCop::Config.new('AllCops' => { 'ActiveSupportExtensionsEnabled' => false }) + end + + it "does not register an offense for duplicate delegate in #{type}" do + expect_no_offenses(<<~RUBY, 'example.rb') + #{opening_line} + def some_method + implement 1 + end + delegate :some_method, to: :foo + end + RUBY + end + end end include_examples('in scope', 'class', 'class A') diff --git a/spec/rubocop/cop/lint/useless_assignment_spec.rb b/spec/rubocop/cop/lint/useless_assignment_spec.rb index cdaea9aa3dd6..a412a9558250 100644 --- a/spec/rubocop/cop/lint/useless_assignment_spec.rb +++ b/spec/rubocop/cop/lint/useless_assignment_spec.rb @@ -1359,6 +1359,23 @@ def some_method end end + context 'when variables are assigned using unary operator in chained assignment and remain unreferenced' do + it 'registers an offense' do + expect_offense(<<~RUBY) + def some_method + foo = -bar = do_something + ^^^ Useless assignment to variable - `foo`. + end + RUBY + + expect_correction(<<~RUBY) + def some_method + -bar = do_something + end + RUBY + end + end + context 'when variables are assigned with sequential assignment using the comma operator and unreferenced' do it 'registers an offense' do expect_offense(<<~RUBY) @@ -1423,12 +1440,48 @@ def some_method end end + context 'when part of a multiple assignment is enclosed in parentheses with splat' do + it 'registers an offense when the variable is not used' do + expect_offense(<<~RUBY) + def some_method + (foo, bar), *baz = do_something + ^^^ Useless assignment to variable - `baz`. Use `_` or `_baz` as a variable name to indicate that it won't be used. + puts foo, bar + end + RUBY + + expect_correction(<<~RUBY) + def some_method + (foo, bar), *_ = do_something + puts foo, bar + end + RUBY + end + + it 'registers an offense when the variable is not used in nested assignment' do + expect_offense(<<~RUBY) + def some_method + foo, (*bar, baz) = do_something + ^^^ Useless assignment to variable - `bar`. Use `_` or `_bar` as a variable name to indicate that it won't be used. + puts foo, baz + end + RUBY + + expect_correction(<<~RUBY) + def some_method + foo, (*_, baz) = do_something + puts foo, baz + end + RUBY + end + end + context 'when a variable is assigned with rest assignment and unreferenced' do it 'registers an offense' do expect_offense(<<~RUBY) def some_method foo, *bar = do_something - ^^^ Useless assignment to variable - `bar`. + ^^^ Useless assignment to variable - `bar`. Use `_` or `_bar` as a variable name to indicate that it won't be used. puts foo end RUBY diff --git a/spec/rubocop/cop/metrics/abc_size_spec.rb b/spec/rubocop/cop/metrics/abc_size_spec.rb index ee6fbb4556c3..7799f343be51 100644 --- a/spec/rubocop/cop/metrics/abc_size_spec.rb +++ b/spec/rubocop/cop/metrics/abc_size_spec.rb @@ -21,7 +21,7 @@ def method_name it 'registers an offense for an if modifier' do expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<0, 2, 1> 2.24/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<0, 2, 1> 2.24/0] call_foo if some_condition # 0 + 2*2 + 1*1 end RUBY @@ -30,7 +30,7 @@ def method_name it 'registers an offense for an assignment of a local variable' do offenses = expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 0, 0> 1/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 0, 0> 1/0] x = 1 end RUBY @@ -41,7 +41,7 @@ def method_name it 'registers an offense for an assignment of an element' do expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 2, 0> 2.24/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 2, 0> 2.24/0] x[0] = 1 end RUBY @@ -50,7 +50,7 @@ def method_name it 'registers an offense for complex content including A, B, and C scores' do expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<3, 4, 5> 7.07/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<3, 4, 5> 7.07/0] my_options = Hash.new if 1 == 1 || 2 == 2 # 1, 1, 4 my_options.each do |key, value| # 2, 1, 1 p key # 0, 1, 0 @@ -63,7 +63,7 @@ def method_name it 'registers an offense for a `define_method`' do expect_offense(<<~RUBY) define_method :method_name do - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 0, 0> 1/0] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 0, 0> 1/0] x = 1 end RUBY @@ -73,7 +73,7 @@ def method_name it 'registers an offense for a `define_method` with numblock' do expect_offense(<<~RUBY) define_method :method_name do - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 0, 0> 1/0] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 0, 0> 1/0] x = _1 end RUBY @@ -84,7 +84,7 @@ def method_name it 'registers an offense for a `define_method` with itblock' do expect_offense(<<~RUBY) define_method :method_name do - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 0, 0> 1/0] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 0, 0> 1/0] x = it end RUBY @@ -94,7 +94,7 @@ def method_name it 'treats safe navigation method calls like regular method calls + a condition' do expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<0, 2, 1> 2.24/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<0, 2, 1> 2.24/0] object&.do_something end RUBY @@ -104,7 +104,7 @@ def method_name it 'registers an offense for an assignment of a local variable' do offenses = expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [<1, 0, 0> 1/0] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [<1, 0, 0> 1/0] x = 1 end RUBY @@ -177,7 +177,7 @@ def self.foo it 'does not count repeated attributes' do expect_offense(<<~RUBY) def foo - ^^^^^^^ Assignment Branch Condition size for foo is too high. [<0, 1, 0> 1/0] + ^^^^^^^ Assignment Branch Condition size for `foo` is too high. [<0, 1, 0> 1/0] bar self.bar bar @@ -192,7 +192,7 @@ def foo it 'counts repeated attributes' do expect_offense(<<~RUBY) def foo - ^^^^^^^ Assignment Branch Condition size for foo is too high. [<0, 3, 0> 3/0] + ^^^^^^^ Assignment Branch Condition size for `foo` is too high. [<0, 3, 0> 3/0] bar self.bar bar @@ -243,7 +243,7 @@ def method_name expect_offense(<<~RUBY) def method_name - ^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [#{presentation}] + ^^^^^^^^^^^^^^^ Assignment Branch Condition size for `method_name` is too high. [#{presentation}] #{code.join("\n ")} end RUBY diff --git a/spec/rubocop/cop/style/access_modifier_declarations_spec.rb b/spec/rubocop/cop/style/access_modifier_declarations_spec.rb index 6340a4dc312e..82fbdc0077f8 100644 --- a/spec/rubocop/cop/style/access_modifier_declarations_spec.rb +++ b/spec/rubocop/cop/style/access_modifier_declarations_spec.rb @@ -63,6 +63,12 @@ class Foo context 'do not allow access modifiers on symbols' do let(:cop_config) { { 'AllowModifiersOnSymbols' => false } } + it "does not register an offense when argument to #{access_modifier} is a symbol and there is no surrounding scope" do + expect_no_offenses(<<~RUBY) + #{access_modifier} :foo + RUBY + end + it "registers an offense when argument to #{access_modifier} is a symbol" do expect_offense(<<~RUBY, access_modifier: access_modifier) class Foo @@ -633,11 +639,13 @@ def bar; end class Test %{access_modifier} ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + def foo; end end RUBY expect_correction(<<~RUBY) class Test + #{access_modifier} def foo; end end RUBY end @@ -647,11 +655,13 @@ class Test class Test %{access_modifier} # hey ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + def foo; end end RUBY expect_correction(<<~RUBY) class Test + #{access_modifier} def foo; end end RUBY end @@ -683,11 +693,7 @@ class TestOne class TestTwo #{access_modifier} ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. - end - - class TestThree - #{access_modifier} - ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + def foo; end end RUBY @@ -697,10 +703,14 @@ class TestOne end class TestTwo + #{access_modifier} def foo; end end + RUBY + end - class TestThree - end + it "does not register an offense for #{access_modifier} without method definitions" do + expect_no_offenses(<<~RUBY) + #{access_modifier} RUBY end @@ -719,7 +729,6 @@ def bar; end expect_correction(<<~RUBY) class Test - #{access_modifier} def foo; end #{access_modifier} def bar; end @@ -728,6 +737,61 @@ class Test end end + context 'with `begin` parent node' do + it 'registers an offense when modifier and method definition are on different lines' do + expect_offense(<<~RUBY, access_modifier: access_modifier) + %{access_modifier}; + ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + def foo + end + + def bar + end + RUBY + + expect_correction(<<~RUBY) + #{access_modifier} def foo + end + + #{access_modifier} def bar + end + RUBY + end + + it 'registers an offense when modifier and method definition are on the same line' do + expect_offense(<<~RUBY, access_modifier: access_modifier) + %{access_modifier}; def foo; end + ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + RUBY + + expect_correction(<<~RUBY) + #{access_modifier} def foo; end + RUBY + end + + it 'registers an offense when modifier and method definitions are on the same line' do + expect_offense(<<~RUBY, access_modifier: access_modifier) + %{access_modifier}; def foo; end; def bar; end + ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + RUBY + + expect_correction(<<~RUBY) + #{access_modifier} def foo; end; #{access_modifier} def bar; end + RUBY + end + + it 'registers an offense when modifier and method definitions and some other node are on the same line' do + expect_offense(<<~RUBY, access_modifier: access_modifier) + %{access_modifier}; def foo; end; some_method; def bar; end + ^{access_modifier} `#{access_modifier}` should be inlined in method definitions. + RUBY + + expect_correction(<<~RUBY) + #{access_modifier} def foo; end; some_method; #{access_modifier} def bar; end + RUBY + end + end + include_examples 'always accepted', access_modifier end end diff --git a/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb index 752dab469b8b..c91973b8ac2c 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb @@ -29,6 +29,24 @@ end end + context 'using nested mofifier' do + it 'registers an offense and corrects' do + expect_offense(<<~RUBY) + condition ? then_part : else_part if inner_condition if outer_condition + ^^ Avoid modifier `if` after another conditional. + ^^ Avoid modifier `if` after another conditional. + RUBY + + expect_correction(<<~RUBY) + if outer_condition + if inner_condition + condition ? then_part : else_part + end + end + RUBY + end + end + context 'conditional with modifier' do it 'registers an offense and corrects' do expect_offense(<<~RUBY) diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index e6819094c678..cebf2465324a 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -426,6 +426,24 @@ def f include_examples 'one-line pattern matching' end + context 'when using endless method definition', :ruby30 do + it 'does not register an offense when using method definition in the branch' do + expect_no_offenses(<<~RUBY) + if condition + def method_name = body + end + RUBY + end + + it 'does not register an offense when using singleton method definition in the branch' do + expect_no_offenses(<<~RUBY) + if condition + def self.method_name = body + end + RUBY + end + end + context 'when using omitted hash values in an assignment', :ruby31 do it 'registers an offense' do expect_offense(<<~RUBY) diff --git a/spec/rubocop/cop/style/map_into_array_spec.rb b/spec/rubocop/cop/style/map_into_array_spec.rb index 5404cc9f3b60..844d3678c930 100644 --- a/spec/rubocop/cop/style/map_into_array_spec.rb +++ b/spec/rubocop/cop/style/map_into_array_spec.rb @@ -221,7 +221,7 @@ RUBY end - it 'registers an offense and corrects when using a itblock', :ruby34parser do + it 'registers an offense and corrects when using a itblock', :ruby34 do expect_offense(<<~RUBY) dest = [] src.each { dest << it * 2 } diff --git a/spec/rubocop/cop/style/multiline_if_modifier_spec.rb b/spec/rubocop/cop/style/multiline_if_modifier_spec.rb index 120776fa2e27..a14b86e4f849 100644 --- a/spec/rubocop/cop/style/multiline_if_modifier_spec.rb +++ b/spec/rubocop/cop/style/multiline_if_modifier_spec.rb @@ -48,6 +48,23 @@ | end RUBY end + + it 'registers an offense when nested modifier' do + expect_offense(<<~RUBY) + [ + ^ Favor a normal if-statement over a modifier clause in a multiline statement. + ] if inner if outer + RUBY + + expect_correction(<<~RUBY) + if outer + if inner + [ + ] + end + end + RUBY + end end context 'unless guard clause' do @@ -97,5 +114,22 @@ | end RUBY end + + it 'registers an offense when nested modifier' do + expect_offense(<<~RUBY) + [ + ^ Favor a normal unless-statement over a modifier clause in a multiline statement. + ] unless inner unless outer + RUBY + + expect_correction(<<~RUBY) + unless outer + unless inner + [ + ] + end + end + RUBY + end end end diff --git a/spec/rubocop/cop/style/percent_q_literals_spec.rb b/spec/rubocop/cop/style/percent_q_literals_spec.rb index 99e0f7f5ba3d..eb09cc62fe54 100644 --- a/spec/rubocop/cop/style/percent_q_literals_spec.rb +++ b/spec/rubocop/cop/style/percent_q_literals_spec.rb @@ -79,6 +79,12 @@ expect_no_offenses('%Q(hi)') end + it 'does not register an offense when correcting leads to a parsing error' do + expect_no_offenses(<<~'RUBY') + %q(\u) + RUBY + end + include_examples 'accepts quote characters' include_examples 'accepts any q string with backslash t' end diff --git a/spec/rubocop/cop/style/sole_nested_conditional_spec.rb b/spec/rubocop/cop/style/sole_nested_conditional_spec.rb index 18cbb6abd889..610051e48f2a 100644 --- a/spec/rubocop/cop/style/sole_nested_conditional_spec.rb +++ b/spec/rubocop/cop/style/sole_nested_conditional_spec.rb @@ -473,6 +473,57 @@ def foo RUBY end + it 'registers an offense and corrects when using nested `if` and `not` in the inner condition' do + expect_offense(<<~RUBY) + if foo + if not bar + ^^ Consider merging nested conditions into outer `if` conditions. + do_something + end + end + RUBY + + expect_correction(<<~RUBY) + if foo && (not bar) + do_something + end + RUBY + end + + it 'registers an offense and corrects when using nested `if` and `not` in the outer condition' do + expect_offense(<<~RUBY) + if not foo + if bar + ^^ Consider merging nested conditions into outer `if` conditions. + do_something + end + end + RUBY + + expect_correction(<<~RUBY) + if (not foo) && bar + do_something + end + RUBY + end + + it 'registers an offense and corrects when using nested `if` and `!` in the inner condition' do + expect_offense(<<~RUBY) + if foo + if !bar + ^^ Consider merging nested conditions into outer `if` conditions. + do_something + end + end + RUBY + + expect_correction(<<~RUBY) + if foo && !bar + do_something + end + RUBY + end + context 'when disabling `Style/IfUnlessModifier`' do let(:config) { RuboCop::Config.new('Style/IfUnlessModifier' => { 'Enabled' => false }) } diff --git a/spec/rubocop/cop/style/string_concatenation_spec.rb b/spec/rubocop/cop/style/string_concatenation_spec.rb index 91edb4e87e49..0b1cd6ece1ba 100644 --- a/spec/rubocop/cop/style/string_concatenation_spec.rb +++ b/spec/rubocop/cop/style/string_concatenation_spec.rb @@ -85,6 +85,17 @@ "abcd" RUBY end + + it 'registers an offense and corrects with string interpolation' do + expect_offense(<<~'RUBY') + "string #{interpolation}" 'foo' + 'bar' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer string interpolation to string concatenation. + RUBY + + expect_correction(<<~'RUBY') + "string #{interpolation}foobar" + RUBY + end end context 'multiline' do diff --git a/spec/rubocop/cop/variable_force/assignment_spec.rb b/spec/rubocop/cop/variable_force/assignment_spec.rb index ac0a0df4a733..275b977e82dd 100644 --- a/spec/rubocop/cop/variable_force/assignment_spec.rb +++ b/spec/rubocop/cop/variable_force/assignment_spec.rb @@ -125,8 +125,8 @@ def some_method RUBY end - it 'returns splat node' do - expect(assignment.meta_assignment_node.type).to eq(:splat) + it 'returns masgn node' do + expect(assignment.meta_assignment_node.type).to eq(:masgn) end end