Merge pull request #415 from crystal-ameba/add-accessor-method-name-rule

Add `Naming/AccessorMethodName` rule

spec/ameba/rule/naming/accessor_method_name_spec.cr

@@ -0,0 +1,93 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Naming
+  subject = AccessorMethodName.new
+
+  describe AccessorMethodName do
+    it "passes if accessor method name is correct" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def self.instance
+          end
+
+          def self.instance=(value)
+          end
+
+          def user
+          end
+
+          def user=(user)
+          end
+        end
+        CRYSTAL
+    end
+
+    it "passes if accessor method is defined in top-level scope" do
+      expect_no_issues subject, <<-CRYSTAL
+        def get_user
+        end
+
+        def set_user(user)
+        end
+        CRYSTAL
+    end
+
+    it "fails if accessor method is defined with receiver in top-level scope" do
+      expect_issue subject, <<-CRYSTAL
+        def Foo.get_user
+              # ^^^^^^^^ error: Favour method name 'user' over 'get_user'
+        end
+
+        def Foo.set_user(user)
+              # ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
+        end
+        CRYSTAL
+    end
+
+    it "fails if accessor method name is wrong" do
+      expect_issue subject, <<-CRYSTAL
+        class Foo
+          def self.get_instance
+                 # ^^^^^^^^^^^^ error: Favour method name 'instance' over 'get_instance'
+          end
+
+          def self.set_instance(value)
+                 # ^^^^^^^^^^^^ error: Favour method name 'instance=' over 'set_instance'
+          end
+
+          def get_user
+            # ^^^^^^^^ error: Favour method name 'user' over 'get_user'
+          end
+
+          def set_user(user)
+            # ^^^^^^^^ error: Favour method name 'user=' over 'set_user'
+          end
+        end
+        CRYSTAL
+    end
+
+    it "ignores if alternative name isn't valid syntax" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def get_404
+          end
+
+          def set_404(value)
+          end
+        end
+        CRYSTAL
+    end
+
+    it "ignores if the method has unexpected arity" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          def get_user(type)
+          end
+
+          def set_user(user, type)
+          end
+        end
+        CRYSTAL
+    end
+  end
+end

src/ameba/rule/naming/accessor_method_name.cr

@@ -0,0 +1,90 @@
+module Ameba::Rule::Naming
+  # A rule that makes sure that accessor methods are named properly.
+  #
+  # Favour this:
+  #
+  # ```
+  # class Foo
+  #   def user
+  #     @user
+  #   end
+  #
+  #   def user=(value)
+  #     @user = value
+  #   end
+  # end
+  # ```
+  #
+  # Over this:
+  #
+  # ```
+  # class Foo
+  #   def get_user
+  #     @user
+  #   end
+  #
+  #   def set_user(value)
+  #     @user = value
+  #   end
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Naming/AccessorMethodName:
+  #   Enabled: true
+  # ```
+  class AccessorMethodName < Base
+    include AST::Util
+
+    properties do
+      description "Makes sure that accessor methods are named properly"
+    end
+
+    MSG = "Favour method name '%s' over '%s'"
+
+    def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
+      defs =
+        case body = node.body
+        when Crystal::Def
+          [body]
+        when Crystal::Expressions
+          body.expressions.select(Crystal::Def)
+        end
+
+      defs.try &.each do |def_node|
+        # skip defs with explicit receiver, as they'll be handled
+        # by the `test(source, node : Crystal::Def)` overload
+        check_issue(source, def_node) unless def_node.receiver
+      end
+    end
+
+    def test(source, node : Crystal::Def)
+      # check only defs with explicit receiver (`def self.foo`)
+      check_issue(source, node) if node.receiver
+    end
+
+    private def check_issue(source, node : Crystal::Def)
+      location = name_location(node)
+      end_location = name_end_location(node)
+
+      case node.name
+      when /^get_([a-z]\w*)$/
+        return unless node.args.empty?
+        if location && end_location
+          issue_for location, end_location, MSG % {$1, node.name}
+        else
+          issue_for node, MSG % {$1, node.name}
+        end
+      when /^set_([a-z]\w*)$/
+        return unless node.args.size == 1
+        if location && end_location
+          issue_for location, end_location, MSG % {"#{$1}=", node.name}
+        else
+          issue_for node, MSG % {"#{$1}=", node.name}
+        end
+      end
+    end
+  end
+end