From 39faf38af2eee59b94cce8b76fd7aa0b0b2e4ab7 Mon Sep 17 00:00:00 2001
From: Stan Lo <stan.lo@shopify.com>
Date: Wed, 12 Jun 2024 18:01:49 +0100
Subject: [PATCH] Fix incorrect parent class assignment for Object and
 BasicObject

When the Object and BasicObject classes are reopened, their parent class
should remain `BasicObject` and `nil` respectively. This change fixes
the current behaviour where the parent class is incorrectly set to
`Object`.
---
 .../lib/ruby_indexer/declaration_listener.rb  | 20 ++++++-
 lib/ruby_indexer/lib/ruby_indexer/entry.rb    |  3 +-
 lib/ruby_indexer/test/index_test.rb           | 57 ++++++++++++++++++-
 3 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
index 5faa27e3c..ab38d17d7 100644
--- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
+++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb
@@ -5,6 +5,9 @@ module RubyIndexer
   class DeclarationListener
     extend T::Sig
 
+    OBJECT_NESTING = T.let(["Object"].freeze, T::Array[String])
+    BASIC_OBJECT_NESTING = T.let(["BasicObject"].freeze, T::Array[String])
+
     sig do
       params(index: Index, dispatcher: Prism::Dispatcher, parse_result: Prism::ParseResult, file_path: String).void
     end
@@ -66,15 +69,26 @@ def on_class_node_enter(node)
       comments = collect_comments(node)
 
       superclass = node.superclass
+
+      nesting = name.start_with?("::") ? [name.delete_prefix("::")] : @stack + [name.delete_prefix("::")]
+
       parent_class = case superclass
       when Prism::ConstantReadNode, Prism::ConstantPathNode
         superclass.slice
       else
-        "::Object"
+        case nesting
+        when OBJECT_NESTING
+          # When Object is reopened, its parent class should still be the top-level BasicObject
+          "::BasicObject"
+        when BASIC_OBJECT_NESTING
+          # When BasicObject is reopened, its parent class should still be nil
+          nil
+        else
+          # Otherwise, the parent class should be the top-level Object
+          "::Object"
+        end
       end
 
-      nesting = name.start_with?("::") ? [name.delete_prefix("::")] : @stack + [name.delete_prefix("::")]
-
       entry = Entry::Class.new(
         nesting,
         @file_path,
diff --git a/lib/ruby_indexer/lib/ruby_indexer/entry.rb b/lib/ruby_indexer/lib/ruby_indexer/entry.rb
index 89eb25e7b..fe400a9e1 100644
--- a/lib/ruby_indexer/lib/ruby_indexer/entry.rb
+++ b/lib/ruby_indexer/lib/ruby_indexer/entry.rb
@@ -152,8 +152,7 @@ class Class < Namespace
       end
       def initialize(nesting, file_path, location, comments, parent_class)
         super(nesting, file_path, location, comments)
-
-        @parent_class = T.let(parent_class, T.nilable(String))
+        @parent_class = parent_class
       end
 
       sig { override.returns(Integer) }
diff --git a/lib/ruby_indexer/test/index_test.rb b/lib/ruby_indexer/test/index_test.rb
index 1c3093456..7cfe427ae 100644
--- a/lib/ruby_indexer/test/index_test.rb
+++ b/lib/ruby_indexer/test/index_test.rb
@@ -1143,8 +1143,32 @@ def test_resolving_unindexed_constant_with_no_nesting
       assert_nil(@index.resolve("RSpec", []))
     end
 
+    def test_object_superclass_indexing_and_resolution_with_reopened_object_class
+      index(<<~RUBY)
+        class Object; end
+      RUBY
+
+      entries = @index["Object"]
+      assert_equal(2, entries.length)
+      reopened_entry = entries.last
+      assert_equal("::BasicObject", reopened_entry.parent_class)
+      assert_equal(["Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Object"))
+    end
+
+    def test_object_superclass_indexing_and_resolution_with_reopened_basic_object_class
+      index(<<~RUBY)
+        class BasicObject; end
+      RUBY
+
+      entries = @index["BasicObject"]
+      assert_equal(2, entries.length)
+      reopened_entry = entries.last
+      assert_nil(reopened_entry.parent_class)
+      assert_equal(["BasicObject"], @index.linearized_ancestors_of("BasicObject"))
+    end
+
     def test_object_superclass_resolution
-      @index.index_single(IndexablePath.new(nil, "/fake/path/foo.rb"), <<~RUBY)
+      index(<<~RUBY)
         module Foo
           class Object; end
 
@@ -1160,8 +1184,25 @@ class Baz < Object; end
       )
     end
 
+    def test_basic_object_superclass_resolution
+      index(<<~RUBY)
+        module Foo
+          class BasicObject; end
+
+          class Bar; end
+          class Baz < BasicObject; end
+        end
+      RUBY
+
+      assert_equal(["Foo::Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Foo::Bar"))
+      assert_equal(
+        ["Foo::Baz", "Foo::BasicObject", "Object", "Kernel", "BasicObject"],
+        @index.linearized_ancestors_of("Foo::Baz"),
+      )
+    end
+
     def test_top_level_object_superclass_resolution
-      @index.index_single(IndexablePath.new(nil, "/fake/path/foo.rb"), <<~RUBY)
+      index(<<~RUBY)
         module Foo
           class Object; end
 
@@ -1172,6 +1213,18 @@ class Bar < ::Object; end
       assert_equal(["Foo::Bar", "Object", "Kernel", "BasicObject"], @index.linearized_ancestors_of("Foo::Bar"))
     end
 
+    def test_top_level_basic_object_superclass_resolution
+      index(<<~RUBY)
+        module Foo
+          class BasicObject; end
+
+          class Bar < ::BasicObject; end
+        end
+      RUBY
+
+      assert_equal(["Foo::Bar", "BasicObject"], @index.linearized_ancestors_of("Foo::Bar"))
+    end
+
     def test_resolving_method_inside_singleton_context
       @index.index_single(IndexablePath.new(nil, "/fake/path/foo.rb"), <<~RUBY)
         module Foo