diff --git a/spec/ameba/rule/layout/line_length_spec.cr b/spec/ameba/rule/layout/line_length_spec.cr index 10392789..71a1b1ea 100644 --- a/spec/ameba/rule/layout/line_length_spec.cr +++ b/spec/ameba/rule/layout/line_length_spec.cr @@ -6,13 +6,15 @@ module Ameba::Rule::Layout describe LineLength do it "passes if all lines are shorter than MaxLength symbols" do - source = Source.new "short line" - subject.catch(source).should be_valid + expect_no_issues subject, <<-CRYSTAL + short line + CRYSTAL end it "passes if line consists of MaxLength symbols" do - source = Source.new "*" * subject.max_length - subject.catch(source).should be_valid + expect_no_issues subject, <<-CRYSTAL + #{"*" * subject.max_length} + CRYSTAL end it "fails if there is at least one line longer than MaxLength symbols" do @@ -33,10 +35,10 @@ module Ameba::Rule::Layout context "properties" do it "allows to configure max length of the line" do - source = Source.new long_line rule = LineLength.new rule.max_length = long_line.size - rule.catch(source).should be_valid + + expect_no_issues rule, long_line end end end diff --git a/spec/ameba/rule/lint/ambiguous_assignment_spec.cr b/spec/ameba/rule/lint/ambiguous_assignment_spec.cr index 153f1796..a808df40 100644 --- a/spec/ameba/rule/lint/ambiguous_assignment_spec.cr +++ b/spec/ameba/rule/lint/ambiguous_assignment_spec.cr @@ -6,163 +6,115 @@ module Ameba::Rule::Lint context "when using `-`" do it "registers an offense with `x`" do - source = Source.new("x =- y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + x =- y + # ^^ error: Suspicious assignment detected. Did you mean `-=`? + CRYSTAL end it "registers an offense with `@x`" do - source = Source.new("@x =- y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" - issue.location.to_s.should eq "source.cr:1:4" - issue.end_location.to_s.should eq "source.cr:1:5" + expect_issue subject, <<-CRYSTAL + @x =- y + # ^^ error: Suspicious assignment detected. Did you mean `-=`? + CRYSTAL end it "registers an offense with `@@x`" do - source = Source.new("@@x =- y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" - issue.location.to_s.should eq "source.cr:1:5" - issue.end_location.to_s.should eq "source.cr:1:6" + expect_issue subject, <<-CRYSTAL + @@x =- y + # ^^ error: Suspicious assignment detected. Did you mean `-=`? + CRYSTAL end it "registers an offense with `X`" do - source = Source.new("X =- y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `-=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + X =- y + # ^^ error: Suspicious assignment detected. Did you mean `-=`? + CRYSTAL end it "does not register an offense when no mistype assignments" do - subject.catch(Source.new(<<-CRYSTAL)).should be_valid + expect_no_issues subject, <<-CRYSTAL x = 1 x -= y x = -y - CRYSTAL + CRYSTAL end end context "when using `+`" do it "registers an offense with `x`" do - source = Source.new("x =+ y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + x =+ y + # ^^ error: Suspicious assignment detected. Did you mean `+=`? + CRYSTAL end it "registers an offense with `@x`" do - source = Source.new("@x =+ y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" - issue.location.to_s.should eq "source.cr:1:4" - issue.end_location.to_s.should eq "source.cr:1:5" + expect_issue subject, <<-CRYSTAL + @x =+ y + # ^^ error: Suspicious assignment detected. Did you mean `+=`? + CRYSTAL end it "registers an offense with `@@x`" do - source = Source.new("@@x =+ y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" - issue.location.to_s.should eq "source.cr:1:5" - issue.end_location.to_s.should eq "source.cr:1:6" + expect_issue subject, <<-CRYSTAL + @@x =+ y + # ^^ error: Suspicious assignment detected. Did you mean `+=`? + CRYSTAL end it "registers an offense with `X`" do - source = Source.new("X =+ y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `+=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + X =+ y + # ^^ error: Suspicious assignment detected. Did you mean `+=`? + CRYSTAL end it "does not register an offense when no mistype assignments" do - subject.catch(Source.new(<<-CRYSTAL)).should be_valid + expect_no_issues subject, <<-CRYSTAL x = 1 x += y x = +y - CRYSTAL + CRYSTAL end end context "when using `!`" do it "registers an offense with `x`" do - source = Source.new("x =! y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + x =! y + # ^^ error: Suspicious assignment detected. Did you mean `!=`? + CRYSTAL end it "registers an offense with `@x`" do - source = Source.new("@x =! y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" - issue.location.to_s.should eq "source.cr:1:4" - issue.end_location.to_s.should eq "source.cr:1:5" + expect_issue subject, <<-CRYSTAL + @x =! y + # ^^ error: Suspicious assignment detected. Did you mean `!=`? + CRYSTAL end it "registers an offense with `@@x`" do - source = Source.new("@@x =! y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" - issue.location.to_s.should eq "source.cr:1:5" - issue.end_location.to_s.should eq "source.cr:1:6" + expect_issue subject, <<-CRYSTAL + @@x =! y + # ^^ error: Suspicious assignment detected. Did you mean `!=`? + CRYSTAL end it "registers an offense with `X`" do - source = Source.new("X =! y", "source.cr") - subject.catch(source).should_not be_valid - source.issues.size.should eq 1 - - issue = source.issues.first - issue.message.should eq "Suspicious assignment detected. Did you mean `!=`?" - issue.location.to_s.should eq "source.cr:1:3" - issue.end_location.to_s.should eq "source.cr:1:4" + expect_issue subject, <<-CRYSTAL + X =! y + # ^^ error: Suspicious assignment detected. Did you mean `!=`? + CRYSTAL end it "does not register an offense when no mistype assignments" do - subject.catch(Source.new(<<-CRYSTAL)).should be_valid + expect_no_issues subject, <<-CRYSTAL x = false x != y x = !y - CRYSTAL + CRYSTAL end end end diff --git a/spec/ameba/rule/lint/bad_directive_spec.cr b/spec/ameba/rule/lint/bad_directive_spec.cr index 5667982d..765a83ec 100644 --- a/spec/ameba/rule/lint/bad_directive_spec.cr +++ b/spec/ameba/rule/lint/bad_directive_spec.cr @@ -5,62 +5,42 @@ module Ameba::Rule::Lint subject = BadDirective.new it "does not report if rule is correct" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL # ameba:disable Lint/BadDirective - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is incorrect action" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL # ameba:foo Lint/BadDirective - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.message.should eq( - "Bad action in comment directive: 'foo'. Possible values: disable, enable" - ) - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "" + # ^{} error: Bad action in comment directive: 'foo'. Possible values: disable, enable + CRYSTAL end it "reports if there are incorrect rule names" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL # ameba:enable BadRule1, BadRule2 - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.message.should eq( - "Such rules do not exist: BadRule1, BadRule2" - ) - issue.location.to_s.should eq "source.cr:1:1" - issue.end_location.to_s.should eq "" + # ^{} error: Such rules do not exist: BadRule1, BadRule2 + CRYSTAL end it "does not report if there no action and rules at all" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL # ameba: - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if there are no rules" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL # ameba:enable # ameba:disable - ) - subject.catch(s).should be_valid + CRYSTAL end it "does not report if there are group names in the directive" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL # ameba:disable Style Performance - ) - subject.catch(s).should be_valid + CRYSTAL end end end diff --git a/spec/ameba/rule/lint/empty_loop_spec.cr b/spec/ameba/rule/lint/empty_loop_spec.cr index 26f63fba..d2a8c21e 100644 --- a/spec/ameba/rule/lint/empty_loop_spec.cr +++ b/spec/ameba/rule/lint/empty_loop_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Lint subject = EmptyLoop.new it "does not report if there are not empty loops" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 while a < 10 @@ -19,54 +19,50 @@ module Ameba::Rule::Lint loop do a += 1 end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is an empty while loop" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL a = 1 while true + # ^^^^^^^^ error: Empty loop detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if while loop has non-literals in cond block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 while a = gets.to_s # nothing here end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there is an empty until loop" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL do_something until false + # ^^^^^^^^^ error: Empty loop detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "doesn't report if until loop has non-literals in cond block" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL until socket_open? end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports if there an empty loop" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL a = 1 loop do - + # ^^^^^ error: Empty loop detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, message and location" do diff --git a/spec/ameba/rule/lint/hash_duplicated_key_spec.cr b/spec/ameba/rule/lint/hash_duplicated_key_spec.cr index 43b5a447..def877f7 100644 --- a/spec/ameba/rule/lint/hash_duplicated_key_spec.cr +++ b/spec/ameba/rule/lint/hash_duplicated_key_spec.cr @@ -5,26 +5,32 @@ module Ameba::Rule::Lint subject = HashDuplicatedKey.new it "passes if there is no duplicated keys in a hash literals" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL h = {"a" => 1, :a => 2, "b" => 3} h = {"a" => 1, "b" => 2, "c" => {"a" => 3, "b" => 4}} h = {} of String => String - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is a duplicated key in a hash literal" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL h = {"a" => 1, "b" => 2, "a" => 3} - ) - subject.catch(s).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "a" + CRYSTAL end it "fails if there is a duplicated key in the inner hash literal" do - s = Source.new %q( + expect_issue subject, <<-CRYSTAL h = {"a" => 1, "b" => {"a" => 3, "b" => 4, "a" => 5}} - ) - subject.catch(s).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "a" + CRYSTAL + end + + it "reports multiple duplicated keys" do + expect_issue subject, <<-CRYSTAL + h = {"key1" => 1, "key1" => 2, "key2" => 3, "key2" => 4} + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "key1", "key2" + CRYSTAL end it "reports rule, location and message" do @@ -38,14 +44,5 @@ module Ameba::Rule::Lint issue.end_location.to_s.should eq "source.cr:1:24" issue.message.should eq %(Duplicated keys in hash literal: "a") end - - it "reports multiple duplicated keys" do - s = Source.new %q( - h = {"key1" => 1, "key1" => 2, "key2" => 3, "key2" => 4} - ) - subject.catch(s).should_not be_valid - issue = s.issues.first - issue.message.should eq %(Duplicated keys in hash literal: "key1", "key2") - end end end diff --git a/spec/ameba/rule/lint/literal_in_interpolation_spec.cr b/spec/ameba/rule/lint/literal_in_interpolation_spec.cr index 218d94d7..78487eec 100644 --- a/spec/ameba/rule/lint/literal_in_interpolation_spec.cr +++ b/spec/ameba/rule/lint/literal_in_interpolation_spec.cr @@ -5,15 +5,14 @@ module Ameba::Rule::Lint describe LiteralInInterpolation do it "passes with good interpolation examples" do - s = Source.new %q( + expect_no_issues subject, <<-CRYSTAL name = "Ary" "Hello, #{name}" "#{name}" "Name size: #{name.size}" - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is useless interpolation" do diff --git a/spec/ameba/rule/lint/rand_zero_spec.cr b/spec/ameba/rule/lint/rand_zero_spec.cr index a86d30f2..cb5cc6d4 100644 --- a/spec/ameba/rule/lint/rand_zero_spec.cr +++ b/spec/ameba/rule/lint/rand_zero_spec.cr @@ -5,22 +5,25 @@ module Ameba::Rule::Lint subject = RandZero.new it "passes if it is not rand(1) or rand(0)" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL rand(1.0) rand(0.11) rand(2) - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if it is rand(0)" do - s = Source.new "rand(0)" - subject.catch(s).should_not be_valid + expect_issue subject, <<-CRYSTAL + rand(0) + # ^^^^^ error: rand(0) always returns 0 + CRYSTAL end it "fails if it is rand(1)" do - s = Source.new "rand(1)" - subject.catch(s).should_not be_valid + expect_issue subject, <<-CRYSTAL + rand(1) + # ^^^^^ error: rand(1) always returns 0 + CRYSTAL end it "reports rule, location and a message" do diff --git a/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr b/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr similarity index 68% rename from spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr rename to spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr index 4b1e5799..5706841f 100644 --- a/spec/ameba/rule/lint/shadowing_local_outer_var_spec.cr +++ b/spec/ameba/rule/lint/shadowing_outer_local_var_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Lint subject = ShadowingOuterLocalVar.new it "doesn't report if there is no shadowing" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL def some_method foo = 1 @@ -14,126 +14,117 @@ module Ameba::Rule::Lint end -> (baz : Int32) {} - -> (bar : String) {} end - ) - - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is a shadowing in a block" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL def some_method foo = 1 3.times do |foo| + # ^ error: Shadowing outer local variable `foo` end end - ) - subject.catch(source).should_not be_valid + CRYSTAL end it "does not report outer vars declared below shadowed block" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL methods = klass.methods.select { |m| m.annotation(MyAnn) } m = methods.last - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is a shadowing in a proc" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL def some_method foo = 1 -> (foo : Int32) {} + # ^ error: Shadowing outer local variable `foo` end - ) - subject.catch(source).should_not be_valid + CRYSTAL end it "reports if there is a shadowing in an inner scope" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL def foo foo = 1 3.times do |i| 3.times { |foo| foo } + # ^ error: Shadowing outer local variable `foo` end end - ) - subject.catch(source).should_not be_valid + CRYSTAL end it "reports if variable is shadowed twice" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL foo = 1 3.times do |foo| + # ^ error: Shadowing outer local variable `foo` -> (foo : Int32) { foo + 1 } + # ^ error: Shadowing outer local variable `foo` end - ) - subject.catch(source).should_not be_valid - - source.issues.size.should eq 2 + CRYSTAL end it "reports if a splat block argument shadows local var" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL foo = 1 3.times do |*foo| + # ^ error: Shadowing outer local variable `foo` end - ) - subject.catch(source).should_not be_valid + CRYSTAL end it "reports if a &block argument is shadowed" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL def method_with_block(a, &block) 3.times do |block| + # ^ error: Shadowing outer local variable `block` end end - ) - subject.catch(source).should_not be_valid - source.issues.first.message.should eq "Shadowing outer local variable `block`" + CRYSTAL end it "reports if there are multiple args and one shadows local var" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL foo = 1 [1, 2, 3].each_with_index do |i, foo| + # ^ error: Shadowing outer local variable `foo` i + foo end - ) - subject.catch(source).should_not be_valid - source.issues.first.message.should eq "Shadowing outer local variable `foo`" + CRYSTAL end it "doesn't report if an outer var is reassigned in a block" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL def foo foo = 1 3.times do |i| foo = 2 end end - ) - subject.catch(source).should be_valid + CRYSTAL end it "doesn't report if an argument is a black hole '_'" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL _ = 1 3.times do |_| end - ) - subject.catch(source).should be_valid + CRYSTAL end it "doesn't report if it shadows record type declaration" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL class FooBar record Foo, index : String @@ -142,12 +133,11 @@ module Ameba::Rule::Lint end end end - ) - subject.catch(source).should be_valid + CRYSTAL end it "doesn't report if it shadows throwaway arguments" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL data = [{1, "a"}, {2, "b"}, {3, "c"}] data.each do |_, string| @@ -155,18 +145,16 @@ module Ameba::Rule::Lint puts string, number end end - ) - subject.catch(source).should be_valid + CRYSTAL end it "does not report if argument shadows an ivar assignment" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def bar(@foo) @foo.try do |foo| end end - ) - subject.catch(s).should be_valid + CRYSTAL end it "reports rule, location and message" do @@ -185,7 +173,7 @@ module Ameba::Rule::Lint context "macro" do it "does not report shadowed vars in outer scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL macro included def foo {% for ivar in instance_vars %} @@ -197,12 +185,11 @@ module Ameba::Rule::Lint {% instance_vars.reject { |ivar| ivar } %} end end - ) - subject.catch(source).should be_valid + CRYSTAL end it "does not report shadowed vars in macro within the same scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL {% methods = klass.methods.select { |m| m.annotation(MyAnn) } %} {% for m, m_idx in methods %} @@ -210,12 +197,11 @@ module Ameba::Rule::Lint {% d %} {% end %} {% end %} - ) - subject.catch(source).should be_valid + CRYSTAL end it "does not report shadowed vars within nested macro" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL module Foo macro included def foo @@ -233,12 +219,11 @@ module Ameba::Rule::Lint end end end - ) - subject.catch(source).should be_valid + CRYSTAL end it "does not report scoped vars to MacroFor" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL struct Test def test {% for ivar in @type.instance_vars %} @@ -248,8 +233,24 @@ module Ameba::Rule::Lint {% ["a", "b"].map { |ivar| puts ivar } %} end end - ) - subject.catch(source).should be_valid + CRYSTAL + end + + # https://github.com/crystal-ameba/ameba/issues/224#issuecomment-822245167 + it "does not report scoped vars to MacroFor (2)" do + expect_no_issues subject, <<-CRYSTAL + struct Test + def test + {% begin %} + {% for ivar in @type.instance_vars %} + {% var_type = ivar %} + {% end %} + + {% ["a", "b"].map { |ivar| puts ivar } %} + {% end %} + end + end + CRYSTAL end end end diff --git a/spec/ameba/rule/lint/syntax_spec.cr b/spec/ameba/rule/lint/syntax_spec.cr index c46e52f3..2412b0b2 100644 --- a/spec/ameba/rule/lint/syntax_spec.cr +++ b/spec/ameba/rule/lint/syntax_spec.cr @@ -5,23 +5,22 @@ module Ameba::Rule::Lint subject = Syntax.new it "passes if there is no invalid syntax" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL def hello puts "totally valid" rescue e: Exception end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is an invalid syntax" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL def hello puts "invalid" rescue Exception => e + # ^ error: expecting any of these tokens: ;, NEWLINE (not '=>') end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, location and message" do diff --git a/spec/ameba/rule/lint/unneeded_disable_directive_spec.cr b/spec/ameba/rule/lint/unneeded_disable_directive_spec.cr index cc21a00f..7186ff2d 100644 --- a/spec/ameba/rule/lint/unneeded_disable_directive_spec.cr +++ b/spec/ameba/rule/lint/unneeded_disable_directive_spec.cr @@ -5,17 +5,15 @@ module Ameba::Rule::Lint subject = UnneededDisableDirective.new it "passes if there are no comments" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 - ) - subject.catch(s).should be_valid + CRYSTAL end it "passes if there is disable directive" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL a = 1 # my super var - ) - subject.catch(s).should be_valid + CRYSTAL end it "doesn't report if there is disable directive and it is needed" do @@ -48,33 +46,27 @@ module Ameba::Rule::Lint end it "fails if there is unneeded directive" do - s = Source.new %Q( + expect_issue subject, <<-CRYSTAL # ameba:disable #{NamedRule.name} + # ^{} error: Unnecessary disabling of #{NamedRule.name} a = 1 - ) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq( - "Unnecessary disabling of #{NamedRule.name}" - ) + CRYSTAL end it "fails if there is inline unneeded directive" do - s = Source.new %Q(a = 1 # ameba:disable #{NamedRule.name}) - subject.catch(s).should_not be_valid - s.issues.first.message.should eq( - "Unnecessary disabling of #{NamedRule.name}" - ) + expect_issue subject, <<-CRYSTAL + a = 1 # ameba:disable #{NamedRule.name} + # ^ error: Unnecessary disabling of #{NamedRule.name} + CRYSTAL end it "detects mixed inline directives" do - s = Source.new %Q( + expect_issue subject, <<-CRYSTAL # ameba:disable Rule1, Rule2 + # ^{} error: Unnecessary disabling of Rule1, Rule2 a = 1 # ameba:disable Rule3 - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 2 - s.issues.first.message.should contain "Rule1, Rule2" - s.issues.last.message.should contain "Rule3" + # ^ error: Unnecessary disabling of Rule3 + CRYSTAL end it "fails if there is disabled UnneededDisableDirective" do diff --git a/spec/ameba/rule/lint/useless_condition_in_when_spec.cr b/spec/ameba/rule/lint/useless_condition_in_when_spec.cr index 3dadc3bb..9fffc4a9 100644 --- a/spec/ameba/rule/lint/useless_condition_in_when_spec.cr +++ b/spec/ameba/rule/lint/useless_condition_in_when_spec.cr @@ -5,25 +5,24 @@ module Ameba::Rule::Lint describe UselessConditionInWhen do it "passes if there is not useless condition" do - s = Source.new %( + expect_no_issues subject, <<-CRYSTAL case when utc? io << " UTC" when local? Format.new(" %:z").format(self, io) if utc? end - ) - subject.catch(s).should be_valid + CRYSTAL end it "fails if there is useless if condition" do - s = Source.new %( + expect_issue subject, <<-CRYSTAL case when utc? io << " UTC" if utc? + # ^^^^ error: Useless condition in when detected end - ) - subject.catch(s).should_not be_valid + CRYSTAL end it "reports rule, location and message" do diff --git a/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr b/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr index eb4f5519..5692ae33 100644 --- a/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr +++ b/spec/ameba/rule/metrics/cyclomatic_complexity_spec.cr @@ -15,33 +15,35 @@ module Ameba::Rule::Metrics end end end - CODE + CODE describe CyclomaticComplexity do it "passes for empty methods" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL def hello end - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports one issue for a complex method" do - subject.max_complexity = 5 + rule = CyclomaticComplexity.new + rule.max_complexity = 5 + source = Source.new(complex_method, "source.cr") - subject.catch(source).should_not be_valid + rule.catch(source).should_not be_valid issue = source.issues.first - issue.rule.should eq subject + issue.rule.should eq rule issue.location.to_s.should eq "source.cr:1:5" issue.end_location.to_s.should eq "source.cr:1:9" issue.message.should eq "Cyclomatic complexity too high [8/5]" end it "doesn't report an issue for an increased threshold" do - subject.max_complexity = 100 - source = Source.new(complex_method, "source.cr") - subject.catch(source).should be_valid + rule = CyclomaticComplexity.new + rule.max_complexity = 100 + + expect_no_issues rule, complex_method end end end diff --git a/spec/ameba/rule/performance/any_after_filter_spec.cr b/spec/ameba/rule/performance/any_after_filter_spec.cr index f95f395a..a7758533 100644 --- a/spec/ameba/rule/performance/any_after_filter_spec.cr +++ b/spec/ameba/rule/performance/any_after_filter_spec.cr @@ -49,6 +49,7 @@ module Ameba::Rule::Performance it "allows to configure object_call_names" do rule = Rule::Performance::AnyAfterFilter.new rule.filter_names = %w(select) + expect_no_issues rule, <<-CRYSTAL [1, 2, 3].reject { |e| e > 2 }.any? CRYSTAL diff --git a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr index ad3cda81..73100d39 100644 --- a/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr +++ b/spec/ameba/rule/performance/chained_call_with_no_bang_spec.cr @@ -47,6 +47,7 @@ module Ameba::Rule::Performance it "allows to configure `call_names`" do rule = ChainedCallWithNoBang.new rule.call_names = %w(uniq) + expect_no_issues rule, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.reverse CRYSTAL diff --git a/spec/ameba/rule/performance/compact_after_map_spec.cr b/spec/ameba/rule/performance/compact_after_map_spec.cr index 3d3a7625..9c7a72e3 100644 --- a/spec/ameba/rule/performance/compact_after_map_spec.cr +++ b/spec/ameba/rule/performance/compact_after_map_spec.cr @@ -5,39 +5,35 @@ module Ameba::Rule::Performance describe CompactAfterMap do it "passes if there is no potential performance improvements" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL (1..3).compact_map(&.itself) - ) - subject.catch(source).should be_valid + CRYSTAL end it "passes if there is map followed by a bang call" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL (1..3).map(&.itself).compact! - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is map followed by compact call" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL (1..3).map(&.itself).compact - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `compact_map {...}` instead of `map {...}.compact` + CRYSTAL end it "does not report if source is a spec" do - source = Source.new %( + expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL (1..3).map(&.itself).compact - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end context "macro" do it "doesn't report in macro scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL {{ [1, 2, 3].map(&.to_s).compact }} - ) - subject.catch(source).should be_valid + CRYSTAL end end diff --git a/spec/ameba/rule/performance/first_last_after_filter_spec.cr b/spec/ameba/rule/performance/first_last_after_filter_spec.cr index a4698c69..55823d9c 100644 --- a/spec/ameba/rule/performance/first_last_after_filter_spec.cr +++ b/spec/ameba/rule/performance/first_last_after_filter_spec.cr @@ -5,74 +5,70 @@ module Ameba::Rule::Performance describe FirstLastAfterFilter do it "passes if there is no potential performance improvements" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 1 } [1, 2, 3].reverse.select { |e| e > 1 } [1, 2, 3].reverse.last [1, 2, 3].reverse.first [1, 2, 3].reverse.first - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is select followed by last" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.last - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last` + CRYSTAL end it "does not report if source is a spec" do - source = Source.new %( + expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.last - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is select followed by last?" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.last? - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last?` + CRYSTAL end it "reports if there is select followed by first" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.first - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first` + CRYSTAL end it "does not report if there is selected followed by first with arguments" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL [1, 2, 3].select { |n| n % 2 == 0 }.first(2) - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is select followed by first?" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.first? - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first?` + CRYSTAL end it "does not report if there is select followed by any other call" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.size [1, 2, 3].select { |e| e > 2 }.any? - ) - subject.catch(source).should be_valid + CRYSTAL end context "properties" do it "allows to configure object_call_names" do - source = Source.new %( - [1, 2, 3].select { |e| e > 2 }.first - ) rule = Rule::Performance::FirstLastAfterFilter.new rule.filter_names = %w(reject) - rule.catch(source).should be_valid + + expect_no_issues rule, <<-CRYSTAL + [1, 2, 3].select { |e| e > 2 }.first + CRYSTAL end end @@ -91,58 +87,12 @@ module Ameba::Rule::Performance issue.message.should eq "Use `find {...}` instead of `select {...}.first`" end - it "reports a correct message for first?" do - s = Source.new %( - [1, 2, 3].select { |e| e > 2 }.first? - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:38" - - issue.message.should eq "Use `find {...}` instead of `select {...}.first?`" - end - - it "reports rule, pos and reverse message" do - s = Source.new %( - [1, 2, 3].select { |e| e > 2 }.last - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:36" - - issue.message.should eq "Use `reverse_each.find {...}` instead of `select {...}.last`" - end - context "macro" do it "doesn't report in macro scope" do - source = Source.new %( - {{[1, 2, 3].select { |e| e > 2 }.last }} - ) - subject.catch(source).should be_valid + expect_no_issues subject, <<-CRYSTAL + {{ [1, 2, 3].select { |e| e > 2 }.last }} + CRYSTAL end end - - it "reports a correct message for last?" do - s = Source.new %( - [1, 2, 3].select { |e| e > 2 }.last? - ), "source.cr" - subject.catch(s).should_not be_valid - s.issues.size.should eq 1 - - issue = s.issues.first - issue.rule.should_not be_nil - issue.location.to_s.should eq "source.cr:1:11" - issue.end_location.to_s.should eq "source.cr:1:37" - - issue.message.should eq "Use `reverse_each.find {...}` instead of `select {...}.last?`" - end end end diff --git a/spec/ameba/rule/performance/flatten_after_map_spec.cr b/spec/ameba/rule/performance/flatten_after_map_spec.cr index 14d63814..54796380 100644 --- a/spec/ameba/rule/performance/flatten_after_map_spec.cr +++ b/spec/ameba/rule/performance/flatten_after_map_spec.cr @@ -5,32 +5,29 @@ module Ameba::Rule::Performance describe FlattenAfterMap do it "passes if there is no potential performance improvements" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL %w[Alice Bob].flat_map(&.chars) - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is map followed by flatten call" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL %w[Alice Bob].map(&.chars).flatten - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^ error: Use `flat_map {...}` instead of `map {...}.flatten` + CRYSTAL end it "does not report is source is a spec" do - source = Source.new %( + expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL %w[Alice Bob].map(&.chars).flatten - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end context "macro" do it "doesn't report in macro scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL {{ %w[Alice Bob].map(&.chars).flatten }} - ) - subject.catch(source).should be_valid + CRYSTAL end end diff --git a/spec/ameba/rule/performance/map_instead_of_block_spec.cr b/spec/ameba/rule/performance/map_instead_of_block_spec.cr index d51f8556..95cfd901 100644 --- a/spec/ameba/rule/performance/map_instead_of_block_spec.cr +++ b/spec/ameba/rule/performance/map_instead_of_block_spec.cr @@ -5,47 +5,44 @@ module Ameba::Rule::Performance describe MapInsteadOfBlock do it "passes if there is no potential performance improvements" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL (1..3).sum(&.*(2)) (1..3).product(&.*(2)) - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is map followed by sum without a block" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + CRYSTAL end it "does not report if source is a spec" do - source = Source.new %( + expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL (1..3).map(&.to_s).join - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is map followed by sum without a block (with argument)" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum(0) - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + CRYSTAL end it "reports if there is map followed by sum with a block" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL (1..3).map(&.to_u64).sum(&.itself) - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum` + CRYSTAL end context "macro" do it "doesn't report in macro scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL {{ [1, 2, 3].map(&.to_u64).sum }} - ) - subject.catch(source).should be_valid + CRYSTAL end end diff --git a/spec/ameba/rule/performance/size_after_filter_spec.cr b/spec/ameba/rule/performance/size_after_filter_spec.cr index a467d551..27034bbf 100644 --- a/spec/ameba/rule/performance/size_after_filter_spec.cr +++ b/spec/ameba/rule/performance/size_after_filter_spec.cr @@ -5,7 +5,7 @@ module Ameba::Rule::Performance describe SizeAfterFilter do it "passes if there is no potential performance improvements" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 } [1, 2, 3].reject { |e| e < 2 } [1, 2, 3].count { |e| e > 2 && e.odd? } @@ -13,55 +13,52 @@ module Ameba::Rule::Performance User.select("field AS name").count Company.select(:value).count - ) - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is a select followed by size" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.size - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `select {...}.size`. + CRYSTAL end it "does not report if source is a spec" do - source = Source.new %( + expect_no_issues subject, path: "source_spec.cr", code: <<-CRYSTAL [1, 2, 3].select { |e| e > 2 }.size - ), "source_spec.cr" - subject.catch(source).should be_valid + CRYSTAL end it "reports if there is a reject followed by size" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].reject { |e| e < 2 }.size - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. + CRYSTAL end it "reports if a block shorthand used" do - source = Source.new %( + expect_issue subject, <<-CRYSTAL [1, 2, 3].reject(&.empty?).size - ) - subject.catch(source).should_not be_valid + # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`. + CRYSTAL end context "properties" do it "allows to configure object caller names" do - source = Source.new %( - [1, 2, 3].reject(&.empty?).size - ) rule = Rule::Performance::SizeAfterFilter.new rule.filter_names = %w(select) - rule.catch(source).should be_valid + + expect_no_issues rule, <<-CRYSTAL + [1, 2, 3].reject(&.empty?).size + CRYSTAL end end context "macro" do it "doesn't report in macro scope" do - source = Source.new %( + expect_no_issues subject, <<-CRYSTAL {{[1, 2, 3].select { |v| v > 1 }.size}} - ) - subject.catch(source).should be_valid + CRYSTAL end end diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr index bacf4258..bcae082c 100644 --- a/src/ameba/ast/scope.cr +++ b/src/ameba/ast/scope.cr @@ -71,7 +71,7 @@ module Ameba::AST ivariables << InstanceVariable.new(node) end - # Returns variable by its name or nil if it does not exist. + # Returns variable by its name or `nil` if it does not exist. # # ``` # scope = Scope.new(class_node, nil) @@ -91,13 +91,13 @@ module Ameba::AST find_variable(name).try &.assign(node, self) end - # Returns true if current scope represents a block (or proc), - # false if not. + # Returns `true` if current scope represents a block (or proc), + # `false` otherwise. def block? node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral) end - # Returns true if current scope represents a spawn block, e. g. + # Returns `true` if current scope represents a spawn block, e. g. # # ``` # spawn do @@ -110,18 +110,19 @@ module Ameba::AST call.try(&.name) == "spawn" end - # Returns true if current scope sits inside a macro. + # Returns `true` if current scope sits inside a macro. def in_macro? - node.is_a?(Crystal::Macro) || !!outer_scope.try(&.in_macro?) + (node.is_a?(Crystal::Macro) || node.is_a?(Crystal::MacroFor)) || + !!outer_scope.try(&.in_macro?) end - # Returns true instance variable assinged in this scope. + # Returns `true` if instance variable is assinged in this scope. def assigns_ivar?(name) arguments.find(&.name.== name) && ivariables.find(&.name.== "@#{name}") end - # Returns true if and only if current scope represents some + # Returns `true` if and only if current scope represents some # type definition, for example a class. def type_definition? node.is_a?(Crystal::ClassDef) || @@ -133,7 +134,7 @@ module Ameba::AST end # Returns true if current scope (or any of inner scopes) references variable, - # false if not. + # `false` otherwise. def references?(variable : Variable, check_inner_scopes = true) variable.references.any? do |reference| return true if reference.scope == self @@ -141,17 +142,17 @@ module Ameba::AST end || variable.used_in_macro? end - # Returns true if current scope is a def, false if not. + # Returns `true` if current scope is a def, `false` otherwise. def def? node.is_a?(Crystal::Def) end - # Returns true if this scope is a top level scope, false if not. + # Returns `true` if this scope is a top level scope, `false` otherwise. def top_level? outer_scope.nil? end - # Returns true if var is an argument in current scope, false if not. + # Returns true if var is an argument in current scope, `false` otherwise. def arg?(var) case current_node = node when Crystal::Def @@ -169,7 +170,7 @@ module Ameba::AST args.any? { |arg| arg.name == var.name && arg.location == var.location } end - # Returns true if the `node` represents exactly + # Returns `true` if the *node* represents exactly # the same Crystal node as `@node`. def eql?(node) node == @node && diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 9119b22c..a37eea08 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -46,7 +46,9 @@ module Ameba::AST {% for name in NODES %} # A visit callback for `Crystal::{{name}}` node. - # Returns true meaning that child nodes will be traversed as well. + # + # Returns `true` if the child nodes should be traversed as well, + # `false` otherwise. def visit(node : Crystal::{{name}}) return false if skip?(node) diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index aaae84ea..06931ff3 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -3,19 +3,39 @@ require "./base_visitor" module Ameba::AST # AST Visitor that traverses the source and constructs scopes. class ScopeVisitor < BaseVisitor + # Non-exhaustive list of nodes to be visited by Ameba's rules. + NODES = [ + ClassDef, + ModuleDef, + EnumDef, + LibDef, + FunDef, + TypeDef, + TypeOf, + CStructOrUnionDef, + ProcLiteral, + Block, + Macro, + MacroFor, + ] + SUPER_NODE_NAME = "super" RECORD_NODE_NAME = "record" @scope_queue = [] of Scope @current_scope : Scope + @current_assign : Crystal::ASTNode? + @skip : Array(Crystal::ASTNode.class)? - def initialize(@rule, @source) + def initialize(@rule, @source, skip = nil) + @skip = skip.try &.map(&.as(Crystal::ASTNode.class)) @current_scope = Scope.new(@source.ast) # top level scope @source.ast.accept self @scope_queue.each { |scope| @rule.test @source, scope.node, scope } end private def on_scope_enter(node) + return if skip?(node) @current_scope = Scope.new(node, @current_scope) end @@ -36,73 +56,18 @@ module Ameba::AST on_scope_end(node) if @current_scope.eql?(node) end - # :nodoc: - def visit(node : Crystal::ClassDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::ModuleDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::EnumDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::LibDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::FunDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::TypeDef) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::TypeOf) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::CStructOrUnionDef) - on_scope_enter(node) - end + {% for name in NODES %} + # :nodoc: + def visit(node : Crystal::{{name}}) + on_scope_enter(node) + end + {% end %} # :nodoc: def visit(node : Crystal::Def) node.name == "->" || on_scope_enter(node) end - # :nodoc: - def visit(node : Crystal::ProcLiteral) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::Block) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::Macro) - on_scope_enter(node) - end - - # :nodoc: - def visit(node : Crystal::MacroFor) - on_scope_enter(node) - end - - @current_assign : Crystal::ASTNode? - # :nodoc: def visit(node : Crystal::Assign | Crystal::OpAssign | Crystal::MultiAssign | Crystal::UninitializedVar) @current_assign = node @@ -182,5 +147,9 @@ module Ameba::AST private def record_macro?(node) node.name == RECORD_NODE_NAME && node.args.first?.is_a?(Crystal::Path) end + + private def skip?(node) + !!@skip.try(&.includes?(node.class)) + end end end diff --git a/src/ameba/rule/lint/shadowing_local_outer_var.cr b/src/ameba/rule/lint/shadowing_outer_local_var.cr similarity index 89% rename from src/ameba/rule/lint/shadowing_local_outer_var.cr rename to src/ameba/rule/lint/shadowing_outer_local_var.cr index 8f70a285..d9abaec8 100644 --- a/src/ameba/rule/lint/shadowing_local_outer_var.cr +++ b/src/ameba/rule/lint/shadowing_outer_local_var.cr @@ -39,7 +39,10 @@ module Ameba::Rule::Lint MSG = "Shadowing outer local variable `%s`" def test(source) - AST::ScopeVisitor.new self, source + AST::ScopeVisitor.new self, source, skip: [ + Crystal::Macro, + Crystal::MacroFor, + ] end def test(source, node : Crystal::ProcLiteral, scope : AST::Scope) @@ -51,9 +54,7 @@ module Ameba::Rule::Lint end private def find_shadowing(source, scope) - outer_scope = scope.outer_scope - - return if outer_scope.nil? || outer_scope.in_macro? + return unless outer_scope = scope.outer_scope scope.arguments.reject(&.ignored?).each do |arg| variable = outer_scope.find_variable(arg.name)