From 849be5261898ccc122ec55996da74213f750fb6a Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 1 Nov 2022 03:17:37 +0100 Subject: [PATCH 1/2] Add `Lint/StaticComparison` rule --- .../ameba/rule/lint/static_comparison_spec.cr | 51 ++++++++++++++++ src/ameba/rule/lint/static_comparison.cr | 58 +++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 spec/ameba/rule/lint/static_comparison_spec.cr create mode 100644 src/ameba/rule/lint/static_comparison.cr diff --git a/spec/ameba/rule/lint/static_comparison_spec.cr b/spec/ameba/rule/lint/static_comparison_spec.cr new file mode 100644 index 00000000..cacc44d6 --- /dev/null +++ b/spec/ameba/rule/lint/static_comparison_spec.cr @@ -0,0 +1,51 @@ +require "../../../spec_helper" + +module Ameba::Rule::Lint + subject = StaticComparison.new + + describe StaticComparison do + it "passes for valid cases" do + expect_no_issues subject, <<-CRYSTAL + /foo/ === "foo" + "foo" == foo + foo == "foo" + CRYSTAL + end + + it "reports if there is a static comparison evaluating to true" do + expect_issue subject, <<-CRYSTAL + "foo" == "foo" + # ^^^^^^^^^^^^ error: Comparison always evaluates to true + CRYSTAL + end + + it "reports if there is a static comparison evaluating to false" do + expect_issue subject, <<-CRYSTAL + "foo" != "foo" + # ^^^^^^^^^^^^ error: Comparison always evaluates to false + CRYSTAL + end + + context "macro" do + it "reports in macro scope" do + expect_issue subject, <<-CRYSTAL + {{ "foo" == "foo" }} + # ^^^^^^^^^^^^^^ error: Comparison always evaluates to true + CRYSTAL + end + end + + it "reports rule, pos and message" do + s = Source.new %( + "foo" == "foo" + ), "source.cr" + subject.catch(s).should_not be_valid + issue = s.issues.first + + issue.rule.should_not be_nil + issue.location.to_s.should eq "source.cr:1:1" + issue.end_location.to_s.should eq "source.cr:1:14" + issue.message.should eq "Comparison always evaluates to true" + end + end +end diff --git a/src/ameba/rule/lint/static_comparison.cr b/src/ameba/rule/lint/static_comparison.cr new file mode 100644 index 00000000..3ea0a285 --- /dev/null +++ b/src/ameba/rule/lint/static_comparison.cr @@ -0,0 +1,58 @@ +module Ameba::Rule::Lint + # This rule is used to identify static comparisons - + # the ones that will always have the same result. + # + # For example, this will be always false: + # + # ``` + # "foo" == 42 + # ``` + # + # YAML configuration example: + # + # ``` + # Lint/StaticComparison: + # Enabled: true + # ``` + class StaticComparison < Base + include AST::Util + + properties do + description "Identifies static comparisons" + end + + OP_NAMES = %w(== !=) + MSG = "Comparison always evaluates to %s" + + PRIMITIVES = { + Crystal::NilLiteral, + Crystal::BoolLiteral, + Crystal::NumberLiteral, + Crystal::CharLiteral, + Crystal::StringLiteral, + Crystal::SymbolLiteral, + Crystal::RangeLiteral, + Crystal::RegexLiteral, + Crystal::TupleLiteral, + Crystal::NamedTupleLiteral, + Crystal::ArrayLiteral, + Crystal::HashLiteral, + Crystal::ProcLiteral, + } + + 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) + + case node.name + when "==" + what = (obj.to_s == arg.to_s).to_s + when "!=" + what = (obj.to_s != arg.to_s).to_s + end + + issue_for node, MSG % what + end + end +end From ea42911c3ceb631cf9d076d063d0140c43b6c7ac Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Tue, 1 Nov 2022 03:35:20 +0100 Subject: [PATCH 2/2] Extend `StaticComparison` w/ support of `===` operator --- spec/ameba/rule/lint/static_comparison_spec.cr | 12 ++++++++++-- src/ameba/rule/lint/static_comparison.cr | 14 +++++++------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/spec/ameba/rule/lint/static_comparison_spec.cr b/spec/ameba/rule/lint/static_comparison_spec.cr index cacc44d6..d905cdc0 100644 --- a/spec/ameba/rule/lint/static_comparison_spec.cr +++ b/spec/ameba/rule/lint/static_comparison_spec.cr @@ -6,13 +6,21 @@ module Ameba::Rule::Lint describe StaticComparison do it "passes for valid cases" do expect_no_issues subject, <<-CRYSTAL - /foo/ === "foo" "foo" == foo + "foo" != foo foo == "foo" + foo != "foo" CRYSTAL end - it "reports if there is a static comparison evaluating to true" do + it "reports if there is a static comparison evaluating to the same" do + expect_issue subject, <<-CRYSTAL + "foo" === "foo" + # ^^^^^^^^^^^^^ error: Comparison always evaluates to the same + CRYSTAL + end + + it "reports if there is a static comparison evaluating to true (2)" do expect_issue subject, <<-CRYSTAL "foo" == "foo" # ^^^^^^^^^^^^ error: Comparison always evaluates to true diff --git a/src/ameba/rule/lint/static_comparison.cr b/src/ameba/rule/lint/static_comparison.cr index 3ea0a285..e66ace5a 100644 --- a/src/ameba/rule/lint/static_comparison.cr +++ b/src/ameba/rule/lint/static_comparison.cr @@ -21,7 +21,7 @@ module Ameba::Rule::Lint description "Identifies static comparisons" end - OP_NAMES = %w(== !=) + OP_NAMES = %w(=== == !=) MSG = "Comparison always evaluates to %s" PRIMITIVES = { @@ -45,12 +45,12 @@ module Ameba::Rule::Lint return unless (obj = node.obj) && (arg = node.args.first?) return unless obj.class.in?(PRIMITIVES) && arg.class.in?(PRIMITIVES) - case node.name - when "==" - what = (obj.to_s == arg.to_s).to_s - when "!=" - what = (obj.to_s != arg.to_s).to_s - end + what = + case node.name + when "===" then "the same" + when "==" then (obj.to_s == arg.to_s).to_s + when "!=" then (obj.to_s != arg.to_s).to_s + end issue_for node, MSG % what end