Change to alternative approach skipping all macro contexts

This commit is contained in:
Sijawusz Pur Rahnama 2022-11-13 01:25:24 +01:00
parent 5491d31b5f
commit 84540d2a22
2 changed files with 18 additions and 13 deletions

View file

@ -41,7 +41,7 @@ module Ameba::Rule::Lint
CRYSTAL
end
pending "reports if there is a static path comparison evaluating to false" do
it "reports if there is a static path comparison evaluating to false" do
expect_issue subject, <<-CRYSTAL
String == Nil
# ^^^^^^^^^^^ error: Comparison always evaluates to false
@ -49,7 +49,7 @@ module Ameba::Rule::Lint
end
context "macro" do
it "reports in macro scope" do
pending "reports in macro scope" do
expect_issue subject, <<-CRYSTAL
{{ "foo" == "foo" }}
# ^^^^^^^^^^^^^^ error: Comparison always evaluates to true

View file

@ -52,6 +52,22 @@ module Ameba::Rule::Lint
LITERAL_TYPES =
PRIMITIVE_LITERAL_TYPES + DYNAMIC_LITERAL_TYPES
# Edge-case: `{{ T == Nil }}`
#
# Current implementation just skips all macro contexts,
# regardless of the free variable being present.
#
# Ideally we should only check whether either of the sides
# is a free var
def test(source)
AST::NodeVisitor.new self, source, skip: [
Crystal::Macro,
Crystal::MacroExpression,
Crystal::MacroIf,
Crystal::MacroFor,
]
end
def test(source, node : Crystal::Call)
return unless node.name.in?(OP_NAMES)
return unless (obj = node.obj) && (arg = node.args.first?)
@ -59,17 +75,6 @@ module Ameba::Rule::Lint
return unless obj.class.in?(LITERAL_TYPES) &&
arg.class.in?(LITERAL_TYPES)
# Edge-case: `{{ T == Nil }}`
#
# Current implementation works for any single name path comparisons:
# `String == Nil`, regardless of the free variable being present.
#
# Ideally we should only check whether either of the sides
# is a free var
if obj.is_a?(Crystal::Path) && arg.is_a?(Crystal::Path)
return if obj.single_name? && arg.single_name? && (obj != arg)
end
is_dynamic = obj.class.in?(DYNAMIC_LITERAL_TYPES) ||
arg.class.in?(DYNAMIC_LITERAL_TYPES)