From 3586b4242fa0c8102db4d79fc2d8ac896d660b99 Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Mon, 19 Dec 2022 15:40:59 +0100
Subject: [PATCH] Extend `Lint/UnusedBlockArgument` rule with corrections

---
 .../rule/lint/unused_block_argument_spec.cr   | 79 +++++++++++--------
 src/ameba/rule/lint/unused_block_argument.cr  | 21 ++++-
 2 files changed, 65 insertions(+), 35 deletions(-)

diff --git a/spec/ameba/rule/lint/unused_block_argument_spec.cr b/spec/ameba/rule/lint/unused_block_argument_spec.cr
index adcdc6a1..dbc95b8c 100644
--- a/spec/ameba/rule/lint/unused_block_argument_spec.cr
+++ b/spec/ameba/rule/lint/unused_block_argument_spec.cr
@@ -5,105 +5,118 @@ module Ameba::Rule::Lint
 
   describe UnusedBlockArgument do
     it "doesn't report if it is an instance var argument" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         class A
           def initialize(&@callback)
           end
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     it "doesn't report if anonymous" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         def method(a, b, c, &)
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     it "doesn't report if argument name starts with a `_`" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         def method(a, b, c, &_block)
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     it "doesn't report if it is a block and used" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         def method(a, b, c, &block)
           block.call
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     it "reports if block arg is not used" do
-      s = Source.new %(
+      source = expect_issue subject, <<-CRYSTAL
         def method(a, b, c, &block)
+                           # ^ error: Unused block argument `block`. [...]
         end
-      )
-      subject.catch(s).should_not be_valid
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        def method(a, b, c, &_block)
+        end
+        CRYSTAL
     end
 
     it "reports if unused and there is yield" do
-      s = Source.new %(
+      source = expect_issue subject, <<-CRYSTAL
         def method(a, b, c, &block)
+                           # ^ error: Use `&` as an argument name to indicate that it won't be referenced.
           3.times do |i|
             i.try do
               yield i
             end
           end
         end
-      )
-      subject.catch(s).should_not be_valid
+        CRYSTAL
+
+      expect_correction source, <<-CRYSTAL
+        def method(a, b, c, &)
+          3.times do |i|
+            i.try do
+              yield i
+            end
+          end
+        end
+        CRYSTAL
     end
 
     it "doesn't report if anonymous and there is yield" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         def method(a, b, c, &)
           yield 1
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     it "doesn't report if variable is referenced implicitly" do
-      s = Source.new %(
+      expect_no_issues subject, <<-CRYSTAL
         class Bar < Foo
           def method(a, b, c, &block)
             super
           end
         end
-      )
-      subject.catch(s).should be_valid
+        CRYSTAL
     end
 
     context "super" do
       it "reports if variable is not referenced implicitly by super" do
-        s = Source.new %(
+        source = expect_issue subject, <<-CRYSTAL
           class Bar < Foo
             def method(a, b, c, &block)
+                               # ^ error: Unused block argument `block`. [...]
               super a, b, c
             end
           end
-        )
-        subject.catch(s).should_not be_valid
-        s.issues.first.message.should eq "Unused block argument `block`. If it's necessary, " \
-                                         "use `_block` as an argument name to indicate " \
-                                         "that it won't be used."
+          CRYSTAL
+
+        expect_correction source, <<-CRYSTAL
+          class Bar < Foo
+            def method(a, b, c, &_block)
+              super a, b, c
+            end
+          end
+          CRYSTAL
       end
     end
 
     context "macro" do
       it "doesn't report if it is a used macro block argument" do
-        s = Source.new %(
+        expect_no_issues subject, <<-CRYSTAL
           macro my_macro(&block)
             {% block %}
           end
-        )
-        subject.catch(s).should be_valid
+          CRYSTAL
       end
     end
   end
diff --git a/src/ameba/rule/lint/unused_block_argument.cr b/src/ameba/rule/lint/unused_block_argument.cr
index 077b9b23..0af82811 100644
--- a/src/ameba/rule/lint/unused_block_argument.cr
+++ b/src/ameba/rule/lint/unused_block_argument.cr
@@ -31,6 +31,8 @@ module Ameba::Rule::Lint
   #   Enabled: true
   # ```
   class UnusedBlockArgument < Base
+    include AST::Util
+
     properties do
       description "Disallows unused block arguments"
     end
@@ -51,11 +53,26 @@ module Ameba::Rule::Lint
       return if block_arg.anonymous?
       return if scope.references?(block_arg.variable)
 
+      location = block_arg.node.location
+      end_location = location.try &.adjust(column_number: block_arg.name.size - 1)
+
       if scope.yields?
-        issue_for block_arg.node, MSG_YIELDED
+        if location && end_location
+          issue_for block_arg.node, MSG_YIELDED do |corrector|
+            corrector.remove(location, end_location)
+          end
+        else
+          issue_for block_arg.node, MSG_YIELDED
+        end
       else
         return if block_arg.ignored?
-        issue_for block_arg.node, MSG_UNUSED % block_arg.name
+        if location && end_location
+          issue_for block_arg.node, MSG_UNUSED % block_arg.name do |corrector|
+            corrector.insert_before(location, '_')
+          end
+        else
+          issue_for block_arg.node, MSG_UNUSED % block_arg.name
+        end
       end
     end
   end