From 1524aad29953ceb8481aabf481d835c5a10ee851 Mon Sep 17 00:00:00 2001
From: Sijawusz Pur Rahnama <sija@sija.pl>
Date: Mon, 28 Nov 2022 06:20:57 +0100
Subject: [PATCH] Add `Style/QueryBoolMethods` rule

---
 .../rule/style/query_bool_methods_spec.cr     | 60 ++++++++++++++++
 src/ameba/ast/util.cr                         |  7 ++
 src/ameba/rule/style/query_bool_methods.cr    | 72 +++++++++++++++++++
 3 files changed, 139 insertions(+)
 create mode 100644 spec/ameba/rule/style/query_bool_methods_spec.cr
 create mode 100644 src/ameba/rule/style/query_bool_methods.cr

diff --git a/spec/ameba/rule/style/query_bool_methods_spec.cr b/spec/ameba/rule/style/query_bool_methods_spec.cr
new file mode 100644
index 00000000..34ea380b
--- /dev/null
+++ b/spec/ameba/rule/style/query_bool_methods_spec.cr
@@ -0,0 +1,60 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Style
+  subject = QueryBoolMethods.new
+
+  describe QueryBoolMethods do
+    it "passes for valid cases" do
+      expect_no_issues subject, <<-CRYSTAL
+        class Foo
+          class_property? foo = true
+          property? foo = true
+          property foo2 : Bool? = true
+          setter panda = true
+        end
+
+        module Bar
+          class_getter? bar : Bool = true
+          getter? bar : Bool
+          getter bar2 : Bool? = true
+          setter panda : Bool = true
+
+          def initialize(@bar = true)
+          end
+        end
+        CRYSTAL
+    end
+
+    {% for call in %w[getter class_getter property class_property] %}
+      it "reports `{{ call.id }}` assign with Bool" do
+        expect_issue subject, <<-CRYSTAL, call: {{ call }}
+          class Foo
+            %{call}   foo = true
+            _{call} # ^^^ error: Consider using '%{call}?' for 'foo'
+          end
+          CRYSTAL
+      end
+
+      it "reports `{{ call.id }}` type declaration assign with Bool" do
+        expect_issue subject, <<-CRYSTAL, call: {{ call }}
+          class Foo
+            %{call}   foo : Bool = true
+            _{call} # ^ error: Consider using '%{call}?' for 'foo'
+          end
+          CRYSTAL
+      end
+
+      it "reports `{{ call.id }}` type declaration with Bool" do
+        expect_issue subject, <<-CRYSTAL, call: {{ call }}
+          class Foo
+            %{call}   foo : Bool
+            _{call} # ^ error: Consider using '%{call}?' for 'foo'
+
+            def initialize(@foo = true)
+            end
+          end
+          CRYSTAL
+      end
+    {% end %}
+  end
+end
diff --git a/src/ameba/ast/util.cr b/src/ameba/ast/util.cr
index e69ee5f8..e1d83bb4 100644
--- a/src/ameba/ast/util.cr
+++ b/src/ameba/ast/util.cr
@@ -58,6 +58,13 @@ module Ameba::AST::Util
     is_literal
   end
 
+  # Returns `true` if current `node` is a `Crystal::Path`
+  # matching given *name*, `false` otherwise.
+  def path_named?(node, name) : Bool
+    node.is_a?(Crystal::Path) &&
+      name == node.names.join("::")
+  end
+
   # Returns a source code for the current node.
   # This method uses `node.location` and `node.end_location`
   # to determine and cut a piece of source of the node.
diff --git a/src/ameba/rule/style/query_bool_methods.cr b/src/ameba/rule/style/query_bool_methods.cr
new file mode 100644
index 00000000..4bd8c370
--- /dev/null
+++ b/src/ameba/rule/style/query_bool_methods.cr
@@ -0,0 +1,72 @@
+module Ameba::Rule::Style
+  # A rule that disallows boolean properties without the `?` suffix - defined
+  # using `Object#(class_)property` or `Object#(class_)getter` macros.
+  #
+  # Favour this:
+  #
+  # ```
+  # class Person
+  #   property? deceased = false
+  #   getter? witty = true
+  # end
+  # ```
+  #
+  # Over this:
+  #
+  # ```
+  # class Person
+  #   property deceased = false
+  #   getter witty = true
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Style/QueryBoolMethods:
+  #   Enabled: true
+  # ```
+  class QueryBoolMethods < Base
+    include AST::Util
+
+    properties do
+      description "Reports boolean properties without the `?` suffix"
+    end
+
+    MSG = "Consider using '%s?' for '%s'"
+
+    CALL_NAMES = %w[getter class_getter property class_property]
+
+    def test(source, node : Crystal::ClassDef | Crystal::ModuleDef)
+      calls =
+        case body = node.body
+        when Crystal::Call
+          [body] if body.name.in?(CALL_NAMES)
+        when Crystal::Expressions
+          body.expressions
+            .select(Crystal::Call)
+            .select!(&.name.in?(CALL_NAMES))
+        end
+
+      return unless calls
+
+      calls.each do |exp|
+        exp.args.each do |arg|
+          name_node, is_bool =
+            case arg
+            when Crystal::Assign
+              {arg.target, arg.value.is_a?(Crystal::BoolLiteral)}
+            when Crystal::TypeDeclaration
+              {arg.var, path_named?(arg.declared_type, "Bool")}
+            else
+              {nil, false}
+            end
+
+          if name_node && is_bool
+            issue_for name_node, MSG % {exp.name, name_node}
+          end
+        end
+      end
+    end
+  end
+end