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 53% rename from src/ameba/rule/lint/static_comparison.cr rename to src/ameba/rule/lint/literals_comparison.cr index e66ace5a..183da4eb 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,53 @@ 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, + Crystal::Path, + } + + 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 +69,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