Merge pull request #39 from Shopify/clarify_const_node_inspector

Clarify ConstNodeInspector code

Author: doodzik

lib/packwerk.rb

@@ -14,7 +14,6 @@
 require "packwerk/configuration"
 require "packwerk/const_node_inspector"
 require "packwerk/constant_discovery"
-require "packwerk/constant_name_inspector"
 require "packwerk/dependency_checker"
 require "packwerk/deprecated_references"
 require "packwerk/files_for_processing"

lib/packwerk/const_node_inspector.rb

@@ -9,20 +9,12 @@ class ConstNodeInspector
     include ConstantNameInspector
 
     def constant_name_from_node(node, ancestors:)
-      return nil unless Node.type(node) == Node::CONSTANT
-
-      # Only process the root `const` node for namespaced constant references. For example, in the
-      # reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`.
+      return nil unless Node.constant?(node)
       parent = ancestors.first
-      return nil if parent && Node.type(parent) == Node::CONSTANT
-
-      if constant_in_module_or_class_definition?(node, parent: parent)
-        # We're defining a class with this name, in which case the constant is implicitly fully qualified by its
-        # enclosing namespace
-        name = Node.parent_module_name(ancestors: ancestors)
-        name ||= Node.enclosing_namespace_path(node, ancestors: ancestors).push(Node.constant_name(node)).join("::")
+      return nil unless root_constant?(parent)
 
-        "::" + name
+      if parent && constant_in_module_or_class_definition?(node, parent: parent)
+        fully_qualify_constant(node, ancestors: ancestors)
       else
         begin
           Node.constant_name(node)
@@ -34,11 +26,29 @@ def constant_name_from_node(node, ancestors:)
 
     private
 
+    # Only process the root `const` node for namespaced constant references. For example, in the
+    # reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`.
+    def root_constant?(parent)
+      !(parent && Node.constant?(parent))
+    end
+
     def constant_in_module_or_class_definition?(node, parent:)
-      if parent
-        parent_name = Node.module_name_from_definition(parent)
-        parent_name && parent_name == Node.constant_name(node)
-      end
+      parent_name = Node.module_name_from_definition(parent)
+      parent_name && parent_name == Node.constant_name(node)
+    end
+
+    def fully_qualify_constant(node, ancestors:)
+      # We're defining a class with this name, in which case the constant is implicitly fully qualified by its
+      # enclosing namespace
+      name = Node.parent_module_name(ancestors: ancestors)
+      name ||= generate_qualified_constant(node, ancestors)
+      "::" + name
+    end
+
+    def generate_qualified_constant(node, ancestors:)
+      namespace_path = Node.enclosing_namespace_path(node, ancestors: ancestors)
+      constant_name = Node.constant_name(node)
+      namespace_path.push(constant_name).join("::")
     end
   end
 end

lib/packwerk/node.rb

@@ -103,6 +103,10 @@ def location(node)
         Location.new(location.line, location.column)
       end
 
+      def constant?(node)
+        type(node) == CONSTANT
+      end
+
       def method_call?(node)
         type(node) == METHOD_CALL
       end
@@ -239,15 +243,15 @@ def module_creation?(node)
         # "Class.new"
         # "Module.new"
         method_call?(node) &&
-          type(receiver(node)) == CONSTANT &&
+          constant?(receiver(node)) &&
           ["Class", "Module"].include?(constant_name(receiver(node))) &&
           method_name(node) == :new
       end
 
       def name_from_block_definition(node)
         if method_name(method_call_node(node)) == :class_eval
           receiver = receiver(node)
-          constant_name(receiver) if receiver && type(receiver) == CONSTANT
+          constant_name(receiver) if receiver && constant?(receiver)
         end
       end