Merge pull request #1601 from ksss/diff-constant-resolver

[rbs diff] Resolve constants name
ruby · Nov 9, 2023 · 41e3902 · 41e3902
2 parents 8b8ac87 + 4d2659a
commit 41e3902
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 23 deletions.
diff --git a/lib/rbs/diff.rb b/lib/rbs/diff.rb
@@ -12,13 +12,13 @@ def initialize(type_name:, library_options:, after_path: [], before_path: [])
     def each_diff(&block)
       return to_enum(:each_diff) unless block
 
-      before_instance_methods, before_singleton_methods, before_constant_decls = build_methods(@before_path)
-      after_instance_methods, after_singleton_methods, after_constant_decls = build_methods(@after_path)
+      before_instance_methods, before_singleton_methods, before_constant_children = build_methods(@before_path)
+      after_instance_methods, after_singleton_methods, after_constant_children = build_methods(@after_path)
 
       each_diff_methods(:instance, before_instance_methods, after_instance_methods, &block)
       each_diff_methods(:singleton, before_singleton_methods, after_singleton_methods, &block)
 
-      each_diff_constants(before_constant_decls, after_constant_decls, &block)
+      each_diff_constants(before_constant_children, after_constant_children, &block)
     end
 
     private
@@ -34,11 +34,11 @@ def each_diff_methods(kind, before_methods, after_methods)
       end
     end
 
-    def each_diff_constants(before_constant_decls, after_constant_decls)
-      all_keys = before_constant_decls.keys.to_set + after_constant_decls.keys.to_set
+    def each_diff_constants(before_constant_children, after_constant_children)
+      all_keys = before_constant_children.keys.to_set + after_constant_children.keys.to_set
       all_keys.each do |key|
-        before = constant_to_s(key, before_constant_decls[key]) or next
-        after = constant_to_s(key, after_constant_decls[key]) or next
+        before = constant_to_s(before_constant_children[key]) or next
+        after = constant_to_s(after_constant_children[key]) or next
         next if before == after
 
         yield before, after
@@ -52,19 +52,25 @@ def build_methods(path)
       instance_methods = begin
         builder.build_instance(@type_name).methods
       rescue => e
-        RBS.logger.warn("#{path}: #{e.message}")
+        RBS.logger.warn("#{path}: (#{e.class}) #{e.message}")
         {}
       end
       singleton_methods = begin
         builder.build_singleton(@type_name).methods
       rescue => e
-        RBS.logger.warn("#{path}: #{e.message}")
+        RBS.logger.warn("#{path}: (#{e.class}) #{e.message}")
         {}
       end
-      type_name_to_s = @type_name.to_s
-      constant_decls = env.constant_decls.select { |key| key.to_s.start_with?(type_name_to_s) }
 
-      [ instance_methods, singleton_methods, constant_decls ]
+      constant_children = begin
+        constant_resolver = RBS::Resolver::ConstantResolver.new(builder: builder)
+        constant_resolver.children(@type_name)
+      rescue => e
+        RBS.logger.warn("#{path}: (#{e.class}) #{e.message}")
+        {}
+      end
+
+      [ instance_methods, singleton_methods, constant_children ]
     end
 
     def build_env(path)
@@ -93,9 +99,9 @@ def definition_method_to_s(key, kind, definition_method)
       end
     end
 
-    def constant_to_s(key, constant)
+    def constant_to_s(constant)
       if constant
-        "#{key}: #{constant.decl.type}"
+        "#{constant.name.name}: #{constant.type}"
       else
         +"-"
       end

diff --git a/lib/rbs/resolver/constant_resolver.rb b/lib/rbs/resolver/constant_resolver.rb
@@ -22,7 +22,7 @@ def initialize(environment)
 
             unless name.namespace.empty?
               parent = name.namespace.to_type_name
-              table = children_table[parent] or raise
+              table = children_table[parent] or raise "#{parent} not found by #{name}"
             else
               table = toplevel
             end

diff --git a/test/rbs/cli_test.rb b/test/rbs/cli_test.rb
@@ -1185,7 +1185,7 @@ def test_diff_markdown
         | `def qux: (untyped) -> untyped` | `-` |
         | `def quux: () -> void` | `alias quux bar` |
         | `def self.baz: () -> (::Integer \\| ::String)` | `def self.baz: (::Integer) -> ::Integer?` |
-        | `::Foo::CONST: Array[Integer]` | `::Foo::CONST: Array[String]` |
+        | `CONST: ::Array[::Integer]` | `CONST: ::Array[::String]` |
       MARKDOWN
     end
   end
@@ -1204,8 +1204,8 @@ def test_diff_diff
         - def self.baz: () -> (::Integer | ::String)
         + def self.baz: (::Integer) -> ::Integer?
 
-        - ::Foo::CONST: Array[Integer]
-        + ::Foo::CONST: Array[String]
+        - CONST: ::Array[::Integer]
+        + CONST: ::Array[::String]
       DIFF
     end
   end

diff --git a/test/rbs/diff_test.rb b/test/rbs/diff_test.rb
@@ -67,11 +67,10 @@ class Foo
         ["def qux: (untyped) -> untyped", "-"],
         ["def quux: () -> void", "alias quux bar"],
         ["def self.baz: () -> (::Integer | ::String)", "def self.baz: (::Integer) -> ::Integer?"],
-        ["::Foo::SAME_MOD_OTHER_VALUE: 2", "::Foo::SAME_MOD_OTHER_VALUE: String"],
-        ["::Foo::SAME_MOD_BEFORE_ONLY: 3", "-"],
-        ["::Foo::OTHER_MOD_SAME_VALUE: 4", "-"],
-        ["::Foo::OTHER_MOD_OTHER_VALUE: 5", "-"],
-        ["-", "::Foo::SAME_MOD_AFTER_ONLY: 3"]
+        ["SAME_MOD_OTHER_VALUE: 2", "SAME_MOD_OTHER_VALUE: ::String"],
+        ["SAME_MOD_BEFORE_ONLY: 3", "-"],
+        ["OTHER_MOD_OTHER_VALUE: 5", "OTHER_MOD_OTHER_VALUE: ::Array[::Integer]"],
+        ["-", "SAME_MOD_AFTER_ONLY: 3"]
       ], results
     end
   end