diff --git a/spec/ameba/rule/lint/literals_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr index 920a133b..595197b8 100644 --- a/spec/ameba/rule/lint/literals_comparison_spec.cr +++ b/spec/ameba/rule/lint/literals_comparison_spec.cr @@ -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 diff --git a/src/ameba/rule/lint/literals_comparison.cr b/src/ameba/rule/lint/literals_comparison.cr index 7bdbfe60..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?) @@ -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)