diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 5f169b781fcf..943366c35c91 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.6 (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..a7fd027c13a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,24 @@ ## master (unreleased) +## 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..bb20c1db644e 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.6 (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_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..289e7c56138d 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,39 @@ 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 +---- + [#lintduplicateregexpcharacterclasselement] == Lint/DuplicateRegexpCharacterClassElement @@ -7285,7 +7318,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/integration_with_other_tools.adoc b/docs/modules/ROOT/pages/integration_with_other_tools.adoc index edfe0e074f16..b01cad4d8522 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.6 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.6 hooks: - id: rubocop additional_dependencies: 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..9b7c08effbbd 100644 --- a/lib/rubocop/cop/lint/duplicate_methods.rb +++ b/lib/rubocop/cop/lint/duplicate_methods.rb @@ -39,9 +39,35 @@ 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 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 +111,25 @@ 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 $_)+ (hash _)) + 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 +154,12 @@ def message_for_dup(node, method_name, key) current: source_location(node)) end + def on_delegate(node, method_names) + method_names.each do |name| + found_instance_method(node, name) + end + 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/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/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/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/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..f676f42b8329 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.6' 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/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/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..e27c4140d0e8 100644 --- a/spec/rubocop/cop/lint/duplicate_methods_spec.rb +++ b/spec/rubocop/cop/lint/duplicate_methods_spec.rb @@ -479,6 +479,78 @@ 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 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 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 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 + 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..d91d27f6993c 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) 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