From 6bf8db81e32d335b25e9b461b8bb74243597cbfe Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 2 Nov 2022 01:39:10 +0100 Subject: [PATCH 1/2] Refactor `Lint/StaticComparison` to `LiteralsComparison` --- ...on_spec.cr => literals_comparison_spec.cr} | 11 ++++-- ...c_comparison.cr => literals_comparison.cr} | 36 +++++++++++++------ 2 files changed, 35 insertions(+), 12 deletions(-) rename spec/ameba/rule/lint/{static_comparison_spec.cr => literals_comparison_spec.cr} (83%) rename src/ameba/rule/lint/{static_comparison.cr => literals_comparison.cr} (54%) diff --git a/spec/ameba/rule/lint/static_comparison_spec.cr b/spec/ameba/rule/lint/literals_comparison_spec.cr similarity index 83% rename from spec/ameba/rule/lint/static_comparison_spec.cr rename to spec/ameba/rule/lint/literals_comparison_spec.cr index d905cdc0..5c039653 100644 --- a/spec/ameba/rule/lint/static_comparison_spec.cr +++ b/spec/ameba/rule/lint/literals_comparison_spec.cr @@ -1,9 +1,9 @@ require "../../../spec_helper" module Ameba::Rule::Lint - subject = StaticComparison.new + subject = LiteralsComparison.new - describe StaticComparison do + describe LiteralsComparison do it "passes for valid cases" do expect_no_issues subject, <<-CRYSTAL "foo" == foo @@ -13,6 +13,13 @@ module Ameba::Rule::Lint CRYSTAL end + it "reports if there is a regex comparison possibly evaluating to the same" do + expect_issue subject, <<-CRYSTAL + /foo/ === "foo" + # ^^^^^^^^^^^^^ error: Comparison most likely evaluates to the same + CRYSTAL + end + it "reports if there is a static comparison evaluating to the same" do expect_issue subject, <<-CRYSTAL "foo" === "foo" diff --git a/src/ameba/rule/lint/static_comparison.cr b/src/ameba/rule/lint/literals_comparison.cr similarity index 54% rename from src/ameba/rule/lint/static_comparison.cr rename to src/ameba/rule/lint/literals_comparison.cr index e66ace5a..df761ffc 100644 --- a/src/ameba/rule/lint/static_comparison.cr +++ b/src/ameba/rule/lint/literals_comparison.cr @@ -1,6 +1,9 @@ module Ameba::Rule::Lint - # This rule is used to identify static comparisons - - # the ones that will always have the same result. + # This rule is used to identify comparisons between two literals. + # + # They usually have the same result - except for non-primitive + # types like containers, range or regex. + # # # For example, this will be always false: # @@ -11,39 +14,52 @@ module Ameba::Rule::Lint # YAML configuration example: # # ``` - # Lint/StaticComparison: + # Lint/LiteralsComparison: # Enabled: true # ``` - class StaticComparison < Base + class LiteralsComparison < Base include AST::Util properties do - description "Identifies static comparisons" + description "Identifies comparisons between literals" end OP_NAMES = %w(=== == !=) - MSG = "Comparison always evaluates to %s" - PRIMITIVES = { + MSG = "Comparison always evaluates to %s" + MSG_LIKELY = "Comparison most likely evaluates to %s" + + PRIMITIVE_LITERAL_TYPES = { Crystal::NilLiteral, Crystal::BoolLiteral, Crystal::NumberLiteral, Crystal::CharLiteral, Crystal::StringLiteral, Crystal::SymbolLiteral, + Crystal::ProcLiteral, + } + + DYNAMIC_LITERAL_TYPES = { Crystal::RangeLiteral, Crystal::RegexLiteral, Crystal::TupleLiteral, Crystal::NamedTupleLiteral, Crystal::ArrayLiteral, Crystal::HashLiteral, - Crystal::ProcLiteral, } + LITERAL_TYPES = + PRIMITIVE_LITERAL_TYPES + DYNAMIC_LITERAL_TYPES + def test(source, node : Crystal::Call) return unless node.name.in?(OP_NAMES) return unless (obj = node.obj) && (arg = node.args.first?) - return unless obj.class.in?(PRIMITIVES) && arg.class.in?(PRIMITIVES) + + return unless obj.class.in?(LITERAL_TYPES) && + arg.class.in?(LITERAL_TYPES) + + is_dynamic = obj.class.in?(DYNAMIC_LITERAL_TYPES) || + arg.class.in?(DYNAMIC_LITERAL_TYPES) what = case node.name @@ -52,7 +68,7 @@ module Ameba::Rule::Lint when "!=" then (obj.to_s != arg.to_s).to_s end - issue_for node, MSG % what + issue_for node, (is_dynamic ? MSG_LIKELY : MSG) % what end end end From aee5de517bc90c5bb0cb26220e125febc45b59a9 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 2 Nov 2022 02:05:27 +0100 Subject: [PATCH 2/2] Add `Crystal::Path` to the list of primitive types So, the comparisons like `Regex == 1=42` will also be detected. --- src/ameba/rule/lint/literals_comparison.cr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ameba/rule/lint/literals_comparison.cr b/src/ameba/rule/lint/literals_comparison.cr index df761ffc..183da4eb 100644 --- a/src/ameba/rule/lint/literals_comparison.cr +++ b/src/ameba/rule/lint/literals_comparison.cr @@ -37,6 +37,7 @@ module Ameba::Rule::Lint Crystal::StringLiteral, Crystal::SymbolLiteral, Crystal::ProcLiteral, + Crystal::Path, } DYNAMIC_LITERAL_TYPES = {