From e113669f922132b3cfa4b86f72e73644f337989c Mon Sep 17 00:00:00 2001 From: Margret Riegert Date: Sat, 11 Jan 2025 22:33:58 -0500 Subject: [PATCH] Add `Typing/MethodParameterTypeRestriction` (#520) --- .../method_parameter_type_restriction_spec.cr | 200 ++++++++++++++++++ .../method_parameter_type_restriction.cr | 89 ++++++++ 2 files changed, 289 insertions(+) create mode 100644 spec/ameba/rule/typing/method_parameter_type_restriction_spec.cr create mode 100644 src/ameba/rule/typing/method_parameter_type_restriction.cr diff --git a/spec/ameba/rule/typing/method_parameter_type_restriction_spec.cr b/spec/ameba/rule/typing/method_parameter_type_restriction_spec.cr new file mode 100644 index 000000000..b37121fc9 --- /dev/null +++ b/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 diff --git a/src/ameba/rule/typing/method_parameter_type_restriction.cr b/src/ameba/rule/typing/method_parameter_type_restriction.cr new file mode 100644 index 000000000..1e4d5df47 --- /dev/null +++ b/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