Expect nested slices to use parent’s namespace (#1240)

For nested slices, define (and expect) their classes to use their parent's namespace. Given either `slices/main/slices/nested/` or `slices/main/config/slices/nested.rb` or `Main::Slice.register_slice(:nested)`, these will all expect or define a `Main::Nested::Slice` class.

It is still possible to opt out of this convention be calling register_slice with an existing slice class, e.g. `register_slice(:nested, SomeOtherNamespace::Slice)`. This kind of opt out requires explicit user intentionality, so at that time I expect them to be aware they're operating outside the framework's ordinary conventions.

lib/hanami/slice.rb

@@ -113,7 +113,7 @@ module Hanami
       # @since 2.0.0
       def config
         @config ||= app.config.dup.tap do |slice_config|
-          # Remove specific values from app that will not apply to this slice
+          # Unset config from app that does not apply to ordinary slices
           slice_config.root = nil
         end
       end

lib/hanami/slice_registrar.rb

@@ -5,6 +5,7 @@ require_relative "constants"
 module Hanami
   # @api private
   class SliceRegistrar
+    VALID_SLICE_NAME_RE = /^[a-z][a-z0-9_]+$/
     SLICE_DELIMITER = CONTAINER_KEY_DELIMITER
 
     attr_reader :parent, :slices
@@ -16,14 +17,16 @@ module Hanami
     end
 
     def register(name, slice_class = nil, &block)
+      unless name.to_s =~ VALID_SLICE_NAME_RE
+        raise ArgumentError, "slice name #{name.inspect} must be lowercase alphanumeric text and underscores only"
+      end
+
       return unless filter_slice_names([name]).any?
 
       if slices.key?(name.to_sym)
         raise SliceLoadError, "Slice '#{name}' is already registered"
       end
 
-      # TODO: raise error unless name meets format (i.e. single level depth only)
-
       slice = slice_class || build_slice(name, &block)
 
       configure_slice(name, slice)
@@ -74,7 +77,9 @@ module Hanami
 
     def with_nested
       to_a.flat_map { |slice|
-        [slice] + slice.slices.with_nested
+        # Return nested slices first so that their more specific namespaces may be picked up first
+        # by SliceConfigurable#slice_for
+        slice.slices.with_nested + [slice]
       }
     end
 
@@ -88,36 +93,39 @@ module Hanami
       parent.inflector
     end
 
-    # Runs when a slice file has been found at `config/slices/[slice_name].rb`, or a slice
-    # directory at `slices/[slice_name]`. Attempts to require the slice class, if defined,
-    # or generates a new slice class for the given slice name.
-    def load_slice(slice_name)
-      slice_const_name = inflector.camelize(slice_name)
-      slice_require_path = root.join(CONFIG_DIR, SLICES_DIR, slice_name).to_s
+    def parent_slice_namespace
+      parent.eql?(parent.app) ? Object : parent.namespace
+    end
 
+    # Runs when a slice file has been found at `config/slices/[slice_name].rb`, or a slice directory
+    # at `slices/[slice_name]`. Attempts to require the slice class, if defined, before registering
+    # the slice. If a slice class is not found, registering the slice will generate the slice class.
+    def load_slice(slice_name)
+      slice_require_path = root.join(CONFIG_DIR, SLICES_DIR, slice_name).to_s
       begin
         require(slice_require_path)
       rescue LoadError => e
         raise e unless e.path == slice_require_path
       end
 
+      slice_module_name = inflector.camelize("#{parent_slice_namespace.name}#{PATH_DELIMITER}#{slice_name}")
       slice_class =
         begin
-          inflector.constantize("#{slice_const_name}::Slice")
+          inflector.constantize("#{slice_module_name}#{MODULE_DELIMITER}Slice")
         rescue NameError => e
-          raise e unless e.name.to_s == slice_const_name || e.name.to_s == :Slice
+          raise e unless e.name.to_s == inflector.camelize(slice_name) || e.name.to_s == :Slice
         end
 
       register(slice_name, slice_class)
     end
 
     def build_slice(slice_name, &block)
+      slice_module_name = inflector.camelize("#{parent_slice_namespace.name}#{PATH_DELIMITER}#{slice_name}")
       slice_module =
         begin
-          slice_module_name = inflector.camelize(slice_name.to_s)
           inflector.constantize(slice_module_name)
         rescue NameError
-          Object.const_set(inflector.camelize(slice_module_name), Module.new)
+          parent_slice_namespace.const_set(inflector.camelize(slice_name), Module.new)
         end
 
       slice_module.const_set(:Slice, Class.new(Hanami::Slice, &block))

spec/integration/slices/slice_loading_spec.rb

@@ -3,7 +3,7 @@
 require "rack/test"
 
 RSpec.describe "Slices / Slice loading", :app_integration, :aggregate_failures do
-  let(:app_modules) { %i[TestApp Admin Editorial Main Shop] }
+  let(:app_modules) { %i[TestApp Admin Main] }
 
   describe "loading specific slices with config.slices" do
     describe "setup app" do
@@ -48,7 +48,7 @@ RSpec.describe "Slices / Slice loading", :app_integration, :aggregate_failures d
 
             expect { Admin::Slice.register_slice :editorial }.not_to(change { Admin::Slice.slices.keys })
             expect { Admin::Slice.register_slice :shop }.to change { Admin::Slice.slices.keys }.to [:shop]
-            expect(Shop::Slice).to be
+            expect(Admin::Shop::Slice).to be
           end
         end
       end
@@ -123,9 +123,9 @@ RSpec.describe "Slices / Slice loading", :app_integration, :aggregate_failures d
             expect(Admin::Slice.slices.keys).to eq [:shop]
 
             expect(Admin::Slice).to be
-            expect(Shop::Slice).to be
+            expect(Admin::Shop::Slice).to be
 
-            expect { Editorial }.to raise_error(NameError)
+            expect { Admin::Editorial }.to raise_error(NameError)
             expect { Main }.to raise_error(NameError)
           end
         end

spec/integration/slices_spec.rb

@@ -55,6 +55,32 @@ RSpec.describe "Slices", :app_integration do
     end
   end
 
+  specify "Loading a nested slice with a defined slice class" do
+    with_tmp_directory(Dir.mktmpdir) do
+      write "config/app.rb", <<~RUBY
+        require "hanami"
+
+        module TestApp
+          class App < Hanami::App
+          end
+        end
+      RUBY
+
+      write "slices/main/config/slices/nested.rb", <<~RUBY
+        module Main
+          module Nested
+            class Slice < Hanami::Slice
+            end
+          end
+        end
+      RUBY
+
+      require "hanami/prepare"
+
+      expect(Hanami.app.slices[:main].slices[:nested]).to be Main::Nested::Slice
+    end
+  end
+
   it "Loading a slice generates a slice class if none is defined" do
     with_tmp_directory(Dir.mktmpdir) do
       write "config/app.rb", <<~RUBY
@@ -78,6 +104,80 @@ RSpec.describe "Slices", :app_integration do
     end
   end
 
+  specify "Registering a slice on the app creates a slice class with a top-level namespace" do
+    with_tmp_directory(Dir.mktmpdir) do
+      write "config/app.rb", <<~RUBY
+        require "hanami"
+
+        module TestApp
+          class App < Hanami::App
+            register_slice :main
+          end
+        end
+      RUBY
+
+      require "hanami/prepare"
+
+      expect(Hanami.app.slices[:main]).to be Main::Slice
+      expect(Main::Slice.ancestors).to include(Hanami::Slice)
+    end
+  end
+
+  specify "Registering a nested slice creates a slice class within the parent's namespace" do
+    with_tmp_directory(Dir.mktmpdir) do
+      write "config/app.rb", <<~RUBY
+        require "hanami"
+
+        module TestApp
+          class App < Hanami::App
+          end
+        end
+      RUBY
+
+      write "config/slices/main.rb", <<~RUBY
+        module Main
+          class Slice < Hanami::Slice
+            register_slice :nested
+          end
+        end
+      RUBY
+
+      require "hanami/prepare"
+
+      expect(Hanami.app.slices[:main].slices[:nested]).to be Main::Nested::Slice
+    end
+  end
+
+  specify "Registering a nested slice with an existing class uses that class' own namespace" do
+    with_tmp_directory(Dir.mktmpdir) do
+      write "config/app.rb", <<~RUBY
+        require "hanami"
+
+        module TestApp
+          class App < Hanami::App
+          end
+        end
+      RUBY
+
+      write "config/slices/main.rb", <<~RUBY
+        module Admin
+          class Slice < Hanami::Slice
+          end
+        end
+
+        module Main
+          class Slice < Hanami::Slice
+            register_slice :nested, Admin::Slice
+          end
+        end
+      RUBY
+
+      require "hanami/prepare"
+
+      expect(Hanami.app.slices[:main].slices[:nested]).to be Admin::Slice
+    end
+  end
+
   it "Registering a slice with a block creates a slice class and evals the block" do
     with_tmp_directory(Dir.mktmpdir) do
       write "config/app.rb", <<~RUBY

spec/unit/hanami/slice_configurable_spec.rb

@@ -106,18 +106,18 @@ RSpec.describe Hanami::SliceConfigurable, :app_integration do
         class Slice
           register_slice :nested
         end
-      end
 
-      module Nested
-        class MySubclass < TestApp::BaseClass
+        module Nested
+          class MySubclass < TestApp::BaseClass
+          end
         end
       end
     end
 
-    subject(:subclass) { Nested::MySubclass }
+    subject(:subclass) { Main::Nested::MySubclass }
 
     it "calls `configure_for_slice` with the nested slice" do
-      expect(subclass.traces).to eq [Nested::Slice]
+      expect(subclass.traces).to eq [Main::Nested::Slice]
     end
   end
 end