Refactor AST::Util#literal? helper

This commit is contained in:
Sijawusz Pur Rahnama 2022-11-15 17:41:37 +01:00
parent cc687d0281
commit 2fb37da80f
4 changed files with 48 additions and 39 deletions

View file

@ -13,10 +13,10 @@ module Ameba::Rule::Lint
CRYSTAL CRYSTAL
end end
it "reports if there is a regex comparison possibly evaluating to the same" do it "reports if there is a dynamic comparison possibly evaluating to the same" do
expect_issue subject, <<-CRYSTAL expect_issue subject, <<-CRYSTAL
/foo/ === "foo" [foo] === ["foo"]
# ^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same # ^^^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same
CRYSTAL CRYSTAL
end end

View file

@ -1,7 +1,10 @@
# Utility module for Ameba's rules. # Utility module for Ameba's rules.
module Ameba::AST::Util module Ameba::AST::Util
# Returns true if current `node` is a literal, false otherwise. # Returns tuple with two bool flags:
def literal?(node) #
# 1. is *node* a literal?
# 2. can *node* be proven static?
protected def literal_kind?(node, include_paths = false) : {Bool, Bool}
case node case node
when Crystal::NilLiteral, when Crystal::NilLiteral,
Crystal::BoolLiteral, Crystal::BoolLiteral,
@ -12,21 +15,49 @@ module Ameba::AST::Util
Crystal::RegexLiteral, Crystal::RegexLiteral,
Crystal::ProcLiteral, Crystal::ProcLiteral,
Crystal::MacroLiteral Crystal::MacroLiteral
true {true, true}
when Crystal::RangeLiteral when Crystal::RangeLiteral
literal?(node.from) && literal?(node.to) {true, static_literal?(node.from, include_paths) &&
static_literal?(node.to, include_paths)}
when Crystal::ArrayLiteral, when Crystal::ArrayLiteral,
Crystal::TupleLiteral Crystal::TupleLiteral
node.elements.all? { |el| literal?(el) } {true, node.elements.all? do |el|
static_literal?(el, include_paths)
end}
when Crystal::HashLiteral when Crystal::HashLiteral
node.entries.all? { |entry| literal?(entry.key) && literal?(entry.value) } {true, node.entries.all? do |entry|
static_literal?(entry.key, include_paths) &&
static_literal?(entry.value, include_paths)
end}
when Crystal::NamedTupleLiteral when Crystal::NamedTupleLiteral
node.entries.all? { |entry| literal?(entry.value) } {true, node.entries.all? do |entry|
static_literal?(entry.value, include_paths)
end}
when Crystal::Path
{include_paths, true}
else else
false {false, false}
end end
end end
# Returns `true` if current `node` is a static literal, `false` otherwise.
def static_literal?(node, include_paths = false) : Bool
is_literal, is_static = literal_kind?(node, include_paths)
is_literal && is_static
end
# Returns `true` if current `node` is a dynamic literal, `false` otherwise.
def dynamic_literal?(node, include_paths = false) : Bool
is_literal, is_static = literal_kind?(node, include_paths)
is_literal && !is_static
end
# Returns `true` if current `node` is a literal, `false` otherwise.
def literal?(node, include_paths = false) : Bool
is_literal, _ = literal_kind?(node, include_paths)
is_literal
end
# Returns a source code for the current node. # Returns a source code for the current node.
# This method uses `node.location` and `node.end_location` # This method uses `node.location` and `node.end_location`
# to determine and cut a piece of source of the node. # to determine and cut a piece of source of the node.

View file

@ -30,7 +30,7 @@ module Ameba::Rule::Lint
MSG = "Literal value found in conditional" MSG = "Literal value found in conditional"
def check_node(source, node) def check_node(source, node)
issue_for node, MSG if literal?(node.cond) issue_for node, MSG if static_literal?(node.cond)
end end
def test(source, node : Crystal::If) def test(source, node : Crystal::If)

View file

@ -29,29 +29,6 @@ module Ameba::Rule::Lint
MSG = "Comparison always evaluates to %s" MSG = "Comparison always evaluates to %s"
MSG_LIKELY = "Comparison most likely evaluates to %s" MSG_LIKELY = "Comparison most likely evaluates to %s"
PRIMITIVE_LITERAL_TYPES = {
Crystal::NilLiteral,
Crystal::BoolLiteral,
Crystal::NumberLiteral,
Crystal::CharLiteral,
Crystal::StringLiteral,
Crystal::SymbolLiteral,
Crystal::ProcLiteral,
Crystal::Path,
}
DYNAMIC_LITERAL_TYPES = {
Crystal::RangeLiteral,
Crystal::RegexLiteral,
Crystal::TupleLiteral,
Crystal::NamedTupleLiteral,
Crystal::ArrayLiteral,
Crystal::HashLiteral,
}
LITERAL_TYPES =
PRIMITIVE_LITERAL_TYPES + DYNAMIC_LITERAL_TYPES
# Edge-case: `{{ T == Nil }}` # Edge-case: `{{ T == Nil }}`
# #
# Current implementation just skips all macro contexts, # Current implementation just skips all macro contexts,
@ -72,11 +49,12 @@ module Ameba::Rule::Lint
return unless node.name.in?(OP_NAMES) return unless node.name.in?(OP_NAMES)
return unless (obj = node.obj) && (arg = node.args.first?) return unless (obj = node.obj) && (arg = node.args.first?)
return unless obj.class.in?(LITERAL_TYPES) && obj_is_literal, obj_is_static = literal_kind?(obj, include_paths: true)
arg.class.in?(LITERAL_TYPES) arg_is_literal, arg_is_static = literal_kind?(arg, include_paths: true)
is_dynamic = obj.class.in?(DYNAMIC_LITERAL_TYPES) || return unless obj_is_literal && arg_is_literal
arg.class.in?(DYNAMIC_LITERAL_TYPES)
is_dynamic = !obj_is_static || !arg_is_static
what = what =
case node.name case node.name