From 5491d31b5ffa94d8d29a58a32f2e8ec8a0047f9e Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Sun, 13 Nov 2022 00:24:58 +0100 Subject: [PATCH] Fix the edge case re: free var comparison --- spec/ameba/rule/lint/literals_comparison_spec.cr | 13 +++++++++++++ src/ameba/rule/lint/literals_comparison.cr | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr index 5c039653..920a133b 100644 --- a/spec/ameba/rule/lint/literals_comparison_spec.cr +++ b/spec/ameba/rule/lint/literals_comparison_spec.cr @@ -41,6 +41,13 @@ module Ameba::Rule::Lint CRYSTAL end + pending "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 expect_issue subject, <<-CRYSTAL @@ -48,6 +55,12 @@ module Ameba::Rule::Lint # ^^^^^^^^^^^^^^ 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..7bdbfe60 100644 --- a/src/ameba/rule/lint/literals_comparison.cr +++ b/src/ameba/rule/lint/literals_comparison.cr @@ -59,6 +59,17 @@ 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)