Skip to content

Commit

Permalink
[Fix #12988] Add new Style/AmbiguousEndlessMethodDefinition cop.
Browse files Browse the repository at this point in the history
  • Loading branch information
dvandersluis authored and bbatsov committed Oct 17, 2024
1 parent bfc974d commit ce37499
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12988](https://github.com/rubocop/rubocop/issues/12988): Add new `Style/AmbiguousEndlessMethodDefinition` cop. ([@dvandersluis][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3143,6 +3143,11 @@ Style/Alias:
- prefer_alias
- prefer_alias_method

Style/AmbiguousEndlessMethodDefinition:
Description: 'Checks for endless methods inside operators of lower precedence.'
Enabled: pending
VersionAdded: '<<next>>'

Style/AndOr:
Description: 'Use &&/|| instead of and/or.'
StyleGuide: '#no-and-or-or'
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
require_relative 'rubocop/cop/mixin/empty_lines_around_body' # relies on range
require_relative 'rubocop/cop/mixin/empty_parameter'
require_relative 'rubocop/cop/mixin/end_keyword_alignment'
require_relative 'rubocop/cop/mixin/endless_method_rewriter'
require_relative 'rubocop/cop/mixin/enforce_superclass'
require_relative 'rubocop/cop/mixin/first_element_line_break'
require_relative 'rubocop/cop/mixin/frozen_string_literal'
Expand Down Expand Up @@ -460,6 +461,7 @@
require_relative 'rubocop/cop/style/access_modifier_declarations'
require_relative 'rubocop/cop/style/accessor_grouping'
require_relative 'rubocop/cop/style/alias'
require_relative 'rubocop/cop/style/ambiguous_endless_method_definition'
require_relative 'rubocop/cop/style/and_or'
require_relative 'rubocop/cop/style/arguments_forwarding'
require_relative 'rubocop/cop/style/array_coercion'
Expand Down
24 changes: 24 additions & 0 deletions lib/rubocop/cop/mixin/endless_method_rewriter.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Common functionality for rewriting endless methods to normal method definitions
module EndlessMethodRewriter
def correct_to_multiline(corrector, node)
replacement = <<~RUBY.strip
def #{node.method_name}#{arguments(node)}
#{node.body.source}
end
RUBY

corrector.replace(node, replacement)
end

private

def arguments(node, missing = '')
node.arguments.any? ? node.arguments.source : missing
end
end
end
end
79 changes: 79 additions & 0 deletions lib/rubocop/cop/style/ambiguous_endless_method_definition.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# Looks for endless methods inside operations of lower precedence (`and`, `or`, and
# modifier forms of `if`, `unless`, `while`, `until`) that are ambiguous due to
# lack of parentheses. This may lead to unexpected behavior as the code may appear
# to use these keywords as part of the method but in fact they modify
# the method definition itself.
#
# In these cases, using a normal method definition is more clear.
#
# @example
#
# # bad
# def foo = true if bar
#
# # good - using a non-endless method is more explicit
# def foo
# true
# end if bar
#
# # ok - method body is explicit
# def foo = (true if bar)
#
# # ok - method definition is explicit
# (def foo = true) if bar
class AmbiguousEndlessMethodDefinition < Base
extend TargetRubyVersion
extend AutoCorrector
include EndlessMethodRewriter
include RangeHelp

minimum_target_ruby_version 3.0

MSG = 'Avoid using `%<keyword>s` statements with endless methods.'

# @!method ambiguous_endless_method_body(node)
def_node_matcher :ambiguous_endless_method_body, <<~PATTERN
^${
(if _ <def _>)
({and or} def _)
({while until} _ def)
}
PATTERN

def on_def(node)
return unless node.endless?

operation = ambiguous_endless_method_body(node)
return unless operation

return unless modifier_form?(operation)

add_offense(operation, message: format(MSG, keyword: keyword(operation))) do |corrector|
correct_to_multiline(corrector, node)
end
end

private

def modifier_form?(operation)
return true if operation.and_type? || operation.or_type?

operation.modifier_form?
end

def keyword(operation)
if operation.respond_to?(:keyword)
operation.keyword
else
operation.operator
end
end
end
end
end
end
15 changes: 1 addition & 14 deletions lib/rubocop/cop/style/endless_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module Style
#
class EndlessMethod < Base
include ConfigurableEnforcedStyle
include EndlessMethodRewriter
extend TargetRubyVersion
extend AutoCorrector

Expand Down Expand Up @@ -81,20 +82,6 @@ def handle_disallow_style(node)

add_offense(node) { |corrector| correct_to_multiline(corrector, node) }
end

def correct_to_multiline(corrector, node)
replacement = <<~RUBY.strip
def #{node.method_name}#{arguments(node)}
#{node.body.source}
end
RUBY

corrector.replace(node, replacement)
end

def arguments(node, missing = '')
node.arguments.any? ? node.arguments.source : missing
end
end
end
end
Expand Down
70 changes: 70 additions & 0 deletions spec/rubocop/cop/style/ambiguous_endless_method_definition_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Style::AmbiguousEndlessMethodDefinition, :config do
context 'Ruby >= 3.0', :ruby30 do
it 'does not register an offense for a non endless method' do
expect_no_offenses(<<~RUBY)
def foo
end
RUBY
end

%i[and or if unless while until].each do |operator|
context "with #{operator}" do
it "does not register an offense for a non endless method followed by `#{operator}`" do
expect_no_offenses(<<~RUBY)
def foo
end #{operator} bar
RUBY
end

it 'does not register an offense when the operator is already wrapped in parens' do
expect_no_offenses(<<~RUBY)
def foo = (true #{operator} bar)
RUBY
end

it 'does not register an offense when the method definition is already wrapped in parens' do
expect_no_offenses(<<~RUBY)
(def foo = true) #{operator} bar
RUBY
end

unless %i[and or].include?(operator)
it "does not register an offense for non-modifier `#{operator}`" do
expect_no_offenses(<<~RUBY)
#{operator} bar
def foo = true
end
RUBY
end
end

it "registers and offense and corrects an endless method followed by `#{operator}`" do
expect_offense(<<~RUBY, operator: operator)
def foo = true #{operator} bar
^^^^^^^^^^^^^^^^{operator}^^^^ Avoid using `#{operator}` statements with endless methods.
RUBY

expect_correction(<<~RUBY)
def foo
true
end #{operator} bar
RUBY
end
end
end

it 'does not register an offense for `&&`' do
expect_no_offenses(<<~RUBY)
def foo = true && bar
RUBY
end

it 'does not register an offense for `||`' do
expect_no_offenses(<<~RUBY)
def foo = true || bar
RUBY
end
end
end

0 comments on commit ce37499

Please sign in to comment.