From 971bff6c277affa2c633dc3e9bf0ce878c907372 Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Fri, 10 Nov 2023 14:27:27 +0100
Subject: [PATCH] Add `Naming/BlockParameterName` rule

---
 .../rule/naming/block_parameter_name_spec.cr  | 100 ++++++++++++++++++
 src/ameba/rule/naming/block_parameter_name.cr |  54 ++++++++++
 2 files changed, 154 insertions(+)
 create mode 100644 spec/ameba/rule/naming/block_parameter_name_spec.cr
 create mode 100644 src/ameba/rule/naming/block_parameter_name.cr

diff --git a/spec/ameba/rule/naming/block_parameter_name_spec.cr b/spec/ameba/rule/naming/block_parameter_name_spec.cr
new file mode 100644
index 00000000..992f7e24
--- /dev/null
+++ b/spec/ameba/rule/naming/block_parameter_name_spec.cr
@@ -0,0 +1,100 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = BlockParameterName.new
+    .tap(&.min_name_length = 3)
+    .tap(&.allowed_names = %w[_ e i j k v])
+
+  describe BlockParameterName do
+    it "passes if block parameter name matches #allowed_names" do
+      subject.allowed_names.each do |name|
+        expect_no_issues subject, <<-CRYSTAL
+          %w[].each { |#{name}| }
+          CRYSTAL
+      end
+    end
+
+    it "fails if block parameter name doesn't match #allowed_names" do
+      expect_issue subject, <<-CRYSTAL
+        %w[].each { |x| }
+                   # ^ error: Disallowed block parameter name found
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#min_name_length" do
+        it "allows setting custom values" do
+          rule = BlockParameterName.new
+          rule.allowed_names = %w[a b c]
+
+          rule.min_name_length = 3
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.min_name_length = 1
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+
+      context "#allow_names_ending_in_numbers" do
+        it "allows setting custom values" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 1
+          rule.allowed_names = %w[]
+
+          rule.allow_names_ending_in_numbers = false
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x1| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.allow_names_ending_in_numbers = true
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x1| }
+            CRYSTAL
+        end
+      end
+
+      context "#allowed_names" do
+        it "allows setting custom names" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 3
+
+          rule.allowed_names = %w[a b c]
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.allowed_names = %w[x y z]
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+
+      context "#forbidden_names" do
+        it "allows setting custom names" do
+          rule = BlockParameterName.new
+          rule.min_name_length = 1
+          rule.allowed_names = %w[]
+
+          rule.forbidden_names = %w[x y z]
+          expect_issue rule, <<-CRYSTAL
+            %w[].each { |x| }
+                       # ^ error: Disallowed block parameter name found
+            CRYSTAL
+
+          rule.forbidden_names = %w[a b c]
+          expect_no_issues rule, <<-CRYSTAL
+            %w[].each { |x| }
+            CRYSTAL
+        end
+      end
+    end
+  end
+end
diff --git a/src/ameba/rule/naming/block_parameter_name.cr b/src/ameba/rule/naming/block_parameter_name.cr
new file mode 100644
index 00000000..c4bf358a
--- /dev/null
+++ b/src/ameba/rule/naming/block_parameter_name.cr
@@ -0,0 +1,54 @@
+module Ameba::Rule::Naming
+  # A rule that reports non-descriptive block parameter names.
+  #
+  # Favour this:
+  #
+  # ```
+  # tokens.each { |token| token.last_accessed_at = Time.utc }
+  # ```
+  #
+  # Over this:
+  #
+  # ```
+  # tokens.each { |t| t.last_accessed_at = Time.utc }
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Naming/BlockParameterName:
+  #   Enabled: true
+  #   MinNameLength: 3
+  #   AllowNamesEndingInNumbers: true
+  #   AllowedNames: [_, e, i, j, k, v, x, y, ex, io, ws, tx, id, k1, k2, v1, v2]
+  #   ForbiddenNames: []
+  # ```
+  class BlockParameterName < Base
+    properties do
+      description "Disallows non-descriptive block parameter names"
+      min_name_length 3
+      allow_names_ending_in_numbers true
+      allowed_names %w[_ e i j k v x y ex io ws tx id k1 k2 v1 v2]
+      forbidden_names %w[]
+    end
+
+    MSG = "Disallowed block parameter name found"
+
+    def test(source, node : Crystal::Call)
+      node.try(&.block).try(&.args).try &.each do |arg|
+        issue_for arg, MSG unless valid_name?(arg.name)
+      end
+    end
+
+    private def valid_name?(name)
+      return true if name.blank? # happens with compound names like `(arg1, arg2)`
+      return true if name.in?(allowed_names)
+
+      return false if name.in?(forbidden_names)
+      return false if name.size < min_name_length
+      return false if name[-1].ascii_number? && !allow_names_ending_in_numbers?
+
+      true
+    end
+  end
+end