From 87dbe9017a4f31779305631fbdd6bfe24ade486f Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Fri, 26 Jul 2024 11:41:19 -0400 Subject: [PATCH 1/3] Add indexing enhancements and included hooks --- .../lib/ruby_indexer/declaration_listener.rb | 13 +- .../lib/ruby_indexer/enhancement.rb | 24 +++ lib/ruby_indexer/lib/ruby_indexer/index.rb | 57 +++++- lib/ruby_indexer/ruby_indexer.rb | 1 + lib/ruby_indexer/test/enhancements_test.rb | 163 ++++++++++++++++++ 5 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 lib/ruby_indexer/lib/ruby_indexer/enhancement.rb create mode 100644 lib/ruby_indexer/test/enhancements_test.rb diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index 412953f22..f01dab09a 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -9,11 +9,18 @@ class DeclarationListener 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 + params( + index: Index, + dispatcher: Prism::Dispatcher, + parse_result: Prism::ParseResult, + file_path: String, + enhancements: T::Array[Enhancement], + ).void end - def initialize(index, dispatcher, parse_result, file_path) + def initialize(index, dispatcher, parse_result, file_path, enhancements: []) @index = index @file_path = file_path + @enhancements = enhancements @visibility_stack = T.let([Entry::Visibility::PUBLIC], T::Array[Entry::Visibility]) @comments_by_line = T.let( parse_result.comments.to_h do |c| @@ -279,6 +286,8 @@ def on_call_node_enter(node) when :private @visibility_stack.push(Entry::Visibility::PRIVATE) end + + @enhancements.each { |aug| aug.on_call_node(@index, @owner_stack.last, node, @file_path) } end sig { params(node: Prism::CallNode).void } diff --git a/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb b/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb new file mode 100644 index 000000000..59336c35a --- /dev/null +++ b/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb @@ -0,0 +1,24 @@ +# typed: strict +# frozen_string_literal: true + +module RubyIndexer + module Enhancement + extend T::Sig + extend T::Helpers + + interface! + + # The `on_extend` indexing enhancement is invoked whenever an extend is encountered in the code. It can be used to + # register for an included callback, similar to what `ActiveSupport::Concern` does in order to auto-extend the + # `ClassMethods` modules + sig do + abstract.params( + index: Index, + owner: T.nilable(Entry::Namespace), + node: Prism::CallNode, + file_path: String, + ).void + end + def on_call_node(index, owner, node, file_path); end + end +end diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index f7adbcd63..803a3913e 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -35,6 +35,27 @@ def initialize # Holds the linearized ancestors list for every namespace @ancestors = T.let({}, T::Hash[String, T::Array[String]]) + + # List of classes that are enhancing the index + @enhancements = T.let([], T::Array[Enhancement]) + + # Map of module name to included hooks that have to be executed when we include the given module + @included_hooks = T.let( + {}, + T::Hash[String, T::Array[T.proc.params(index: Index, base: Entry::Namespace).void]], + ) + end + + # Register an enhancement to the index. Enhancements must conform to the `Enhancement` interface + sig { params(enhancement: Enhancement).void } + def register_enhancement(enhancement) + @enhancements << enhancement + end + + # Register an included `hook` that will be executed when `module_name` is included into any namespace + sig { params(module_name: String, hook: T.proc.params(index: Index, base: Entry::Namespace).void).void } + def register_included_hook(module_name, &hook) + (@included_hooks[module_name] ||= []) << hook end sig { params(indexable: IndexablePath).void } @@ -296,7 +317,7 @@ def index_single(indexable_path, source = nil) dispatcher = Prism::Dispatcher.new result = Prism.parse(content) - DeclarationListener.new(self, dispatcher, result, indexable_path.full_path) + DeclarationListener.new(self, dispatcher, result, indexable_path.full_path, enhancements: @enhancements) dispatcher.dispatch(result.value) require_path = indexable_path.require_path @@ -457,6 +478,12 @@ def linearized_ancestors_of(fully_qualified_name) end end + # We only need to run included hooks when linearizing singleton classes. Included hooks are typically used to add + # new singleton methods or to extend a module through an include. There's no need to support instance methods, the + # inclusion of another module or the prepending of another module, because those features are already a part of + # Ruby and can be used directly without any metaprogramming + run_included_hooks(attached_class_name, nesting) if singleton_levels > 0 + linearize_mixins(ancestors, namespaces, nesting) linearize_superclass( ancestors, @@ -570,6 +597,34 @@ def existing_or_new_singleton_class(name) private + # Runs the registered included hooks + sig { params(fully_qualified_name: String, nesting: T::Array[String]).void } + def run_included_hooks(fully_qualified_name, nesting) + return if @included_hooks.empty? + + namespaces = self[fully_qualified_name]&.grep(Entry::Namespace) + return unless namespaces + + namespaces.each do |namespace| + namespace.mixin_operations.each do |operation| + next unless operation.is_a?(Entry::Include) + + # First we resolve the include name, so that we know the actual module being referred to in the include + resolved_modules = resolve(operation.module_name, nesting) + next unless resolved_modules + + module_name = T.must(resolved_modules.first).name + + # Then we grab any hooks registered for that module + hooks = @included_hooks[module_name] + next unless hooks + + # We invoke the hooks with the index and the namespace that included the module + hooks.each { |hook| hook.call(self, namespace) } + end + end + end + # Linearize mixins for an array of namespace entries. This method will mutate the `ancestors` array with the # linearized ancestors of the mixins sig do diff --git a/lib/ruby_indexer/ruby_indexer.rb b/lib/ruby_indexer/ruby_indexer.rb index 17b25c049..6690f40ab 100644 --- a/lib/ruby_indexer/ruby_indexer.rb +++ b/lib/ruby_indexer/ruby_indexer.rb @@ -6,6 +6,7 @@ require "ruby_indexer/lib/ruby_indexer/indexable_path" require "ruby_indexer/lib/ruby_indexer/declaration_listener" +require "ruby_indexer/lib/ruby_indexer/enhancement" require "ruby_indexer/lib/ruby_indexer/index" require "ruby_indexer/lib/ruby_indexer/entry" require "ruby_indexer/lib/ruby_indexer/configuration" diff --git a/lib/ruby_indexer/test/enhancements_test.rb b/lib/ruby_indexer/test/enhancements_test.rb new file mode 100644 index 000000000..2e366d896 --- /dev/null +++ b/lib/ruby_indexer/test/enhancements_test.rb @@ -0,0 +1,163 @@ +# typed: true +# frozen_string_literal: true + +require_relative "test_case" + +module RubyIndexer + class EnhancementTest < TestCase + def test_enhancing_indexing_included_hook + enhancement_class = Class.new do + include Enhancement + + def on_call_node(index, owner, node, file_path) + return unless owner + return unless node.name == :extend + + arguments = node.arguments&.arguments + return unless arguments + + location = node.location + + arguments.each do |node| + next unless node.is_a?(Prism::ConstantReadNode) || node.is_a?(Prism::ConstantPathNode) + + module_name = node.full_name + next unless module_name == "ActiveSupport::Concern" + + index.register_included_hook(owner.name) do |index, base| + class_methods_name = "#{owner.name}::ClassMethods" + + if index.indexed?(class_methods_name) + singleton = index.existing_or_new_singleton_class(base.name) + singleton.mixin_operations << Entry::Include.new(class_methods_name) + end + end + + index.add(Entry::Method.new( + "new_method", + file_path, + location, + location, + [], + [Entry::Signature.new([Entry::RequiredParameter.new(name: :a)])], + Entry::Visibility::PUBLIC, + owner, + )) + rescue Prism::ConstantPathNode::DynamicPartsInConstantPathError, + Prism::ConstantPathNode::MissingNodesInConstantPathError + # Do nothing + end + end + end + + @index.register_enhancement(enhancement_class.new) + index(<<~RUBY) + module ActiveSupport + module Concern + def self.extended(base) + base.class_eval("def new_method(a); end") + end + end + end + + module ActiveRecord + module Associations + extend ActiveSupport::Concern + + module ClassMethods + def belongs_to(something); end + end + end + + class Base + include Associations + end + end + + class User < ActiveRecord::Base + end + RUBY + + assert_equal( + [ + "User::", + "ActiveRecord::Base::", + "ActiveRecord::Associations::ClassMethods", + "Object::", + "BasicObject::", + "Class", + "Module", + "Object", + "Kernel", + "BasicObject", + ], + @index.linearized_ancestors_of("User::"), + ) + + assert_entry("new_method", Entry::Method, "/fake/path/foo.rb:10-4:10-33") + end + + def test_enhancing_indexing_configuration_dsl + enhancement_class = Class.new do + include Enhancement + + def on_call_node(index, owner, node, file_path) + return unless owner + + name = node.name + return unless name == :has_many + + arguments = node.arguments&.arguments + return unless arguments + + association_name = arguments.first + return unless association_name.is_a?(Prism::SymbolNode) + + location = association_name.location + + index.add(Entry::Method.new( + T.must(association_name.value), + file_path, + location, + location, + [], + [], + Entry::Visibility::PUBLIC, + owner, + )) + end + end + + @index.register_enhancement(enhancement_class.new) + index(<<~RUBY) + module ActiveSupport + module Concern + def self.extended(base) + base.class_eval("def new_method(a); end") + end + end + end + + module ActiveRecord + module Associations + extend ActiveSupport::Concern + + module ClassMethods + def belongs_to(something); end + end + end + + class Base + include Associations + end + end + + class User < ActiveRecord::Base + has_many :posts + end + RUBY + + assert_entry("posts", Entry::Method, "/fake/path/foo.rb:23-11:23-17") + end + end +end From 949da8bdcc77eb1e7fb43d18b6cc6bc83c96689f Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Mon, 5 Aug 2024 15:09:01 +0100 Subject: [PATCH 2/3] Make indexing enhancement error-tolerant This change rescues errors raised when indexing with enhancements and logs them to stderr when the file's indexing is finished. --- .../lib/ruby_indexer/declaration_listener.rb | 10 +++++- .../lib/ruby_indexer/enhancement.rb | 2 ++ lib/ruby_indexer/lib/ruby_indexer/index.rb | 16 ++++++++- lib/ruby_indexer/test/enhancements_test.rb | 34 +++++++++++++++++++ 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index f01dab09a..327c96fee 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -8,6 +8,9 @@ class DeclarationListener OBJECT_NESTING = T.let(["Object"].freeze, T::Array[String]) BASIC_OBJECT_NESTING = T.let(["BasicObject"].freeze, T::Array[String]) + sig { returns(T::Array[String]) } + attr_reader :indexing_errors + sig do params( index: Index, @@ -36,6 +39,7 @@ def initialize(index, dispatcher, parse_result, file_path, enhancements: []) # A stack of namespace entries that represent where we currently are. Used to properly assign methods to an owner @owner_stack = T.let([], T::Array[Entry::Namespace]) + @indexing_errors = T.let([], T::Array[String]) dispatcher.register( self, @@ -287,7 +291,11 @@ def on_call_node_enter(node) @visibility_stack.push(Entry::Visibility::PRIVATE) end - @enhancements.each { |aug| aug.on_call_node(@index, @owner_stack.last, node, @file_path) } + @enhancements.each do |enhancement| + enhancement.on_call_node(@index, @owner_stack.last, node, @file_path) + rescue StandardError + @indexing_errors << "Error occurred when indexing #{@file_path} with '#{enhancement.class.name}' enhancement" + end end sig { params(node: Prism::CallNode).void } diff --git a/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb b/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb index 59336c35a..fc06c8db1 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/enhancement.rb @@ -8,6 +8,8 @@ module Enhancement interface! + requires_ancestor { Object } + # The `on_extend` indexing enhancement is invoked whenever an extend is encountered in the code. It can be used to # register for an included callback, similar to what `ActiveSupport::Concern` does in order to auto-extend the # `ClassMethods` modules diff --git a/lib/ruby_indexer/lib/ruby_indexer/index.rb b/lib/ruby_indexer/lib/ruby_indexer/index.rb index 803a3913e..c556e10a2 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/index.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/index.rb @@ -317,11 +317,25 @@ def index_single(indexable_path, source = nil) dispatcher = Prism::Dispatcher.new result = Prism.parse(content) - DeclarationListener.new(self, dispatcher, result, indexable_path.full_path, enhancements: @enhancements) + listener = DeclarationListener.new( + self, + dispatcher, + result, + indexable_path.full_path, + enhancements: @enhancements, + ) dispatcher.dispatch(result.value) + indexing_errors = listener.indexing_errors.uniq + require_path = indexable_path.require_path @require_paths_tree.insert(require_path, indexable_path) if require_path + + if indexing_errors.any? + indexing_errors.each do |error| + $stderr.puts error + end + end rescue Errno::EISDIR, Errno::ENOENT # If `path` is a directory, just ignore it and continue indexing. If the file doesn't exist, then we also ignore # it diff --git a/lib/ruby_indexer/test/enhancements_test.rb b/lib/ruby_indexer/test/enhancements_test.rb index 2e366d896..ba3101205 100644 --- a/lib/ruby_indexer/test/enhancements_test.rb +++ b/lib/ruby_indexer/test/enhancements_test.rb @@ -159,5 +159,39 @@ class User < ActiveRecord::Base assert_entry("posts", Entry::Method, "/fake/path/foo.rb:23-11:23-17") end + + def test_error_handling_in_enhancement + enhancement_class = Class.new do + include Enhancement + + def on_call_node(index, owner, node, file_path) + raise "Error" + end + + class << self + def name + "TestEnhancement" + end + end + end + + @index.register_enhancement(enhancement_class.new) + + _stdout, stderr = capture_io do + index(<<~RUBY) + module ActiveSupport + module Concern + def self.extended(base) + base.class_eval("def new_method(a); end") + end + end + end + RUBY + end + + assert_match(%r{Error occurred when indexing /fake/path/foo\.rb with 'TestEnhancement' enhancement}, stderr) + # The module should still be indexed + assert_entry("ActiveSupport::Concern", Entry::Module, "/fake/path/foo.rb:1-2:5-5") + end end end From 5269522c4b730bca0afb2248848c764f11c49277 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Wed, 7 Aug 2024 16:11:08 -0400 Subject: [PATCH 3/3] Include error message for enhancements --- lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb | 4 ++-- lib/ruby_indexer/test/enhancements_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb index 327c96fee..53c37a172 100644 --- a/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb +++ b/lib/ruby_indexer/lib/ruby_indexer/declaration_listener.rb @@ -293,8 +293,8 @@ def on_call_node_enter(node) @enhancements.each do |enhancement| enhancement.on_call_node(@index, @owner_stack.last, node, @file_path) - rescue StandardError - @indexing_errors << "Error occurred when indexing #{@file_path} with '#{enhancement.class.name}' enhancement" + rescue StandardError => e + @indexing_errors << "Indexing error in #{@file_path} with '#{enhancement.class.name}' enhancement: #{e.message}" end end diff --git a/lib/ruby_indexer/test/enhancements_test.rb b/lib/ruby_indexer/test/enhancements_test.rb index ba3101205..df33ff568 100644 --- a/lib/ruby_indexer/test/enhancements_test.rb +++ b/lib/ruby_indexer/test/enhancements_test.rb @@ -189,7 +189,7 @@ def self.extended(base) RUBY end - assert_match(%r{Error occurred when indexing /fake/path/foo\.rb with 'TestEnhancement' enhancement}, stderr) + assert_match(%r{Indexing error in /fake/path/foo\.rb with 'TestEnhancement' enhancement}, stderr) # The module should still be indexed assert_entry("ActiveSupport::Concern", Entry::Module, "/fake/path/foo.rb:1-2:5-5") end