diff --git a/spec/ameba/ast/util_spec.cr b/spec/ameba/ast/util_spec.cr index 17f5649b..be47dd70 100644 --- a/spec/ameba/ast/util_spec.cr +++ b/spec/ameba/ast/util_spec.cr @@ -39,31 +39,31 @@ module Ameba::AST describe "#node_source" do it "returns original source of the node" do - s = %( + s = <<-CRYSTAL a = 1 - ) + CRYSTAL node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") source.should eq "a = 1" end it "returns original source of multiline node" do - s = %( + s = <<-CRYSTAL if () :ok end - ) + CRYSTAL node = Crystal::Parser.new(s).parse source = subject.node_source node, s.split("\n") source.should eq <<-CRYSTAL if () - :ok - end + :ok + end CRYSTAL end it "does not report source of node which has incorrect location" do - s = %q( + s = <<-'CRYSTAL' module MyModule macro conditional_error_for_inline_callbacks \{% @@ -74,7 +74,7 @@ module Ameba::AST macro before_save(x = nil) end end - ) + CRYSTAL node = as_nodes(s).nil_literal_nodes.first source = subject.node_source node, s.split("\n") @@ -140,29 +140,29 @@ module Ameba::AST end it "returns true if this is if-else consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL if foo return :foo else return :bar end - ) + CRYSTAL subject.flow_expression?(node, false).should eq true end it "returns true if this is unless-else consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL unless foo return :foo else return :bar end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this is case consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL case when 1 return 1 @@ -171,71 +171,71 @@ module Ameba::AST else return 3 end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this is exception handler consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL begin raise "exp" rescue e return e end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns true if this while consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL while true return end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false if this while with break" do - node = as_node %( + node = as_node <<-CRYSTAL while true break end - ) + CRYSTAL subject.flow_expression?(node).should eq false end it "returns true if this until consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL until false return end - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false if this until with break" do - node = as_node %( + node = as_node <<-CRYSTAL until false break end - ) + CRYSTAL subject.flow_expression?(node).should eq false end it "returns true if this expressions consumed by flow expressions" do - node = as_node %( + node = as_node <<-CRYSTAL exp1 exp2 return - ) + CRYSTAL subject.flow_expression?(node).should eq true end it "returns false otherwise" do - node = as_node %( + node = as_node <<-CRYSTAL exp1 exp2 - ) + CRYSTAL subject.flow_expression?(node).should eq false end end @@ -322,5 +322,28 @@ module Ameba::AST subject.loop?(node).should eq false 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 diff --git a/spec/ameba/rule/style/constant_names_spec.cr b/spec/ameba/rule/style/constant_names_spec.cr index f3181496..0053ecf5 100644 --- a/spec/ameba/rule/style/constant_names_spec.cr +++ b/spec/ameba/rule/style/constant_names_spec.cr @@ -3,17 +3,19 @@ require "../../../spec_helper" module Ameba 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 - s = Source.new code - Rule::Style::ConstantNames.new.catch(s).should_not be_valid - s.issues.first.message.should contain expected + rule = Rule::Style::ConstantNames.new + expect_issue rule, <<-CRYSTAL, name: name + %{name} = #{value} + # ^{name} error: Constant name should be screaming-cased: #{expected}, not #{name} + CRYSTAL end end describe Rule::Style::ConstantNames do it "passes if type names are screaming-cased" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL LUCKY_NUMBERS = [3, 7, 11] DOCUMENTATION_URL = "http://crystal-lang.org/docs" @@ -29,13 +31,12 @@ module Ameba a = 1 myVar = 2 m_var = 3 - ) - subject.catch(s).should be_valid + CRYSTAL end - # it_reports_constant "MyBadConstant=1", "MYBADCONSTANT" - it_reports_constant "Wrong_NAME=2", "WRONG_NAME" - it_reports_constant "Wrong_Name=3", "WRONG_NAME" + # it_reports_constant "MyBadConstant", "1", "MYBADCONSTANT" + it_reports_constant "Wrong_NAME", "2", "WRONG_NAME" + it_reports_constant "Wrong_Name", "3", "WRONG_NAME" it "reports rule, pos and message" do s = Source.new %( diff --git a/spec/ameba/rule/style/is_a_nil_spec.cr b/spec/ameba/rule/style/is_a_nil_spec.cr index a3bf0ee6..f857a9db 100644 --- a/spec/ameba/rule/style/is_a_nil_spec.cr +++ b/spec/ameba/rule/style/is_a_nil_spec.cr @@ -5,27 +5,26 @@ module Ameba::Rule::Style subject = IsANil.new it "doesn't report if there are no is_a?(Nil) calls" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 a.nil? a.is_a?(NilLiteral) a.is_a?(Custom::Nil) - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is a call to is_a?(Nil) without receiver" do - s = Source.new %( - is_a?(Nil) - ) - subject.catch(s).should_not be_valid + expect_issue subject, <<-CRYSTAL + a = is_a?(Nil) + # ^^^ error: Use `nil?` instead of `is_a?(Nil)` + CRYSTAL end 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) - ) - subject.catch(s).should_not be_valid + # ^^^ error: Use `nil?` instead of `is_a?(Nil)` + CRYSTAL end it "reports rule, location and message" do diff --git a/spec/ameba/rule/style/large_numbers_spec.cr b/spec/ameba/rule/style/large_numbers_spec.cr index 7397603f..1b2ea4ca 100644 --- a/spec/ameba/rule/style/large_numbers_spec.cr +++ b/spec/ameba/rule/style/large_numbers_spec.cr @@ -20,7 +20,7 @@ module Ameba describe Rule::Style::LargeNumbers 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 16 17 18 19 20 30 40 50 60 70 80 90 100 @@ -41,7 +41,7 @@ module Ameba 900_000 1_000_000 - -9_223_372_036_854_775_808 + -9_223_372_036_854_775_808 9_223_372_036_854_775_807 141_592_654 @@ -80,8 +80,7 @@ module Ameba 1200.0 1200.01 1200.012 - ) - subject.catch(s).should be_valid + CRYSTAL end it_transforms "10000", "10_000" @@ -131,10 +130,9 @@ module Ameba context "properties" do it "allows to configure integer min digits" do - s = Source.new %q(1200000) rule = Rule::Style::LargeNumbers.new rule.int_min_digits = 10 - rule.catch(s).should be_valid + expect_no_issues rule, %q(1200000) end end end diff --git a/spec/ameba/rule/style/method_names_spec.cr b/spec/ameba/rule/style/method_names_spec.cr index 4f828b58..02893ee5 100644 --- a/spec/ameba/rule/style/method_names_spec.cr +++ b/spec/ameba/rule/style/method_names_spec.cr @@ -3,17 +3,19 @@ require "../../../spec_helper" module Ameba 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 - s = Source.new code - Rule::Style::MethodNames.new.catch(s).should_not be_valid - s.issues.first.message.should contain expected + rule = Rule::Style::MethodNames.new + expect_issue rule, <<-CRYSTAL, name: name + def %{name}; end + # ^{name} error: Method name should be underscore-cased: #{expected}, not %{name} + CRYSTAL end end describe Rule::Style::MethodNames do it "passes if method names are underscore-cased" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Person def first_name end @@ -30,13 +32,12 @@ module Ameba def name end end - ) - subject.catch(s).should be_valid + CRYSTAL end - it_reports_method_name %(def firstName; end), "first_name" - it_reports_method_name %(def date_of_Birth; end), "date_of_birth" - it_reports_method_name %(def homepageURL; end), "homepage_url" + it_reports_method_name "firstName", "first_name" + it_reports_method_name "date_of_Birth", "date_of_birth" + it_reports_method_name "homepageURL", "homepage_url" it "reports rule, pos and message" do s = Source.new %( diff --git a/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr b/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr index e8a042dc..19251c54 100644 --- a/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr +++ b/spec/ameba/rule/style/negated_conditions_in_unless_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Style describe NegatedConditionsInUnless do it "passes with a unless without negated condition" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL unless a :ok end @@ -15,44 +15,43 @@ module Ameba::Rule::Style unless s.empty? :ok end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is a negated condition in unless" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL unless !a + # ^^^^^^^ error: Avoid negated conditions in unless blocks :nok end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "fails if one of AND conditions is negated" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL unless a && !b + # ^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks :nok end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "fails if one of OR conditions is negated" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL unless a || !b + # ^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks :nok end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "fails if one of inner conditions is negated" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL unless a && (b || !c) + # ^^^^^^^^^^^^^^^^^^^ error: Avoid negated conditions in unless blocks :nok end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, pos and message" do diff --git a/spec/ameba/rule/style/predicate_name_spec.cr b/spec/ameba/rule/style/predicate_name_spec.cr index eb797a22..128e6a3b 100644 --- a/spec/ameba/rule/style/predicate_name_spec.cr +++ b/spec/ameba/rule/style/predicate_name_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Style describe PredicateName do it "passes if predicate name is correct" do - s = Source.new %q( + expect_no_issues subject, <<-CRYSTAL def valid?(x) end @@ -16,16 +16,15 @@ module Ameba::Rule::Style def allow_this_picture? end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if predicate name is wrong" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL def is_valid?(x) + # ^^^^^^^^^^^^^^ error: Favour method name 'valid?' over 'is_valid?' end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, pos and message" do @@ -47,14 +46,13 @@ module Ameba::Rule::Style end it "ignores if alternative name isn't valid syntax" do - s = Source.new %q( + expect_no_issues subject, <<-CRYSTAL class Image def is_404?(x) true end end - ) - subject.catch(s).should be_valid + CRYSTAL end end end diff --git a/spec/ameba/rule/style/redundant_begin_spec.cr b/spec/ameba/rule/style/redundant_begin_spec.cr index cbd18eb4..7fe188b8 100644 --- a/spec/ameba/rule/style/redundant_begin_spec.cr +++ b/spec/ameba/rule/style/redundant_begin_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Style subject = RedundantBegin.new it "passes if there is no redundant begin blocks" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method do_something rescue @@ -27,12 +27,11 @@ module Ameba::Rule::Style def method; end def method; a = 1; rescue; end def method; begin; rescue; end; end - ) - subject.catch(s).should be_valid + CRYSTAL end 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 begin open_file @@ -80,13 +79,13 @@ module Ameba::Rule::Style end expr end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is a redundant begin block" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL def method(a : String) : String + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected begin open_file do_some_stuff @@ -94,39 +93,39 @@ module Ameba::Rule::Style close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end 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 + # ^^^^^^^^ error: Redundant `begin` block detected begin open_file ensure close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end 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 + # ^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected begin open_file ensure close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end 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, + # ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected b : String) begin open_file @@ -134,13 +133,13 @@ module Ameba::Rule::Style close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end 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, + # ^^^^^^^^^^^^^^^^^^^^ error: Redundant `begin` block detected b : String ) begin @@ -149,12 +148,11 @@ module Ameba::Rule::Style close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if there is an inner redundant block" do - s = Source.new %q( + expect_no_issues subject, <<-CRYSTAL def method begin open_file @@ -163,37 +161,36 @@ module Ameba::Rule::Style end rescue end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is a redundant block with yield" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL def method + # ^^^^^^^^ error: Redundant `begin` block detected begin yield ensure close_file end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "fails if there is top level redundant block in a method" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL def method + # ^^^^^^^^ error: Redundant `begin` block detected begin a = 1 b = 2 end end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if begin-end block in a proc literal" do - s = Source.new %q( + expect_no_issues subject, <<-CRYSTAL foo = ->{ begin raise "Foo!" @@ -201,8 +198,7 @@ module Ameba::Rule::Style pp ex end } - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports rule, pos and message" do diff --git a/spec/ameba/rule/style/redundant_next_spec.cr b/spec/ameba/rule/style/redundant_next_spec.cr index 460d2dca..689894d1 100644 --- a/spec/ameba/rule/style/redundant_next_spec.cr +++ b/spec/ameba/rule/style/redundant_next_spec.cr @@ -5,149 +5,183 @@ module Ameba::Rule::Style describe RedundantNext 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 } - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next with argument in the block" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |v| next v + 1 + # ^^^^^^^^^^ error: Redundant `next` detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |v| + v + 1 + end + CRYSTAL end context "if" 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| next if v > 10 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next in if/else branch" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |a| if a > 0 next a + 1 + # ^^^^^^^^^^ error: Redundant `next` detected else next a + 2 + # ^^^^^^^^^^ error: Redundant `next` detected end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":3:5" - s.issues.last.location.to_s.should eq ":5:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a| + if a > 0 + a + 1 + else + a + 2 + end + end + CRYSTAL end end context "unless" 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| next unless v > 10 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next in unless/else branch" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |a| unless a > 0 next a + 1 + # ^^^^^^^^^^ error: Redundant `next` detected else next a + 2 + # ^^^^^^^^^^ error: Redundant `next` detected end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":3:5" - s.issues.last.location.to_s.should eq ":5:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a| + unless a > 0 + a + 1 + else + a + 2 + end + end + CRYSTAL end end context "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| a = 1 a + v end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next in expressions" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |a| a = 1 next a + # ^^^^^^ error: Redundant `next` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":3:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a| + a = 1 + a + end + CRYSTAL end end context "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| a && v end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next in binary op" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |a| a && next a + # ^^^^^^ error: Redundant `next` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:8" + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a| + a && a + end + CRYSTAL end end context "expception 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| v + 1 rescue e next v if v > 0 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant next in exception handler" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL block do |a| next a + 1 + # ^^^^^^^^^^ error: Redundant `next` detected rescue ArgumentError next a + 2 + # ^^^^^^^^^^ error: Redundant `next` detected rescue Exception a + 2 next a + # ^^^^^^ error: Redundant `next` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 3 - s.issues[0].location.to_s.should eq ":2:3" - s.issues[1].location.to_s.should eq ":4:3" - s.issues[2].location.to_s.should eq ":7:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a| + a + 1 + rescue ArgumentError + a + 2 + rescue Exception + a + 2 + a + end + CRYSTAL end end @@ -169,49 +203,51 @@ module Ameba::Rule::Style context "properties" do context "#allow_multi_next=" do it "allows multi next statements by default" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL block do |a, b| next a, b end - ) - subject.catch(s).should be_valid + CRYSTAL end 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.allow_multi_next = false - rule.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + source = expect_issue rule, <<-CRYSTAL + block do |a, b| + next a, b + # ^^^^^^^^^ error: Redundant `next` detected + end + CRYSTAL + + expect_correction source, <<-CRYSTAL + block do |a, b| + {a, b} + end + CRYSTAL end end context "#allow_empty_next" do it "allows empty next statements by default" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL block do next end - ) - subject.catch(s).should be_valid + CRYSTAL end it "allows to configure empty next statements" do - s = Source.new %( - block do - next - end - ) rule = Rule::Style::RedundantNext.new rule.allow_empty_next = false - rule.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + source = expect_issue rule, <<-CRYSTAL + block do + next + # ^^^^ error: Redundant `next` detected + end + CRYSTAL + + expect_no_corrections source end end end diff --git a/spec/ameba/rule/style/redundant_return_spec.cr b/spec/ameba/rule/style/redundant_return_spec.cr index 4638dfc9..6fe207b8 100644 --- a/spec/ameba/rule/style/redundant_return_spec.cr +++ b/spec/ameba/rule/style/redundant_return_spec.cr @@ -5,36 +5,38 @@ module Ameba::Rule::Style describe RedundantReturn do it "does not report if there is no return" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def inc(a) a + 1 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is redundant return in method body" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def inc(a) return a + 1 + # ^^^^^^^^^^^^ error: Redundant `return` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def inc(a) + a + 1 + end + CRYSTAL end it "doesn't report if it returns tuple literal" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def foo(a) return a, a + 2 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if there are other expressions after control flow" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a) case a when true then return true @@ -44,109 +46,141 @@ module Ameba::Rule::Style rescue nil end - ) - subject.catch(s).should be_valid + CRYSTAL end context "if" do it "doesn't report if there is return in if branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def inc(a) return a + 1 if a > 0 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there are returns in if/else branch" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def inc(a) do_something(a) if a > 0 return :positive + # ^^^^^^^^^^^^^^^^ error: Redundant `return` detected else return :negative + # ^^^^^^^^^^^^^^^^ error: Redundant `return` detected end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":4:5" - s.issues.last.location.to_s.should eq ":6:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def inc(a) + do_something(a) + if a > 0 + :positive + else + :negative + end + end + CRYSTAL end end context "unless" do it "doesn't report if there is return in unless branch" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def inc(a) return a + 1 unless a > 0 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there are returns in unless/else branch" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def inc(a) do_something(a) unless a < 0 return :positive + # ^^^^^^^^^^^^^^^^ error: Redundant `return` detected else return :negative + # ^^^^^^^^^^^^^^^^ error: Redundant `return` detected end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":4:5" - s.issues.last.location.to_s.should eq ":6:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def inc(a) + do_something(a) + unless a < 0 + :positive + else + :negative + end + end + CRYSTAL end end context "binary op" 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) valid? && a > 0 end - ) - subject.catch(s).should be_valid + CRYSTAL end 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) valid? && return a > 0 + # ^^^^^^^^^^^^ error: Redundant `return` detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL + + expect_correction source, <<-CRYSTAL + def can_create?(a) + valid? && a > 0 + end + CRYSTAL end end context "case" do it "reports if there are returns in whens" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def foo(a) case a when .nil? puts "blah" return nil + # ^^^^^^^^^^ error: Redundant `return` detected when .blank? return "" + # ^^^^^^^^^ error: Redundant `return` detected when true true end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":5:5" - s.issues.last.location.to_s.should eq ":7:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def foo(a) + case a + when .nil? + puts "blah" + nil + when .blank? + "" + when true + true + end + end + CRYSTAL end it "reports if there is return in else" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def foo(a) do_something_with(a) @@ -155,49 +189,76 @@ module Ameba::Rule::Style true else return false + # ^^^^^^^^^^^^ error: Redundant `return` detected end end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":8:5" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def foo(a) + do_something_with(a) + + case a + when true + true + else + false + end + end + CRYSTAL end end context "exception handler" do it "reports if there are returns in body" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def foo(a) return true + # ^^^^^^^^^^^ error: Redundant `return` detected rescue false end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def foo(a) + true + rescue + false + end + CRYSTAL end it "reports if there are returns in rescues" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def foo(a) true rescue ArgumentError return false + # ^^^^^^^^^^^^ error: Redundant `return` detected rescue RuntimeError "" rescue Exception return nil + # ^^^^^^^^^^ error: Redundant `return` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.location.to_s.should eq ":4:3" - s.issues.last.location.to_s.should eq ":8:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def foo(a) + true + rescue ArgumentError + false + rescue RuntimeError + "" + rescue Exception + nil + end + CRYSTAL end it "reports if there are returns in else" do - s = Source.new %( + source = expect_issue subject, <<-CRYSTAL def foo(a) true rescue Exception @@ -205,60 +266,71 @@ module Ameba::Rule::Style else puts "else branch" return :bar + # ^^^^^^^^^^^ error: Redundant `return` detected end - ) - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":7:3" + CRYSTAL + + expect_correction source, <<-CRYSTAL + def foo(a) + true + rescue Exception + nil + else + puts "else branch" + :bar + end + CRYSTAL end end context "properties" do context "#allow_multi_return=" do it "allows multi returns by default" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method(a, b) return a, b end - ) - subject.catch(s).should be_valid + CRYSTAL end it "allows to configure multi returns" do - s = Source.new %( - def method(a, b) - return a, b - end - ) rule = Rule::Style::RedundantReturn.new rule.allow_multi_return = false - rule.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + source = expect_issue rule, <<-CRYSTAL + def method(a, b) + return a, b + # ^^^^^^^^^^^ error: Redundant `return` detected + end + CRYSTAL + + expect_correction source, <<-CRYSTAL + def method(a, b) + {a, b} + end + CRYSTAL end end context "#allow_empty_return" do it "allows empty returns by default" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def method return end - ) - subject.catch(s).should be_valid + CRYSTAL end it "allows to configure empty returns" do - s = Source.new %( - def method - return - end - ) rule = Rule::Style::RedundantReturn.new rule.allow_empty_return = false - rule.catch(s).should_not be_valid - s.issues.size.should eq 1 - s.issues.first.location.to_s.should eq ":2:3" + source = expect_issue rule, <<-CRYSTAL + def method + return + # ^^^^^^ error: Redundant `return` detected + end + CRYSTAL + + expect_no_corrections source end end end diff --git a/spec/ameba/rule/style/type_names_spec.cr b/spec/ameba/rule/style/type_names_spec.cr index b5064411..b856ed2b 100644 --- a/spec/ameba/rule/style/type_names_spec.cr +++ b/spec/ameba/rule/style/type_names_spec.cr @@ -3,17 +3,19 @@ require "../../../spec_helper" module Ameba 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 - s = Source.new code - Rule::Style::TypeNames.new.catch(s).should_not be_valid - s.issues.first.message.should contain expected + rule = Rule::Style::TypeNames.new + expect_issue rule, <<-CRYSTAL, type: type, name: name + %{type} %{name}; end + # ^{type}^{name}^^^^ error: Type name should be camelcased: #{expected}, but it was %{name} + CRYSTAL end end describe Rule::Style::TypeNames do it "passes if type names are camelcased" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class ParseError < Exception end @@ -32,16 +34,21 @@ module Ameba enum Time::DayOfWeek end - ) - subject.catch(s).should be_valid + CRYSTAL end - it_reports_name "class My_class; end", "MyClass" - it_reports_name "module HTT_p; end", "HTTP" - it_reports_name "alias Numeric_value = Int32", "NumericValue" - it_reports_name "lib Lib_YAML; end", "LibYAML" - it_reports_name "struct Tag_directive; end", "TagDirective" - it_reports_name "enum Time_enum::Day_of_week; end", "TimeEnum::DayOfWeek" + it_reports_name "class", "My_class", "MyClass" + it_reports_name "module", "HTT_p", "HTTP" + it_reports_name "lib", "Lib_YAML", "LibYAML" + it_reports_name "struct", "Tag_directive", "TagDirective" + it_reports_name "enum", "Time_enum::Day_of_week", "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 s = Source.new %( diff --git a/spec/ameba/rule/style/unless_else_spec.cr b/spec/ameba/rule/style/unless_else_spec.cr index bb8b6b25..cf1c065d 100644 --- a/spec/ameba/rule/style/unless_else_spec.cr +++ b/spec/ameba/rule/style/unless_else_spec.cr @@ -5,23 +5,22 @@ module Ameba::Rule::Style describe UnlessElse do it "passes if unless hasn't else" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL unless something :ok end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if unless has else" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL unless something + # ^^^^^^^^^^^^^^ error: Favour if over unless with else :one else :two end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, pos and message" do diff --git a/spec/ameba/rule/style/variable_names_spec.cr b/spec/ameba/rule/style/variable_names_spec.cr index 6ec6ae9f..2e28cf9c 100644 --- a/spec/ameba/rule/style/variable_names_spec.cr +++ b/spec/ameba/rule/style/variable_names_spec.cr @@ -3,17 +3,19 @@ require "../../../spec_helper" module Ameba subject = Rule::Style::VariableNames.new - private def it_reports_var_name(code, expected) - it "reports method name #{expected}" do - s = Source.new code - Rule::Style::VariableNames.new.catch(s).should_not be_valid - s.issues.first.message.should contain expected + private def it_reports_var_name(name, value, expected) + it "reports variable name #{expected}" do + rule = Rule::Style::VariableNames.new + expect_issue rule, <<-CRYSTAL, name: name + %{name} = #{value} + # ^{name} error: Var name should be underscore-cased: #{expected}, not %{name} + CRYSTAL end end describe Rule::Style::VariableNames do it "passes if var names are underscore-cased" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL class Greeting @@default_greeting = "Hello world" @@ -25,25 +27,41 @@ module Ameba puts greeting end end - ) - subject.catch(s).should be_valid + CRYSTAL end - it_reports_var_name %(myBadNamedVar = 1), "my_bad_named_var" - it_reports_var_name %(wrong_Name = 'y'), "wrong_name" + it_reports_var_name "myBadNamedVar", "1", "my_bad_named_var" + it_reports_var_name "wrong_Name", "'y'", "wrong_name" - it_reports_var_name %( - class Greeting - def initialize(@badNamed = nil) + it "reports instance variable name" do + expect_issue subject, <<-CRYSTAL + class Greeting + def initialize(@badNamed = nil) + # ^ error: Var name should be underscore-cased: @bad_named, not @badNamed + end end - end - ), "bad_named" + CRYSTAL + end - it_reports_var_name %( - class Greeting - @@defaultGreeting = "Hello world" - end - ), "default_greeting" + 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 + @@defaultGreeting = "Hello world" + # ^^^^^^^^^^^^^^^^^ error: Var name should be underscore-cased: @@default_greeting, not @@defaultGreeting + end + CRYSTAL + end it "reports rule, pos and message" do s = Source.new %( diff --git a/spec/ameba/rule/style/verbose_block_spec.cr b/spec/ameba/rule/style/verbose_block_spec.cr index f324263e..c2280d80 100644 --- a/spec/ameba/rule/style/verbose_block_spec.cr +++ b/spec/ameba/rule/style/verbose_block_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Style describe VerboseBlock 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).join('.', &.to_s) (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.to_s.split.join('.') } (1..3).map { :foo } - ) - subject.catch(source).should be_valid + CRYSTAL end 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 { |j| j * j.to_i64 } (1..3).map { |k| k.to_i64 * k } (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 { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join } - ) - subject.catch(source).should be_valid + CRYSTAL end 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? } - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` + CRYSTAL end 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 } - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `join('.', &.to_s)` + CRYSTAL end 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 } - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `map(&.to_s.split.reverse.join.strip)` + CRYSTAL end context "properties" 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 - .tap(&.exclude_calls_with_block = true) - .catch(source).should be_valid - rule - .tap(&.exclude_calls_with_block = false) - .catch(source).should_not be_valid + rule.exclude_calls_with_block = true + expect_no_issues rule, <<-CRYSTAL + (1..3).in_groups_of(1) { |i| i.map(&.to_s) } + CRYSTAL + rule.exclude_calls_with_block = false + 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 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| i.odd? end - ) - rule = VerboseBlock.new - rule - .tap(&.exclude_multiple_line_blocks = true) - .catch(source).should be_valid - rule - .tap(&.exclude_multiple_line_blocks = false) - .catch(source).should_not be_valid + CRYSTAL + rule.exclude_multiple_line_blocks = false + expect_issue rule, <<-CRYSTAL + (1..3).any? do |i| + # ^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` + i.odd? + end + CRYSTAL end 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 } - ) - rule = VerboseBlock.new - rule - .tap(&.exclude_prefix_operators = true) - .catch(source).should be_valid - rule - .tap(&.exclude_prefix_operators = false) - .tap(&.exclude_operators = false) - .catch(source).should_not be_valid + CRYSTAL + rule.exclude_prefix_operators = false + rule.exclude_operators = false + expect_issue rule, <<-CRYSTAL + (1..3).sum { |i| +i } + # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.+)` + (1..3).sum { |i| -i } + # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.-)` + (1..3).sum { |i| ~i } + # ^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.~)` + CRYSTAL end it "#exclude_operators" do - source = Source.new %( - (1..3).sum { |i| i * 2 } - ) rule = VerboseBlock.new - rule - .tap(&.exclude_operators = true) - .catch(source).should be_valid - rule - .tap(&.exclude_operators = false) - .catch(source).should_not be_valid + rule.exclude_operators = true + expect_no_issues rule, <<-CRYSTAL + (1..3).sum { |i| i * 2 } + CRYSTAL + rule.exclude_operators = false + expect_issue rule, <<-CRYSTAL + (1..3).sum { |i| i * 2 } + # ^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `sum(&.*(2))` + CRYSTAL end it "#exclude_setters" do - source = Source.new %( - Char::Reader.new("abc").tap { |reader| reader.pos = 0 } - ) rule = VerboseBlock.new - rule - .tap(&.exclude_setters = true) - .catch(source).should be_valid - rule - .tap(&.exclude_setters = false) - .catch(source).should_not be_valid + rule.exclude_setters = true + expect_no_issues rule, <<-CRYSTAL + Char::Reader.new("abc").tap { |reader| reader.pos = 0 } + CRYSTAL + rule.exclude_setters = false + expect_issue rule, <<-CRYSTAL + Char::Reader.new("abc").tap { |reader| reader.pos = 0 } + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `tap(&.pos=(0))` + CRYSTAL end 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| i.to_s.reverse.strip.blank? end - ) - rule = VerboseBlock.new - .tap(&.exclude_multiple_line_blocks = false) - rule - .tap(&.max_line_length = 60) - .catch(source).should be_valid - rule - .tap(&.max_line_length = nil) - .catch(source).should_not be_valid + CRYSTAL + rule.max_line_length = nil + expect_issue rule, <<-CRYSTAL + (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| + # ^^^^^^^^^^ error: Use short block notation instead: `tap(&.to_s.reverse.strip.blank?)` + i.to_s.reverse.strip.blank? + end + CRYSTAL end it "#max_length" do - source = Source.new %( - (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } - ) rule = VerboseBlock.new - rule - .tap(&.max_length = 30) - .catch(source).should be_valid - rule - .tap(&.max_length = nil) - .catch(source).should_not be_valid + rule.max_length = 30 + expect_no_issues rule, <<-CRYSTAL + (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } + CRYSTAL + rule.max_length = nil + 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 context "macro" do it "reports in macro scope" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL {{ (1..3).any? { |i| i.odd? } }} - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^ error: Use short block notation instead: `any?(&.odd?)` + CRYSTAL end end 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 - .tap(&.exclude_operators = false) - .catch(source).should_not be_valid - source.issues.size.should eq(short_block_variants.size) - - source.issues.each_with_index do |issue, i| - issue.message.should eq(VerboseBlock::MSG % short_block_variants[i]) - end + rule.exclude_operators = false + expect_issue rule, <<-CRYSTAL + (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? } + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[start: 0.to_i64, count: 3]?)` + (1..3).map { |i| i.to_s[0.to_i64, count: 3]? } + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: [...] `map(&.to_s.[0.to_i64, count: 3]?)` + (1..3).map { |i| i.to_s[0.to_i64, 3]? } + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 it "reports rule, pos and message" do diff --git a/spec/ameba/rule/style/while_true_spec.cr b/spec/ameba/rule/style/while_true_spec.cr index 6d9d85da..3464e272 100644 --- a/spec/ameba/rule/style/while_true_spec.cr +++ b/spec/ameba/rule/style/while_true_spec.cr @@ -1,43 +1,36 @@ 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 subject = WhileTrue.new describe WhileTrue do it "passes if there is no `while true`" do - source = Source.new valid_source - subject.catch(source).should be_valid + expect_no_issues subject, <<-CRYSTAL + a = 1 + loop do + a += 1 + break if a > 5 + end + CRYSTAL end it "fails if there is `while true`" do - source = Source.new invalid_source - subject.catch(source).should_not be_valid - end + source = expect_issue subject, <<-CRYSTAL + a = 1 + while true + # ^^^^^^^^ error: While statement using true literal as condition + a += 1 + break if a > 5 + end + CRYSTAL - it "reports rule, pos and message" do - source = Source.new invalid_source, "source.cr" - subject.catch(source).should_not be_valid - - issue = source.issues.first - issue.location.to_s.should eq "source.cr:2:1" - issue.end_location.to_s.should eq "source.cr:5:3" - issue.message.should eq "While statement using true literal as condition" + expect_correction source, <<-CRYSTAL + a = 1 + loop do + a += 1 + break if a > 5 + end + CRYSTAL end end end diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr index 5b87608a..2965247f 100644 --- a/src/ameba/ast/util.cr +++ b/src/ameba/ast/util.cr @@ -158,4 +158,16 @@ module Ameba::AST::Util false 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 diff --git a/src/ameba/rule/style/redundant_next.cr b/src/ameba/rule/style/redundant_next.cr index 5decb740..214690b5 100644 --- a/src/ameba/rule/style/redundant_next.cr +++ b/src/ameba/rule/style/redundant_next.cr @@ -97,6 +97,8 @@ module Ameba::Rule::Style # AllowEmptyNext: true # ``` class RedundantNext < Base + include AST::Util + properties do 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_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 diff --git a/src/ameba/rule/style/redundant_return.cr b/src/ameba/rule/style/redundant_return.cr index e8ec7dc9..77eafe73 100644 --- a/src/ameba/rule/style/redundant_return.cr +++ b/src/ameba/rule/style/redundant_return.cr @@ -94,6 +94,8 @@ module Ameba::Rule::Style # AllowEmptyReturn: true # ``` class RedundantReturn < Base + include AST::Util + properties do 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_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 diff --git a/src/ameba/rule/style/variable_names.cr b/src/ameba/rule/style/variable_names.cr index e8b999c1..2176dbec 100644 --- a/src/ameba/rule/style/variable_names.cr +++ b/src/ameba/rule/style/variable_names.cr @@ -35,6 +35,10 @@ module Ameba::Rule::Style issue_for node, MSG % {expected, node.name} end + def test(source : Source) + VarVisitor.new self, source + end + def test(source, node : Crystal::Var) check_node source, node end @@ -46,5 +50,20 @@ module Ameba::Rule::Style def test(source, node : Crystal::ClassVar) check_node source, node 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 diff --git a/src/ameba/rule/style/verbose_block.cr b/src/ameba/rule/style/verbose_block.cr index 62d52ca0..233bbf44 100644 --- a/src/ameba/rule/style/verbose_block.cr +++ b/src/ameba/rule/style/verbose_block.cr @@ -169,7 +169,7 @@ module Ameba::Rule::Style end # 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_multiple_line_blocks && !same_location_lines?(call, 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_length?(call_code) - issue_for call.name_location, call.end_location, + issue_for call.name_location, block.end_location, MSG % call_code end @@ -218,7 +218,7 @@ module Ameba::Rule::Style return if reference_count(body, arg) > 1 # 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 diff --git a/src/ameba/rule/style/while_true.cr b/src/ameba/rule/style/while_true.cr index 4026c217..be83d62b 100644 --- a/src/ameba/rule/style/while_true.cr +++ b/src/ameba/rule/style/while_true.cr @@ -33,7 +33,13 @@ module Ameba::Rule::Style MSG = "While statement using true literal as condition" 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