Skip to content

Commit

Permalink
Make Style/GuardClause a bit more lenient when the replacement woul…
Browse files Browse the repository at this point in the history
…d make the code more verbose.
  • Loading branch information
dvandersluis authored and bbatsov committed Jun 24, 2022
1 parent 92385a5 commit 1c25af3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 15 deletions.
@@ -0,0 +1 @@
* [#10740](https://github.com/rubocop/rubocop/pull/10740): Make `Style/GuardClause` a bit more lenient when the replacement would make the code more verbose. ([@dvandersluis][])
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -3703,7 +3703,7 @@ Style/GuardClause:
StyleGuide: '#no-nested-conditionals'
Enabled: true
VersionAdded: '0.20'
VersionChanged: '1.28'
VersionChanged: '<<next>>'
# `MinBodyLength` defines the number of lines of the a body of an `if` or `unless`
# needs to have to trigger this cop
MinBodyLength: 1
Expand Down
14 changes: 8 additions & 6 deletions lib/rubocop/cop/style/guard_clause.rb
Expand Up @@ -110,7 +110,7 @@ def on_if(node)
kw = if guard_clause_in_if
node.loc.keyword.source
else
opposite_keyword(node)
node.inverse_keyword
end

register_offense(node, guard_clause_source(guard_clause), kw)
Expand All @@ -123,7 +123,7 @@ def check_ending_if(node)
return if allowed_consecutive_conditionals? &&
consecutive_conditionals?(node.parent, node)

register_offense(node, 'return', opposite_keyword(node))
register_offense(node, 'return', node.inverse_keyword)
end

def consecutive_conditionals?(parent, node)
Expand All @@ -134,14 +134,12 @@ def consecutive_conditionals?(parent, node)
end
end

def opposite_keyword(node)
node.if? ? 'unless' : 'if'
end

def register_offense(node, scope_exiting_keyword, conditional_keyword)
condition, = node.node_parts
example = [scope_exiting_keyword, conditional_keyword, condition.source].join(' ')
if too_long_for_single_line?(node, example)
return if trivial?(node)

example = "#{conditional_keyword} #{condition.source}; #{scope_exiting_keyword}; end"
end

Expand All @@ -167,6 +165,10 @@ def accepted_form?(node, ending: false)
accepted_if?(node, ending) || node.condition.multiline? || node.parent&.assignment?
end

def trivial?(node)
node.branches.one? && !node.if_branch.if_type? && !node.if_branch.begin_type?
end

def accepted_if?(node, ending)
return true if node.modifier_form? || node.ternary?

Expand Down
48 changes: 40 additions & 8 deletions spec/rubocop/cop/style/guard_clause_spec.rb
Expand Up @@ -407,15 +407,47 @@ def func
end

context 'with Metrics/MaxLineLength enabled' do
it 'registers an offense with non-modifier example code if too long for single line' do
expect_offense(<<~RUBY)
def test
if something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line
^^ Use a guard clause (`unless something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line; return; end`) instead of wrapping the code inside a conditional expression.
work
end
context 'when the correction is too long for a single line' do
context 'with a trivial body' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
def test
if something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line
work
end
end
RUBY
end
RUBY
end

context 'with a nested `if` node' do
it 'does registers an offense' do
expect_offense(<<~RUBY)
def test
if something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line
^^ Use a guard clause (`unless something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line; return; end`) instead of wrapping the code inside a conditional expression.
if something_else
work
end
end
end
RUBY
end
end

context 'with a nested `begin` node' do
it 'does registers an offense' do
expect_offense(<<~RUBY)
def test
if something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line
^^ Use a guard clause (`unless something && something_that_makes_the_guard_clause_too_long_to_fit_on_one_line; return; end`) instead of wrapping the code inside a conditional expression.
work
more_work
end
end
RUBY
end
end
end
end

Expand Down

0 comments on commit 1c25af3

Please sign in to comment.