diff --git a/spec/ameba/rule/lint/unreachable_code_spec.cr b/spec/ameba/rule/lint/unreachable_code_spec.cr index 4a30272d..e3ef9a89 100644 --- a/spec/ameba/rule/lint/unreachable_code_spec.cr +++ b/spec/ameba/rule/lint/unreachable_code_spec.cr @@ -33,13 +33,46 @@ module Ameba::Rule::Lint it "doesn't report if there are returns in if-then-else" do s = Source.new %( if a > 0 - return :positivie + return :positive else return :negative end ) subject.catch(s).should be_valid end + + it "doesn't report if return is used in a block" do + s = Source.new %( + def foo + bar = obj.try do + if something + a = 1 + end + return nil + end + + bar + end + ) + subject.catch(s).should be_valid + end + + pending "reports if there is unreachable code after if-then-else" do + s = Source.new %( + def foo + if a > 0 + return :positive + else + return :negative + end + + :unreachable + end + ) + subject.catch(s).should_not be_valid + issue = s.issues.first + issue.location.to_s.should eq ":8:4" + end end context "break" do @@ -98,5 +131,98 @@ module Ameba::Rule::Lint subject.catch(s).should be_valid end end + + context "raise" do + it "reports if there is unreachable code after raise" do + s = Source.new %( + a = 1 + raise "exception" + b = 2 + ) + subject.catch(s).should_not be_valid + + issue = s.issues.first + issue.location.to_s.should eq ":3:1" + end + + it "doesn't report if raise is in a condition" do + s = Source.new %( + a = 1 + raise "exception" if a > 0 + b = 2 + ) + subject.catch(s).should be_valid + end + end + + context "exit" do + it "reports if there is unreachable code after exit without args" do + s = Source.new %( + a = 1 + exit + b = 2 + ) + subject.catch(s).should_not be_valid + + issue = s.issues.first + issue.location.to_s.should eq ":3:1" + end + + it "reports if there is unreachable code after exit with exit code" do + s = Source.new %( + a = 1 + exit 1 + b = 2 + ) + subject.catch(s).should_not be_valid + + issue = s.issues.first + issue.location.to_s.should eq ":3:1" + end + + it "doesn't report if exit is in a condition" do + s = Source.new %( + a = 1 + exit if a > 0 + b = 2 + ) + subject.catch(s).should be_valid + end + end + + context "abort" do + it "reports if there is unreachable code after abort with one argument" do + s = Source.new %( + a = 1 + abort "abort" + b = 2 + ) + subject.catch(s).should_not be_valid + + issue = s.issues.first + issue.location.to_s.should eq ":3:1" + end + + it "reports if there is unreachable code after abort with two args" do + s = Source.new %( + a = 1 + abort "abort", 1 + b = 2 + ) + subject.catch(s).should_not be_valid + + issue = s.issues.first + issue.location.to_s.should eq ":3:1" + end + + it "doesn't report if abort is in a condition" do + s = Source.new %( + a = 1 + abort "abort" if a > 0 + b = 2 + ) + subject.catch(s).should be_valid + end + end end end diff --git a/src/ameba/ast/visitors/flow_expression_visitor.cr b/src/ameba/ast/visitors/flow_expression_visitor.cr index 88b3c72d..d632a70b 100644 --- a/src/ameba/ast/visitors/flow_expression_visitor.cr +++ b/src/ameba/ast/visitors/flow_expression_visitor.cr @@ -4,6 +4,7 @@ module Ameba::AST # AST Visitor that traverses all the flow expressions. class FlowExpressionVisitor < BaseVisitor @node_stack = Array(Crystal::ASTNode).new + @flow_expression : FlowExpression? def initialize(@rule, @source) @source.ast.accept self @@ -14,20 +15,46 @@ module Ameba::AST end def end_visit(node) - @node_stack.pop + if @flow_expression.nil? + @node_stack.pop unless @node_stack.empty? + else + @flow_expression = nil + end end def visit(node : Crystal::ControlExpression) - if parent_node = @node_stack.last? - flow_expression = FlowExpression.new(node, parent_node) - @rule.test @source, node, flow_expression + on_flow_expression_start(node) + + true + end + + def visit(node : Crystal::Call) + if raise?(node) || exit?(node) || abort?(node) + on_flow_expression_start(node) + else + @node_stack.push node end true end - def end_visit(node : Crystal::ControlExpression) - # + private def on_flow_expression_start(node) + if parent_node = @node_stack.last? + @flow_expression = FlowExpression.new(node, parent_node) + @rule.test @source, node, @flow_expression + end + end + + private def raise?(node) + node.name == "raise" && node.args.size == 1 && node.obj.nil? + end + + private def exit?(node) + node.name == "exit" && node.args.size <= 1 && node.obj.nil? + end + + private def abort?(node) + node.name == "abort" && node.args.size <= 2 && node.obj.nil? end end end