New rule: useless condition in when

thanks to @hugoabonizio for suggesstion
This commit is contained in:
Vitalii Elenhaupt 2017-12-02 21:44:25 +02:00
parent 8cbdd0de4d
commit 96c1af4e35
No known key found for this signature in database
GPG key ID: 7558EF3A4056C706
3 changed files with 130 additions and 0 deletions

View file

@ -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

View file

@ -23,6 +23,7 @@ module Ameba::AST
StringInterpolation,
Unless,
Var,
When,
While,
]

View file

@ -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