Merge pull request #568 from Shopify/at-deadcode-poset

Allow dead code plugins to reason on the classes hierarchy

lib/spoom/cli/deadcode.rb

@@ -100,6 +100,7 @@ def deadcode(*paths)
           next
         end
 
+        model.finalize!
         index.apply_plugins!(plugins)
         index.finalize!
 

lib/spoom/deadcode/plugins/actionpack.rb

@@ -7,6 +7,8 @@ module Plugins
       class ActionPack < Base
         extend T::Sig
 
+        ignore_classes_inheriting_from("ApplicationController")
+
         CALLBACKS = T.let(
           [
             "after_action",
@@ -25,14 +27,12 @@ class ActionPack < Base
           T::Array[String],
         )
 
-        ignore_classes_named(/Controller$/)
-
         sig { override.params(definition: Model::Method).void }
         def on_define_method(definition)
           owner = definition.owner
           return unless owner.is_a?(Model::Class)
 
-          @index.ignore(definition) if ignored_class_name?(owner.name)
+          @index.ignore(definition) if ignored_subclass?(owner)
         end
 
         sig { override.params(send: Send).void }

lib/spoom/deadcode/plugins/active_model.rb

@@ -7,7 +7,7 @@ module Plugins
       class ActiveModel < Base
         extend T::Sig
 
-        ignore_classes_inheriting_from(/^(::)?ActiveModel::EachValidator$/)
+        ignore_classes_inheriting_from("ActiveModel::EachValidator")
         ignore_methods_named("validate_each")
 
         sig { override.params(send: Send).void }

lib/spoom/deadcode/plugins/active_support.rb

@@ -5,7 +5,7 @@ module Spoom
   module Deadcode
     module Plugins
       class ActiveSupport < Base
-        ignore_classes_inheriting_from(/^(::)?ActiveSupport::TestCase$/)
+        ignore_classes_inheriting_from("ActiveSupport::TestCase")
 
         ignore_methods_named(
           "after_all",

lib/spoom/deadcode/plugins/base.rb

@@ -179,7 +179,7 @@ def on_define_class(definition)
         def internal_on_define_class(definition)
           if ignored_class_name?(definition.name)
             @index.ignore(definition)
-          elsif ignored_subclass?(definition.superclass_name&.delete_prefix("::"))
+          elsif ignored_subclass?(definition)
             @index.ignore(definition)
           end
 
@@ -286,18 +286,31 @@ def on_send(send)
 
         # DSL support
 
+        sig { params(definition: Model::Namespace, superclass_name: String).returns(T::Boolean) }
+        def subclass_of?(definition, superclass_name)
+          superclass_symbol = @index.model.symbols[superclass_name]
+          return false unless superclass_symbol
+
+          @index.model.symbols_hierarchy.edge?(definition.symbol, superclass_symbol)
+        end
+
         sig { params(name: T.nilable(String)).returns(T::Boolean) }
         def ignored_class_name?(name)
           return false unless name
 
           ignored_name?(name, :@ignored_class_names, :@ignored_class_patterns)
         end
 
-        sig { params(superclass_name: T.nilable(String)).returns(T::Boolean) }
-        def ignored_subclass?(superclass_name)
-          return false unless superclass_name
+        sig { params(definition: Model::Class).returns(T::Boolean) }
+        def ignored_subclass?(definition)
+          superclass_name = definition.superclass_name
+          return true if superclass_name && ignored_name?(
+            superclass_name,
+            :@ignored_subclasses_of_names,
+            :@ignored_subclasses_of_patterns,
+          )
 
-          ignored_name?(superclass_name, :@ignored_subclasses_of_names, :@ignored_subclasses_of_patterns)
+          names(:@ignored_subclasses_of_names).any? { |superclass_name| subclass_of?(definition, superclass_name) }
         end
 
         sig { params(name: String).returns(T::Boolean) }

lib/spoom/deadcode/plugins/graphql.rb

@@ -8,10 +8,10 @@ class GraphQL < Base
         extend T::Sig
 
         ignore_classes_inheriting_from(
-          /^(::)?GraphQL::Schema::Enum$/,
-          /^(::)?GraphQL::Schema::Object$/,
-          /^(::)?GraphQL::Schema::Scalar$/,
-          /^(::)?GraphQL::Schema::Union$/,
+          "GraphQL::Schema::Enum",
+          "GraphQL::Schema::Object",
+          "GraphQL::Schema::Scalar",
+          "GraphQL::Schema::Union",
         )
 
         ignore_methods_named(

lib/spoom/deadcode/plugins/rubocop.rb

@@ -10,19 +10,16 @@ class Rubocop < Base
         RUBOCOP_CONSTANTS = T.let(["MSG", "RESTRICT_ON_SEND"].to_set.freeze, T::Set[String])
 
         ignore_classes_inheriting_from(
-          /^(::)?RuboCop::Cop::Cop$/,
-          /^(::)?RuboCop::Cop::Base$/,
+          "RuboCop::Cop::Cop",
+          "RuboCop::Cop::Base",
         )
 
         sig { override.params(definition: Model::Constant).void }
         def on_define_constant(definition)
           owner = definition.owner
           return false unless owner.is_a?(Model::Class)
 
-          superclass_name = owner.superclass_name
-          return false unless superclass_name
-
-          @index.ignore(definition) if ignored_subclass?(superclass_name) && RUBOCOP_CONSTANTS.include?(definition.name)
+          @index.ignore(definition) if ignored_subclass?(owner) && RUBOCOP_CONSTANTS.include?(definition.name)
         end
 
         sig { override.params(definition: Model::Method).void }
@@ -32,10 +29,7 @@ def on_define_method(definition)
           owner = definition.owner
           return unless owner.is_a?(Model::Class)
 
-          superclass_name = owner.superclass_name
-          return unless superclass_name
-
-          @index.ignore(definition) if ignored_subclass?(superclass_name)
+          @index.ignore(definition) if ignored_subclass?(owner)
         end
       end
     end

lib/spoom/deadcode/plugins/sorbet.rb

@@ -29,10 +29,7 @@ def sorbet_enum_constant?(definition)
           owner = definition.owner
           return false unless owner.is_a?(Model::Class)
 
-          superclass_name = owner.superclass_name
-          return false unless superclass_name
-
-          superclass_name.match?(/^(::)?T::Enum$/)
+          subclass_of?(owner, "T::Enum")
         end
       end
     end

lib/spoom/deadcode/plugins/thor.rb

@@ -14,10 +14,7 @@ def on_define_method(definition)
           owner = definition.owner
           return unless owner.is_a?(Model::Class)
 
-          superclass_name = owner.superclass_name
-          return unless superclass_name
-
-          @index.ignore(definition) if superclass_name =~ /^(::)?Thor$/
+          @index.ignore(definition) if subclass_of?(owner, "Thor")
         end
       end
     end

test/helpers/deadcode_helper.rb

@@ -34,6 +34,7 @@ def deadcode_index(plugin_classes: [])
             end
           end
 
+          model.finalize!
           index.apply_plugins!(plugins)
           index.finalize!
           index

test/spoom/deadcode/plugins/action_mailer_preview_test.rb

@@ -41,7 +41,7 @@ def bar; end
 
           # Document current less then ideal behavior:
           assert_ignored(index, "some_unused_private_method")
-          refute_ignored(index, "MyIndirectInheritanceMailerPreview")
+          assert_ignored(index, "MyIndirectInheritanceMailerPreview")
         end
 
         private

test/spoom/deadcode/plugins/actionpack_test.rb

@@ -12,7 +12,7 @@ class ActionPackTest < TestWithProject
 
         def test_ignore_actionpack_controllers
           @project.write!("app/controllers/foo.rb", <<~RB)
-            class FooController
+            class FooController < ApplicationController
               def foo; end
             end
 

test/spoom/deadcode/plugins/base_test.rb

@@ -240,14 +240,16 @@ def test_ignore_classes_inheriting_from
             class Subclass1 < ::Superclass1; end
             class Subclass2 < Superclass2; end
             class Subclass3 < Superclass3; end
+            class Subclass4 < Subclass2; end
             class SubclassRE1 < SuperclassRE1; end
             class SubclassRE2 < SuperclassRE2; end
           RB
 
           index = deadcode_index(plugin_classes: [plugin])
           assert_ignored(index, "Subclass1")
-          assert_ignored(index, "Subclass2")
+          assert_alive(index, "Subclass2")
           refute_ignored(index, "Subclass3")
+          assert_ignored(index, "Subclass4")
           assert_ignored(index, "SubclassRE1")
           assert_ignored(index, "SubclassRE2")
         end