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

Merge pull request #250 from FnControlOption/spec/style

Browse source 
Use `expect_issue` in `Style` specs
This commit is contained in:
Sijawusz Pur Rahnama 2021-11-09 22:35:38 +01:00 committed by GitHub
commit d843afd962
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 663 additions and 474 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
spec/ameba/ast/util_spec.cr
View file

@ -39,20 +39,20 @@ module Ameba::AST
describe "#node_source" do describe "#node_source" do
it "returns original source of the node" do it "returns original source of the node" do
s = %( s = <<-CRYSTAL
a = 1 a = 1
) CRYSTAL
node = Crystal::Parser.new(s).parse node = Crystal::Parser.new(s).parse
source = subject.node_source node, s.split("\n") source = subject.node_source node, s.split("\n")
source.should eq "a = 1" source.should eq "a = 1"
end end
it "returns original source of multiline node" do it "returns original source of multiline node" do
s = %( s = <<-CRYSTAL
if () if ()
:ok :ok
end end
) CRYSTAL
node = Crystal::Parser.new(s).parse node = Crystal::Parser.new(s).parse
source = subject.node_source node, s.split("\n") source = subject.node_source node, s.split("\n")
source.should eq <<-CRYSTAL source.should eq <<-CRYSTAL
@ -63,7 +63,7 @@ module Ameba::AST
end end
it "does not report source of node which has incorrect location" do it "does not report source of node which has incorrect location" do
s = %q( s = <<-'CRYSTAL'
module MyModule module MyModule
macro conditional_error_for_inline_callbacks macro conditional_error_for_inline_callbacks
\{% \{%
@ -74,7 +74,7 @@ module Ameba::AST
macro before_save(x = nil) macro before_save(x = nil)
end end
end end
) CRYSTAL
node = as_nodes(s).nil_literal_nodes.first node = as_nodes(s).nil_literal_nodes.first
source = subject.node_source node, s.split("\n") source = subject.node_source node, s.split("\n")
@ -140,29 +140,29 @@ module Ameba::AST
end end
it "returns true if this is if-else consumed by flow expressions" do it "returns true if this is if-else consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
if foo if foo
return :foo return :foo
else else
return :bar return :bar
end end
) CRYSTAL
subject.flow_expression?(node, false).should eq true subject.flow_expression?(node, false).should eq true
end end
it "returns true if this is unless-else consumed by flow expressions" do it "returns true if this is unless-else consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
unless foo unless foo
return :foo return :foo
else else
return :bar return :bar
end end
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns true if this is case consumed by flow expressions" do it "returns true if this is case consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
case case
when 1 when 1
return 1 return 1
@ -171,71 +171,71 @@ module Ameba::AST
else else
return 3 return 3
end end
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns true if this is exception handler consumed by flow expressions" do it "returns true if this is exception handler consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
begin begin
raise "exp" raise "exp"
rescue e rescue e
return e return e
end end
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns true if this while consumed by flow expressions" do it "returns true if this while consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
while true while true
return return
end end
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns false if this while with break" do it "returns false if this while with break" do
node = as_node %( node = as_node <<-CRYSTAL
while true while true
break break
end end
) CRYSTAL
subject.flow_expression?(node).should eq false subject.flow_expression?(node).should eq false
end end
it "returns true if this until consumed by flow expressions" do it "returns true if this until consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
until false until false
return return
end end
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns false if this until with break" do it "returns false if this until with break" do
node = as_node %( node = as_node <<-CRYSTAL
until false until false
break break
end end
) CRYSTAL
subject.flow_expression?(node).should eq false subject.flow_expression?(node).should eq false
end end
it "returns true if this expressions consumed by flow expressions" do it "returns true if this expressions consumed by flow expressions" do
node = as_node %( node = as_node <<-CRYSTAL
exp1 exp1
exp2 exp2
return return
) CRYSTAL
subject.flow_expression?(node).should eq true subject.flow_expression?(node).should eq true
end end
it "returns false otherwise" do it "returns false otherwise" do
node = as_node %( node = as_node <<-CRYSTAL
exp1 exp1
exp2 exp2
) CRYSTAL
subject.flow_expression?(node).should eq false subject.flow_expression?(node).should eq false
end end
end end
@ -322,5 +322,28 @@ module Ameba::AST
subject.loop?(node).should eq false subject.loop?(node).should eq false
end end
end end
describe "#control_exp_code" do
it "returns the exp code of a control expression" do
s = "return 1"
node = as_node(s).as Crystal::ControlExpression
exp_code = subject.control_exp_code node, [s]
exp_code.should eq "1"
end
it "wraps implicit tuple literal with curly brackets" do
s = "return 1, 2"
node = as_node(s).as Crystal::ControlExpression
exp_code = subject.control_exp_code node, [s]
exp_code.should eq "{1, 2}"
end
it "accepts explicit tuple literal" do
s = "return {1, 2}"
node = as_node(s).as Crystal::ControlExpression
exp_code = subject.control_exp_code node, [s]
exp_code.should eq "{1, 2}"
end
end
end end
end end

21
spec/ameba/rule/style/constant_names_spec.cr
View file

@ -3,17 +3,19 @@ require "../../../spec_helper"
module Ameba module Ameba
subject = Rule::Style::ConstantNames.new subject = Rule::Style::ConstantNames.new
private def it_reports_constant(code, expected) private def it_reports_constant(name, value, expected)
it "reports constant name #{expected}" do it "reports constant name #{expected}" do
s = Source.new code rule = Rule::Style::ConstantNames.new
Rule::Style::ConstantNames.new.catch(s).should_not be_valid expect_issue rule, <<-CRYSTAL, name: name
s.issues.first.message.should contain expected %{name} = #{value}
# ^{name} error: Constant name should be screaming-cased: #{expected}, not #{name}
CRYSTAL
end end
end end
describe Rule::Style::ConstantNames do describe Rule::Style::ConstantNames do
it "passes if type names are screaming-cased" do it "passes if type names are screaming-cased" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
LUCKY_NUMBERS = [3, 7, 11] LUCKY_NUMBERS = [3, 7, 11]
DOCUMENTATION_URL = "http://crystal-lang.org/docs" DOCUMENTATION_URL = "http://crystal-lang.org/docs"
@ -29,13 +31,12 @@ module Ameba
a = 1 a = 1
myVar = 2 myVar = 2
m_var = 3 m_var = 3
) CRYSTAL
subject.catch(s).should be_valid
end end
# it_reports_constant "MyBadConstant=1", "MYBADCONSTANT" # it_reports_constant "MyBadConstant", "1", "MYBADCONSTANT"
it_reports_constant "Wrong_NAME=2", "WRONG_NAME" it_reports_constant "Wrong_NAME", "2", "WRONG_NAME"
it_reports_constant "Wrong_Name=3", "WRONG_NAME" it_reports_constant "Wrong_Name", "3", "WRONG_NAME"
it "reports rule, pos and message" do it "reports rule, pos and message" do
s = Source.new %( s = Source.new %(

19
spec/ameba/rule/style/is_a_nil_spec.cr
View file

@ -5,27 +5,26 @@ module Ameba::Rule::Style
subject = IsANil.new subject = IsANil.new
it "doesn't report if there are no is_a?(Nil) calls" do it "doesn't report if there are no is_a?(Nil) calls" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = 1 a = 1
a.nil? a.nil?
a.is_a?(NilLiteral) a.is_a?(NilLiteral)
a.is_a?(Custom::Nil) a.is_a?(Custom::Nil)
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is a call to is_a?(Nil) without receiver" do it "reports if there is a call to is_a?(Nil) without receiver" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
is_a?(Nil) a = is_a?(Nil)
) # ^^^ error: Use `nil?` instead of `is_a?(Nil)`
subject.catch(s).should_not be_valid CRYSTAL
end end
it "reports if there is a call to is_a?(Nil) with receiver" do it "reports if there is a call to is_a?(Nil) with receiver" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
a.is_a?(Nil) a.is_a?(Nil)
) # ^^^ error: Use `nil?` instead of `is_a?(Nil)`
subject.catch(s).should_not be_valid CRYSTAL
end end
it "reports rule, location and message" do it "reports rule, location and message" do

8
spec/ameba/rule/style/large_numbers_spec.cr
View file

@ -20,7 +20,7 @@ module Ameba
describe Rule::Style::LargeNumbers do describe Rule::Style::LargeNumbers do
it "passes if large number does not require underscore" do it "passes if large number does not require underscore" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
16 17 18 19 20 30 40 50 60 70 80 90 16 17 18 19 20 30 40 50 60 70 80 90
100 100
@ -80,8 +80,7 @@ module Ameba
1200.0 1200.0
1200.01 1200.01
1200.012 1200.012
) CRYSTAL
subject.catch(s).should be_valid
end end
it_transforms "10000", "10_000" it_transforms "10000", "10_000"
@ -131,10 +130,9 @@ module Ameba
context "properties" do context "properties" do
it "allows to configure integer min digits" do it "allows to configure integer min digits" do
s = Source.new %q(1200000)
rule = Rule::Style::LargeNumbers.new rule = Rule::Style::LargeNumbers.new
rule.int_min_digits = 10 rule.int_min_digits = 10
rule.catch(s).should be_valid expect_no_issues rule, %q(1200000)
end end
end end
end end

21
spec/ameba/rule/style/method_names_spec.cr
View file

@ -3,17 +3,19 @@ require "../../../spec_helper"
module Ameba module Ameba
subject = Rule::Style::MethodNames.new subject = Rule::Style::MethodNames.new
private def it_reports_method_name(code, expected) private def it_reports_method_name(name, expected)
it "reports method name #{expected}" do it "reports method name #{expected}" do
s = Source.new code rule = Rule::Style::MethodNames.new
Rule::Style::MethodNames.new.catch(s).should_not be_valid expect_issue rule, <<-CRYSTAL, name: name
s.issues.first.message.should contain expected def %{name}; end
# ^{name} error: Method name should be underscore-cased: #{expected}, not %{name}
CRYSTAL
end end
end end
describe Rule::Style::MethodNames do describe Rule::Style::MethodNames do
it "passes if method names are underscore-cased" do it "passes if method names are underscore-cased" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
class Person class Person
def first_name def first_name
end end
@ -30,13 +32,12 @@ module Ameba
def name def name
end end
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it_reports_method_name %(def firstName; end), "first_name" it_reports_method_name "firstName", "first_name"
it_reports_method_name %(def date_of_Birth; end), "date_of_birth" it_reports_method_name "date_of_Birth", "date_of_birth"
it_reports_method_name %(def homepageURL; end), "homepage_url" it_reports_method_name "homepageURL", "homepage_url"
it "reports rule, pos and message" do it "reports rule, pos and message" do
s = Source.new %( s = Source.new %(

29
spec/ameba/rule/style/negated_conditions_in_unless_spec.cr
View file

@ -5,7 +5,7 @@ module Ameba::Rule::Style
describe NegatedConditionsInUnless do describe NegatedConditionsInUnless do
it "passes with a unless without negated condition" do it "passes with a unless without negated condition" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
unless a unless a
:ok :ok
end end
@ -15,44 +15,43 @@ module Ameba::Rule::Style
unless s.empty? unless s.empty?
:ok :ok
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is a negated condition in unless" do it "fails if there is a negated condition in unless" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
unless !a unless !a
# ^^^^^^^ error: Avoid negated conditions in unless blocks
:nok :nok
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if one of AND conditions is negated" do it "fails if one of AND conditions is negated" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
unless a && !b unless a && !b
# ^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks
:nok :nok
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if one of OR conditions is negated" do it "fails if one of OR conditions is negated" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
unless a || !b unless a || !b
# ^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks
:nok :nok
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if one of inner conditions is negated" do it "fails if one of inner conditions is negated" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
unless a && (b || !c) unless a && (b || !c)
# ^^^^^^^^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks
:nok :nok
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

16
spec/ameba/rule/style/predicate_name_spec.cr
View file

@ -5,7 +5,7 @@ module Ameba::Rule::Style
describe PredicateName do describe PredicateName do
it "passes if predicate name is correct" do it "passes if predicate name is correct" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
def valid?(x) def valid?(x)
end end
@ -16,16 +16,15 @@ module Ameba::Rule::Style
def allow_this_picture? def allow_this_picture?
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if predicate name is wrong" do it "fails if predicate name is wrong" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def is_valid?(x) def is_valid?(x)
# ^^^^^^^^^^^^^^ error: Favour method name 'valid?' over 'is_valid?'
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do
@ -47,14 +46,13 @@ module Ameba::Rule::Style
end end
it "ignores if alternative name isn't valid syntax" do it "ignores if alternative name isn't valid syntax" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
class Image class Image
def is_404?(x) def is_404?(x)
true true
end end
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
end end
end end

62
spec/ameba/rule/style/redundant_begin_spec.cr
View file

@ -5,7 +5,7 @@ module Ameba::Rule::Style
subject = RedundantBegin.new subject = RedundantBegin.new
it "passes if there is no redundant begin blocks" do it "passes if there is no redundant begin blocks" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def method def method
do_something do_something
rescue rescue
@ -27,12 +27,11 @@ module Ameba::Rule::Style
def method; end def method; end
def method; a = 1; rescue; end def method; a = 1; rescue; end
def method; begin; rescue; end; end def method; begin; rescue; end; end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "passes if there is a correct begin block in a handler" do it "passes if there is a correct begin block in a handler" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
def handler_and_expression def handler_and_expression
begin begin
open_file open_file
@ -80,13 +79,13 @@ module Ameba::Rule::Style
end end
expr expr
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is a redundant begin block" do it "fails if there is a redundant begin block" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method(a : String) : String def method(a : String) : String
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
begin begin
open_file open_file
do_some_stuff do_some_stuff
@ -94,39 +93,39 @@ module Ameba::Rule::Style
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if there is a redundant begin block in a method without args" do it "fails if there is a redundant begin block in a method without args" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
open_file open_file
ensure ensure
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if there is a redundant block in a method with return type" do it "fails if there is a redundant block in a method with return type" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method : String def method : String
# ^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
begin begin
open_file open_file
ensure ensure
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if there is a redundant block in a method with multiple args" do it "fails if there is a redundant block in a method with multiple args" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method(a : String, def method(a : String,
# ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
b : String) b : String)
begin begin
open_file open_file
@ -134,13 +133,13 @@ module Ameba::Rule::Style
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if there is a redundant block in a method with multiple args" do it "fails if there is a redundant block in a method with multiple args" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method(a : String, def method(a : String,
# ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected
b : String b : String
) )
begin begin
@ -149,12 +148,11 @@ module Ameba::Rule::Style
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "doesn't report if there is an inner redundant block" do it "doesn't report if there is an inner redundant block" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
def method def method
begin begin
open_file open_file
@ -163,37 +161,36 @@ module Ameba::Rule::Style
end end
rescue rescue
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is a redundant block with yield" do it "fails if there is a redundant block with yield" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
yield yield
ensure ensure
close_file close_file
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "fails if there is top level redundant block in a method" do it "fails if there is top level redundant block in a method" do
s = Source.new %q( expect_issue subject, <<-CRYSTAL
def method def method
# ^^^^^^^^ error: Redundant `begin` block detected
begin begin
a = 1 a = 1
b = 2 b = 2
end end
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "doesn't report if begin-end block in a proc literal" do it "doesn't report if begin-end block in a proc literal" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
foo = ->{ foo = ->{
begin begin
raise "Foo!" raise "Foo!"
@ -201,8 +198,7 @@ module Ameba::Rule::Style
pp ex pp ex
end end
} }
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

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

33
spec/ameba/rule/style/type_names_spec.cr
View file

@ -3,17 +3,19 @@ require "../../../spec_helper"
module Ameba module Ameba
subject = Rule::Style::TypeNames.new subject = Rule::Style::TypeNames.new
private def it_reports_name(code, expected) private def it_reports_name(type, name, expected)
it "reports type name #{expected}" do it "reports type name #{expected}" do
s = Source.new code rule = Rule::Style::TypeNames.new
Rule::Style::TypeNames.new.catch(s).should_not be_valid expect_issue rule, <<-CRYSTAL, type: type, name: name
s.issues.first.message.should contain expected %{type} %{name}; end
# ^{type}^{name}^^^^ error: Type name should be camelcased: #{expected}, but it was %{name}
CRYSTAL
end end
end end
describe Rule::Style::TypeNames do describe Rule::Style::TypeNames do
it "passes if type names are camelcased" do it "passes if type names are camelcased" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
class ParseError < Exception class ParseError < Exception
end end
@ -32,16 +34,21 @@ module Ameba
enum Time::DayOfWeek enum Time::DayOfWeek
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it_reports_name "class My_class; end", "MyClass" it_reports_name "class", "My_class", "MyClass"
it_reports_name "module HTT_p; end", "HTTP" it_reports_name "module", "HTT_p", "HTTP"
it_reports_name "alias Numeric_value = Int32", "NumericValue" it_reports_name "lib", "Lib_YAML", "LibYAML"
it_reports_name "lib Lib_YAML; end", "LibYAML" it_reports_name "struct", "Tag_directive", "TagDirective"
it_reports_name "struct Tag_directive; end", "TagDirective" it_reports_name "enum", "Time_enum::Day_of_week", "TimeEnum::DayOfWeek"
it_reports_name "enum Time_enum::Day_of_week; end", "TimeEnum::DayOfWeek"
it "reports alias name" do
expect_issue subject, <<-CRYSTAL
alias Numeric_value = Int32
# ^{} error: Type name should be camelcased: NumericValue, but it was Numeric_value
CRYSTAL
end
it "reports rule, pos and message" do it "reports rule, pos and message" do
s = Source.new %( s = Source.new %(

11
spec/ameba/rule/style/unless_else_spec.cr
View file

@ -5,23 +5,22 @@ module Ameba::Rule::Style
describe UnlessElse do describe UnlessElse do
it "passes if unless hasn't else" do it "passes if unless hasn't else" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
unless something unless something
:ok :ok
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if unless has else" do it "fails if unless has else" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
unless something unless something
# ^^^^^^^^^^^^^^ error: Favour if over unless with else
:one :one
else else
:two :two
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

46
spec/ameba/rule/style/variable_names_spec.cr
View file

@ -3,17 +3,19 @@ require "../../../spec_helper"
module Ameba module Ameba
subject = Rule::Style::VariableNames.new subject = Rule::Style::VariableNames.new
private def it_reports_var_name(code, expected) private def it_reports_var_name(name, value, expected)
it "reports method name #{expected}" do it "reports variable name #{expected}" do
s = Source.new code rule = Rule::Style::VariableNames.new
Rule::Style::VariableNames.new.catch(s).should_not be_valid expect_issue rule, <<-CRYSTAL, name: name
s.issues.first.message.should contain expected %{name} = #{value}
# ^{name} error: Var name should be underscore-cased: #{expected}, not %{name}
CRYSTAL
end end
end end
describe Rule::Style::VariableNames do describe Rule::Style::VariableNames do
it "passes if var names are underscore-cased" do it "passes if var names are underscore-cased" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
class Greeting class Greeting
@@default_greeting = "Hello world" @@default_greeting = "Hello world"
@ -25,25 +27,41 @@ module Ameba
puts greeting puts greeting
end end
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it_reports_var_name %(myBadNamedVar = 1), "my_bad_named_var" it_reports_var_name "myBadNamedVar", "1", "my_bad_named_var"
it_reports_var_name %(wrong_Name = 'y'), "wrong_name" it_reports_var_name "wrong_Name", "'y'", "wrong_name"
it_reports_var_name %( it "reports instance variable name" do
expect_issue subject, <<-CRYSTAL
class Greeting class Greeting
def initialize(@badNamed = nil) def initialize(@badNamed = nil)
# ^ error: Var name should be underscore-cased: @bad_named, not @badNamed
end end
end end
), "bad_named" CRYSTAL
end
it_reports_var_name %( it "reports method with multiple instance variables" do
expect_issue subject, <<-CRYSTAL
class Location
def at(@startLocation = nil, @endLocation = nil)
# ^ error: Var name should be underscore-cased: @start_location, not @startLocation
# ^ error: Var name should be underscore-cased: @end_location, not @endLocation
end
end
CRYSTAL
end
it "reports class variable name" do
expect_issue subject, <<-CRYSTAL
class Greeting class Greeting
@@defaultGreeting = "Hello world" @@defaultGreeting = "Hello world"
# ^^^^^^^^^^^^^^^^^ error: Var name should be underscore-cased: @@default_greeting, not @@defaultGreeting
end
CRYSTAL
end end
), "default_greeting"
it "reports rule, pos and message" do it "reports rule, pos and message" do
s = Source.new %( s = Source.new %(

236
spec/ameba/rule/style/verbose_block_spec.cr
View file

@ -5,7 +5,7 @@ module Ameba::Rule::Style
describe VerboseBlock do describe VerboseBlock do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).any?(&.odd?) (1..3).any?(&.odd?)
(1..3).join('.', &.to_s) (1..3).join('.', &.to_s)
(1..3).each_with_index { |i, idx| i * idx } (1..3).each_with_index { |i, idx| i * idx }
@ -14,192 +14,188 @@ module Ameba::Rule::Style
(1..3).map { |i| :foo } (1..3).map { |i| :foo }
(1..3).map { |i| :foo.to_s.split.join('.') } (1..3).map { |i| :foo.to_s.split.join('.') }
(1..3).map { :foo } (1..3).map { :foo }
) CRYSTAL
subject.catch(source).should be_valid
end end
it "passes if the block argument is used within the body" do it "passes if the block argument is used within the body" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).map { |i| i * i } (1..3).map { |i| i * i }
(1..3).map { |j| j * j.to_i64 } (1..3).map { |j| j * j.to_i64 }
(1..3).map { |k| k.to_i64 * k } (1..3).map { |k| k.to_i64 * k }
(1..3).map { |l| l.to_i64 * l.to_i64 } (1..3).map { |l| l.to_i64 * l.to_i64 }
(1..3).map { |m| m.to_s[start: m.to_i64, count: 3]? } (1..3).map { |m| m.to_s[start: m.to_i64, count: 3]? }
(1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join } (1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join }
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a call with a collapsible block" do it "reports if there is a call with a collapsible block" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).any? { |i| i.odd? } (1..3).any? { |i| i.odd? }
) # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is a call with an argument + collapsible block" do it "reports if there is a call with an argument + collapsible block" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).join('.') { |i| i.to_s } (1..3).join('.') { |i| i.to_s }
) # ^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `join('.', &.to_s)`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is a call with a collapsible block (with chained call)" do it "reports if there is a call with a collapsible block (with chained call)" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).map { |i| i.to_s.split.reverse.join.strip } (1..3).map { |i| i.to_s.split.reverse.join.strip }
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `map(&.to_s.split.reverse.join.strip)`
subject.catch(source).should_not be_valid CRYSTAL
end end
context "properties" do context "properties" do
it "#exclude_calls_with_block" do it "#exclude_calls_with_block" do
source = Source.new %(
(1..3).in_groups_of(1) { |i| i.map(&.to_s) }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_calls_with_block = true
.tap(&.exclude_calls_with_block = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).in_groups_of(1) { |i| i.map(&.to_s) }
rule CRYSTAL
.tap(&.exclude_calls_with_block = false) rule.exclude_calls_with_block = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).in_groups_of(1) { |i| i.map(&.to_s) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `in_groups_of(1, &.map {...})`
CRYSTAL
end end
it "#exclude_multiple_line_blocks" do it "#exclude_multiple_line_blocks" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_multiple_line_blocks = true
expect_no_issues rule, <<-CRYSTAL
(1..3).any? do |i| (1..3).any? do |i|
i.odd? i.odd?
end end
) CRYSTAL
rule = VerboseBlock.new rule.exclude_multiple_line_blocks = false
rule expect_issue rule, <<-CRYSTAL
.tap(&.exclude_multiple_line_blocks = true) (1..3).any? do |i|
.catch(source).should be_valid # ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
rule i.odd?
.tap(&.exclude_multiple_line_blocks = false) end
.catch(source).should_not be_valid CRYSTAL
end end
it "#exclude_prefix_operators" do it "#exclude_prefix_operators" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_prefix_operators = true
expect_no_issues rule, <<-CRYSTAL
(1..3).sum { |i| +i } (1..3).sum { |i| +i }
(1..3).sum { |i| -i } (1..3).sum { |i| -i }
(1..3).sum { |i| ~i } (1..3).sum { |i| ~i }
) CRYSTAL
rule = VerboseBlock.new rule.exclude_prefix_operators = false
rule rule.exclude_operators = false
.tap(&.exclude_prefix_operators = true) expect_issue rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).sum { |i| +i }
rule # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)`
.tap(&.exclude_prefix_operators = false) (1..3).sum { |i| -i }
.tap(&.exclude_operators = false) # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.-)`
.catch(source).should_not be_valid (1..3).sum { |i| ~i }
# ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)`
CRYSTAL
end end
it "#exclude_operators" do it "#exclude_operators" do
source = Source.new %(
(1..3).sum { |i| i * 2 }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_operators = true
.tap(&.exclude_operators = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).sum { |i| i * 2 }
rule CRYSTAL
.tap(&.exclude_operators = false) rule.exclude_operators = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).sum { |i| i * 2 }
# ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))`
CRYSTAL
end end
it "#exclude_setters" do it "#exclude_setters" do
source = Source.new %(
Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_setters = true
.tap(&.exclude_setters = true) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
rule CRYSTAL
.tap(&.exclude_setters = false) rule.exclude_setters = false
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
Char::Reader.new("abc").tap { |reader| reader.pos = 0 }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))`
CRYSTAL
end end
it "#max_line_length" do it "#max_line_length" do
source = Source.new %( rule = VerboseBlock.new
rule.exclude_multiple_line_blocks = false
rule.max_line_length = 60
expect_no_issues rule, <<-CRYSTAL
(1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
i.to_s.reverse.strip.blank? i.to_s.reverse.strip.blank?
end end
) CRYSTAL
rule = VerboseBlock.new rule.max_line_length = nil
.tap(&.exclude_multiple_line_blocks = false) expect_issue rule, <<-CRYSTAL
rule (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i|
.tap(&.max_line_length = 60) # ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)`
.catch(source).should be_valid i.to_s.reverse.strip.blank?
rule end
.tap(&.max_line_length = nil) CRYSTAL
.catch(source).should_not be_valid
end end
it "#max_length" do it "#max_length" do
source = Source.new %(
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.max_length = 30
.tap(&.max_length = 30) expect_no_issues rule, <<-CRYSTAL
.catch(source).should be_valid (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
rule CRYSTAL
.tap(&.max_length = nil) rule.max_length = nil
.catch(source).should_not be_valid expect_issue rule, <<-CRYSTAL
(1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `tap(&.to_s.split.reverse.join.strip.blank?)`
CRYSTAL
end end
end end
context "macro" do context "macro" do
it "reports in macro scope" do it "reports in macro scope" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
{{ (1..3).any? { |i| i.odd? } }} {{ (1..3).any? { |i| i.odd? } }}
) # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)`
subject.catch(source).should_not be_valid CRYSTAL
end end
end end
it "reports call args and named_args" do it "reports call args and named_args" do
short_block_variants = {
%|map(&.to_s.[start: 0.to_i64, count: 3]?)|,
%|map(&.to_s.[0.to_i64, count: 3]?)|,
%|map(&.to_s.[0.to_i64, 3]?)|,
%|map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))|,
%|map(&.to_s.[0.to_i64, count: 3]=("foo"))|,
%|map(&.to_s.[0.to_i64, 3]=("foo"))|,
%|map(&.to_s.camelcase(lower: true))|,
%|map(&.to_s.camelcase)|,
%|map(&.to_s.gsub('_', '-'))|,
%|map(&.in?(*{1, 2, 3}, **{foo: :bar}))|,
%|map(&.in?(1, *foo, 3, **bar))|,
%|join(separator: '.', &.to_s)|,
}
source = Source.new path: "source.cr", code: %(
(1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? }
(1..3).map { |i| i.to_s[0.to_i64, count: 3]? }
(1..3).map { |i| i.to_s[0.to_i64, 3]? }
(1..3).map { |i| i.to_s[start: 0.to_i64, count: 3] = "foo" }
(1..3).map { |i| i.to_s[0.to_i64, count: 3] = "foo" }
(1..3).map { |i| i.to_s[0.to_i64, 3] = "foo" }
(1..3).map { |i| i.to_s.camelcase(lower: true) }
(1..3).map { |i| i.to_s.camelcase }
(1..3).map { |i| i.to_s.gsub('_', '-') }
(1..3).map { |i| i.in?(*{1, 2, 3}, **{foo: :bar}) }
(1..3).map { |i| i.in?(1, *foo, 3, **bar) }
(1..3).join(separator: '.') { |i| i.to_s }
)
rule = VerboseBlock.new rule = VerboseBlock.new
rule rule.exclude_operators = false
.tap(&.exclude_operators = false) expect_issue rule, <<-CRYSTAL
.catch(source).should_not be_valid (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? }
source.issues.size.should eq(short_block_variants.size) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 0.to_i64, count: 3]?)`
(1..3).map { |i| i.to_s[0.to_i64, count: 3]? }
source.issues.each_with_index do |issue, i| # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[0.to_i64, count: 3]?)`
issue.message.should eq(VerboseBlock::MSG % short_block_variants[i]) (1..3).map { |i| i.to_s[0.to_i64, 3]? }
end # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[0.to_i64, 3]?)`
(1..3).map { |i| i.to_s[start: 0.to_i64, count: 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))`
(1..3).map { |i| i.to_s[0.to_i64, count: 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[0.to_i64, count: 3]=("foo"))`
(1..3).map { |i| i.to_s[0.to_i64, 3] = "foo" }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[0.to_i64, 3]=("foo"))`
(1..3).map { |i| i.to_s.camelcase(lower: true) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.camelcase(lower: true))`
(1..3).map { |i| i.to_s.camelcase }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.camelcase)`
(1..3).map { |i| i.to_s.gsub('_', '-') }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.gsub('_', '-'))`
(1..3).map { |i| i.in?(*{1, 2, 3}, **{foo: :bar}) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.in?(*{1, 2, 3}, **{foo: :bar}))`
(1..3).map { |i| i.in?(1, *foo, 3, **bar) }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.in?(1, *foo, 3, **bar))`
(1..3).join(separator: '.') { |i| i.to_s }
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `join(separator: '.', &.to_s)`
CRYSTAL
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

49
spec/ameba/rule/style/while_true_spec.cr
View file

@ -1,43 +1,36 @@
require "../../../spec_helper" require "../../../spec_helper"
valid_source = <<-EOF
a = 1
loop do
a += 1
break if a > 5
end
EOF
invalid_source = <<-EOF
a = 1
while true
a += 1
break if a > 5
end
EOF
module Ameba::Rule::Style module Ameba::Rule::Style
subject = WhileTrue.new subject = WhileTrue.new
describe WhileTrue do describe WhileTrue do
it "passes if there is no `while true`" do it "passes if there is no `while true`" do
source = Source.new valid_source expect_no_issues subject, <<-CRYSTAL
subject.catch(source).should be_valid a = 1
loop do
a += 1
break if a > 5
end
CRYSTAL
end end
it "fails if there is `while true`" do it "fails if there is `while true`" do
source = Source.new invalid_source source = expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid a = 1
while true
# ^^^^^^^^ error: While statement using true literal as condition
a += 1
break if a > 5
end end
CRYSTAL
it "reports rule, pos and message" do expect_correction source, <<-CRYSTAL
source = Source.new invalid_source, "source.cr" a = 1
subject.catch(source).should_not be_valid loop do
a += 1
issue = source.issues.first break if a > 5
issue.location.to_s.should eq "source.cr:2:1" end
issue.end_location.to_s.should eq "source.cr:5:3" CRYSTAL
issue.message.should eq "While statement using true literal as condition"
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(node : Crystal::ControlExpression, code_lines)
return unless (exp = node.exp)
return unless (exp_code = node_source(exp, code_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, code_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(node, source.lines))
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(node, source.lines))
issue_for node, MSG do |corrector|
corrector.replace(node, exp_code)
end
else
issue_for node, MSG
end
end end
end end
end end

19
src/ameba/rule/style/variable_names.cr
View file

@ -35,6 +35,10 @@ module Ameba::Rule::Style
issue_for node, MSG % {expected, node.name} issue_for node, MSG % {expected, node.name}
end end
def test(source : Source)
VarVisitor.new self, source
end
def test(source, node : Crystal::Var) def test(source, node : Crystal::Var)
check_node source, node check_node source, node
end end
@ -46,5 +50,20 @@ module Ameba::Rule::Style
def test(source, node : Crystal::ClassVar) def test(source, node : Crystal::ClassVar)
check_node source, node check_node source, node
end end
private class VarVisitor < AST::NodeVisitor
private getter var_locations = [] of Crystal::Location
def visit(node : Crystal::Var)
!var_locations.includes?(node.location) && super
end
def visit(node : Crystal::InstanceVar | Crystal::ClassVar)
if (location = node.location)
var_locations << location
end
super
end
end
end end
end end

6
src/ameba/rule/style/verbose_block.cr
View file

@ -169,7 +169,7 @@ module Ameba::Rule::Style
end end
# ameba:disable Metrics/CyclomaticComplexity # ameba:disable Metrics/CyclomaticComplexity
protected def issue_for_valid(source, call : Crystal::Call, body : Crystal::Call) protected def issue_for_valid(source, call : Crystal::Call, block : Crystal::Block, body : Crystal::Call)
return if exclude_calls_with_block && body.block return if exclude_calls_with_block && body.block
return if exclude_multiple_line_blocks && !same_location_lines?(call, body) return if exclude_multiple_line_blocks && !same_location_lines?(call, body)
return if exclude_prefix_operators && prefix_operator?(body) return if exclude_prefix_operators && prefix_operator?(body)
@ -182,7 +182,7 @@ module Ameba::Rule::Style
return unless valid_line_length?(call, call_code) return unless valid_line_length?(call, call_code)
return unless valid_length?(call_code) return unless valid_length?(call_code)
issue_for call.name_location, call.end_location, issue_for call.name_location, block.end_location,
MSG % call_code MSG % call_code
end end
@ -218,7 +218,7 @@ module Ameba::Rule::Style
return if reference_count(body, arg) > 1 return if reference_count(body, arg) > 1
# add issue if the given nodes pass all of the checks # add issue if the given nodes pass all of the checks
issue_for_valid source, node, body issue_for_valid source, node, block, body
end end
end end
end end

8
src/ameba/rule/style/while_true.cr
View file

@ -33,7 +33,13 @@ module Ameba::Rule::Style
MSG = "While statement using true literal as condition" MSG = "While statement using true literal as condition"
def test(source, node : Crystal::While) def test(source, node : Crystal::While)
issue_for node, MSG if node.cond.true_literal? return unless node.cond.true_literal?
return unless (location = node.location)
return unless (end_location = node.cond.end_location)
issue_for node, MSG do |corrector|
corrector.replace(location, end_location, "loop do")
end
end end
end end
end end