diff --git a/spec/ameba/rule/useless_condition_in_when_spec.cr b/spec/ameba/rule/useless_condition_in_when_spec.cr new file mode 100644 index 00000000..d504793c --- /dev/null +++ b/spec/ameba/rule/useless_condition_in_when_spec.cr @@ -0,0 +1,45 @@ +require "../../spec_helper" + +module Ameba::Rule + subject = UselessConditionInWhen.new + + describe UselessConditionInWhen do + it "passes if there is not useless condition" do + s = Source.new %( + case + when utc? + io << " UTC" + when local? + Format.new(" %:z").format(self, io) if utc? + end + ) + subject.catch(s).should be_valid + end + + it "fails if there is useless if condition" do + s = Source.new %( + case + when utc? + io << " UTC" if utc? + end + ) + subject.catch(s).should_not be_valid + end + + it "reports rule, location and message" do + s = Source.new %( + case + when String + puts "hello" + when can_generate? + generate if can_generate? + 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:6:23" + error.message.should eq "Useless condition in when detected" + end + end +end diff --git a/src/ameba/ast/traverse.cr b/src/ameba/ast/traverse.cr index 2ee803da..b42140aa 100644 --- a/src/ameba/ast/traverse.cr +++ b/src/ameba/ast/traverse.cr @@ -23,6 +23,7 @@ module Ameba::AST StringInterpolation, Unless, Var, + When, While, ] diff --git a/src/ameba/rule/useless_condition_in_when.cr b/src/ameba/rule/useless_condition_in_when.cr new file mode 100644 index 00000000..8d0bd69f --- /dev/null +++ b/src/ameba/rule/useless_condition_in_when.cr @@ -0,0 +1,84 @@ +module Ameba::Rule + # A rule that disallows useless conditions in when clause + # where it is guaranteed to always return the same result. + # + # For example, this is considered invalid: + # + # ``` + # case + # when utc? + # io << " UTC" + # when local? + # Format.new(" %:z").format(self, io) if local? + # end + # ``` + # + # And has to be written as the following: + # + # ``` + # case + # when utc? + # io << " UTC" + # when local? + # Format.new(" %:z").format(self, io) + # end + # ``` + # + # YAML configuration example: + # + # ``` + # UselessConditionInWhen: + # Enabled: true + # ``` + # + struct UselessConditionInWhen < Base + properties do + description = "Disallows useless conditions in when" + end + + # TODO: condition.cond may be a complex ASTNode with + # useless inner conditions. We might need to improve this + # simple implementation in future. + protected def check_node(source, when_node, cond) + cond_s = cond.to_s + return if when_node + .conds + .map(&.to_s) + .none? { |c| c == cond_s } + + source.error self, cond.location, "Useless condition in when detected" + end + + def test(source) + AST::Visitor.new self, source + end + + def test(source, node : Crystal::When) + ConditionInWhenVisitor.new self, source, node + end + + class ConditionInWhenVisitor < Crystal::Visitor + @source : Source + @rule : UselessConditionInWhen + @parent : Crystal::When + + def initialize(@rule, @source, @parent) + @parent.accept self + end + + def visit(node : Crystal::If) + @rule.check_node(@source, @parent, node.cond) + true + end + + def visit(node : Crystal::Unless) + @rule.check_node(@source, @parent, node.cond) + true + end + + def visit(node : Crystal::ASTNode) + true + end + end + end +end