From 4f10b32154a01f3330210518b82a55d66f068087 Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Thu, 13 Jun 2024 09:41:21 -0400 Subject: [PATCH 1/3] Deadcode Index takes a Model Signed-off-by: Alexandre Terrasa --- lib/spoom/cli/deadcode.rb | 7 +++++-- lib/spoom/deadcode.rb | 1 + lib/spoom/deadcode/index.rb | 12 ++++++++---- test/helpers/deadcode_helper.rb | 5 +++-- test/spoom/deadcode/remover_test.rb | 8 +++++--- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/spoom/cli/deadcode.rb b/lib/spoom/cli/deadcode.rb index c5e8518c..bc30315c 100644 --- a/lib/spoom/cli/deadcode.rb +++ b/lib/spoom/cli/deadcode.rb @@ -80,7 +80,8 @@ def deadcode(*paths) $stderr.puts end - index = Spoom::Deadcode::Index.new + model = Spoom::Model.new + index = Spoom::Deadcode::Index.new(model) $stderr.puts "Indexing #{blue(files.size.to_s)} files..." files.each do |file| @@ -88,6 +89,7 @@ def deadcode(*paths) content = Spoom::Deadcode::ERB.new(content).src if file.end_with?(".erb") tree = Spoom.parse_ruby(content, file: file) + Spoom::Model::Builder.new(model, file).visit(tree) Spoom::Deadcode.index_node(index, tree, content, file: file, plugins: plugins) rescue ParseError => e say_error("Error parsing #{file}: #{e.message}") @@ -97,6 +99,8 @@ def deadcode(*paths) next end + index.finalize!(plugins: plugins) + if options[:show_defs] $stderr.puts "\nDefinitions:" index.definitions.each do |name, definitions| @@ -123,7 +127,6 @@ def deadcode(*paths) references_count = index.references.size.to_s $stderr.puts "Analyzing #{blue(definitions_count)} definitions against #{blue(references_count)} references..." - index.finalize! dead = index.definitions.values.flatten.select(&:dead?) if options[:sort] == "name" diff --git a/lib/spoom/deadcode.rb b/lib/spoom/deadcode.rb index 8f5f9970..bd9ca581 100644 --- a/lib/spoom/deadcode.rb +++ b/lib/spoom/deadcode.rb @@ -58,6 +58,7 @@ def index_node(index, node, ruby, file:, plugins: []) sig { params(index: Index, ruby: String, file: String, plugins: T::Array[Deadcode::Plugins::Base]).void } def index_ruby(index, ruby, file:, plugins: []) node = Spoom.parse_ruby(ruby, file: file) + Model::Builder.new(index.model, file).visit(node) index_node(index, node, ruby, file: file, plugins: plugins) end diff --git a/lib/spoom/deadcode/index.rb b/lib/spoom/deadcode/index.rb index 63a7f3df..5b497a51 100644 --- a/lib/spoom/deadcode/index.rb +++ b/lib/spoom/deadcode/index.rb @@ -6,14 +6,18 @@ module Deadcode class Index extend T::Sig + sig { returns(Model) } + attr_reader :model + sig { returns(T::Hash[String, T::Array[Definition]]) } attr_reader :definitions sig { returns(T::Hash[String, T::Array[Reference]]) } attr_reader :references - sig { void } - def initialize + sig { params(model: Model).void } + def initialize(model) + @model = model @definitions = T.let({}, T::Hash[String, T::Array[Definition]]) @references = T.let({}, T::Hash[String, T::Array[Reference]]) end @@ -33,8 +37,8 @@ def reference(reference) # Mark all definitions having a reference of the same name as `alive` # # To be called once all the files have been indexed and all the definitions and references discovered. - sig { void } - def finalize! + sig { params(plugins: T::Array[Plugins::Base]).void } + def finalize!(plugins: []) @references.keys.each do |name| definitions_for_name(name).each(&:alive!) end diff --git a/test/helpers/deadcode_helper.rb b/test/helpers/deadcode_helper.rb index 96dccb57..bbd72f73 100644 --- a/test/helpers/deadcode_helper.rb +++ b/test/helpers/deadcode_helper.rb @@ -21,7 +21,8 @@ def deadcode_index(plugins: []) allow_mime_types: ["text/x-ruby", "text/x-ruby-script"], ) - index = Deadcode::Index.new + model = Model.new + index = Deadcode::Index.new(model) files.each do |file| content = project.read(file) @@ -32,7 +33,7 @@ def deadcode_index(plugins: []) end end - index.finalize! + index.finalize!(plugins: plugins) index end diff --git a/test/spoom/deadcode/remover_test.rb b/test/spoom/deadcode/remover_test.rb index c81b1de8..d55feb8a 100644 --- a/test/spoom/deadcode/remover_test.rb +++ b/test/spoom/deadcode/remover_test.rb @@ -1611,11 +1611,13 @@ def foo sig { params(ruby_string: String, def_name: String).returns(String) } def remove(ruby_string, def_name) + file = "file.rb" context = Context.mktmp! - context.write!("file.rb", ruby_string) + context.write!(file, ruby_string) - index = Index.new - Deadcode.index_ruby(index, ruby_string, file: "file.rb") + model = Model.new + index = Index.new(model) + Deadcode.index_ruby(index, ruby_string, file: file) index.finalize! definitions = definitions_for_name(index, def_name) From 9eefe03e4595e72c66dc5559e7f7857b5c6359ba Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Wed, 19 Jun 2024 16:22:45 -0400 Subject: [PATCH 2/3] Deadcode Indexer relies on Model for class definitions Signed-off-by: Alexandre Terrasa --- lib/spoom/deadcode/index.rb | 15 +++++++++++++++ lib/spoom/deadcode/indexer.rb | 15 --------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/spoom/deadcode/index.rb b/lib/spoom/deadcode/index.rb index 5b497a51..46552825 100644 --- a/lib/spoom/deadcode/index.rb +++ b/lib/spoom/deadcode/index.rb @@ -39,6 +39,21 @@ def reference(reference) # To be called once all the files have been indexed and all the definitions and references discovered. sig { params(plugins: T::Array[Plugins::Base]).void } def finalize!(plugins: []) + @model.symbols.each do |_full_name, symbol| + symbol.definitions.each do |symbol_def| + case symbol_def + when Model::Class + definition = Definition.new( + kind: Definition::Kind::Class, + name: symbol.name, + full_name: symbol.full_name, + location: symbol_def.location, + ) + define(definition) + plugins.each { |plugin| plugin.internal_on_define_class(symbol_def, definition) } + end + end + end @references.keys.each do |name| definitions_for_name(name).each(&:alive!) end diff --git a/lib/spoom/deadcode/indexer.rb b/lib/spoom/deadcode/indexer.rb index cf33c777..0f8dd301 100644 --- a/lib/spoom/deadcode/indexer.rb +++ b/lib/spoom/deadcode/indexer.rb @@ -112,8 +112,6 @@ def visit_class_node(node) @names_nesting.clear @names_nesting << full_name - define_class(T.must(constant_path.split("::").last), full_name, node) - # We do not call `super` here because we don't want to visit the `constant` again visit(node.superclass) if node.superclass visit(node.body) @@ -123,7 +121,6 @@ def visit_class_node(node) @names_nesting = old_nesting else @names_nesting << constant_path - define_class(T.must(constant_path.split("::").last), @names_nesting.join("::"), node) # We do not call `super` here because we don't want to visit the `constant` again visit(node.superclass) if node.superclass @@ -351,18 +348,6 @@ def define_attr_writer(name, full_name, node) @plugins.each { |plugin| plugin.internal_on_define_accessor(self, definition) } end - sig { params(name: String, full_name: String, node: Prism::Node).void } - def define_class(name, full_name, node) - definition = Definition.new( - kind: Definition::Kind::Class, - name: name, - full_name: full_name, - location: node_location(node), - ) - @index.define(definition) - @plugins.each { |plugin| plugin.internal_on_define_class(self, definition) } - end - sig { params(name: String, full_name: String, node: Prism::Node).void } def define_constant(name, full_name, node) definition = Definition.new( From 49c816fb5dadbda139646c36b889b2d8f3d70f0b Mon Sep 17 00:00:00 2001 From: Alexandre Terrasa Date: Wed, 19 Jun 2024 16:23:17 -0400 Subject: [PATCH 3/3] Pass SymbolDefs to plugin `on_class` Signed-off-by: Alexandre Terrasa --- lib/spoom/deadcode/plugins/base.rb | 18 +++++++++--------- lib/spoom/deadcode/plugins/namespaces.rb | 6 +++--- lib/spoom/deadcode/plugins/rails.rb | 6 +++--- test/spoom/deadcode/plugins/base_test.rb | 4 ++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/spoom/deadcode/plugins/base.rb b/lib/spoom/deadcode/plugins/base.rb index d8fa7d0e..bb173b38 100644 --- a/lib/spoom/deadcode/plugins/base.rb +++ b/lib/spoom/deadcode/plugins/base.rb @@ -159,26 +159,26 @@ def internal_on_define_accessor(indexer, definition) # # ~~~rb # class MyPlugin < Spoom::Deadcode::Plugins::Base - # def on_define_class(indexer, definition) - # definition.ignored! if definition.name == "Foo" + # def on_define_class(symbol_def, definition) + # definition.ignored! if symbol_def.name == "Foo" # end # end # ~~~ - sig { params(indexer: Indexer, definition: Definition).void } - def on_define_class(indexer, definition) + sig { params(symbol_def: Model::Class, definition: Definition).void } + def on_define_class(symbol_def, definition) # no-op end # Do not override this method, use `on_define_class` instead. - sig { params(indexer: Indexer, definition: Definition).void } - def internal_on_define_class(indexer, definition) - if ignored_class_name?(definition.name) + sig { params(symbol_def: Model::Class, definition: Definition).void } + def internal_on_define_class(symbol_def, definition) + if ignored_class_name?(symbol_def.name) definition.ignored! - elsif ignored_subclass?(indexer.nesting_class_superclass_name) + elsif ignored_subclass?(symbol_def.superclass_name&.delete_prefix("::")) definition.ignored! end - on_define_class(indexer, definition) + on_define_class(symbol_def, definition) end # Called when a constant is defined. diff --git a/lib/spoom/deadcode/plugins/namespaces.rb b/lib/spoom/deadcode/plugins/namespaces.rb index d3484b60..72fca835 100644 --- a/lib/spoom/deadcode/plugins/namespaces.rb +++ b/lib/spoom/deadcode/plugins/namespaces.rb @@ -7,9 +7,9 @@ module Plugins class Namespaces < Base extend T::Sig - sig { override.params(indexer: Indexer, definition: Definition).void } - def on_define_class(indexer, definition) - definition.ignored! if used_as_namespace?(indexer) + sig { override.params(symbol_def: Model::Class, definition: Definition).void } + def on_define_class(symbol_def, definition) + definition.ignored! unless symbol_def.children.empty? end sig { override.params(indexer: Indexer, definition: Definition).void } diff --git a/lib/spoom/deadcode/plugins/rails.rb b/lib/spoom/deadcode/plugins/rails.rb index 56fce785..0ef71cbe 100644 --- a/lib/spoom/deadcode/plugins/rails.rb +++ b/lib/spoom/deadcode/plugins/rails.rb @@ -9,9 +9,9 @@ class Rails < Base ignore_constants_named("APP_PATH", "ENGINE_PATH", "ENGINE_ROOT") - sig { override.params(indexer: Indexer, definition: Definition).void } - def on_define_class(indexer, definition) - definition.ignored! if file_is_helper?(indexer) + sig { override.params(symbol_def: Model::Class, definition: Definition).void } + def on_define_class(symbol_def, definition) + definition.ignored! if symbol_def.location.file.match?(%r{app/helpers/.*\.rb$}) end sig { override.params(indexer: Indexer, definition: Definition).void } diff --git a/test/spoom/deadcode/plugins/base_test.rb b/test/spoom/deadcode/plugins/base_test.rb index 1c40dc8c..1cbccc40 100644 --- a/test/spoom/deadcode/plugins/base_test.rb +++ b/test/spoom/deadcode/plugins/base_test.rb @@ -31,8 +31,8 @@ def on_define_accessor(indexer, definition) def test_on_define_class plugin = Class.new(Base) do - def on_define_class(indexer, definition) - definition.ignored! if definition.name == "Class1" + def on_define_class(symbol_def, definition) + definition.ignored! if symbol_def.name == "Class1" end end