Add Lint/UnusedComparison and Lint/UnusedLiteral

[nobodywasishere]

Closes #235
Closes #508

This is still a little WIP, I need to go through the code again and write more / better specs. Maybe the visitor can be moved to the main visitors, as it may be useful for writing other Lint/Useless* rules. It's basically useful for checking if the return value from a node will be captured or not, which is how I determined whether the comparison operations were useful.

[Sija]

Nice one ❤️ I'd rename the rule to UnusedComparison though, useless IMO brings some negative connotations.

[Sija]

I'll do the code review once you mark the PR as ready for review, just FYI.

[nobodywasishere]

I'll do the code review once you mark the PR as ready for review, just FYI.

Yeah that's why I marked it as draft, so you wouldn't waste time reviewing it only for me to completely change it.

[nobodywasishere]

Running these rules over the ameba, Kagi, and crystal compiler codebases, didn't turn up any violations. Don't know if that's due to a bug in the rules or violations being rare, though I'm leaning towards the latter.

[nobodywasishere]

The only literal that isn't supported is RegexLiteral, and that's due to them not having a location.

[straight-shoota]

Adding location to RegexLiteral in crystal-lang/crystal#15235

[nobodywasishere]

If desired, I can add support for regex literals gated behind whether Ameba is built with Crystal 1.15.0 or later. This way, support can be added and merged now without requiring ameba to update its minimum version from 1.10

diff --git a/spec/ameba/rule/lint/unused_literal_spec.cr b/spec/ameba/rule/lint/unused_literal_spec.cr
index 5c22ea7b..7494a0ba 100644
--- a/spec/ameba/rule/lint/unused_literal_spec.cr
+++ b/spec/ameba/rule/lint/unused_literal_spec.cr
@@ -252,5 +252,16 @@ module Ameba::Rule::Lint
           end
       CRYSTAL
     end
+
+    # Locations for Regex literals were added in Crystal v1.15.0
+    {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %}
+      it "fails if a regex literal is unused" do
+        expect_issue subject, <<-CRYSTAL
+            a = /hello world/
+            /goodnight moon/
+          # ^^^^^^^^^^^^^^^^ error: Literal value is not used
+          CRYSTAL
+      end
+    {% end %}
   end
 end
diff --git a/src/ameba/rule/lint/unused_literal.cr b/src/ameba/rule/lint/unused_literal.cr
index 50b3fdd6..cdf30201 100644
--- a/src/ameba/rule/lint/unused_literal.cr
+++ b/src/ameba/rule/lint/unused_literal.cr
@@ -57,6 +57,14 @@ module Ameba::Rule::Lint
       AST::ImplicitReturnVisitor.new(self, source)
     end
 
+    # Locations for Regex literals were added in Crystal v1.15.0
+    {% if compare_versions(Crystal::VERSION, "1.15.0") >= 0 %}
+      def test(source, node : Crystal::RegexLiteral, last_is_used : Bool) : Bool
+        issue_for node, MSG unless last_is_used
+        true
+      end
+    {% end %}
+
     def test(
       source,
       node : Crystal::BoolLiteral | Crystal::CharLiteral | Crystal::HashLiteral |

[Sija]

@nobodywasishere That would be great 🚀

[veelenga]

LGTM. Great job 👏🏻

[carlhoerberg]

Seems to think that IO.copy(in, out, len) == len || raise IO::EOFError.new is an unused comparison, and that { error: "abc" }.to_json(context.response) is an unused literal.

[nobodywasishere]

Good catch! I'll do a follow-up PR trying to limit these edge-cases. Since it's impossible to know whether a method has side-effects, I may not catch || and && if one of them is a call, and not catch literals if they're the object of a call. I'll also do some tests to see if there are other edge-cases like this that need to be handled.