Merge pull request #297 from crystal-ameba/Sija/fix-issue-295

Fix the edge case re: free var comparison
This commit is contained in:
Sijawusz Pur Rahnama 2022-11-13 17:35:43 +01:00 committed by GitHub
commit df4b49798e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 1 deletions

View file

@ -41,13 +41,26 @@ module Ameba::Rule::Lint
CRYSTAL CRYSTAL
end 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 context "macro" do
it "reports in macro scope" do pending "reports in macro scope" do
expect_issue subject, <<-CRYSTAL expect_issue subject, <<-CRYSTAL
{{ "foo" == "foo" }} {{ "foo" == "foo" }}
# ^^^^^^^^^^^^^^ error: Comparison always evaluates to true # ^^^^^^^^^^^^^^ error: Comparison always evaluates to true
CRYSTAL CRYSTAL
end end
it "passes for free variables comparisons in macro scope" do
expect_no_issues subject, <<-CRYSTAL
{{ T == Nil }}
CRYSTAL
end
end end
it "reports rule, pos and message" do it "reports rule, pos and message" do

View file

@ -52,6 +52,22 @@ module Ameba::Rule::Lint
LITERAL_TYPES = LITERAL_TYPES =
PRIMITIVE_LITERAL_TYPES + DYNAMIC_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) def test(source, node : Crystal::Call)
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?)