From 866256cb0a867f5aa988af6bd9029db05de121fb Mon Sep 17 00:00:00 2001
From: Alexander Momchilov <alexander.momchilov@shopify.com>
Date: Thu, 6 Jun 2024 12:00:01 -0400
Subject: [PATCH] Rewrite `attr_*` into `def` methods

---
 lib/rbi.rb                                 |   1 +
 lib/rbi/model.rb                           |   4 +
 lib/rbi/rewriters/attr_to_methods.rb       | 151 +++++++++++
 test/rbi/rewriters/attr_to_methods_test.rb | 292 +++++++++++++++++++++
 4 files changed, 448 insertions(+)
 create mode 100644 lib/rbi/rewriters/attr_to_methods.rb
 create mode 100644 test/rbi/rewriters/attr_to_methods_test.rb

diff --git a/lib/rbi.rb b/lib/rbi.rb
index e2ad99fd..f9881960 100644
--- a/lib/rbi.rb
+++ b/lib/rbi.rb
@@ -16,6 +16,7 @@
 require "rbi/rewriters/nest_non_public_methods"
 require "rbi/rewriters/group_nodes"
 require "rbi/rewriters/remove_known_definitions"
+require "rbi/rewriters/attr_to_methods"
 require "rbi/rewriters/sort_nodes"
 require "rbi/parser"
 require "rbi/printer"
diff --git a/lib/rbi/model.rb b/lib/rbi/model.rb
index ea579190..8da7e759 100644
--- a/lib/rbi/model.rb
+++ b/lib/rbi/model.rb
@@ -380,6 +380,10 @@ def initialize(name, names, visibility: Public.new, sigs: [], loc: nil, comments
 
     sig { abstract.returns(T::Array[String]) }
     def fully_qualified_names; end
+
+    # Convert this attribute declaration into method declarations, adjusting the sigs as necessary.
+    sig { abstract.returns(T::Array[Method]) }
+    def convert_to_methods; end
   end
 
   class AttrAccessor < Attr
diff --git a/lib/rbi/rewriters/attr_to_methods.rb b/lib/rbi/rewriters/attr_to_methods.rb
new file mode 100644
index 00000000..601623c4
--- /dev/null
+++ b/lib/rbi/rewriters/attr_to_methods.rb
@@ -0,0 +1,151 @@
+# typed: strict
+# frozen_string_literal: true
+
+module RBI
+  module Rewriters
+    class AttrToMethods < Visitor
+      extend T::Sig
+
+      sig { override.params(node: T.nilable(Node)).void }
+      def visit(node)
+        case node
+        when Tree
+          visit_all(node.nodes) # Why doesn't this need a dup?!?!?!?
+
+        when Attr
+          new_methods = node.convert_to_methods
+          replace(node, with: new_methods)
+        end
+      end
+
+      private
+
+      sig { params(node: Node, with: T::Array[Node]).void }
+      def replace(node, with:)
+        tree = node.parent_tree
+        raise unless tree
+
+        with.each { |node| tree << node }
+        node.detach
+      end
+    end
+  end
+
+  class Tree
+    extend T::Sig
+
+    sig { void }
+    def replace_attributes_with_methods!
+      visitor = Rewriters::AttrToMethods.new
+      visitor.visit(self)
+    end
+  end
+
+  class Attr
+    private
+
+    sig(:final) { returns([T.nilable(Sig), T.nilable(String)]) }
+    def parse_sig
+      raise "Attributes cannot have more than 1 sig" if 1 < sigs.count
+
+      sig = sigs.first
+      return [nil, nil] unless sig
+
+      attribute_type = case self
+      when AttrReader, AttrAccessor then sig.return_type
+      when AttrWriter then sig.params.first&.type
+      end
+
+      [sig, attribute_type]
+    end
+
+    sig do
+      params(
+        name: String,
+        sig: T.nilable(Sig),
+        visibility: Visibility,
+        loc: T.nilable(Loc),
+        comments: T::Array[Comment],
+      ).returns(Method)
+    end
+    def create_getter_method(name, sig, visibility, loc, comments)
+      Method.new(
+        name,
+        params: [],
+        visibility: visibility,
+        sigs: sig ? [sig] : [],
+        loc: loc,
+        comments: comments,
+      )
+    end
+
+    sig do
+      params(
+        name: String,
+        sig: T.nilable(Sig),
+        attribute_type: T.nilable(String),
+        visibility: Visibility,
+        loc: T.nilable(Loc),
+        comments: T::Array[Comment],
+      ).returns(Method)
+    end
+    def create_setter_method(name, sig, attribute_type, visibility, loc, comments) # rubocop:disable Metrics/ParameterLists
+      sig = if sig # Modify the original sig to correct the name, and remove the return type
+        params = attribute_type ? [SigParam.new(name, attribute_type)] : []
+
+        Sig.new(
+          params: params,
+          return_type: "void",
+          is_abstract: sig.is_abstract,
+          is_override: sig.is_override,
+          is_overridable: sig.is_overridable,
+          is_final: sig.is_final,
+          type_params: sig.type_params,
+          checked: sig.checked,
+          loc: sig.loc,
+        )
+      end
+
+      Method.new(
+        "#{name}=",
+        params: [ReqParam.new(name)],
+        visibility: visibility,
+        sigs: sig ? [sig] : [],
+        loc: loc,
+        comments: comments,
+      )
+    end
+  end
+
+  class AttrAccessor
+    sig { override.returns(T::Array[Method]) }
+    def convert_to_methods
+      sig, attribute_type = parse_sig
+
+      names.flat_map do |name|
+        [
+          create_getter_method(name.to_s, sig, visibility, loc, comments),
+          create_setter_method(name.to_s, sig, attribute_type, visibility, loc, comments),
+        ]
+      end
+    end
+  end
+
+  class AttrReader
+    sig { override.returns(T::Array[Method]) }
+    def convert_to_methods
+      sig, _ = parse_sig
+
+      names.map { |name| create_getter_method(name.to_s, sig, visibility, loc, comments) }
+    end
+  end
+
+  class AttrWriter
+    sig { override.returns(T::Array[Method]) }
+    def convert_to_methods
+      sig, attribute_type = parse_sig
+
+      names.map { |name| create_setter_method(name.to_s, sig, attribute_type, visibility, loc, comments) }
+    end
+  end
+end
diff --git a/test/rbi/rewriters/attr_to_methods_test.rb b/test/rbi/rewriters/attr_to_methods_test.rb
new file mode 100644
index 00000000..609f25d4
--- /dev/null
+++ b/test/rbi/rewriters/attr_to_methods_test.rb
@@ -0,0 +1,292 @@
+# typed: true
+# frozen_string_literal: true
+
+require "test_helper"
+
+module RBI
+  class AttrToMethodsTest < Minitest::Test
+    def test_replaces_attr_reader_with_method
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        attr_reader :a
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def a; end
+      RBI
+    end
+
+    def test_replaces_attr_writer_with_setter_method
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        attr_writer :a
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        def a=(a); end
+      RBI
+    end
+
+    def test_replaces_attr_writer_with_return_type_with_setter_method
+      # Sorbet allows either `.void` or `.returns(TheType)`.
+      # We'll support both, until Sorbet starts to prefer one or the other.
+
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { params(a: Integer).returns(Integer) }
+        attr_writer :a
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        def a=(a); end
+      RBI
+    end
+
+    def test_replaces_attr_accessor_with_getter_and_setter_methods
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        attr_accessor :a
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def a; end
+
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        def a=(a); end
+      RBI
+    end
+
+    ### Testing for multiple attributes defined in a single declaration
+
+    def test_replaces_multi_attr_reader_with_methods
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        attr_reader :a, :b, :c
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def a; end
+
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def b; end
+
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def c; end
+      RBI
+    end
+
+    def test_replaces_multi_attr_writer_with_methods
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        attr_writer :a, :b, :c
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        def a=(a); end
+
+        # Lorum ipsum...
+        sig { params(b: Integer).void }
+        def b=(b); end
+
+        # Lorum ipsum...
+        sig { params(c: Integer).void }
+        def c=(c); end
+      RBI
+    end
+
+    def test_replaces_multi_attr_accessor_with_methods
+      rbi = Parser.parse_string(<<~RBI)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        attr_accessor :a, :b, :c
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def a; end
+
+        # Lorum ipsum...
+        sig { params(a: Integer).void }
+        def a=(a); end
+
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def b; end
+
+        # Lorum ipsum...
+        sig { params(b: Integer).void }
+        def b=(b); end
+
+        # Lorum ipsum...
+        sig { returns(Integer) }
+        def c; end
+
+        # Lorum ipsum...
+        sig { params(c: Integer).void }
+        def c=(c); end
+      RBI
+    end
+
+    ### Testing for sig modifiers
+    # We don't test for Abstract, because attribute declarations are treated as though
+    # they have always have a method body, and so they could never be abstract.
+
+    def test_replacing_attr_reader_copies_sig_modifiers
+      rbi = Parser.parse_string(<<~RBI)
+        class GrandParent
+          sig { overridable.returns(Integer) }
+          attr_reader :a
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.returns(Integer) }
+          attr_reader :a
+        end
+
+        class Child < Parent
+          sig(:final) { override.returns(Integer) }
+          attr_reader :a
+        end
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        class GrandParent
+          sig { overridable.returns(Integer) }
+          def a; end
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.returns(Integer) }
+          def a; end
+        end
+
+        class Child < Parent
+          sig(:final) { override.returns(Integer) }
+          def a; end
+        end
+      RBI
+    end
+
+    def test_replacing_attr_writer_copies_sig_modifiers
+      rbi = Parser.parse_string(<<~RBI)
+        class GrandParent
+          sig { overridable.params(a: Integer).void }
+          attr_writer :a
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.params(a: Integer).void }
+          attr_writer :a
+        end
+
+        class Child < Parent
+          sig(:final) { override.params(a: Integer).void }
+          attr_writer :a
+        end
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        class GrandParent
+          sig { overridable.params(a: Integer).void }
+          def a=(a); end
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.params(a: Integer).void }
+          def a=(a); end
+        end
+
+        class Child < Parent
+          sig(:final) { override.params(a: Integer).void }
+          def a=(a); end
+        end
+      RBI
+    end
+
+    def test_replacing_attr_accessor_copies_sig_modifiers
+      rbi = Parser.parse_string(<<~RBI)
+        class GrandParent
+          sig { overridable.returns(Integer) }
+          attr_accessor :a
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.returns(Integer) }
+          attr_accessor :a
+        end
+
+        class Child < Parent
+          sig(:final) { override.returns(Integer) }
+          attr_accessor :a
+        end
+      RBI
+
+      rbi.replace_attributes_with_methods!
+
+      assert_equal(<<~RBI, rbi.string)
+        class GrandParent
+          sig { overridable.returns(Integer) }
+          def a; end
+
+          sig { overridable.params(a: Integer).void }
+          def a=(a); end
+        end
+
+        class Parent < GrandParent
+          sig { override.overridable.returns(Integer) }
+          def a; end
+
+          sig { override.overridable.params(a: Integer).void }
+          def a=(a); end
+        end
+
+        class Child < Parent
+          sig(:final) { override.returns(Integer) }
+          def a; end
+
+          sig(:final) { override.params(a: Integer).void }
+          def a=(a); end
+        end
+      RBI
+    end
+  end
+end