Parsing of nested classes

[alisnic]

this is somehow related to #87

Introduction

In standard Ruby, when you define a class without defining its parent module first, this fails:

2.5.8 :001 > class Foo::Bar
2.5.8 :002?>   end
Traceback (most recent call last):
        2: from /Users/alisnic/.rvm/rubies/ruby-2.5.8/bin/irb:11:in `<main>'
        1: from (irb):1
NameError (uninitialized constant Foo)

However, in Rails that's perfectly fine, for example in nested controllers:

class Api::StatusesController < ApplicationController
   #
end

Problem report

When solargraph parses such class definitons, it does not generate intermediary pin entries. This leads to the fact that autocomplete for queries like Api:: will not lead to anything. I wrote a failing spec:

diff --git a/spec/api_map_spec.rb b/spec/api_map_spec.rb
index 3d86ebf3..a080a7bc 100755
--- a/spec/api_map_spec.rb
+++ b/spec/api_map_spec.rb
@@ -88,6 +89,18 @@ describe Solargraph::ApiMap do
     expect(paths).to include('Foo::Baz')
   end
 
+  it "finds nested namespaces inside classes" do
+    map = Solargraph::SourceMap.load_string(%(
+      class Foo::Bar
+      end
+    ))
+
+    @api_map.index map.pins
+    pins = @api_map.get_constants('Foo')
+    paths = pins.map(&:path)
+    expect(paths).to include('Foo::Bar')
+  end
+

Please provide guidelines on how to better fix this. I have a dirty patch that kind of works, but I think this is better done in a plugin, like solargraph-rails. Any thoughts / ideas @castwide?

Dirty patch

diff --git a/lib/solargraph/parser/legacy/node_processors/namespace_node.rb b/lib/solargraph/parser/legacy/node_processors/namespace_node.rb
index a46a82b7..c77b2cda 100644
--- a/lib/solargraph/parser/legacy/node_processors/namespace_node.rb
+++ b/lib/solargraph/parser/legacy/node_processors/namespace_node.rb
@@ -9,20 +9,27 @@ module Solargraph
 
           def process
             sc = nil
+
             if node.type == :class and !node.children[1].nil?
               sc = unpack_name(node.children[1])
             end
             loc = get_node_location(node)
-            nspin = Solargraph::Pin::Namespace.new(
-              type: node.type,
-              location: loc,
-              closure: region.closure,
-              name: unpack_name(node.children[0]),
-              comments: comments_for(node),
-              visibility: :public,
-              gates: region.closure.gates.freeze
-            )
-            pins.push nspin
+            parts = pack_name(node.children[0])
+            names = parts.each_with_index.reduce([]) { |acc, (_, i)| acc + [parts[0..i].join("::")] }
+
+            names.each do |name|
+              nspin = Solargraph::Pin::Namespace.new(
+                type: node.type,
+                location: loc,
+                closure: region.closure,
+                name: name,
+                comments: comments_for(node),
+                visibility: :public,
+                gates: region.closure.gates.freeze
+              )
+              pins.push nspin
+            end
+
             unless sc.nil?
               pins.push Pin::Reference::Superclass.new(
                 location: loc,
@@ -30,7 +37,7 @@ module Solargraph
                 name: sc
               )
             end
-            process_children region.update(closure: nspin, visibility: :public)
+            process_children region.update(closure: pins.last, visibility: :public)
           end
         end
       end

[reneweteling]

Same issue here, would like to know if its a best practice to define the module somewhere before using it in a rails context or that a change to solargraph needs to be created.

[alisnic]

Same issue here, would like to know if its a best practice to define the module somewhere before using it in a rails context or that a change to solargraph needs to be created.

I was able to write a plugin that works around this limitation at https://github.com/alisnic/solar-rails. Test and see if it works.

[castwide]

Since this isn't core Ruby behavior, I would definitely recommend implementing it in a plugin (or at least making it optional). A couple possibilities:

• Implement it directly in solargraph-rails
• Make a separate plugin (e.g., solargraph-zeitwerk) and add it to solargraph-rails dependencies

/cc @iftheshoefritz

[alisnic]

Since this isn't core Ruby behavior, I would definitely recommend implementing it in a plugin (or at least making it optional). A couple possibilities:

• Implement it directly in solargraph-rails
• Make a separate plugin (e.g., solargraph-zeitwerk) and add it to solargraph-rails dependencies

/cc @iftheshoefritz

That's what I thought. Thanks! Closing here

[reneweteling]

If someone reads this, just use nested class and module definitions, its the default in rubocop, also using a nested module or class before the parent module is declared results in an error in plain ruby. Rails has some magic to overcome this, but that does not make it correct.

just enforce this Cop: https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/ClassAndModuleChildren to nested and you are golden.