iv-org-mirror/shard-ameba
1
0
Fork 0
mirror of https://gitea.invidious.io/iv-org/shard-ameba.git synced 2024-08-15 00:53:29 +00:00
Code Issues Projects Releases Packages Wiki Activity

Autocorrect Stye/RedundantNext and Style/RedundantReturn

Browse source
This commit is contained in:
fn ⌃ ⌥ 2021-11-06 11:42:29 -07:00
parent 7b437fbd2f
commit e7cfe387d6
5 changed files with 297 additions and 161 deletions
Download patch file Download diff file Expand all files Collapse all files

180
spec/ameba/rule/style/redundant_next_spec.cr
View file

@ -5,149 +5,183 @@ module Ameba::Rule::Style
describe RedundantNext do describe RedundantNext do
it "does not report if there is no redundant next" do it "does not report if there is no redundant next" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
array.map { |x| x + 1 } array.map { |x| x + 1 }
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next with argument in the block" do it "reports if there is redundant next with argument in the block" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |v| block do |v|
next v + 1 next v + 1
# ^^^^^^^^^^ error: Redundant `next` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
expect_correction source, <<-CRYSTAL
block do |v|
v + 1
end
CRYSTAL
end end
context "if" do context "if" do
it "doesn't report if there is not redundant next in if branch" do it "doesn't report if there is not redundant next in if branch" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |v| block do |v|
next if v > 10 next if v > 10
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next in if/else branch" do it "reports if there is redundant next in if/else branch" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |a| block do |a|
if a > 0 if a > 0
next a + 1 next a + 1
# ^^^^^^^^^^ error: Redundant `next` detected
else else
next a + 2 next a + 2
# ^^^^^^^^^^ error: Redundant `next` detected
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":3:5" block do |a|
s.issues.last.location.to_s.should eq ":5:5" if a > 0
a + 1
else
a + 2
end
end
CRYSTAL
end end
end end
context "unless" do context "unless" do
it "doesn't report if there is no redundant next in unless branch" do it "doesn't report if there is no redundant next in unless branch" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |v| block do |v|
next unless v > 10 next unless v > 10
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next in unless/else branch" do it "reports if there is redundant next in unless/else branch" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |a| block do |a|
unless a > 0 unless a > 0
next a + 1 next a + 1
# ^^^^^^^^^^ error: Redundant `next` detected
else else
next a + 2 next a + 2
# ^^^^^^^^^^ error: Redundant `next` detected
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":3:5" block do |a|
s.issues.last.location.to_s.should eq ":5:5" unless a > 0
a + 1
else
a + 2
end
end
CRYSTAL
end end
end end
context "expressions" do context "expressions" do
it "doesn't report if there is no redundant next in expressions" do it "doesn't report if there is no redundant next in expressions" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |v| block do |v|
a = 1 a = 1
a + v a + v
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next in expressions" do it "reports if there is redundant next in expressions" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |a| block do |a|
a = 1 a = 1
next a next a
# ^^^^^^ error: Redundant `next` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":3:3" block do |a|
a = 1
a
end
CRYSTAL
end end
end end
context "binary-op" do context "binary-op" do
it "doesn't report if there is no redundant next in binary op" do it "doesn't report if there is no redundant next in binary op" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |v| block do |v|
a && v a && v
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next in binary op" do it "reports if there is redundant next in binary op" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |a| block do |a|
a && next a a && next a
# ^^^^^^ error: Redundant `next` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":2:8" block do |a|
a && a
end
CRYSTAL
end end
end end
context "expception handler" do context "expception handler" do
it "doesn't report if there is no redundant next in exception handler" do it "doesn't report if there is no redundant next in exception handler" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |v| block do |v|
v + 1 v + 1
rescue e rescue e
next v if v > 0 next v if v > 0
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant next in exception handler" do it "reports if there is redundant next in exception handler" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
block do |a| block do |a|
next a + 1 next a + 1
# ^^^^^^^^^^ error: Redundant `next` detected
rescue ArgumentError rescue ArgumentError
next a + 2 next a + 2
# ^^^^^^^^^^ error: Redundant `next` detected
rescue Exception rescue Exception
a + 2 a + 2
next a next a
# ^^^^^^ error: Redundant `next` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 3 expect_correction source, <<-CRYSTAL
s.issues[0].location.to_s.should eq ":2:3" block do |a|
s.issues[1].location.to_s.should eq ":4:3" a + 1
s.issues[2].location.to_s.should eq ":7:3" rescue ArgumentError
a + 2
rescue Exception
a + 2
a
end
CRYSTAL
end end
end end
@ -169,49 +203,51 @@ module Ameba::Rule::Style
context "properties" do context "properties" do
context "#allow_multi_next=" do context "#allow_multi_next=" do
it "allows multi next statements by default" do it "allows multi next statements by default" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do |a, b| block do |a, b|
next a, b next a, b
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "allows to configure multi next statements" do it "allows to configure multi next statements" do
s = Source.new %(
block do |a, b|
next a, b
end
)
rule = Rule::Style::RedundantNext.new rule = Rule::Style::RedundantNext.new
rule.allow_multi_next = false rule.allow_multi_next = false
rule.catch(s).should_not be_valid source = expect_issue rule, <<-CRYSTAL
s.issues.size.should eq 1 block do |a, b|
s.issues.first.location.to_s.should eq ":2:3" next a, b
# ^^^^^^^^^ error: Redundant `next` detected
end
CRYSTAL
expect_correction source, <<-CRYSTAL
block do |a, b|
{a, b}
end
CRYSTAL
end end
end end
context "#allow_empty_next" do context "#allow_empty_next" do
it "allows empty next statements by default" do it "allows empty next statements by default" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
block do block do
next next
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "allows to configure empty next statements" do it "allows to configure empty next statements" do
s = Source.new %(
block do
next
end
)
rule = Rule::Style::RedundantNext.new rule = Rule::Style::RedundantNext.new
rule.allow_empty_next = false rule.allow_empty_next = false
rule.catch(s).should_not be_valid source = expect_issue rule, <<-CRYSTAL
s.issues.size.should eq 1 block do
s.issues.first.location.to_s.should eq ":2:3" next
# ^^^^ error: Redundant `next` detected
end
CRYSTAL
expect_no_corrections source
end end
end end
end end

246
spec/ameba/rule/style/redundant_return_spec.cr
View file

@ -5,36 +5,38 @@ module Ameba::Rule::Style
describe RedundantReturn do describe RedundantReturn do
it "does not report if there is no return" do it "does not report if there is no return" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def inc(a) def inc(a)
a + 1 a + 1
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is redundant return in method body" do it "reports if there is redundant return in method body" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def inc(a) def inc(a)
return a + 1 return a + 1
# ^^^^^^^^^^^^ error: Redundant `return` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":2:3" def inc(a)
a + 1
end
CRYSTAL
end end
it "doesn't report if it returns tuple literal" do it "doesn't report if it returns tuple literal" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def foo(a) def foo(a)
return a, a + 2 return a, a + 2
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "doesn't report if there are other expressions after control flow" do it "doesn't report if there are other expressions after control flow" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def method(a) def method(a)
case a case a
when true then return true when true then return true
@ -44,109 +46,141 @@ module Ameba::Rule::Style
rescue rescue
nil nil
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
context "if" do context "if" do
it "doesn't report if there is return in if branch" do it "doesn't report if there is return in if branch" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def inc(a) def inc(a)
return a + 1 if a > 0 return a + 1 if a > 0
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there are returns in if/else branch" do it "reports if there are returns in if/else branch" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def inc(a) def inc(a)
do_something(a) do_something(a)
if a > 0 if a > 0
return :positive return :positive
# ^^^^^^^^^^^^^^^^ error: Redundant `return` detected
else else
return :negative return :negative
# ^^^^^^^^^^^^^^^^ error: Redundant `return` detected
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":4:5" def inc(a)
s.issues.last.location.to_s.should eq ":6:5" do_something(a)
if a > 0
:positive
else
:negative
end
end
CRYSTAL
end end
end end
context "unless" do context "unless" do
it "doesn't report if there is return in unless branch" do it "doesn't report if there is return in unless branch" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def inc(a) def inc(a)
return a + 1 unless a > 0 return a + 1 unless a > 0
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there are returns in unless/else branch" do it "reports if there are returns in unless/else branch" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def inc(a) def inc(a)
do_something(a) do_something(a)
unless a < 0 unless a < 0
return :positive return :positive
# ^^^^^^^^^^^^^^^^ error: Redundant `return` detected
else else
return :negative return :negative
# ^^^^^^^^^^^^^^^^ error: Redundant `return` detected
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":4:5" def inc(a)
s.issues.last.location.to_s.should eq ":6:5" do_something(a)
unless a < 0
:positive
else
:negative
end
end
CRYSTAL
end end
end end
context "binary op" do context "binary op" do
it "doesn't report if there is no return in the right binary op node" do it "doesn't report if there is no return in the right binary op node" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def can_create?(a) def can_create?(a)
valid? && a > 0 valid? && a > 0
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is return in the right binary op node" do it "reports if there is return in the right binary op node" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def can_create?(a) def can_create?(a)
valid? && return a > 0 valid? && return a > 0
# ^^^^^^^^^^^^ error: Redundant `return` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
expect_correction source, <<-CRYSTAL
def can_create?(a)
valid? && a > 0
end
CRYSTAL
end end
end end
context "case" do context "case" do
it "reports if there are returns in whens" do it "reports if there are returns in whens" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def foo(a) def foo(a)
case a case a
when .nil? when .nil?
puts "blah" puts "blah"
return nil return nil
# ^^^^^^^^^^ error: Redundant `return` detected
when .blank? when .blank?
return "" return ""
# ^^^^^^^^^ error: Redundant `return` detected
when true when true
true true
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":5:5" def foo(a)
s.issues.last.location.to_s.should eq ":7:5" case a
when .nil?
puts "blah"
nil
when .blank?
""
when true
true
end
end
CRYSTAL
end end
it "reports if there is return in else" do it "reports if there is return in else" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def foo(a) def foo(a)
do_something_with(a) do_something_with(a)
@ -155,49 +189,76 @@ module Ameba::Rule::Style
true true
else else
return false return false
# ^^^^^^^^^^^^ error: Redundant `return` detected
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":8:5" def foo(a)
do_something_with(a)
case a
when true
true
else
false
end
end
CRYSTAL
end end
end end
context "exception handler" do context "exception handler" do
it "reports if there are returns in body" do it "reports if there are returns in body" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def foo(a) def foo(a)
return true return true
# ^^^^^^^^^^^ error: Redundant `return` detected
rescue rescue
false false
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":2:3" def foo(a)
true
rescue
false
end
CRYSTAL
end end
it "reports if there are returns in rescues" do it "reports if there are returns in rescues" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def foo(a) def foo(a)
true true
rescue ArgumentError rescue ArgumentError
return false return false
# ^^^^^^^^^^^^ error: Redundant `return` detected
rescue RuntimeError rescue RuntimeError
"" ""
rescue Exception rescue Exception
return nil return nil
# ^^^^^^^^^^ error: Redundant `return` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 2 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":4:3" def foo(a)
s.issues.last.location.to_s.should eq ":8:3" true
rescue ArgumentError
false
rescue RuntimeError
""
rescue Exception
nil
end
CRYSTAL
end end
it "reports if there are returns in else" do it "reports if there are returns in else" do
s = Source.new %( source = expect_issue subject, <<-CRYSTAL
def foo(a) def foo(a)
true true
rescue Exception rescue Exception
@ -205,60 +266,71 @@ module Ameba::Rule::Style
else else
puts "else branch" puts "else branch"
return :bar return :bar
# ^^^^^^^^^^^ error: Redundant `return` detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.size.should eq 1 expect_correction source, <<-CRYSTAL
s.issues.first.location.to_s.should eq ":7:3" def foo(a)
true
rescue Exception
nil
else
puts "else branch"
:bar
end
CRYSTAL
end end
end end
context "properties" do context "properties" do
context "#allow_multi_return=" do context "#allow_multi_return=" do
it "allows multi returns by default" do it "allows multi returns by default" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def method(a, b) def method(a, b)
return a, b return a, b
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "allows to configure multi returns" do it "allows to configure multi returns" do
s = Source.new %(
def method(a, b)
return a, b
end
)
rule = Rule::Style::RedundantReturn.new rule = Rule::Style::RedundantReturn.new
rule.allow_multi_return = false rule.allow_multi_return = false
rule.catch(s).should_not be_valid source = expect_issue rule, <<-CRYSTAL
s.issues.size.should eq 1 def method(a, b)
s.issues.first.location.to_s.should eq ":2:3" return a, b
# ^^^^^^^^^^^ error: Redundant `return` detected
end
CRYSTAL
expect_correction source, <<-CRYSTAL
def method(a, b)
{a, b}
end
CRYSTAL
end end
end end
context "#allow_empty_return" do context "#allow_empty_return" do
it "allows empty returns by default" do it "allows empty returns by default" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def method def method
return return
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "allows to configure empty returns" do it "allows to configure empty returns" do
s = Source.new %(
def method
return
end
)
rule = Rule::Style::RedundantReturn.new rule = Rule::Style::RedundantReturn.new
rule.allow_empty_return = false rule.allow_empty_return = false
rule.catch(s).should_not be_valid source = expect_issue rule, <<-CRYSTAL
s.issues.size.should eq 1 def method
s.issues.first.location.to_s.should eq ":2:3" return
# ^^^^^^ error: Redundant `return` detected
end
CRYSTAL
expect_no_corrections source
end end
end end
end end

12
src/ameba/ast/util.cr
View file

@ -158,4 +158,16 @@ module Ameba::AST::Util
false false
end end
end end
# Returns the exp code of a control expression.
# Wraps implicit tuple literal with curly brackets (e.g. multi-return).
def control_exp_code(source, node : Crystal::ControlExpression)
return unless (exp = node.exp)
return unless (exp_code = node_source(exp, source.lines))
return exp_code unless exp.is_a?(Crystal::TupleLiteral) && exp_code[0] != '{'
return unless (exp_start = exp.elements.first.location)
return unless (exp_end = exp.end_location)
"{#{source_between(exp_start, exp_end, source.lines)}}"
end
end end

10
src/ameba/rule/style/redundant_next.cr
View file

@ -97,6 +97,8 @@ module Ameba::Rule::Style
# AllowEmptyNext: true # AllowEmptyNext: true
# ``` # ```
class RedundantNext < Base class RedundantNext < Base
include AST::Util
properties do properties do
description "Reports redundant next expressions" description "Reports redundant next expressions"
@ -114,7 +116,13 @@ module Ameba::Rule::Style
return if allow_multi_next && node.exp.is_a?(Crystal::TupleLiteral) return if allow_multi_next && node.exp.is_a?(Crystal::TupleLiteral)
return if allow_empty_next && (node.exp.nil? || node.exp.not_nil!.nop?) return if allow_empty_next && (node.exp.nil? || node.exp.not_nil!.nop?)
source.try &.add_issue self, node, MSG if (exp_code = control_exp_code(source, node))
issue_for node, MSG do |corrector|
corrector.replace(node, exp_code)
end
else
issue_for node, MSG
end
end end
end end
end end

10
src/ameba/rule/style/redundant_return.cr
View file

@ -94,6 +94,8 @@ module Ameba::Rule::Style
# AllowEmptyReturn: true # AllowEmptyReturn: true
# ``` # ```
class RedundantReturn < Base class RedundantReturn < Base
include AST::Util
properties do properties do
description "Reports redundant return expressions" description "Reports redundant return expressions"
@ -111,7 +113,13 @@ module Ameba::Rule::Style
return if allow_multi_return && node.exp.is_a?(Crystal::TupleLiteral) return if allow_multi_return && node.exp.is_a?(Crystal::TupleLiteral)
return if allow_empty_return && (node.exp.nil? || node.exp.not_nil!.nop?) return if allow_empty_return && (node.exp.nil? || node.exp.not_nil!.nop?)
source.try &.add_issue self, node, MSG if (exp_code = control_exp_code(source, node))
issue_for node, MSG do |corrector|
corrector.replace(node, exp_code)
end
else
issue_for node, MSG
end
end end
end end
end end