diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr index 5c039653..595197b8 100644 --- a/spec/ameba/rule/lint/literals_comparison_spec.cr +++ b/spec/ameba/rule/lint/literals_comparison_spec.cr @@ -41,13 +41,26 @@ module Ameba::Rule::Lint CRYSTAL end + 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 + CRYSTAL + 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 CRYSTAL end + + it "passes for free variables comparisons in macro scope" do + expect_no_issues subject, <<-CRYSTAL + {{ T == Nil }} + CRYSTAL + end end it "reports rule, pos and message" do diff --git a/src/ameba/rule/lint/literals_comparison.cr b/src/ameba/rule/lint/literals_comparison.cr index 183da4eb..59ecc0a8 100644 --- a/src/ameba/rule/lint/literals_comparison.cr +++ b/src/ameba/rule/lint/literals_comparison.cr @@ -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?)