diff --git a/spec/ameba/ast/branch_spec.cr b/spec/ameba/ast/branch_spec.cr new file mode 100644 index 00000000..484ffe8c --- /dev/null +++ b/spec/ameba/ast/branch_spec.cr @@ -0,0 +1,364 @@ +require "../../spec_helper" + +private def branch_of_assign_in_def(source) + nodes = as_nodes source + Ameba::AST::Branch.of(nodes.assign_nodes.first, nodes.def_nodes.first) +end + +module Ameba::AST + describe Branch do + describe ".of" do + context "Crystal::If" do + it "constructs a branch in If.cond" do + branch = branch_of_assign_in_def %( + def method + if a = get_something # --> Crystal::Assign + puts a + end + end + ) + branch.to_s.should eq "a = get_something" + end + + it "constructs a branch in If.then" do + branch = branch_of_assign_in_def %( + def method + if true + a = 2 # --> Crystal::Assign + end + end + ) + branch.to_s.should eq "a = 2" + end + + it "constructs a branch in If.else" do + branch = branch_of_assign_in_def %( + def method + if true + nil + else + a = 2 # --> Crystal::Assign + end + end + ) + branch.to_s.should eq "a = 2" + end + + it "constructs a branch in inline If" do + branch = branch_of_assign_in_def %( + def method(a) + a = 0 if a == 2 # --> Crystal::Assign + end + ) + branch.to_s.should eq "a = 0" + end + end + + context "Crystal::Unless" do + it "constructs a branch in Unless.cond" do + branch = branch_of_assign_in_def %( + def method + unless a = get_something # --> Crystal::Assign + puts a + end + end + ) + branch.to_s.should eq "a = get_something" + end + + it "constructs a branch in Unless.then" do + branch = branch_of_assign_in_def %( + def method + unless true + a = 2 # --> Crystal::Assign + end + end + ) + branch.to_s.should eq "a = 2" + end + + it "constructs a new branch in Unless.else" do + branch = branch_of_assign_in_def %( + def method + unless true + nil + else + a = 2 # --> Crystal::Assign + end + end + ) + branch.to_s.should eq "a = 2" + end + + it "constructs a branch in inline Unless" do + branch = branch_of_assign_in_def %( + def method(a) + (a = 0; b = 3) unless a == 2 # --> Crystal::Expressions + end + ) + branch.to_s.should eq "(a = 0\nb = 3)" + end + end + + context "Crystal::BinaryOp" do + it "constructs a branch in left node" do + branch = branch_of_assign_in_def %( + def method(a) + (a = 2) && do_something + end + ) + branch.to_s.should eq "(a = 2)" + end + + it "constructs a branch in right node" do + branch = branch_of_assign_in_def %( + def method(a) + do_something || (a = 0) + end + ) + branch.to_s.should eq "(a = 0)" + end + end + + context "Crystal::Case" do + # FIXME: + # https://github.com/crystal-lang/crystal/pull/6032 + # it "constructs a branch in cond" do + # branch = branch_of_assign_in_def %( + # def method(a) + # case (a = 2) + # when true then nil + # end + # end + # ) + # branch.to_s.should eq "(a = 2)" + # end + + it "constructs a branch in when" do + branch = branch_of_assign_in_def %( + def method(a) + case a + when a = 3 then nil + end + end + ) + branch.to_s.should eq "when a = 3\n nil\n" + end + + it "constructs a branch in else" do + branch = branch_of_assign_in_def %( + def method(a) + case a + when true then nil + else a = 4 + end + end + ) + branch.to_s.should eq "a = 4" + end + end + + context "Crystal::While" do + it "constructs a branch in cond" do + branch = branch_of_assign_in_def %( + def method(a) + while a = 1 + nil + end + end + ) + branch.to_s.should eq "a = 1" + end + + it "constructs a branch in body" do + branch = branch_of_assign_in_def %( + def method(a) + while true + b = (a = 1) + end + end + ) + branch.to_s.should eq "b = (a = 1)" + end + end + + context "Crystal::Until" do + it "constructs a branch in cond" do + branch = branch_of_assign_in_def %( + def method(a) + until a = 1 + nil + end + end + ) + branch.to_s.should eq "a = 1" + end + + it "constructs a branch in body" do + branch = branch_of_assign_in_def %( + def method(a) + until false + b = (a = 1) + end + end + ) + branch.to_s.should eq "b = (a = 1)" + end + end + + context "Crystal::ExceptionHandler" do + it "constructs a branch in body" do + branch = branch_of_assign_in_def %( + def method(a) + a = 1 + rescue + nil + end + ) + branch.to_s.should eq "a = 1" + end + + it "constructs a branch in a rescue" do + branch = branch_of_assign_in_def %( + def method(a) + rescue + a = 1 + end + ) + branch.to_s.should eq "a = 1" + end + + it "constructs a branch in else" do + branch = branch_of_assign_in_def %( + def method(a) + rescue + else + a = 1 + end + ) + branch.to_s.should eq "a = 1" + end + + it "constructs a branch in ensure" do + branch = branch_of_assign_in_def %( + def method(a) + rescue + ensure + a = 1 + end + ) + branch.to_s.should eq "a = 1" + end + end + + context "Crystal::MacroIf" do + it "constructs a branch in cond" do + branch = branch_of_assign_in_def %( + def method(a) + {% if a = 2 %} + {% end %} + end + ) + branch.to_s.should eq "a = 2" + end + + it "constructs a branch in then" do + nodes = as_nodes %( + def method(a) + {% if true %} + a = 2 + {% end %} + end + ) + branch = Branch.of(nodes.macro_literal_nodes.first, nodes.def_nodes.first) + branch.to_s.strip.should eq "a = 2" + end + end + + context "Crystal::MacroFor" do + it "constructs a branch in body" do + nodes = as_nodes %( + def method(a) + {% for x in [1, 2, 3] %} + a = 2 + {% end %} + end + ) + branch = Branch.of(nodes.macro_literal_nodes.first, nodes.def_nodes.first) + branch.to_s.strip.should eq "a = 2" + end + end + + it "returns nil if branch does not exist" do + nodes = as_nodes %( + def method + a = 2 + end + ) + branch = Branch.of(nodes.assign_nodes.first, nodes.def_nodes.first) + branch.should be_nil + end + end + + describe "#initialize" do + it "creates new branch" do + nodes = as_nodes %( + if true + a = 2 + end + ) + branchable = Branchable.new nodes.if_nodes.first + branch = Branch.new nodes.assign_nodes.first, branchable + branch.node.should_not be_nil + end + end + + describe "delegation" do + it "delegates to_s to node" do + nodes = as_nodes %( + if true + a = 2 + end + ) + branchable = Branchable.new nodes.if_nodes.first + branch = Branch.new nodes.assign_nodes.first, branchable + branch.to_s.should eq branch.node.to_s + end + + it "delegates location to node" do + nodes = as_nodes %( + if true + a = 2 + end + ) + branchable = Branchable.new nodes.if_nodes.first + branch = Branch.new nodes.assign_nodes.first, branchable + branch.location.should eq branch.node.location + end + end + + describe "#in_loop?" do + it "returns true if branch is in a loop" do + nodes = as_nodes %( + while true + a = 1 + end + ) + branchable = Branchable.new nodes.while_nodes.first + branch = Branch.new nodes.assign_nodes.first, branchable + branch.in_loop?.should be_true + end + + it "returns false if branch is not in a loop" do + nodes = as_nodes %( + if a > 2 + a = 1 + end + ) + branchable = Branchable.new nodes.if_nodes.first + branch = Branch.new nodes.assign_nodes.first, branchable + branch.in_loop?.should be_false + end + end + end +end diff --git a/spec/ameba/ast/branchable_spec.cr b/spec/ameba/ast/branchable_spec.cr new file mode 100644 index 00000000..9aa50638 --- /dev/null +++ b/spec/ameba/ast/branchable_spec.cr @@ -0,0 +1,48 @@ +require "../../spec_helper" + +module Ameba::AST + describe Branchable do + describe "#initialize" do + it "creates a new branchable" do + branchable = Branchable.new as_node %(a = 2 if true) + branchable.node.should_not be_nil + end + end + + describe "delegation" do + it "delegates to_s to @node" do + node = as_node %(a = 2 if true) + branchable = Branchable.new node + branchable.to_s.should eq node.to_s + end + + it "delegates location to @node" do + node = as_node %(a = 2 if true) + branchable = Branchable.new node + branchable.location.should eq node.location + end + end + + describe "#loop?" do + it "returns true if it is a while loop" do + branchable = Branchable.new as_node %(while true; a = 2; end) + branchable.loop?.should be_true + end + + it "returns true if it is the until loop" do + branchable = Branchable.new as_node %(until false; a = 2; end) + branchable.loop?.should be_true + end + + it "returns true if it is loop" do + branchable = Branchable.new as_node %(loop {}) + branchable.loop?.should be_true + end + + it "returns false otherwise" do + branchable = Branchable.new as_node %(a = 2 if true) + branchable.loop?.should be_false + end + end + end +end diff --git a/spec/ameba/ast/scope_spec.cr b/spec/ameba/ast/scope_spec.cr new file mode 100644 index 00000000..81189049 --- /dev/null +++ b/spec/ameba/ast/scope_spec.cr @@ -0,0 +1,72 @@ +require "../../spec_helper" + +module Ameba::AST + describe Scope do + describe "#initialize" do + source = "a = 2" + + it "assigns outer scope" do + root = Scope.new as_node(source) + child = Scope.new as_node(source), root + child.outer_scope.should_not be_nil + end + + it "assigns node" do + scope = Scope.new as_node(source) + scope.node.should_not be_nil + end + end + end + + describe "#add_variable" do + it "adds a new variable to the scope" do + scope = Scope.new as_node("") + scope.add_variable(Crystal::Var.new "foo") + scope.variables.any?.should be_true + end + end + + describe "#find_variable" do + it "returns the variable in the scope by name" do + scope = Scope.new as_node("foo = 1") + scope.add_variable Crystal::Var.new "foo" + scope.find_variable("foo").should_not be_nil + end + + it "returns nil if variable not exist in this scope" do + scope = Scope.new as_node("foo = 1") + scope.find_variable("bar").should be_nil + end + end + + describe "#assign_variable" do + it "creates a new assignment" do + scope = Scope.new as_node("foo = 1") + scope.add_variable Crystal::Var.new "foo" + scope.assign_variable(Crystal::Var.new "foo") + scope.find_variable("foo").not_nil!.assignments.size.should eq 1 + end + + it "does not create the assignment if variable is wrong" do + scope = Scope.new as_node("foo = 1") + scope.add_variable Crystal::Var.new "foo" + scope.assign_variable(Crystal::Var.new "bar") + scope.find_variable("foo").not_nil!.assignments.size.should eq 0 + end + end + + describe "#block?" do + it "returns true if Crystal::Block" do + nodes = as_nodes %( + 3.times {} + ) + scope = Scope.new nodes.block_nodes.first + scope.block?.should be_true + end + + it "returns false otherwise" do + scope = Scope.new as_node "a = 1" + scope.block?.should be_false + end + end +end diff --git a/spec/ameba/ast/variabling/assignment_spec.cr b/spec/ameba/ast/variabling/assignment_spec.cr new file mode 100644 index 00000000..ee13e2e1 --- /dev/null +++ b/spec/ameba/ast/variabling/assignment_spec.cr @@ -0,0 +1,90 @@ +require "../../../spec_helper" + +module Ameba::AST + describe Assignment do + node = Crystal::NilLiteral.new + scope = Scope.new as_node "foo = 1" + variable = Variable.new(Crystal::Var.new("foo"), scope) + + describe "#initialize" do + it "creates a new assignment with node and var" do + assignment = Assignment.new(node, variable) + assignment.node.should_not be_nil + end + end + + describe "#reference=" do + it "creates a new reference" do + assignment = Assignment.new(node, variable) + assignment.referenced = true + assignment.referenced?.should be_true + end + end + + describe "delegation" do + it "delegates location" do + assignment = Assignment.new(node, variable) + assignment.location.should eq node.location + end + + it "delegates to_s" do + assignment = Assignment.new(node, variable) + assignment.to_s.should eq node.to_s + end + + it "delegates scope" do + assignment = Assignment.new(node, variable) + assignment.scope.should eq variable.scope + end + end + + describe "#branch" do + it "returns the branch of the assignment" do + nodes = as_nodes %( + def method(a) + if a + a = 3 # --> Crystal::Expressions + puts a + end + end + ) + + scope = Scope.new nodes.def_nodes.first + variable = Variable.new(nodes.var_nodes.first, scope) + assignment = Assignment.new(nodes.assign_nodes.first, variable) + assignment.branch.should_not be_nil + assignment.branch.not_nil!.node.class.should eq Crystal::Expressions + end + + it "returns inner branch" do + nodes = as_nodes %( + def method(a, b) + if a + if b + a = 3 # --> Crystal::Assign + end + end + end + ) + scope = Scope.new nodes.def_nodes.first + variable = Variable.new(nodes.var_nodes.first, scope) + assignment = Assignment.new(nodes.assign_nodes.first, variable) + assignment.branch.should_not be_nil + assignment.branch.not_nil!.node.class.should eq Crystal::Assign + end + + it "returns nil if assignment does not have a branch" do + nodes = as_nodes %( + def method(a) + a = 2 + end + ) + + scope = Scope.new nodes.def_nodes.first + variable = Variable.new(nodes.var_nodes.first, scope) + assignment = Assignment.new(nodes.assign_nodes.first, variable) + assignment.branch.should be_nil + end + end + end +end diff --git a/spec/ameba/ast/variabling/reference_spec.cr b/spec/ameba/ast/variabling/reference_spec.cr new file mode 100644 index 00000000..15527091 --- /dev/null +++ b/spec/ameba/ast/variabling/reference_spec.cr @@ -0,0 +1,10 @@ +require "../../../spec_helper" + +module Ameba::AST + describe Reference do + it "is derived from a Variable" do + node = Crystal::Var.new "foo" + Reference.new(node, Scope.new as_node "foo = 1").is_a?(Variable).should be_true + end + end +end diff --git a/spec/ameba/ast/variabling/variable_spec.cr b/spec/ameba/ast/variabling/variable_spec.cr new file mode 100644 index 00000000..761b013e --- /dev/null +++ b/spec/ameba/ast/variabling/variable_spec.cr @@ -0,0 +1,154 @@ +require "../../../spec_helper" + +module Ameba::AST + describe Variable do + var_node = Crystal::Var.new("foo") + scope = Scope.new as_node "foo = 1" + + describe "#initialize" do + it "creates a new variable" do + variable = Variable.new(var_node, scope) + variable.node.should_not be_nil + end + end + + describe "delegation" do + it "delegates location" do + variable = Variable.new(var_node, scope) + variable.location.should eq var_node.location + end + + it "delegates name" do + variable = Variable.new(var_node, scope) + variable.name.should eq var_node.name + end + + it "delegates to_s" do + variable = Variable.new(var_node, scope) + variable.to_s.should eq var_node.to_s + end + end + + describe "#special?" do + it "returns truthy if it is a special `$?` var" do + variable = Variable.new Crystal::Var.new("$?"), scope + variable.special?.should be_truthy + end + + it "returns falsey otherwise" do + variable = Variable.new Crystal::Var.new("a"), scope + variable.special?.should be_falsey + end + end + + describe "#assign" do + assign_node = as_node("foo=1") + + it "assigns the variable (creates a new assignment)" do + variable = Variable.new(var_node, scope) + variable.assign(assign_node) + variable.assignments.any?.should be_true + end + + it "can create multiple assignments" do + variable = Variable.new(var_node, scope) + variable.assign(assign_node) + variable.assign(assign_node) + variable.assignments.size.should eq 2 + end + end + + describe "#reference" do + it "references the existed assignment" do + variable = Variable.new(var_node, scope) + variable.assign(as_node "foo=1") + variable.reference(var_node, scope) + variable.references.any?.should be_true + end + end + + describe "#captured_by_block?" do + it "returns truthy if the variable is captured by block" do + nodes = as_nodes %( + def method + a = 2 + 3.times { |i| a = a + i } + end + ) + scope = Scope.new nodes.def_nodes.first + var_node = nodes.var_nodes.first + scope.add_variable var_node + scope.inner_scopes << Scope.new(nodes.block_nodes.first, scope) + + variable = Variable.new(var_node, scope) + variable.reference nodes.var_nodes.last, scope.inner_scopes.last + variable.captured_by_block?.should be_truthy + end + + it "returns falsey if the variable is not captured by the block" do + scope = Scope.new as_node %( + def method + a = 1 + end + ) + scope.add_variable Crystal::Var.new "a" + variable = scope.variables.first + variable.captured_by_block?.should be_falsey + end + end + + describe "#target_of?" do + it "returns true if the variable is a target of Crystal::Assign node" do + assign_node = as_nodes("foo=1").assign_nodes.last + variable = Variable.new assign_node.target.as(Crystal::Var), scope + variable.target_of?(assign_node).should be_true + end + + it "returns true if the variable is a target of Crystal::OpAssign node" do + assign_node = as_nodes("foo=1;foo+=1").op_assign_nodes.last + variable = Variable.new assign_node.target.as(Crystal::Var), scope + variable.target_of?(assign_node).should be_true + end + + it "returns true if the variable is a target of Crystal::MultiAssign node" do + assign_node = as_nodes("a,b,c={1,2,3}").multi_assign_nodes.last + assign_node.targets.size.should_not eq 0 + assign_node.targets.each do |target| + variable = Variable.new target.as(Crystal::Var), scope + variable.target_of?(assign_node).should be_true + end + end + + it "returns false if the node is not assign" do + variable = Variable.new(Crystal::Var.new("v"), scope) + variable.target_of?(as_node "nil").should be_false + end + + it "returns false if the variable is not a target of the assign" do + variable = Variable.new(Crystal::Var.new("foo"), scope) + variable.target_of?(as_node("bar = 1")).should be_false + end + end + + describe "#eql?" do + var = Crystal::Var.new("foo").at(Crystal::Location.new(nil, 1, 2)) + variable = Variable.new var, scope + + it "is false if node is not a Crystal::Var" do + variable.eql?(as_node("nil")).should be_false + end + + it "is false if node name is different" do + variable.eql?(Crystal::Var.new "bar").should be_false + end + + it "is false if node has a different location" do + variable.eql?(Crystal::Var.new "foo").should be_false + end + + it "is true otherwise" do + variable.eql?(variable.node).should be_true + end + end + end +end diff --git a/spec/ameba/ast/traverse_spec.cr b/spec/ameba/ast/visitors/node_visitor_spec.cr similarity index 73% rename from spec/ameba/ast/traverse_spec.cr rename to spec/ameba/ast/visitors/node_visitor_spec.cr index 7dc31b19..4946adfe 100644 --- a/spec/ameba/ast/traverse_spec.cr +++ b/spec/ameba/ast/visitors/node_visitor_spec.cr @@ -1,14 +1,14 @@ -require "../../spec_helper" +require "../../../spec_helper" module Ameba::AST rule = DummyRule.new source = Source.new "" - describe "Traverse" do + describe NodeVisitor do {% for name in NODES %} describe "{{name}}" do it "allow to visit {{name}} node" do - visitor = Visitor.new rule, source + visitor = NodeVisitor.new rule, source nodes = Crystal::Parser.new("").parse nodes.accept visitor end diff --git a/spec/ameba/ast/visitors/scope_visitor_spec.cr b/spec/ameba/ast/visitors/scope_visitor_spec.cr new file mode 100644 index 00000000..98818fa5 --- /dev/null +++ b/spec/ameba/ast/visitors/scope_visitor_spec.cr @@ -0,0 +1,58 @@ +require "../../../spec_helper" + +module Ameba::AST + describe ScopeVisitor do + it "creates a scope for the def" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new %( + def method + end + ) + rule.scopes.size.should eq 1 + end + + it "creates a scope for the proc" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new %( + -> {} + ) + rule.scopes.size.should eq 1 + end + + it "creates a scope for the block" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new %( + 3.times {} + ) + rule.scopes.size.should eq 2 + end + + context "inner scopes" do + it "creates scope for block inside def" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new %( + def method + 3.times {} + end + ) + rule.scopes.size.should eq 2 + rule.scopes.last.outer_scope.should_not be_nil + rule.scopes.first.outer_scope.should eq rule.scopes.last + end + + it "creates scope for block inside block" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new %( + 3.times do + 2.times {} + end + ) + rule.scopes.size.should eq 3 + inner_block = rule.scopes.first + outer_block = rule.scopes.last + inner_block.outer_scope.should_not eq outer_block + outer_block.outer_scope.should be_nil + end + end + end +end diff --git a/spec/ameba/rule/unneded_disable_directive_spec.cr b/spec/ameba/rule/unneded_disable_directive_spec.cr index 689b4575..0061e261 100644 --- a/spec/ameba/rule/unneded_disable_directive_spec.cr +++ b/spec/ameba/rule/unneded_disable_directive_spec.cr @@ -35,7 +35,7 @@ module Ameba::Rule subject.catch(s).should be_valid end - it "failes if there is unneeded directive" do + it "fails if there is unneeded directive" do s = Source.new %Q( # ameba:disable #{NamedRule.name} a = 1 diff --git a/spec/ameba/rule/useless_assign_spec.cr b/spec/ameba/rule/useless_assign_spec.cr new file mode 100644 index 00000000..9bc2d596 --- /dev/null +++ b/spec/ameba/rule/useless_assign_spec.cr @@ -0,0 +1,813 @@ +require "../../spec_helper" + +module Ameba::Rule + describe UselessAssign do + subject = UselessAssign.new + + it "does not report used assigments" do + s = Source.new %( + def method + a = 2 + a + end + ) + subject.catch(s).should be_valid + end + + it "reports a useless assignment in a method" do + s = Source.new %( + def method + a = 2 + end + ) + subject.catch(s).should_not be_valid + end + + it "reports a useless assignment in a proc" do + s = Source.new %( + ->() { + a = 2 + } + ) + subject.catch(s).should_not be_valid + end + + it "reports a useless assignment in a block" do + s = Source.new %( + def method + 3.times do + a = 1 + end + end + ) + subject.catch(s).should_not be_valid + end + + it "reports a useless assignment in a proc inside def" do + s = Source.new %( + def method + ->() { + a = 2 + } + end + ) + subject.catch(s).should_not be_valid + end + + it "reports a useless assignment in a proc inside a block" do + s = Source.new %( + def method + 3.times do + ->() { + a = 2 + } + end + end + ) + subject.catch(s).should_not be_valid + end + + it "reports rule, position and a message" do + s = Source.new %( + def method + a = 2 + end + ), "source.cr" + subject.catch(s).should_not be_valid + + error = s.errors.first + error.rule.should_not be_nil + error.location.to_s.should eq "source.cr:3:11" + error.message.should eq "Useless assignment to variable `a`" + end + + it "does not report useless assignment of instance var" do + s = Source.new %( + class Cls + def initialize(@name) + end + end + ) + subject.catch(s).should be_valid + end + + it "does not report if assignment used in the inner block scope" do + s = Source.new %( + def method + var = true + 3.times { var = false } + end + ) + subject.catch(s).should be_valid + end + + it "reports if assigned is not referenced in the inner block scope" do + s = Source.new %( + def method + var = true + 3.times {} + end + ) + subject.catch(s).should_not be_valid + end + + it "doesn't report if assignment in referenced in inner block" do + s = Source.new %( + def method + two = true + + 3.times do + mutex.synchronize do + two = 2 + end + end + + two.should be_true + end + ) + subject.catch(s).should be_valid + end + + it "reports if first assignment is useless" do + s = Source.new %( + def method + var = true + var = false + var + end + ) + subject.catch(s).should_not be_valid + s.errors.first.location.to_s.should eq ":3:11" + end + + it "reports if variable reassigned and not used" do + s = Source.new %( + def method + var = true + var = false + end + ) + subject.catch(s).should_not be_valid + end + + it "does not report if variable used in a condition" do + s = Source.new %( + def method + a = 1 + if a + nil + end + end + ) + subject.catch(s).should be_valid + end + + it "reports second assignment as useless" do + s = Source.new %( + def method + a = 1 + a = a + 1 + end + ) + subject.catch(s).should_not be_valid + end + + it "does not report if variable is referenced in other assignment" do + s = Source.new %( + def method + if f = get_something + @f = f + end + end + ) + subject.catch(s).should be_valid + end + + it "does not report if variable is referenced in a setter" do + s = Source.new %( + def method + foo = 2 + table[foo] ||= "bar" + end + ) + subject.catch(s).should be_valid + end + + it "does not report if variable is reassigned but not referenced" do + s = Source.new %( + def method + foo = 1 + puts foo + foo = 2 + end + ) + subject.catch(s).should_not be_valid + end + + it "does not report if variable is referenced in a call" do + s = Source.new %( + def method + if f = FORMATTER + @formatter = f.new + end + end + ) + subject.catch(s).should be_valid + end + + it "does not report if a setter is invoked with operator assignment" do + s = Source.new %( + def method + obj = {} of Symbol => Int32 + obj[:name] = 3 + end + ) + subject.catch(s).should be_valid + end + + it "does not report if global var" do + s = Source.new %( + def method + $? = 3 + end + ) + subject.catch(s).should be_valid + end + + it "does not report if assignment is referenced in a proc" do + s = Source.new %( + def method + called = false + ->() { called = true } + called + end + ) + subject.catch(s).should be_valid + end + + it "reports if variable is shadowed in inner scope" do + s = Source.new %( + def method + i = 1 + 3.times do |i| + i + 1 + end + end + ) + subject.catch(s).should_not be_valid + end + + it "does not report if parameter is referenced after the branch" do + s = Source.new %( + def method(param) + 3.times do + param = 3 + end + param + end + ) + subject.catch(s).should be_valid + end + + context "op assigns" do + it "does not report if variable is referenced below the op assign" do + s = Source.new %( + def method + a = 1 + a += 1 + a + end + ) + subject.catch(s).should be_valid + end + + it "does not report if variable is referenced in op assign few times" do + s = Source.new %( + def method + a = 1 + a += 1 + a += 1 + a = a + 1 + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if variable is not referenced below the op assign" do + s = Source.new %( + def method + a = 1 + a += 1 + end + ) + subject.catch(s).should_not be_valid + end + + it "reports rule, location and a message" do + s = Source.new %( + def method + b = 2 + a = 3 + a += 1 + end + ), "source.cr" + subject.catch(s).should_not be_valid + + error = s.errors.last + error.rule.should_not be_nil + error.location.to_s.should eq "source.cr:5:13" + error.message.should eq "Useless assignment to variable `a`" + end + end + + context "multi assigns" do + it "does not report if all assigns are referenced" do + s = Source.new %( + def method + a, b = {1, 2} + a + b + end + ) + subject.catch(s).should be_valid + end + + it "reports if one assign is not referenced" do + s = Source.new %( + def method + a, b = {1, 2} + a + end + ) + subject.catch(s).should_not be_valid + error = s.errors.first + error.location.to_s.should eq ":3:16" + error.message.should eq "Useless assignment to variable `b`" + end + + it "reports if both assigns are reassigned and useless" do + s = Source.new %( + def method + a, b = {1, 2} + a, b = {3, 4} + end + ) + subject.catch(s).should_not be_valid + end + + it "reports if both assigns are not referenced" do + s = Source.new %( + def method + a, b = {1, 2} + end + ) + subject.catch(s).should_not be_valid + + error = s.errors.first + error.location.to_s.should eq ":3:13" + error.message.should eq "Useless assignment to variable `a`" + + error = s.errors.last + error.location.to_s.should eq ":3:16" + error.message.should eq "Useless assignment to variable `b`" + end + end + + context "top level" do + it "reports if assignment is not referenced" do + s = Source.new %( + a = 1 + a = 2 + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 2 + s.errors.first.location.to_s.should eq ":2:11" + s.errors.last.location.to_s.should eq ":3:11" + end + + it "doesn't report if assignments are referenced" do + s = Source.new %( + a = 1 + a += 1 + a + + b, c = {1, 2} + b + c + ) + subject.catch(s).should be_valid + end + + it "doesn't report if assignment is captured by block" do + s = Source.new %( + a = 1 + + 3.times do + a = 2 + end + ) + subject.catch(s).should be_valid + end + end + + context "branching" do + context "if-then-else" do + it "doesn't report if assignment is consumed by branches" do + s = Source.new %( + def method + a = 0 + if something + a = 1 + else + a = 2 + end + a + end + ) + subject.catch(s).should be_valid + end + + it "doesn't report if assignment is in one branch" do + s = Source.new %( + def method + a = 0 + if something + a = 1 + else + nil + end + a + end + ) + subject.catch(s).should be_valid + end + + it "doesn't report if assignment is in one line branch" do + s = Source.new %( + def method + a = 0 + a = 1 if something + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless in the branch" do + s = Source.new %( + def method(a) + if a + a = 2 + end + end + ) + subject.catch(s).should_not be_valid + end + + it "reports if only last assignment is referenced in a branch" do + s = Source.new %( + def method(a) + a = 1 + if a + a = 2 + a = 3 + end + a + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":5:17" + end + + it "does not report of assignments are referenced in all branches" do + s = Source.new %( + def method + if matches + matches = owner.lookup_matches signature + else + matches = owner.lookup_matches signature + end + + matches + end + ) + subject.catch(s).should be_valid + end + + it "does not report referenced assignments in inner branches" do + s = Source.new %( + def method + has_newline = false + + if something + do_something unless false + has_newline = false + else + do_something if true + has_newline = true + end + + has_newline + end + ) + subject.catch(s).should be_valid + end + end + + context "unless-then-else" do + it "doesn't report if assignment is consumed by branches" do + s = Source.new %( + def method + a = 0 + unless something + a = 1 + else + a = 2 + end + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if there is a useless assignment in a branch" do + s = Source.new %( + def method + a = 0 + unless something + a = 1 + a = 2 + else + a = 2 + end + a + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":5:17" + end + end + + context "case" do + it "does not report if assignment is referenced" do + s = Source.new %( + def method(a) + case a + when /foo/ + a = 1 + when /bar/ + a = 2 + end + puts a + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless" do + s = Source.new %( + def method(a) + case a + when /foo/ + a = 1 + when /bar/ + a = 2 + end + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 2 + s.errors.first.location.to_s.should eq ":5:17" + s.errors.last.location.to_s.should eq ":7:17" + end + end + + context "binary operator" do + it "does not report if assignment is referenced" do + s = Source.new %( + def method(a) + (a = 1) && (b = 1) + a + b + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless" do + s = Source.new %( + def method(a) + (a = 1) || (b = 1) + a + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":3:27" + end + end + + context "while" do + it "does not report if assignment is referenced" do + s = Source.new %( + def method(a) + while a < 10 + a = a + 1 + end + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless" do + s = Source.new %( + def method(a) + while a < 10 + b = a + end + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":4:17" + end + + it "does not report if assignment is referenced in a loop" do + s = Source.new %( + def method + a = 3 + result = 0 + + while result < 10 + result += a + a = a + 1 + end + result + end + ) + subject.catch(s).should be_valid + end + + it "does not report if assignment is referenced as param in a loop" do + s = Source.new %( + def method(a) + result = 0 + + while result < 10 + result += a + a = a + 1 + end + result + end + ) + subject.catch(s).should be_valid + end + + it "does not report if assignment is referenced in loop and inner branch" do + s = Source.new %( + def method(a) + result = 0 + + while result < 10 + result += a + if result > 0 + a = a + 1 + else + a = 3 + end + end + result + end + ) + subject.catch(s).should be_valid + end + + it "works properly if there is branch with blank node" do + s = Source.new %( + def visit + count = 0 + while true + break if count == 1 + case something + when :any + else + :anything_else + end + count += 1 + end + end + ) + subject.catch(s).should be_valid + end + end + + context "until" do + it "does not report if assignment is referenced" do + s = Source.new %( + def method(a) + until a > 10 + a = a + 1 + end + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless" do + s = Source.new %( + def method(a) + until a > 10 + b = a + 1 + end + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":4:17" + end + end + + context "exception handler" do + it "does not report if assignment is referenced in body" do + s = Source.new %( + def method(a) + a = 2 + rescue + a + end + ) + subject.catch(s).should be_valid + end + + it "doesn't report if assignment is referenced in ensure" do + s = Source.new %( + def method(a) + a = 2 + ensure + a + end + ) + subject.catch(s).should be_valid + end + + it "doesn't report if assignment is referenced in else" do + s = Source.new %( + def method(a) + a = 2 + rescue + else + a + end + ) + subject.catch(s).should be_valid + end + + it "reports if assignment is useless" do + s = Source.new %( + def method(a) + rescue + a = 2 + end + ) + subject.catch(s).should_not be_valid + s.errors.size.should eq 1 + s.errors.first.location.to_s.should eq ":4:15" + end + end + end + + context "macro" do + it "doesn't report if assignment is referenced in macro" do + s = Source.new %( + def method + a = 2 + {% if flag?(:bits64) %} + a.to_s + {% else %} + a + {% end %} + end + ) + subject.catch(s).should be_valid + end + + it "doesn't report referenced assignments in macro literal" do + s = Source.new %( + def method + a = 2 + {% if flag?(:bits64) %} + a = 3 + {% else %} + a = 4 + {% end %} + puts a + end + ) + subject.catch(s).should be_valid + end + end + end +end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index dc31189c..93d593a5 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -30,6 +30,17 @@ module Ameba end end + struct ScopeRule < Rule::Base + getter scopes = [] of AST::Scope + + def test(source) + end + + def test(source, node : Crystal::ASTNode, scope : AST::Scope) + @scopes << scope + end + end + class DummyFormatter < Formatter::BaseFormatter property started_sources : Array(Source)? property finished_sources : Array(Source)? @@ -71,8 +82,49 @@ module Ameba "Source expected to be invalid, but it is valid." end end + + class TestNodeVisitor < Crystal::Visitor + NODES = [ + Crystal::Var, + Crystal::Assign, + Crystal::OpAssign, + Crystal::MultiAssign, + Crystal::Block, + Crystal::Def, + Crystal::If, + Crystal::While, + Crystal::MacroLiteral, + ] + + def initialize(node) + node.accept self + end + + def visit(node : Crystal::ASTNode) + true + end + + {% for node in NODES %} + {{getter_name = node.stringify.split("::").last.underscore + "_nodes"}} + + getter {{getter_name.id}} = [] of {{node}} + + def visit(node : {{node}}) + {{getter_name.id}} << node + true + end + {% end %} + end end def be_valid Ameba::BeValidExpectation.new end + +def as_node(source) + Crystal::Parser.new(source).parse +end + +def as_nodes(source) + Ameba::TestNodeVisitor.new(as_node source) +end diff --git a/src/ameba.cr b/src/ameba.cr index d4792e2c..3c4ce27f 100644 --- a/src/ameba.cr +++ b/src/ameba.cr @@ -1,5 +1,5 @@ require "./ameba/*" -require "./ameba/ast/*" +require "./ameba/ast/**" require "./ameba/rule/*" require "./ameba/formatter/*" diff --git a/src/ameba/ast/branch.cr b/src/ameba/ast/branch.cr new file mode 100644 index 00000000..081b3031 --- /dev/null +++ b/src/ameba/ast/branch.cr @@ -0,0 +1,193 @@ +module Ameba::AST + # Represents the branch in Crystal code. + # Branch is a part of a branchable statement. + # For example, the branchable if statement contains 3 branches: + # + # ``` + # if a = something # --> Branch A + # a = 1 # --> Branch B + # put a if out # --> Branch C + # else + # do_something a # --> Branch D + # end + # ``` + # + class Branch + # The actual branch node. + getter node : Crystal::ASTNode + + # The parent branchable. + getter parent : Branchable + + delegate to_s, to: @node + delegate location, to: @node + + def_equals_and_hash node, location + + # Creates a new branch. + # + # ``` + # Branch.new(if_node) + # ``` + def initialize(@node, @parent) + end + + # Returns true if current branch is in a loop, false - otherwise. + # For example, this branch is in a loop: + # + # ``` + # while true + # handle_input # this branch is in a loop + # if wrong_input + # show_message # this branch is also in a loop. + # end + # end + # ``` + # + def in_loop? + @parent.loop? + end + + # Constructs a new branch based on the node in scope. + # + # ``` + # Branch.of(assign_node, scope) + # ``` + def self.of(node : Crystal::ASTNode, scope : Scope) + of(node, scope.node) + end + + # Constructs a new branch based on the node some parent scope. + # + # ``` + # Branch.of(assign_node, def_node) + # ``` + def self.of(node : Crystal::ASTNode, parent_node : Crystal::ASTNode) + BranchVisitor.new(node).tap(&.accept parent_node).branch + end + + # :nodoc: + private class BranchVisitor < Crystal::Visitor + @current_branch : Crystal::ASTNode? + + property branchable : Branchable? + property branch : Branch? + + def initialize(@node : Crystal::ASTNode) + end + + private def on_branchable_start(node, *branches) + on_branchable_start(node, branches) + end + + private def on_branchable_start(node, branches : Array | Tuple) + @branchable = Branchable.new(node, @branchable) + + branches.each do |node| + break if branch # branch found + @current_branch = node if node && !node.nop? + node.try &.accept(self) + end + + false + end + + private def on_branchable_end(node) + @branchable = @branchable.try &.parent + end + + def visit(node : Crystal::ASTNode) + return false if branch + + if node.class == @node.class && + node.location == @node.location && + (branchable = @branchable) && + (branch = @current_branch) + @branch = Branch.new(branch, branchable) + end + + true + end + + def visit(node : Crystal::If) + on_branchable_start node, node.cond, node.then, node.else + end + + def end_visit(node : Crystal::If) + on_branchable_end node + end + + def visit(node : Crystal::Unless) + on_branchable_start node, node.cond, node.then, node.else + end + + def end_visit(node : Crystal::Unless) + on_branchable_end node + end + + def visit(node : Crystal::BinaryOp) + on_branchable_start node, node.left, node.right + end + + def end_visit(node : Crystal::BinaryOp) + on_branchable_end node + end + + def visit(node : Crystal::Case) + on_branchable_start node, [node.cond, node.whens, node.else].flatten + end + + def end_visit(node : Crystal::Case) + on_branchable_end node + end + + def visit(node : Crystal::While) + on_branchable_start node, node.cond, node.body + end + + def end_visit(node : Crystal::While) + on_branchable_end node + end + + def visit(node : Crystal::Until) + on_branchable_start node, node.cond, node.body + end + + def end_visit(node : Crystal::Until) + on_branchable_end node + end + + def visit(node : Crystal::ExceptionHandler) + on_branchable_start node, [node.body, node.rescues, node.else, node.ensure].flatten + end + + def end_visit(node : Crystal::ExceptionHandler) + on_branchable_end node + end + + def visit(node : Crystal::Rescue) + on_branchable_start node, node.body + end + + def end_visit(node : Crystal::Rescue) + on_branchable_end node + end + + def visit(node : Crystal::MacroIf) + on_branchable_start node, node.cond, node.then, node.else + end + + def end_visit(node : Crystal::MacroIf) + on_branchable_end node + end + + def visit(node : Crystal::MacroFor) + on_branchable_start node, node.exp, node.body + end + + def end_visit(node : Crystal::MacroFor) + on_branchable_end node + end + end + end +end diff --git a/src/ameba/ast/branchable.cr b/src/ameba/ast/branchable.cr new file mode 100644 index 00000000..035cb46e --- /dev/null +++ b/src/ameba/ast/branchable.cr @@ -0,0 +1,42 @@ +module Ameba::AST + # A generic entity to represent a branchable Crystal node. + # For example, `Crystal::If`, `Crystal::Unless`, `Crystal::While` + # are branchable. + # + # ``` + # white a > 100 # Branchable A + # if b > 2 # Branchable B + # a += 1 + # end + # end + # ``` + class Branchable + getter branches = [] of Crystal::ASTNode + + # The actual Crystal node. + getter node : Crystal::ASTNode + + # Parent branchable (if any) + getter parent : Branchable? + + delegate to_s, to: @node + delegate location, to: @node + + # Creates a new branchable + # + # ``` + # Branchable.new(node, parent_branchable) + # ``` + def initialize(@node, @parent = nil) + end + + # Returns true if this node or one of the parent branchables is a loop, false otherwise. + def loop? + return true if node.is_a? Crystal::While || + node.is_a? Crystal::Until || + ((n = node) && n.is_a?(Crystal::Call) && n.name == "loop") + + parent.try(&.loop?) || false + end + end +end diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr new file mode 100644 index 00000000..6c8e014a --- /dev/null +++ b/src/ameba/ast/scope.cr @@ -0,0 +1,110 @@ +require "./variabling/*" + +module Ameba::AST + # Represents a context of the local variable visibility. + # This is where the local variables belong to. + class Scope + # Link to local variables + getter variables = [] of Variable + + # Link to the outer scope + getter outer_scope : Scope? + + # List of inner scopes + getter inner_scopes = [] of Scope + + # The actual AST node that represents a current scope. + getter node : Crystal::ASTNode + + delegate to_s, to: node + delegate location, to: node + + def_equals_and_hash node, location + + # Creates a new scope. Accepts the AST node and the outer scope. + # + # ``` + # scope = Scope.new(class_node, nil) + # ``` + def initialize(@node, @outer_scope = nil) + @outer_scope.try &.inner_scopes.<<(self) + end + + # Creates a new variable in the current scope. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.add_variable(var_node) + # ``` + def add_variable(node) + variables << Variable.new(node, self) + end + + # Returns variable by its name or nil if it does not exist. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.find_variable("foo") + # ``` + def find_variable(name : String) + variables.find { |v| v.name == name } || outer_scope.try &.find_variable(name) + end + + # Creates a new assignment for the variable. + # + # ``` + # scope = Scope.new(class_node, nil) + # scope.assign_variable(var_node) + # ``` + def assign_variable(node) + node.is_a?(Crystal::Var) && find_variable(node.name).try &.assign(node) + end + + # Returns true if current scope represents a block (or proc), + # false if not. + def block? + node.is_a?(Crystal::Block) || node.is_a?(Crystal::ProcLiteral) + end + + # Returns true if and only if current scope represents some + # type definition, for example a class. + def type_definition? + node.is_a?(Crystal::ClassDef) || + node.is_a?(Crystal::ModuleDef) || + node.is_a?(Crystal::LibDef) || + node.is_a?(Crystal::FunDef) || + node.is_a?(Crystal::TypeDef) || + node.is_a?(Crystal::CStructOrUnionDef) + end + + # Returns true if current scope references variable, false if not. + def references?(variable : Variable) + variable.references.any? { |reference| reference.scope == self } + end + + # Returns arguments of this scope (if any). + def args + case current_node = node + when Crystal::Block, Crystal::Def then current_node.args + when Crystal::ProcLiteral then current_node.def.args + else + [] of Crystal::Var + end + end + + # Returns true if variable is an argument in current scope, false if not. + def arg?(var : Crystal::Var) + args.any? do |arg| + arg.is_a?(Crystal::Var) && + arg.name == var.name && + arg.location == var.location + end + end + + # Returns true if the `node` represents exactly + # the same Crystal node as `@node`. + def eql?(node) + node == @node && node.location == @node.location + end + end +end diff --git a/src/ameba/ast/traverse.cr b/src/ameba/ast/traverse.cr deleted file mode 100644 index b42140aa..00000000 --- a/src/ameba/ast/traverse.cr +++ /dev/null @@ -1,68 +0,0 @@ -require "compiler/crystal/syntax/*" - -# A module that helps to traverse Crystal AST using `Crystal::Visitor`. -module Ameba::AST - # List of nodes to be visited by Ameba's rules. - NODES = [ - Alias, - Assign, - Call, - Case, - ClassDef, - ClassVar, - Def, - EnumDef, - ExceptionHandler, - Expressions, - HashLiteral, - If, - InstanceVar, - LibDef, - ModuleDef, - NilLiteral, - StringInterpolation, - Unless, - Var, - When, - While, - ] - - # An AST Visitor used by rules. - # - # ``` - # visitor = Ameba::AST::Visitor.new(rule, source) - # ``` - # - class Visitor < Crystal::Visitor - # A corresponding rule that uses this visitor. - @rule : Rule::Base - - # A source that needs to be traversed. - @source : Source - - # Creates instance of this visitor. - # - # ``` - # visitor = Ameba::AST::Visitor.new(rule, source) - # ``` - # - def initialize(@rule, @source) - @source.ast.accept self - end - - # A main visit method that accepts `Crystal::ASTNode`. - # Returns true meaning all child nodes will be traversed. - def visit(node : Crystal::ASTNode) - true - end - - {% for name in NODES %} - # A visit callback for `Crystal::{{name}}` node. - # Returns true meaning that child nodes will be traversed as well. - def visit(node : Crystal::{{name}}) - @rule.test @source, node - true - end - {% end %} - end -end diff --git a/src/ameba/ast/variabling/assignment.cr b/src/ameba/ast/variabling/assignment.cr new file mode 100644 index 00000000..86c7b756 --- /dev/null +++ b/src/ameba/ast/variabling/assignment.cr @@ -0,0 +1,42 @@ +require "./reference" +require "./variable" + +module Ameba::AST + # Represents the assignment to the variable. + # Holds the assign node and the variable. + class Assignment + property? referenced = false + + # The actual assignment node. + getter node : Crystal::ASTNode + + # Variable of this assignment. + getter variable : Variable + + # Branch of this assignment. + getter branch : Branch? + + delegate location, to: @node + delegate to_s, to: @node + delegate scope, to: @variable + + # Creates a new assignment. + # + # ``` + # Assignment.new(node, variable) + # ``` + # + def initialize(@node, @variable) + if scope = @variable.scope + @branch = Branch.of(@node, scope) + @referenced = true if @variable.special? || + @variable.scope.type_definition? || + referenced_in_loop? + end + end + + def referenced_in_loop? + @variable.referenced? && @branch.try &.in_loop? + end + end +end diff --git a/src/ameba/ast/variabling/reference.cr b/src/ameba/ast/variabling/reference.cr new file mode 100644 index 00000000..e85b3225 --- /dev/null +++ b/src/ameba/ast/variabling/reference.cr @@ -0,0 +1,9 @@ +require "./variable" + +module Ameba::AST + # Represents a reference to the variable. + # It behaves like a variable is used to distinguish a + # the variable from its reference. + class Reference < Variable + end +end diff --git a/src/ameba/ast/variabling/variable.cr b/src/ameba/ast/variabling/variable.cr new file mode 100644 index 00000000..3d3ce705 --- /dev/null +++ b/src/ameba/ast/variabling/variable.cr @@ -0,0 +1,138 @@ +module Ameba::AST + # Represents the existence of the local variable. + # Holds the var node and variable assigments. + class Variable + # List of the assigments of this variable. + getter assignments = [] of Assignment + + # List of the references of this variable. + getter references = [] of Reference + + # The actual var node. + getter node : Crystal::Var + + # Scope of this variable. + getter scope : Scope + + delegate location, to: @node + delegate name, to: @node + delegate to_s, to: @node + + # Creates a new variable(in the scope). + # + # ``` + # Variable.new(node, scope) + # ``` + # + def initialize(@node, @scope) + end + + # Returns true if it is a special variable, i.e `$?`. + def special? + name =~ /^\$/ + end + + # Assigns the variable (creates a new assignment). + # Variable may have multiple assignments. + # + # ``` + # variable = Variable.new(node, scope) + # variable.assign(node1) + # variable.assign(node2) + # variable.assignment.size # => 2 + # ``` + # + def assign(node) + assignments << Assignment.new(node, self) + end + + # Returns true if variable has any reference. + # + # ``` + # variable = Variable.new(node, scope) + # variable.reference(var_node) + # variable.referenced? # => true + # ``` + def referenced? + references.any? + end + + # Creates a reference to this variable in some scope. + # + # ``` + # variable = Variable.new(node, scope) + # variable.reference(var_node, some_scope) + # ``` + # + def reference(node : Crystal::Var, scope : Scope) + Reference.new(node, scope).tap do |reference| + references << reference + end + end + + # Reference variable's assignments. + # + # ``` + # variable = Variable.new(node, scope) + # variable.assign(assign_node) + # variable.reference_assignments! + # ``` + def reference_assignments! + consumed_branches = Set(Branch).new + + assignments.reverse_each do |assignment| + next if consumed_branches.includes?(assignment.branch) + assignment.referenced = true + + break unless assignment.branch + consumed_branches << assignment.branch.not_nil! + end + end + + # Returns true if the current assignment is referenced in + # in the block. For example this variable is captured + # by block: + # + # ``` + # a = 1 + # 3.times { |i| a = a + i } + # ``` + # + # And this variable is not captured by block. + # + # ``` + # i = 1 + # 3.times { |i| i + 1 } + # ``` + def captured_by_block?(scope = @scope) + return false unless scope + + scope.inner_scopes.each do |inner_scope| + return true if inner_scope.block? && inner_scope.references?(self) + return true if captured_by_block?(inner_scope) + end + + false + end + + # Returns true if the variable is a target (on the left) of the assignment, + # false otherwise. + def target_of?(assign) + case assign + when Crystal::Assign then eql?(assign.target) + when Crystal::OpAssign then eql?(assign.target) + when Crystal::MultiAssign then assign.targets.any? { |t| eql?(t) } + else + false + end + end + + # Returns true if the `node` represents exactly + # the same Crystal node as `@node`. + def eql?(node) + node.is_a?(Crystal::Var) && + node.name == @node.name && + node.location == @node.location + end + end +end diff --git a/src/ameba/ast/visitors/base_visitor.cr b/src/ameba/ast/visitors/base_visitor.cr new file mode 100644 index 00000000..20a2ef5a --- /dev/null +++ b/src/ameba/ast/visitors/base_visitor.cr @@ -0,0 +1,29 @@ +require "compiler/crystal/syntax/*" + +# A module that helps to traverse Crystal AST using `Crystal::Visitor`. +module Ameba::AST + # An abstract base visitor that utilizes general logic for all visitors. + abstract class BaseVisitor < Crystal::Visitor + # A corresponding rule that uses this visitor. + @rule : Rule::Base + + # A source that needs to be traversed. + @source : Source + + # Creates instance of this visitor. + # + # ``` + # visitor = Ameba::AST::NodeVisitor.new(rule, source) + # ``` + # + def initialize(@rule, @source) + @source.ast.accept self + end + + # A main visit method that accepts `Crystal::ASTNode`. + # Returns true meaning all child nodes will be traversed. + def visit(node : Crystal::ASTNode) + true + end + end +end diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr new file mode 100644 index 00000000..872cb6f9 --- /dev/null +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -0,0 +1,46 @@ +require "./base_visitor" + +module Ameba::AST + # List of nodes to be visited by Ameba's rules. + NODES = [ + Alias, + Assign, + Call, + Case, + ClassDef, + ClassVar, + Def, + EnumDef, + ExceptionHandler, + Expressions, + HashLiteral, + If, + InstanceVar, + LibDef, + ModuleDef, + NilLiteral, + StringInterpolation, + Unless, + Var, + When, + While, + ] + + # An AST Visitor that traverses the source and allows all nodes + # to be inspected by rules. + # + # ``` + # visitor = Ameba::AST::NodeVisitor.new(rule, source) + # ``` + # + class NodeVisitor < BaseVisitor + {% for name in NODES %} + # A visit callback for `Crystal::{{name}}` node. + # Returns true meaning that child nodes will be traversed as well. + def visit(node : Crystal::{{name}}) + @rule.test @source, node + true + end + {% end %} + end +end diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr new file mode 100644 index 00000000..a878a261 --- /dev/null +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -0,0 +1,140 @@ +require "./base_visitor" + +module Ameba::AST + # AST Visitor that traverses the source and constructs scopes. + class ScopeVisitor < BaseVisitor + @current_scope : Scope + + def initialize(@rule, @source) + @current_scope = Scope.new(@source.ast) # top level scope + @source.ast.accept self + end + + private def on_scope_enter(node) + @current_scope = Scope.new(node, @current_scope) + end + + private def on_scope_end(node) + @rule.test @source, node, @current_scope + + # go up, if this is not a top level scope + if outer_scope = @current_scope.outer_scope + @current_scope = outer_scope + end + end + + # :nodoc: + def end_visit(node : Crystal::ASTNode) + 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::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::CStructOrUnionDef) + on_scope_enter(node) + 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 + + @current_assign : Crystal::ASTNode? + + # :nodoc: + def visit(node : Crystal::Assign | Crystal::OpAssign | Crystal::MultiAssign) + @current_assign = node + end + + # :nodoc: + def end_visit(node : Crystal::Assign | Crystal::OpAssign) + @current_scope.assign_variable(node.target) + @current_assign = nil + end + + # :nodoc: + def end_visit(node : Crystal::MultiAssign) + node.targets.each { |target| @current_scope.assign_variable(target) } + @current_assign = nil + end + + # :nodoc: + def visit(node : Crystal::Arg) + @current_scope.add_variable Crystal::Var.new(node.name).at(node.location) + end + + # :nodoc: + def visit(node : Crystal::Var) + if !@current_scope.arg?(node) && (variable = @current_scope.find_variable node.name) + reference = variable.reference node, @current_scope + + if @current_assign.is_a?(Crystal::OpAssign) || !reference.target_of?(@current_assign) + variable.reference_assignments! + end + else + @current_scope.add_variable(node) + end + end + + # :nodoc: + def visit(node : Crystal::MacroLiteral) + MacroLiteralVarVisitor.new(node).vars.each { |var| visit(var) } + end + end + + private class MacroLiteralVarVisitor < Crystal::Visitor + getter vars = [] of Crystal::Var + + def initialize(literal) + Crystal::Parser.new(literal.value).parse.accept self + rescue + nil + end + + def visit(node : Crystal::ASTNode) + true + end + + def visit(node : Crystal::Var) + vars << node + end + + def visit(node : Crystal::Call) + vars << Crystal::Var.new(node.name).at(node.location) + end + end +end diff --git a/src/ameba/config.cr b/src/ameba/config.cr index 53172fa0..7e7a8ae8 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -126,12 +126,16 @@ class Ameba::Config {% definitions = [] of NamedTuple %} {% if block.body.is_a? Assign %} {% definitions << {var: block.body.target, value: block.body.value} %} + {% elsif block.body.is_a? Call %} + {% definitions << {var: block.body.name, value: block.body.args.first} %} {% elsif block.body.is_a? TypeDeclaration %} {% definitions << {var: block.body.var, value: block.body.value, type: block.body.type} %} {% elsif block.body.is_a? Expressions %} {% for prop in block.body.expressions %} {% if prop.is_a? Assign %} {% definitions << {var: prop.target, value: prop.value} %} + {% elsif prop.is_a? Call %} + {% definitions << {var: prop.name, value: prop.args.first} %} {% elsif prop.is_a? TypeDeclaration %} {% definitions << {var: prop.var, value: prop.value, type: prop.type} %} {% end %} diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index cdb4dde0..1e2d9355 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -34,7 +34,7 @@ module Ameba::Rule # that are tested by this rule, it should add an error. abstract def test(source : Source) - def test(source : Source, node : Crystal::ASTNode) + def test(source : Source, node : Crystal::ASTNode, *opts) # can't be abstract end diff --git a/src/ameba/rule/comparison_to_boolean.cr b/src/ameba/rule/comparison_to_boolean.cr index afec9f2f..ff38bfc5 100644 --- a/src/ameba/rule/comparison_to_boolean.cr +++ b/src/ameba/rule/comparison_to_boolean.cr @@ -22,14 +22,14 @@ module Ameba::Rule # struct ComparisonToBoolean < Base properties do - enabled = false - description = "Disallows comparison to booleans" + enabled false + description "Disallows comparison to booleans" end MSG = "Comparison to a boolean is pointless" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/constant_names.cr b/src/ameba/rule/constant_names.cr index e6cbf4fb..30cef1a0 100644 --- a/src/ameba/rule/constant_names.cr +++ b/src/ameba/rule/constant_names.cr @@ -24,13 +24,13 @@ module Ameba::Rule # struct ConstantNames < Base properties do - description = "Enforces constant names to be in screaming case" + description "Enforces constant names to be in screaming case" end MSG = "Constant name should be screaming-cased: %s, not %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Assign) diff --git a/src/ameba/rule/debugger_statement.cr b/src/ameba/rule/debugger_statement.cr index 55eb0919..1c6c76d1 100644 --- a/src/ameba/rule/debugger_statement.cr +++ b/src/ameba/rule/debugger_statement.cr @@ -13,13 +13,13 @@ module Ameba::Rule # struct DebuggerStatement < Base properties do - description = "Disallows calls to debugger" + description "Disallows calls to debugger" end MSG = "Possible forgotten debugger statement detected" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/empty_ensure.cr b/src/ameba/rule/empty_ensure.cr index 45cff0ab..b56da975 100644 --- a/src/ameba/rule/empty_ensure.cr +++ b/src/ameba/rule/empty_ensure.cr @@ -34,13 +34,13 @@ module Ameba::Rule # struct EmptyEnsure < Base properties do - description = "Disallows empty ensure statement" + description "Disallows empty ensure statement" end MSG = "Empty `ensure` block detected" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::ExceptionHandler) diff --git a/src/ameba/rule/empty_expression.cr b/src/ameba/rule/empty_expression.cr index 99470cae..3b4b6ce0 100644 --- a/src/ameba/rule/empty_expression.cr +++ b/src/ameba/rule/empty_expression.cr @@ -32,14 +32,14 @@ module Ameba::Rule include AST::Util properties do - description = "Disallows empty expressions" + description "Disallows empty expressions" end MSG = "Avoid empty expression %s" MSG_EXRS = "Avoid empty expressions" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::NilLiteral) diff --git a/src/ameba/rule/hash_duplicated_key.cr b/src/ameba/rule/hash_duplicated_key.cr index 550a366b..828e2dae 100644 --- a/src/ameba/rule/hash_duplicated_key.cr +++ b/src/ameba/rule/hash_duplicated_key.cr @@ -22,13 +22,13 @@ module Ameba::Rule # struct HashDuplicatedKey < Base properties do - description = "Disallows duplicated keys in hash literals" + description "Disallows duplicated keys in hash literals" end MSG = "Duplicated keys in hash literal: %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::HashLiteral) diff --git a/src/ameba/rule/large_numbers.cr b/src/ameba/rule/large_numbers.cr index 18e1742a..fc8c9317 100644 --- a/src/ameba/rule/large_numbers.cr +++ b/src/ameba/rule/large_numbers.cr @@ -29,8 +29,8 @@ module Ameba::Rule # struct LargeNumbers < Base properties do - description = "Disallows usage of large numbers without underscore" - int_min_digits = 5 + description "Disallows usage of large numbers without underscore" + int_min_digits 5 end MSG = "Large numbers should be written with underscores: %s" diff --git a/src/ameba/rule/line_length.cr b/src/ameba/rule/line_length.cr index 83094f71..7ab2ccf1 100644 --- a/src/ameba/rule/line_length.cr +++ b/src/ameba/rule/line_length.cr @@ -11,9 +11,9 @@ module Ameba::Rule # struct LineLength < Base properties do - enabled = false - description = "Disallows lines longer than `MaxLength` number of symbols" - max_length = 80 + enabled false + description "Disallows lines longer than `MaxLength` number of symbols" + max_length 80 end MSG = "Line too long" diff --git a/src/ameba/rule/literal_in_condition.cr b/src/ameba/rule/literal_in_condition.cr index 7d3433c4..ef558e56 100644 --- a/src/ameba/rule/literal_in_condition.cr +++ b/src/ameba/rule/literal_in_condition.cr @@ -24,14 +24,14 @@ module Ameba::Rule include AST::Util properties do - description = "Disallows useless conditional statements that contain \ + description "Disallows useless conditional statements that contain \ a literal in place of a variable or predicate function" end MSG = "Literal value found in conditional" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def check_node(source, node) diff --git a/src/ameba/rule/literal_in_interpolation.cr b/src/ameba/rule/literal_in_interpolation.cr index cfdf407a..4d118ff7 100644 --- a/src/ameba/rule/literal_in_interpolation.cr +++ b/src/ameba/rule/literal_in_interpolation.cr @@ -20,13 +20,13 @@ module Ameba::Rule include AST::Util properties do - description = "Disallows useless string interpolations" + description "Disallows useless string interpolations" end MSG = "Literal value found in interpolation" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::StringInterpolation) diff --git a/src/ameba/rule/method_names.cr b/src/ameba/rule/method_names.cr index 8e3dfd13..bb58989e 100644 --- a/src/ameba/rule/method_names.cr +++ b/src/ameba/rule/method_names.cr @@ -40,13 +40,13 @@ module Ameba::Rule # struct MethodNames < Base properties do - description = "Enforces method names to be in underscored case" + description "Enforces method names to be in underscored case" end MSG = "Method name should be underscore-cased: %s, not %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Def) diff --git a/src/ameba/rule/negated_conditions_in_unless.cr b/src/ameba/rule/negated_conditions_in_unless.cr index 74d307df..e9e4e0c0 100644 --- a/src/ameba/rule/negated_conditions_in_unless.cr +++ b/src/ameba/rule/negated_conditions_in_unless.cr @@ -29,13 +29,13 @@ module Ameba::Rule # struct NegatedConditionsInUnless < Base properties do - description = "Disallows negated conditions in unless" + description "Disallows negated conditions in unless" end MSG = "Avoid negated conditions in unless blocks" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Unless) diff --git a/src/ameba/rule/percent_arrays.cr b/src/ameba/rule/percent_arrays.cr index 8d8558f8..30335490 100644 --- a/src/ameba/rule/percent_arrays.cr +++ b/src/ameba/rule/percent_arrays.cr @@ -26,9 +26,9 @@ module Ameba::Rule # struct PercentArrays < Base properties do - description = "Disallows some unwanted symbols in percent array literals" - string_array_unwanted_symbols = ",\"" - symbol_array_unwanted_symbols = ",:" + description "Disallows some unwanted symbols in percent array literals" + string_array_unwanted_symbols ",\"" + symbol_array_unwanted_symbols ",:" end MSG = "Symbols `%s` may be unwanted in %s array literals" diff --git a/src/ameba/rule/predicate_name.cr b/src/ameba/rule/predicate_name.cr index 492d45de..7f95e227 100644 --- a/src/ameba/rule/predicate_name.cr +++ b/src/ameba/rule/predicate_name.cr @@ -31,13 +31,13 @@ module Ameba::Rule # struct PredicateName < Base properties do - description = "Disallows tautological predicate names" + description "Disallows tautological predicate names" end MSG = "Favour method name '%s?' over '%s'" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Def) diff --git a/src/ameba/rule/rand_zero.cr b/src/ameba/rule/rand_zero.cr index 6c449e50..df80ed93 100644 --- a/src/ameba/rule/rand_zero.cr +++ b/src/ameba/rule/rand_zero.cr @@ -25,13 +25,13 @@ module Ameba::Rule # struct RandZero < Base properties do - description = "Disallows rand zero calls" + description "Disallows rand zero calls" end MSG = "%s always returns 0" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Call) diff --git a/src/ameba/rule/redundant_begin.cr b/src/ameba/rule/redundant_begin.cr index 972623fc..bf5ec4fc 100644 --- a/src/ameba/rule/redundant_begin.cr +++ b/src/ameba/rule/redundant_begin.cr @@ -59,13 +59,13 @@ module Ameba::Rule struct RedundantBegin < Base include AST::Util properties do - description = "Disallows redundant begin blocks" + description "Disallows redundant begin blocks" end MSG = "Redundant `begin` block detected" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Def) diff --git a/src/ameba/rule/shadowed_exception.cr b/src/ameba/rule/shadowed_exception.cr index fda70711..7af41f8b 100644 --- a/src/ameba/rule/shadowed_exception.cr +++ b/src/ameba/rule/shadowed_exception.cr @@ -36,13 +36,13 @@ module Ameba::Rule # struct ShadowedException < Base properties do - description = "Disallows rescued exception that get shadowed" + description "Disallows rescued exception that get shadowed" end MSG = "Exception handler has shadowed exceptions: %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::ExceptionHandler) diff --git a/src/ameba/rule/trailing_blank_lines.cr b/src/ameba/rule/trailing_blank_lines.cr index 8e579292..3b6663e7 100644 --- a/src/ameba/rule/trailing_blank_lines.cr +++ b/src/ameba/rule/trailing_blank_lines.cr @@ -10,7 +10,7 @@ module Ameba::Rule # struct TrailingBlankLines < Base properties do - description = "Disallows trailing blank lines" + description "Disallows trailing blank lines" end MSG = "Blank lines detected at the end of the file" diff --git a/src/ameba/rule/trailing_whitespace.cr b/src/ameba/rule/trailing_whitespace.cr index 1c39397b..6ef481f6 100644 --- a/src/ameba/rule/trailing_whitespace.cr +++ b/src/ameba/rule/trailing_whitespace.cr @@ -10,7 +10,7 @@ module Ameba::Rule # struct TrailingWhitespace < Base properties do - description = "Disallows trailing whitespaces" + description "Disallows trailing whitespaces" end MSG = "Trailing whitespace detected" diff --git a/src/ameba/rule/type_names.cr b/src/ameba/rule/type_names.cr index f8f56d86..1b274f6b 100644 --- a/src/ameba/rule/type_names.cr +++ b/src/ameba/rule/type_names.cr @@ -54,13 +54,13 @@ module Ameba::Rule # struct TypeNames < Base properties do - description = "Enforces type names in camelcase manner" + description "Enforces type names in camelcase manner" end MSG = "Type name should be camelcased: %s, but it was %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end private def check_node(source, node) diff --git a/src/ameba/rule/unless_else.cr b/src/ameba/rule/unless_else.cr index 2e5ff054..afab396f 100644 --- a/src/ameba/rule/unless_else.cr +++ b/src/ameba/rule/unless_else.cr @@ -45,13 +45,13 @@ module Ameba::Rule # struct UnlessElse < Base properties do - description = "Disallows the use of an `else` block with the `unless`" + description "Disallows the use of an `else` block with the `unless`" end MSG = "Favour if over unless with else" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::Unless) diff --git a/src/ameba/rule/unneeded_disable_directive.cr b/src/ameba/rule/unneeded_disable_directive.cr index 49cfd2f8..4d22f3dc 100644 --- a/src/ameba/rule/unneeded_disable_directive.cr +++ b/src/ameba/rule/unneeded_disable_directive.cr @@ -20,7 +20,7 @@ module Ameba::Rule # struct UnneededDisableDirective < Base properties do - description = "Reports unneeded disable directives in comments" + description "Reports unneeded disable directives in comments" end MSG = "Unnecessary disabling of %s" diff --git a/src/ameba/rule/useless_assign.cr b/src/ameba/rule/useless_assign.cr new file mode 100644 index 00000000..b01eaf60 --- /dev/null +++ b/src/ameba/rule/useless_assign.cr @@ -0,0 +1,51 @@ +module Ameba::Rule + # A rule that disallows useless assignments. + # + # For example, this is considered invalid: + # + # ``` + # def method + # var = 1 + # do_something + # end + # ``` + # + # And has to be written as the following: + # + # ``` + # def method + # var = 1 + # do_something(var) + # end + # ``` + # + # YAML configuration example: + # + # ``` + # UselessAssign: + # Enabled: true + # ``` + # + struct UselessAssign < Base + properties do + description "Disallows useless variable assignments" + end + + MSG = "Useless assignment to variable `%s`" + + def test(source) + AST::ScopeVisitor.new self, source + end + + def test(source, node, scope : AST::Scope) + scope.variables.each do |var| + next if var.captured_by_block? + + var.assignments.each do |assign| + next if assign.referenced? + source.error self, assign.location, MSG % var.name + end + end + end + end +end diff --git a/src/ameba/rule/useless_condition_in_when.cr b/src/ameba/rule/useless_condition_in_when.cr index 930605a6..2bca16cd 100644 --- a/src/ameba/rule/useless_condition_in_when.cr +++ b/src/ameba/rule/useless_condition_in_when.cr @@ -33,7 +33,7 @@ module Ameba::Rule # struct UselessConditionInWhen < Base properties do - description = "Disallows useless conditions in when" + description "Disallows useless conditions in when" end MSG = "Useless condition in when detected" @@ -52,7 +52,7 @@ module Ameba::Rule end def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::When) diff --git a/src/ameba/rule/variable_names.cr b/src/ameba/rule/variable_names.cr index 0a99060f..8b4dbf4e 100644 --- a/src/ameba/rule/variable_names.cr +++ b/src/ameba/rule/variable_names.cr @@ -33,13 +33,13 @@ module Ameba::Rule # struct VariableNames < Base properties do - description = "Enforces variable names to be in underscored case" + description "Enforces variable names to be in underscored case" end MSG = "Var name should be underscore-cased: %s, not %s" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end private def check_node(source, node) diff --git a/src/ameba/rule/while_true.cr b/src/ameba/rule/while_true.cr index 03b31620..22ab5585 100644 --- a/src/ameba/rule/while_true.cr +++ b/src/ameba/rule/while_true.cr @@ -28,13 +28,13 @@ module Ameba::Rule # struct WhileTrue < Base properties do - description = "Disallows while statements with a true literal as condition" + description "Disallows while statements with a true literal as condition" end MSG = "While statement using true literal as condition" def test(source) - AST::Visitor.new self, source + AST::NodeVisitor.new self, source end def test(source, node : Crystal::While)