From 6dde0a92ba4caf20a11ea4da97a4d24ba6f5eabb Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 19 Feb 2025 02:50:14 +0100 Subject: [PATCH 1/2] Report all literals in `Lint/LiteralInCondition`, except for the `case` expression --- .../rule/lint/literal_in_condition_spec.cr | 20 +++++++++++++++++-- src/ameba/rule/lint/literal_in_condition.cr | 11 +++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/spec/ameba/rule/lint/literal_in_condition_spec.cr b/spec/ameba/rule/lint/literal_in_condition_spec.cr index 5d602f5a2..67d338885 100644 --- a/spec/ameba/rule/lint/literal_in_condition_spec.cr +++ b/spec/ameba/rule/lint/literal_in_condition_spec.cr @@ -26,7 +26,23 @@ module Ameba::Rule::Lint subject.catch(s).should be_valid end - it "fails if there is a predicate in if conditional" do + it "fails if there is a predicate with non-literals" do + s = Source.new %( + :ok if [foo, bar] + :ok unless [foo, bar] + + while [foo, bar] + :ok + end + + until [foo, bar] + :ok + end + ) + subject.catch(s).should_not be_valid + end + + it "fails if there is a predicate in `if` conditional" do s = Source.new %( if "string" :ok @@ -35,7 +51,7 @@ module Ameba::Rule::Lint subject.catch(s).should_not be_valid end - it "fails if there is a predicate in unless conditional" do + it "fails if there is a predicate in `unless` conditional" do s = Source.new %( unless true :ok diff --git a/src/ameba/rule/lint/literal_in_condition.cr b/src/ameba/rule/lint/literal_in_condition.cr index 4678ecba1..150b549ce 100644 --- a/src/ameba/rule/lint/literal_in_condition.cr +++ b/src/ameba/rule/lint/literal_in_condition.cr @@ -3,7 +3,7 @@ module Ameba::Rule::Lint # in place of a variable or predicate function. # # This is because a conditional construct with a literal predicate will - # always result in the same behaviour at run time, meaning it can be + # always result in the same behavior at run time, meaning it can be # replaced with either the body of the construct, or deleted entirely. # # This is considered invalid: @@ -31,10 +31,15 @@ module Ameba::Rule::Lint MSG = "Literal value found in conditional" - def test(source, node : Crystal::If | Crystal::Unless | Crystal::Case) + def test(source, node : Crystal::If | Crystal::Unless) + issue_for node.cond, MSG if literal?(node.cond) + end + + def test(source, node : Crystal::Case) return unless cond = node.cond + return unless static_literal?(cond) - issue_for cond, MSG if static_literal?(cond) + issue_for cond, MSG end end end From d5ec13a0a329159b48baf3d11665e0ce726af104 Mon Sep 17 00:00:00 2001 From: Sijawusz Pur Rahnama Date: Wed, 19 Feb 2025 02:51:38 +0100 Subject: [PATCH 2/2] Report conditions in `while` and `until` expressions as well --- .../rule/lint/literal_in_condition_spec.cr | 36 +++++++++++++++++++ src/ameba/rule/lint/literal_in_condition.cr | 10 +++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/spec/ameba/rule/lint/literal_in_condition_spec.cr b/spec/ameba/rule/lint/literal_in_condition_spec.cr index 67d338885..1388d960f 100644 --- a/spec/ameba/rule/lint/literal_in_condition_spec.cr +++ b/spec/ameba/rule/lint/literal_in_condition_spec.cr @@ -60,6 +60,42 @@ module Ameba::Rule::Lint subject.catch(s).should_not be_valid end + it "fails if there is a predicate in `while` conditional" do + s = Source.new %( + while 1 + :ok + end + ) + subject.catch(s).should_not be_valid + end + + it "fails if there is a `false` predicate in `while` conditional" do + s = Source.new %( + while false + :ok + end + ) + subject.catch(s).should_not be_valid + end + + it "passes if there is a `true` predicate in `while` conditional" do + s = Source.new %( + while true + :ok + end + ) + subject.catch(s).should be_valid + end + + it "fails if there is a predicate in `until` conditional" do + s = Source.new %( + until true + :foo + end + ) + subject.catch(s).should_not be_valid + end + describe "range" do it "reports range with literals" do s = Source.new %( diff --git a/src/ameba/rule/lint/literal_in_condition.cr b/src/ameba/rule/lint/literal_in_condition.cr index 150b549ce..80c55abf0 100644 --- a/src/ameba/rule/lint/literal_in_condition.cr +++ b/src/ameba/rule/lint/literal_in_condition.cr @@ -31,7 +31,7 @@ module Ameba::Rule::Lint MSG = "Literal value found in conditional" - def test(source, node : Crystal::If | Crystal::Unless) + def test(source, node : Crystal::If | Crystal::Unless | Crystal::Until) issue_for node.cond, MSG if literal?(node.cond) end @@ -41,5 +41,13 @@ module Ameba::Rule::Lint issue_for cond, MSG end + + def test(source, node : Crystal::While) + return unless literal?(cond = node.cond) + # allow `while true` + return if cond.is_a?(Crystal::BoolLiteral) && cond.value + + issue_for cond, MSG + end end end