Merge branch 'master' into fix/crystal-nightly

This commit is contained in:
Sijawusz Pur Rahnama 2022-04-05 00:37:48 +02:00 committed by GitHub
commit e2faffacfe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 352 additions and 518 deletions

View file

@ -6,13 +6,15 @@ module Ameba::Rule::Layout
describe LineLength do describe LineLength do
it "passes if all lines are shorter than MaxLength symbols" do it "passes if all lines are shorter than MaxLength symbols" do
source = Source.new "short line" expect_no_issues subject, <<-CRYSTAL
subject.catch(source).should be_valid short line
CRYSTAL
end end
it "passes if line consists of MaxLength symbols" do it "passes if line consists of MaxLength symbols" do
source = Source.new "*" * subject.max_length expect_no_issues subject, <<-CRYSTAL
subject.catch(source).should be_valid #{"*" * subject.max_length}
CRYSTAL
end end
it "fails if there is at least one line longer than MaxLength symbols" do 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 context "properties" do
it "allows to configure max length of the line" do it "allows to configure max length of the line" do
source = Source.new long_line
rule = LineLength.new rule = LineLength.new
rule.max_length = long_line.size rule.max_length = long_line.size
rule.catch(source).should be_valid
expect_no_issues rule, long_line
end end
end end
end end

View file

@ -6,163 +6,115 @@ module Ameba::Rule::Lint
context "when using `-`" do context "when using `-`" do
it "registers an offense with `x`" do it "registers an offense with `x`" do
source = Source.new("x =- y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid x =- y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `-=`?
CRYSTAL
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"
end end
it "registers an offense with `@x`" do it "registers an offense with `@x`" do
source = Source.new("@x =- y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @x =- y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `-=`?
CRYSTAL
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"
end end
it "registers an offense with `@@x`" do it "registers an offense with `@@x`" do
source = Source.new("@@x =- y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @@x =- y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `-=`?
CRYSTAL
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"
end end
it "registers an offense with `X`" do it "registers an offense with `X`" do
source = Source.new("X =- y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid X =- y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `-=`?
CRYSTAL
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"
end end
it "does not register an offense when no mistype assignments" do 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 = 1
x -= y x -= y
x = -y x = -y
CRYSTAL CRYSTAL
end end
end end
context "when using `+`" do context "when using `+`" do
it "registers an offense with `x`" do it "registers an offense with `x`" do
source = Source.new("x =+ y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid x =+ y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `+=`?
CRYSTAL
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"
end end
it "registers an offense with `@x`" do it "registers an offense with `@x`" do
source = Source.new("@x =+ y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @x =+ y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `+=`?
CRYSTAL
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"
end end
it "registers an offense with `@@x`" do it "registers an offense with `@@x`" do
source = Source.new("@@x =+ y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @@x =+ y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `+=`?
CRYSTAL
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"
end end
it "registers an offense with `X`" do it "registers an offense with `X`" do
source = Source.new("X =+ y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid X =+ y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `+=`?
CRYSTAL
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"
end end
it "does not register an offense when no mistype assignments" do 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 = 1
x += y x += y
x = +y x = +y
CRYSTAL CRYSTAL
end end
end end
context "when using `!`" do context "when using `!`" do
it "registers an offense with `x`" do it "registers an offense with `x`" do
source = Source.new("x =! y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid x =! y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `!=`?
CRYSTAL
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"
end end
it "registers an offense with `@x`" do it "registers an offense with `@x`" do
source = Source.new("@x =! y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @x =! y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `!=`?
CRYSTAL
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"
end end
it "registers an offense with `@@x`" do it "registers an offense with `@@x`" do
source = Source.new("@@x =! y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid @@x =! y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `!=`?
CRYSTAL
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"
end end
it "registers an offense with `X`" do it "registers an offense with `X`" do
source = Source.new("X =! y", "source.cr") expect_issue subject, <<-CRYSTAL
subject.catch(source).should_not be_valid X =! y
source.issues.size.should eq 1 # ^^ error: Suspicious assignment detected. Did you mean `!=`?
CRYSTAL
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"
end end
it "does not register an offense when no mistype assignments" do 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 = false
x != y x != y
x = !y x = !y
CRYSTAL CRYSTAL
end end
end end
end end

View file

@ -5,62 +5,42 @@ module Ameba::Rule::Lint
subject = BadDirective.new subject = BadDirective.new
it "does not report if rule is correct" do it "does not report if rule is correct" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
# ameba:disable Lint/BadDirective # ameba:disable Lint/BadDirective
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is incorrect action" do it "reports if there is incorrect action" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
# ameba:foo Lint/BadDirective # ameba:foo Lint/BadDirective
), "source.cr" # ^{} error: Bad action in comment directive: 'foo'. Possible values: disable, enable
subject.catch(s).should_not be_valid CRYSTAL
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 ""
end end
it "reports if there are incorrect rule names" do it "reports if there are incorrect rule names" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
# ameba:enable BadRule1, BadRule2 # ameba:enable BadRule1, BadRule2
), "source.cr" # ^{} error: Such rules do not exist: BadRule1, BadRule2
subject.catch(s).should_not be_valid CRYSTAL
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 ""
end end
it "does not report if there no action and rules at all" do it "does not report if there no action and rules at all" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
# ameba: # ameba:
) CRYSTAL
subject.catch(s).should be_valid
end end
it "does not report if there are no rules" do it "does not report if there are no rules" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
# ameba:enable # ameba:enable
# ameba:disable # ameba:disable
) CRYSTAL
subject.catch(s).should be_valid
end end
it "does not report if there are group names in the directive" do 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 # ameba:disable Style Performance
) CRYSTAL
subject.catch(s).should be_valid
end end
end end
end end

View file

@ -5,7 +5,7 @@ module Ameba::Rule::Lint
subject = EmptyLoop.new subject = EmptyLoop.new
it "does not report if there are not empty loops" do it "does not report if there are not empty loops" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = 1 a = 1
while a < 10 while a < 10
@ -19,54 +19,50 @@ module Ameba::Rule::Lint
loop do loop do
a += 1 a += 1
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is an empty while loop" do it "reports if there is an empty while loop" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
a = 1 a = 1
while true while true
# ^^^^^^^^ error: Empty loop detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "doesn't report if while loop has non-literals in cond block" do it "doesn't report if while loop has non-literals in cond block" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = 1 a = 1
while a = gets.to_s while a = gets.to_s
# nothing here # nothing here
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there is an empty until loop" do it "reports if there is an empty until loop" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
do_something do_something
until false until false
# ^^^^^^^^^ error: Empty loop detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "doesn't report if until loop has non-literals in cond block" do 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? until socket_open?
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports if there an empty loop" do it "reports if there an empty loop" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
a = 1 a = 1
loop do loop do
# ^^^^^ error: Empty loop detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, message and location" do it "reports rule, message and location" do

View file

@ -5,26 +5,32 @@ module Ameba::Rule::Lint
subject = HashDuplicatedKey.new subject = HashDuplicatedKey.new
it "passes if there is no duplicated keys in a hash literals" do 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, :a => 2, "b" => 3}
h = {"a" => 1, "b" => 2, "c" => {"a" => 3, "b" => 4}} h = {"a" => 1, "b" => 2, "c" => {"a" => 3, "b" => 4}}
h = {} of String => String h = {} of String => String
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is a duplicated key in a hash literal" do 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} h = {"a" => 1, "b" => 2, "a" => 3}
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "a"
subject.catch(s).should_not be_valid CRYSTAL
end end
it "fails if there is a duplicated key in the inner hash literal" do 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}} h = {"a" => 1, "b" => {"a" => 3, "b" => 4, "a" => 5}}
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Duplicated keys in hash literal: "a"
subject.catch(s).should_not be_valid 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 end
it "reports rule, location and message" do 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.end_location.to_s.should eq "source.cr:1:24"
issue.message.should eq %(Duplicated keys in hash literal: "a") issue.message.should eq %(Duplicated keys in hash literal: "a")
end 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
end end

View file

@ -5,15 +5,14 @@ module Ameba::Rule::Lint
describe LiteralInInterpolation do describe LiteralInInterpolation do
it "passes with good interpolation examples" do it "passes with good interpolation examples" do
s = Source.new %q( expect_no_issues subject, <<-CRYSTAL
name = "Ary" name = "Ary"
"Hello, #{name}" "Hello, #{name}"
"#{name}" "#{name}"
"Name size: #{name.size}" "Name size: #{name.size}"
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is useless interpolation" do it "fails if there is useless interpolation" do

View file

@ -5,22 +5,25 @@ module Ameba::Rule::Lint
subject = RandZero.new subject = RandZero.new
it "passes if it is not rand(1) or rand(0)" do it "passes if it is not rand(1) or rand(0)" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
rand(1.0) rand(1.0)
rand(0.11) rand(0.11)
rand(2) rand(2)
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if it is rand(0)" do it "fails if it is rand(0)" do
s = Source.new "rand(0)" expect_issue subject, <<-CRYSTAL
subject.catch(s).should_not be_valid rand(0)
# ^^^^^ error: rand(0) always returns 0
CRYSTAL
end end
it "fails if it is rand(1)" do it "fails if it is rand(1)" do
s = Source.new "rand(1)" expect_issue subject, <<-CRYSTAL
subject.catch(s).should_not be_valid rand(1)
# ^^^^^ error: rand(1) always returns 0
CRYSTAL
end end
it "reports rule, location and a message" do it "reports rule, location and a message" do

View file

@ -5,7 +5,7 @@ module Ameba::Rule::Lint
subject = ShadowingOuterLocalVar.new subject = ShadowingOuterLocalVar.new
it "doesn't report if there is no shadowing" do it "doesn't report if there is no shadowing" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
def some_method def some_method
foo = 1 foo = 1
@ -14,126 +14,117 @@ module Ameba::Rule::Lint
end end
-> (baz : Int32) {} -> (baz : Int32) {}
-> (bar : String) {} -> (bar : String) {}
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a shadowing in a block" do it "reports if there is a shadowing in a block" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
def some_method def some_method
foo = 1 foo = 1
3.times do |foo| 3.times do |foo|
# ^ error: Shadowing outer local variable `foo`
end end
end end
) CRYSTAL
subject.catch(source).should_not be_valid
end end
it "does not report outer vars declared below shadowed block" do 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) } methods = klass.methods.select { |m| m.annotation(MyAnn) }
m = methods.last m = methods.last
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a shadowing in a proc" do it "reports if there is a shadowing in a proc" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
def some_method def some_method
foo = 1 foo = 1
-> (foo : Int32) {} -> (foo : Int32) {}
# ^ error: Shadowing outer local variable `foo`
end end
) CRYSTAL
subject.catch(source).should_not be_valid
end end
it "reports if there is a shadowing in an inner scope" do it "reports if there is a shadowing in an inner scope" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
def foo def foo
foo = 1 foo = 1
3.times do |i| 3.times do |i|
3.times { |foo| foo } 3.times { |foo| foo }
# ^ error: Shadowing outer local variable `foo`
end end
end end
) CRYSTAL
subject.catch(source).should_not be_valid
end end
it "reports if variable is shadowed twice" do it "reports if variable is shadowed twice" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
foo = 1 foo = 1
3.times do |foo| 3.times do |foo|
# ^ error: Shadowing outer local variable `foo`
-> (foo : Int32) { foo + 1 } -> (foo : Int32) { foo + 1 }
# ^ error: Shadowing outer local variable `foo`
end end
) CRYSTAL
subject.catch(source).should_not be_valid
source.issues.size.should eq 2
end end
it "reports if a splat block argument shadows local var" do it "reports if a splat block argument shadows local var" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
foo = 1 foo = 1
3.times do |*foo| 3.times do |*foo|
# ^ error: Shadowing outer local variable `foo`
end end
) CRYSTAL
subject.catch(source).should_not be_valid
end end
it "reports if a &block argument is shadowed" do it "reports if a &block argument is shadowed" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
def method_with_block(a, &block) def method_with_block(a, &block)
3.times do |block| 3.times do |block|
# ^ error: Shadowing outer local variable `block`
end end
end end
) CRYSTAL
subject.catch(source).should_not be_valid
source.issues.first.message.should eq "Shadowing outer local variable `block`"
end end
it "reports if there are multiple args and one shadows local var" do it "reports if there are multiple args and one shadows local var" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
foo = 1 foo = 1
[1, 2, 3].each_with_index do |i, foo| [1, 2, 3].each_with_index do |i, foo|
# ^ error: Shadowing outer local variable `foo`
i + foo i + foo
end end
) CRYSTAL
subject.catch(source).should_not be_valid
source.issues.first.message.should eq "Shadowing outer local variable `foo`"
end end
it "doesn't report if an outer var is reassigned in a block" do it "doesn't report if an outer var is reassigned in a block" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
def foo def foo
foo = 1 foo = 1
3.times do |i| 3.times do |i|
foo = 2 foo = 2
end end
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "doesn't report if an argument is a black hole '_'" do it "doesn't report if an argument is a black hole '_'" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
_ = 1 _ = 1
3.times do |_| 3.times do |_|
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "doesn't report if it shadows record type declaration" do it "doesn't report if it shadows record type declaration" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
class FooBar class FooBar
record Foo, index : String record Foo, index : String
@ -142,12 +133,11 @@ module Ameba::Rule::Lint
end end
end end
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "doesn't report if it shadows throwaway arguments" do 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 = [{1, "a"}, {2, "b"}, {3, "c"}]
data.each do |_, string| data.each do |_, string|
@ -155,18 +145,16 @@ module Ameba::Rule::Lint
puts string, number puts string, number
end end
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "does not report if argument shadows an ivar assignment" do it "does not report if argument shadows an ivar assignment" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def bar(@foo) def bar(@foo)
@foo.try do |foo| @foo.try do |foo|
end end
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "reports rule, location and message" do it "reports rule, location and message" do
@ -185,7 +173,7 @@ module Ameba::Rule::Lint
context "macro" do context "macro" do
it "does not report shadowed vars in outer scope" do it "does not report shadowed vars in outer scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
macro included macro included
def foo def foo
{% for ivar in instance_vars %} {% for ivar in instance_vars %}
@ -197,12 +185,11 @@ module Ameba::Rule::Lint
{% instance_vars.reject { |ivar| ivar } %} {% instance_vars.reject { |ivar| ivar } %}
end end
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "does not report shadowed vars in macro within the same scope" do 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) } %} {% methods = klass.methods.select { |m| m.annotation(MyAnn) } %}
{% for m, m_idx in methods %} {% for m, m_idx in methods %}
@ -210,12 +197,11 @@ module Ameba::Rule::Lint
{% d %} {% d %}
{% end %} {% end %}
{% end %} {% end %}
) CRYSTAL
subject.catch(source).should be_valid
end end
it "does not report shadowed vars within nested macro" do it "does not report shadowed vars within nested macro" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
module Foo module Foo
macro included macro included
def foo def foo
@ -233,12 +219,11 @@ module Ameba::Rule::Lint
end end
end end
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "does not report scoped vars to MacroFor" do it "does not report scoped vars to MacroFor" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
struct Test struct Test
def test def test
{% for ivar in @type.instance_vars %} {% for ivar in @type.instance_vars %}
@ -248,8 +233,24 @@ module Ameba::Rule::Lint
{% ["a", "b"].map { |ivar| puts ivar } %} {% ["a", "b"].map { |ivar| puts ivar } %}
end end
end end
) CRYSTAL
subject.catch(source).should be_valid 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 end
end end

View file

@ -5,23 +5,22 @@ module Ameba::Rule::Lint
subject = Syntax.new subject = Syntax.new
it "passes if there is no invalid syntax" do it "passes if there is no invalid syntax" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
def hello def hello
puts "totally valid" puts "totally valid"
rescue e: Exception rescue e: Exception
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is an invalid syntax" do it "fails if there is an invalid syntax" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
def hello def hello
puts "invalid" puts "invalid"
rescue Exception => e rescue Exception => e
# ^ error: expecting any of these tokens: ;, NEWLINE (not '=>')
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, location and message" do it "reports rule, location and message" do

View file

@ -5,17 +5,15 @@ module Ameba::Rule::Lint
subject = UnneededDisableDirective.new subject = UnneededDisableDirective.new
it "passes if there are no comments" do it "passes if there are no comments" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = 1 a = 1
) CRYSTAL
subject.catch(s).should be_valid
end end
it "passes if there is disable directive" do it "passes if there is disable directive" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
a = 1 # my super var a = 1 # my super var
) CRYSTAL
subject.catch(s).should be_valid
end end
it "doesn't report if there is disable directive and it is needed" do it "doesn't report if there is disable directive and it is needed" do
@ -48,33 +46,27 @@ module Ameba::Rule::Lint
end end
it "fails if there is unneeded directive" do it "fails if there is unneeded directive" do
s = Source.new %Q( expect_issue subject, <<-CRYSTAL
# ameba:disable #{NamedRule.name} # ameba:disable #{NamedRule.name}
# ^{} error: Unnecessary disabling of #{NamedRule.name}
a = 1 a = 1
) CRYSTAL
subject.catch(s).should_not be_valid
s.issues.first.message.should eq(
"Unnecessary disabling of #{NamedRule.name}"
)
end end
it "fails if there is inline unneeded directive" do it "fails if there is inline unneeded directive" do
s = Source.new %Q(a = 1 # ameba:disable #{NamedRule.name}) expect_issue subject, <<-CRYSTAL
subject.catch(s).should_not be_valid a = 1 # ameba:disable #{NamedRule.name}
s.issues.first.message.should eq( # ^ error: Unnecessary disabling of #{NamedRule.name}
"Unnecessary disabling of #{NamedRule.name}" CRYSTAL
)
end end
it "detects mixed inline directives" do it "detects mixed inline directives" do
s = Source.new %Q( expect_issue subject, <<-CRYSTAL
# ameba:disable Rule1, Rule2 # ameba:disable Rule1, Rule2
# ^{} error: Unnecessary disabling of Rule1, Rule2
a = 1 # ameba:disable Rule3 a = 1 # ameba:disable Rule3
), "source.cr" # ^ error: Unnecessary disabling of Rule3
subject.catch(s).should_not be_valid CRYSTAL
s.issues.size.should eq 2
s.issues.first.message.should contain "Rule1, Rule2"
s.issues.last.message.should contain "Rule3"
end end
it "fails if there is disabled UnneededDisableDirective" do it "fails if there is disabled UnneededDisableDirective" do

View file

@ -5,25 +5,24 @@ module Ameba::Rule::Lint
describe UselessConditionInWhen do describe UselessConditionInWhen do
it "passes if there is not useless condition" do it "passes if there is not useless condition" do
s = Source.new %( expect_no_issues subject, <<-CRYSTAL
case case
when utc? when utc?
io << " UTC" io << " UTC"
when local? when local?
Format.new(" %:z").format(self, io) if utc? Format.new(" %:z").format(self, io) if utc?
end end
) CRYSTAL
subject.catch(s).should be_valid
end end
it "fails if there is useless if condition" do it "fails if there is useless if condition" do
s = Source.new %( expect_issue subject, <<-CRYSTAL
case case
when utc? when utc?
io << " UTC" if utc? io << " UTC" if utc?
# ^^^^ error: Useless condition in when detected
end end
) CRYSTAL
subject.catch(s).should_not be_valid
end end
it "reports rule, location and message" do it "reports rule, location and message" do

View file

@ -15,33 +15,35 @@ module Ameba::Rule::Metrics
end end
end end
end end
CODE CODE
describe CyclomaticComplexity do describe CyclomaticComplexity do
it "passes for empty methods" do it "passes for empty methods" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
def hello def hello
end end
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports one issue for a complex method" do 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") 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 = source.issues.first
issue.rule.should eq subject issue.rule.should eq rule
issue.location.to_s.should eq "source.cr:1:5" issue.location.to_s.should eq "source.cr:1:5"
issue.end_location.to_s.should eq "source.cr:1:9" issue.end_location.to_s.should eq "source.cr:1:9"
issue.message.should eq "Cyclomatic complexity too high [8/5]" issue.message.should eq "Cyclomatic complexity too high [8/5]"
end end
it "doesn't report an issue for an increased threshold" do it "doesn't report an issue for an increased threshold" do
subject.max_complexity = 100 rule = CyclomaticComplexity.new
source = Source.new(complex_method, "source.cr") rule.max_complexity = 100
subject.catch(source).should be_valid
expect_no_issues rule, complex_method
end end
end end
end end

View file

@ -49,6 +49,7 @@ module Ameba::Rule::Performance
it "allows to configure object_call_names" do it "allows to configure object_call_names" do
rule = Rule::Performance::AnyAfterFilter.new rule = Rule::Performance::AnyAfterFilter.new
rule.filter_names = %w(select) rule.filter_names = %w(select)
expect_no_issues rule, <<-CRYSTAL expect_no_issues rule, <<-CRYSTAL
[1, 2, 3].reject { |e| e > 2 }.any? [1, 2, 3].reject { |e| e > 2 }.any?
CRYSTAL CRYSTAL

View file

@ -47,6 +47,7 @@ module Ameba::Rule::Performance
it "allows to configure `call_names`" do it "allows to configure `call_names`" do
rule = ChainedCallWithNoBang.new rule = ChainedCallWithNoBang.new
rule.call_names = %w(uniq) rule.call_names = %w(uniq)
expect_no_issues rule, <<-CRYSTAL expect_no_issues rule, <<-CRYSTAL
[1, 2, 3].select { |e| e > 2 }.reverse [1, 2, 3].select { |e| e > 2 }.reverse
CRYSTAL CRYSTAL

View file

@ -5,39 +5,35 @@ module Ameba::Rule::Performance
describe CompactAfterMap do describe CompactAfterMap do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).compact_map(&.itself) (1..3).compact_map(&.itself)
) CRYSTAL
subject.catch(source).should be_valid
end end
it "passes if there is map followed by a bang call" do 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! (1..3).map(&.itself).compact!
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is map followed by compact call" do it "reports if there is map followed by compact call" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
(1..3).map(&.itself).compact (1..3).map(&.itself).compact
) # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `compact_map {...}` instead of `map {...}.compact`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if source is a spec" do 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 (1..3).map(&.itself).compact
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
context "macro" do context "macro" do
it "doesn't report in macro scope" do it "doesn't report in macro scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
{{ [1, 2, 3].map(&.to_s).compact }} {{ [1, 2, 3].map(&.to_s).compact }}
) CRYSTAL
subject.catch(source).should be_valid
end end
end end

View file

@ -5,74 +5,70 @@ module Ameba::Rule::Performance
describe FirstLastAfterFilter do describe FirstLastAfterFilter do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
[1, 2, 3].select { |e| e > 1 } [1, 2, 3].select { |e| e > 1 }
[1, 2, 3].reverse.select { |e| e > 1 } [1, 2, 3].reverse.select { |e| e > 1 }
[1, 2, 3].reverse.last [1, 2, 3].reverse.last
[1, 2, 3].reverse.first [1, 2, 3].reverse.first
[1, 2, 3].reverse.first [1, 2, 3].reverse.first
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is select followed by last" do 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 [1, 2, 3].select { |e| e > 2 }.last
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if source is a spec" do 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 [1, 2, 3].select { |e| e > 2 }.last
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is select followed by last?" do 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? [1, 2, 3].select { |e| e > 2 }.last?
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `reverse_each.find {...}` instead of `select {...}.last?`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is select followed by first" do 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 [1, 2, 3].select { |e| e > 2 }.first
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if there is selected followed by first with arguments" do 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) [1, 2, 3].select { |n| n % 2 == 0 }.first(2)
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is select followed by first?" do 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? [1, 2, 3].select { |e| e > 2 }.first?
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `find {...}` instead of `select {...}.first?`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if there is select followed by any other call" do 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 }.size
[1, 2, 3].select { |e| e > 2 }.any? [1, 2, 3].select { |e| e > 2 }.any?
) CRYSTAL
subject.catch(source).should be_valid
end end
context "properties" do context "properties" do
it "allows to configure object_call_names" 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 = Rule::Performance::FirstLastAfterFilter.new
rule.filter_names = %w(reject) 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
end end
@ -91,58 +87,12 @@ module Ameba::Rule::Performance
issue.message.should eq "Use `find {...}` instead of `select {...}.first`" issue.message.should eq "Use `find {...}` instead of `select {...}.first`"
end 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 context "macro" do
it "doesn't report in macro scope" do it "doesn't report in macro scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
{{[1, 2, 3].select { |e| e > 2 }.last }} {{ [1, 2, 3].select { |e| e > 2 }.last }}
) CRYSTAL
subject.catch(source).should be_valid
end end
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
end end

View file

@ -5,32 +5,29 @@ module Ameba::Rule::Performance
describe FlattenAfterMap do describe FlattenAfterMap do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
%w[Alice Bob].flat_map(&.chars) %w[Alice Bob].flat_map(&.chars)
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is map followed by flatten call" do it "reports if there is map followed by flatten call" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
%w[Alice Bob].map(&.chars).flatten %w[Alice Bob].map(&.chars).flatten
) # ^^^^^^^^^^^^^^^^^^^^^ error: Use `flat_map {...}` instead of `map {...}.flatten`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report is source is a spec" do 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 %w[Alice Bob].map(&.chars).flatten
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
context "macro" do context "macro" do
it "doesn't report in macro scope" do it "doesn't report in macro scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
{{ %w[Alice Bob].map(&.chars).flatten }} {{ %w[Alice Bob].map(&.chars).flatten }}
) CRYSTAL
subject.catch(source).should be_valid
end end
end end

View file

@ -5,47 +5,44 @@ module Ameba::Rule::Performance
describe MapInsteadOfBlock do describe MapInsteadOfBlock do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
(1..3).sum(&.*(2)) (1..3).sum(&.*(2))
(1..3).product(&.*(2)) (1..3).product(&.*(2))
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is map followed by sum without a block" do 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 (1..3).map(&.to_u64).sum
) # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if source is a spec" do 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 (1..3).map(&.to_s).join
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is map followed by sum without a block (with argument)" do 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) (1..3).map(&.to_u64).sum(0)
) # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum`
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if there is map followed by sum with a block" do 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) (1..3).map(&.to_u64).sum(&.itself)
) # ^^^^^^^^^^^^^^^^^^ error: Use `sum {...}` instead of `map {...}.sum`
subject.catch(source).should_not be_valid CRYSTAL
end end
context "macro" do context "macro" do
it "doesn't report in macro scope" do it "doesn't report in macro scope" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
{{ [1, 2, 3].map(&.to_u64).sum }} {{ [1, 2, 3].map(&.to_u64).sum }}
) CRYSTAL
subject.catch(source).should be_valid
end end
end end

View file

@ -5,7 +5,7 @@ module Ameba::Rule::Performance
describe SizeAfterFilter do describe SizeAfterFilter do
it "passes if there is no potential performance improvements" do it "passes if there is no potential performance improvements" do
source = Source.new %( expect_no_issues subject, <<-CRYSTAL
[1, 2, 3].select { |e| e > 2 } [1, 2, 3].select { |e| e > 2 }
[1, 2, 3].reject { |e| e < 2 } [1, 2, 3].reject { |e| e < 2 }
[1, 2, 3].count { |e| e > 2 && e.odd? } [1, 2, 3].count { |e| e > 2 && e.odd? }
@ -13,55 +13,52 @@ module Ameba::Rule::Performance
User.select("field AS name").count User.select("field AS name").count
Company.select(:value).count Company.select(:value).count
) CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a select followed by size" do 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 [1, 2, 3].select { |e| e > 2 }.size
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `select {...}.size`.
subject.catch(source).should_not be_valid CRYSTAL
end end
it "does not report if source is a spec" do 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 [1, 2, 3].select { |e| e > 2 }.size
), "source_spec.cr" CRYSTAL
subject.catch(source).should be_valid
end end
it "reports if there is a reject followed by size" do 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 [1, 2, 3].reject { |e| e < 2 }.size
) # ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`.
subject.catch(source).should_not be_valid CRYSTAL
end end
it "reports if a block shorthand used" do it "reports if a block shorthand used" do
source = Source.new %( expect_issue subject, <<-CRYSTAL
[1, 2, 3].reject(&.empty?).size [1, 2, 3].reject(&.empty?).size
) # ^^^^^^^^^^^^^^^^^^^^^^ error: Use `count {...}` instead of `reject {...}.size`.
subject.catch(source).should_not be_valid CRYSTAL
end end
context "properties" do context "properties" do
it "allows to configure object caller names" do it "allows to configure object caller names" do
source = Source.new %(
[1, 2, 3].reject(&.empty?).size
)
rule = Rule::Performance::SizeAfterFilter.new rule = Rule::Performance::SizeAfterFilter.new
rule.filter_names = %w(select) 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
end end
context "macro" do context "macro" do
it "doesn't report in macro scope" 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}} {{[1, 2, 3].select { |v| v > 1 }.size}}
) CRYSTAL
subject.catch(source).should be_valid
end end
end end

View file

@ -71,7 +71,7 @@ module Ameba::AST
ivariables << InstanceVariable.new(node) ivariables << InstanceVariable.new(node)
end 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) # scope = Scope.new(class_node, nil)
@ -91,13 +91,13 @@ module Ameba::AST
find_variable(name).try &.assign(node, self) find_variable(name).try &.assign(node, self)
end end
# Returns true if current scope represents a block (or proc), # Returns `true` if current scope represents a block (or proc),
# false if not. # `false` otherwise.
def block? def block?
node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral) node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral)
end 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 # spawn do
@ -110,18 +110,19 @@ module Ameba::AST
call.try(&.name) == "spawn" call.try(&.name) == "spawn"
end end
# Returns true if current scope sits inside a macro. # Returns `true` if current scope sits inside a macro.
def in_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 end
# Returns true instance variable assinged in this scope. # Returns `true` if instance variable is assinged in this scope.
def assigns_ivar?(name) def assigns_ivar?(name)
arguments.find(&.name.== name) && arguments.find(&.name.== name) &&
ivariables.find(&.name.== "@#{name}") ivariables.find(&.name.== "@#{name}")
end 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. # type definition, for example a class.
def type_definition? def type_definition?
node.is_a?(Crystal::ClassDef) || node.is_a?(Crystal::ClassDef) ||
@ -133,7 +134,7 @@ module Ameba::AST
end end
# Returns true if current scope (or any of inner scopes) references variable, # 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) def references?(variable : Variable, check_inner_scopes = true)
variable.references.any? do |reference| variable.references.any? do |reference|
return true if reference.scope == self return true if reference.scope == self
@ -141,17 +142,17 @@ module Ameba::AST
end || variable.used_in_macro? end || variable.used_in_macro?
end end
# Returns true if current scope is a def, false if not. # Returns `true` if current scope is a def, `false` otherwise.
def def? def def?
node.is_a?(Crystal::Def) node.is_a?(Crystal::Def)
end 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? def top_level?
outer_scope.nil? outer_scope.nil?
end 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) def arg?(var)
case current_node = node case current_node = node
when Crystal::Def when Crystal::Def
@ -169,7 +170,7 @@ module Ameba::AST
args.any? { |arg| arg.name == var.name && arg.location == var.location } args.any? { |arg| arg.name == var.name && arg.location == var.location }
end end
# Returns true if the `node` represents exactly # Returns `true` if the *node* represents exactly
# the same Crystal node as `@node`. # the same Crystal node as `@node`.
def eql?(node) def eql?(node)
node == @node && node == @node &&

View file

@ -46,7 +46,9 @@ module Ameba::AST
{% for name in NODES %} {% for name in NODES %}
# A visit callback for `Crystal::{{name}}` node. # 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}}) def visit(node : Crystal::{{name}})
return false if skip?(node) return false if skip?(node)

View file

@ -3,19 +3,39 @@ require "./base_visitor"
module Ameba::AST module Ameba::AST
# AST Visitor that traverses the source and constructs scopes. # AST Visitor that traverses the source and constructs scopes.
class ScopeVisitor < BaseVisitor 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" SUPER_NODE_NAME = "super"
RECORD_NODE_NAME = "record" RECORD_NODE_NAME = "record"
@scope_queue = [] of Scope @scope_queue = [] of Scope
@current_scope : 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 @current_scope = Scope.new(@source.ast) # top level scope
@source.ast.accept self @source.ast.accept self
@scope_queue.each { |scope| @rule.test @source, scope.node, scope } @scope_queue.each { |scope| @rule.test @source, scope.node, scope }
end end
private def on_scope_enter(node) private def on_scope_enter(node)
return if skip?(node)
@current_scope = Scope.new(node, @current_scope) @current_scope = Scope.new(node, @current_scope)
end end
@ -36,73 +56,18 @@ module Ameba::AST
on_scope_end(node) if @current_scope.eql?(node) on_scope_end(node) if @current_scope.eql?(node)
end end
# :nodoc: {% for name in NODES %}
def visit(node : Crystal::ClassDef) # :nodoc:
on_scope_enter(node) def visit(node : Crystal::{{name}})
end on_scope_enter(node)
end
# :nodoc: {% end %}
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
# :nodoc: # :nodoc:
def visit(node : Crystal::Def) def visit(node : Crystal::Def)
node.name == "->" || on_scope_enter(node) node.name == "->" || on_scope_enter(node)
end 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: # :nodoc:
def visit(node : Crystal::Assign | Crystal::OpAssign | Crystal::MultiAssign | Crystal::UninitializedVar) def visit(node : Crystal::Assign | Crystal::OpAssign | Crystal::MultiAssign | Crystal::UninitializedVar)
@current_assign = node @current_assign = node
@ -182,5 +147,9 @@ module Ameba::AST
private def record_macro?(node) private def record_macro?(node)
node.name == RECORD_NODE_NAME && node.args.first?.is_a?(Crystal::Path) node.name == RECORD_NODE_NAME && node.args.first?.is_a?(Crystal::Path)
end end
private def skip?(node)
!!@skip.try(&.includes?(node.class))
end
end end
end end

View file

@ -39,7 +39,10 @@ module Ameba::Rule::Lint
MSG = "Shadowing outer local variable `%s`" MSG = "Shadowing outer local variable `%s`"
def test(source) def test(source)
AST::ScopeVisitor.new self, source AST::ScopeVisitor.new self, source, skip: [
Crystal::Macro,
Crystal::MacroFor,
]
end end
def test(source, node : Crystal::ProcLiteral, scope : AST::Scope) def test(source, node : Crystal::ProcLiteral, scope : AST::Scope)
@ -51,9 +54,7 @@ module Ameba::Rule::Lint
end end
private def find_shadowing(source, scope) private def find_shadowing(source, scope)
outer_scope = scope.outer_scope return unless outer_scope = scope.outer_scope
return if outer_scope.nil? || outer_scope.in_macro?
scope.arguments.reject(&.ignored?).each do |arg| scope.arguments.reject(&.ignored?).each do |arg|
variable = outer_scope.find_variable(arg.name) variable = outer_scope.find_variable(arg.name)