Merge pull request #418 from crystal-ameba/add-rescued-exceptions-variable-name-rule

Add `Naming/RescuedExceptionsVariableName` rule

spec/ameba/rule/naming/rescued_exceptions_variable_name_spec.cr

@@ -0,0 +1,53 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = RescuedExceptionsVariableName.new
+
+  describe RescuedExceptionsVariableName do
+    it "passes if exception handler variable name matches #allowed_names" do
+      subject.allowed_names.each do |name|
+        expect_no_issues subject, <<-CRYSTAL
+          def foo
+            raise "foo"
+          rescue #{name}
+            nil
+          end
+          CRYSTAL
+      end
+    end
+
+    it "fails if exception handler variable name doesn't match #allowed_names" do
+      expect_issue subject, <<-CRYSTAL
+        def foo
+          raise "foo"
+        rescue wtf
+        # ^^^^^^^^ error: Disallowed variable name, use one of these instead: 'e', 'ex', 'exception'
+          nil
+        end
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#allowed_names" do
+        it "returns sensible defaults" do
+          rule = RescuedExceptionsVariableName.new
+          rule.allowed_names.should eq %w[e ex exception]
+        end
+
+        it "allows setting custom names" do
+          rule = RescuedExceptionsVariableName.new
+          rule.allowed_names = %w[foo]
+
+          expect_issue rule, <<-CRYSTAL
+            def foo
+              raise "foo"
+            rescue e
+            # ^^^^^^ error: Disallowed variable name, use 'foo' instead
+              nil
+            end
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

src/ameba/rule/naming/rescued_exceptions_variable_name.cr

@@ -0,0 +1,51 @@
+module Ameba::Rule::Naming
+  # A rule that makes sure that rescued exceptions variables are named as expected.
+  #
+  # For example, these are considered valid:
+  #
+  #     def foo
+  #       # potentially raising computations
+  #     rescue e
+  #       Log.error(exception: e) { "Error" }
+  #     end
+  #
+  # And these are invalid variable names:
+  #
+  #     def foo
+  #       # potentially raising computations
+  #     rescue wtf
+  #       Log.error(exception: wtf) { "Error" }
+  #     end
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Naming/RescuedExceptionsVariableName:
+  #   Enabled: true
+  #   AllowedNames: [e, ex, exception]
+  # ```
+  class RescuedExceptionsVariableName < Base
+    properties do
+      description "Makes sure that rescued exceptions variables are named as expected"
+      allowed_names %w[e ex exception]
+    end
+
+    MSG          = "Disallowed variable name, use one of these instead: '%s'"
+    MSG_SINGULAR = "Disallowed variable name, use '%s' instead"
+
+    def test(source, node : Crystal::ExceptionHandler)
+      node.rescues.try &.each do |r|
+        next if valid_name?(r.name)
+
+        message =
+          allowed_names.size == 1 ? MSG_SINGULAR : MSG
+
+        issue_for r, message % allowed_names.join("', '")
+      end
+    end
+
+    private def valid_name?(name)
+      !name || name.in?(allowed_names)
+    end
+  end
+end