Add Typing/MethodParameterTypeRestriction (#520)

spec/ameba/rule/typing/method_parameter_type_restriction_spec.cr

@@ -0,0 +1,200 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Typing
+  describe MethodParameterTypeRestriction do
+    subject = MethodParameterTypeRestriction.new
+
+    it "passes if a method parameter has a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        def hello(a : String, b : _) : String
+          "hello world" + a + b
+        end
+
+        def hello(*a : String) : String
+          "hello world" + a.join(", ")
+        end
+        CRYSTAL
+    end
+
+    it "fails if a splat method parameter with a name doesn't have a type restriction" do
+      expect_issue subject, <<-CRYSTAL
+        def hello(*a) : String
+                 # ^ error: Method parameter should have a type restriction
+          "hello world" + a.join(", ")
+        end
+        CRYSTAL
+    end
+
+    it "passes if a splat parameter without a name doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        def hello(hello : String, *, world : String = "world") : String
+          hello + world
+        end
+        CRYSTAL
+    end
+
+    it "passes if a double splat method parameter doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        def hello(a : String, **world) : String
+          "hello world" + a
+        end
+        CRYSTAL
+    end
+
+    it "passes if a private method parameter doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        private def hello(a)
+          "hello world" + a
+        end
+        CRYSTAL
+    end
+
+    it "passes if a protected method parameter doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        protected def hello(a)
+          "hello world" + a
+        end
+        CRYSTAL
+    end
+
+    it "fails if a public method parameter doesn't have a type restriction" do
+      expect_issue subject, <<-CRYSTAL
+        def hello(a)
+                # ^ error: Method parameter should have a type restriction
+          "hello world" + a
+        end
+
+        def hello(a, ext b)
+                # ^ error: Method parameter should have a type restriction
+                   # ^ error: Method parameter should have a type restriction
+          "hello world" + a + b
+        end
+        CRYSTAL
+    end
+
+    it "passes if a method parameter with a default value doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        def hello(a = "jim")
+          "hello there, " + a
+        end
+        CRYSTAL
+    end
+
+    it "passes if a block parameter doesn't have a type restriction" do
+      expect_no_issues subject, <<-CRYSTAL
+        def hello(&)
+          "hello there"
+        end
+        CRYSTAL
+    end
+
+    context "properties" do
+      context "#private_methods" do
+        rule = MethodParameterTypeRestriction.new
+        rule.private_methods = true
+
+        it "passes if a method has a parameter type restriction" do
+          expect_no_issues rule, <<-CRYSTAL
+            private def hello(a : String) : String
+              "hello world" + a
+            end
+            CRYSTAL
+        end
+
+        it "passes if a protected method parameter doesn't have a type restriction" do
+          expect_no_issues rule, <<-CRYSTAL
+            protected def hello(a)
+              "hello world"
+            end
+            CRYSTAL
+        end
+
+        it "fails if a public or private method doesn't have a parameter type restriction" do
+          expect_issue rule, <<-CRYSTAL
+            def hello(a)
+                    # ^ error: Method parameter should have a type restriction
+              "hello world"
+            end
+
+            private def hello(a)
+                            # ^ error: Method parameter should have a type restriction
+              "hello world"
+            end
+            CRYSTAL
+        end
+      end
+
+      context "#protected_methods" do
+        rule = MethodParameterTypeRestriction.new
+        rule.protected_methods = true
+
+        it "passes if a method has a parameter type restriction" do
+          expect_no_issues rule, <<-CRYSTAL
+            protected def hello(a : String) : String
+              "hello world" + a
+            end
+            CRYSTAL
+        end
+
+        it "passes if a private method parameter doesn't have a type restriction" do
+          expect_no_issues rule, <<-CRYSTAL
+            private def hello(a)
+              "hello world"
+            end
+            CRYSTAL
+        end
+
+        it "fails if a public or protected method doesn't have a parameter type restriction" do
+          expect_issue rule, <<-CRYSTAL
+            def hello(a)
+                    # ^ error: Method parameter should have a type restriction
+              "hello world"
+            end
+
+            protected def hello(a)
+                              # ^ error: Method parameter should have a type restriction
+              "hello world"
+            end
+            CRYSTAL
+        end
+      end
+
+      context "#default_value" do
+        it "fails if a method parameter with a default value doesn't have a type restriction" do
+          rule = MethodParameterTypeRestriction.new
+          rule.default_value = true
+
+          expect_issue rule, <<-CRYSTAL
+            def hello(a = "world")
+                    # ^ error: Method parameter should have a type restriction
+              "hello \#{a}"
+            end
+            CRYSTAL
+        end
+      end
+
+      context "#block_parameters" do
+        rule = MethodParameterTypeRestriction.new
+        rule.block_parameters = true
+
+        it "fails if a block parameter without a name doesn't have a type restriction" do
+          expect_issue rule, <<-CRYSTAL
+            def hello(&)
+                     # ^ error: Method parameter should have a type restriction
+              "hello"
+            end
+            CRYSTAL
+        end
+
+        it "fails if a block parameter with a name doesn't have a type restriction" do
+          expect_issue rule, <<-CRYSTAL
+            def hello(&a)
+                     # ^ error: Method parameter should have a type restriction
+              "hello, \#{a.call}"
+            end
+            CRYSTAL
+        end
+      end
+    end
+  end
+end

src/ameba/rule/typing/method_parameter_type_restriction.cr

@@ -0,0 +1,89 @@
+module Ameba::Rule::Typing
+  # A rule that enforces method parameters have type restrictions, with optional enforcement of block parameters.
+  #
+  # For example, this is considered invalid:
+  #
+  # ```
+  # def add(a, b)
+  #   a + b
+  # end
+  # ```
+  #
+  # And this is considered valid:
+  #
+  # ```
+  # def add(a : String, b : String)
+  #   a + b
+  # end
+  # ```
+  #
+  # When the config options `PrivateMethods` and `ProtectedMethods`
+  # are true, this rule is also applied to private and protected methods, respectively.
+  #
+  # The `BlockParameters` configuration option will extend this to block parameters, where these are invalid:
+  #
+  # ```
+  # def exec(&)
+  # end
+  #
+  # def exec(&block)
+  # end
+  # ```
+  #
+  # And this is valid:
+  #
+  # ```
+  # def exec(&block : String -> String)
+  #   yield "cmd"
+  # end
+  # ```
+  #
+  # The config option `DefaultValue` controls whether this rule applies to parameters that have a default value.
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Typing/MethodParameterTypeRestriction:
+  #   Enabled: false
+  #   DefaultValue: false
+  #   PrivateMethods: false
+  #   ProtectedMethods: false
+  #   BlockParameters: false
+  # ```
+  class MethodParameterTypeRestriction < Base
+    properties do
+      since_version "1.7.0"
+      description "Recommends that method parameters have type restrictions"
+      enabled false
+      default_value false
+      private_methods false
+      protected_methods false
+      block_parameters false
+    end
+
+    MSG = "Method parameter should have a type restriction"
+
+    def test(source, node : Crystal::Def)
+      return if valid?(node)
+
+      node.args.each do |arg|
+        next if arg.restriction || arg.name.empty? || (!default_value? && arg.default_value)
+
+        issue_for arg, MSG
+      end
+
+      if block_parameters?
+        node.block_arg.try do |block_arg|
+          next if block_arg.restriction
+
+          issue_for block_arg, MSG
+        end
+      end
+    end
+
+    private def valid?(node : Crystal::ASTNode) : Bool
+      (!private_methods? && node.visibility.private?) ||
+        (!protected_methods? && node.visibility.protected?)
+    end
+  end
+end