From 8e296f6a4e8333d3bcc8c4473575a53d57af76d0 Mon Sep 17 00:00:00 2001
From: Tom Finill <tom@kalohq.com>
Date: Wed, 11 Dec 2019 18:00:58 +0100
Subject: [PATCH] Allow for nested `no_commands` blocks.

In certain circumstances we may wish to nest `no_commands` blocks.
For example, if we want to import any module which includes both methods
and `attr_reader` invocations.

However, since when exiting the no_commands block the @no_commands
instance var is reset to `false` we get unexpected behaviour.

This change uses a `NestedContext` object to track the depth of the
no_command blocks and ensure that we only start creating commands again
once we've left all of them.
---
 lib/thor/base.rb            | 22 ++++++++++++++--------
 lib/thor/nested_context.rb  | 29 +++++++++++++++++++++++++++++
 spec/base_spec.rb           |  1 +
 spec/fixtures/script.thor   |  7 ++++++-
 spec/nested_context_spec.rb | 20 ++++++++++++++++++++
 5 files changed, 70 insertions(+), 9 deletions(-)
 create mode 100644 lib/thor/nested_context.rb
 create mode 100644 spec/nested_context_spec.rb

diff --git a/lib/thor/base.rb b/lib/thor/base.rb
index 2b1dab597..8a990a721 100644
--- a/lib/thor/base.rb
+++ b/lib/thor/base.rb
@@ -2,6 +2,7 @@
 require_relative "core_ext/hash_with_indifferent_access"
 require_relative "error"
 require_relative "invocation"
+require_relative "nested_context"
 require_relative "parser"
 require_relative "shell"
 require_relative "line_editor"
@@ -418,14 +419,20 @@ def remove_command(*names)
       #     remove_command :this_is_not_a_command
       #   end
       #
-      def no_commands
-        @no_commands = true
-        yield
-      ensure
-        @no_commands = false
+      def no_commands(&block)
+        no_commands_context.enter(&block)
       end
+
       alias_method :no_tasks, :no_commands
 
+      def no_commands_context
+        @no_commands_context ||= NestedContext.new
+      end
+
+      def no_commands?
+        no_commands_context.entered?
+      end
+
       # Sets the namespace for the Thor or Thor::Group class. By default the
       # namespace is retrieved from the class name. If your Thor class is named
       # Scripts::MyScript, the help method, for example, will be called as:
@@ -607,7 +614,7 @@ def find_and_refresh_command(name) #:nodoc:
       def inherited(klass)
         super(klass)
         Thor::Base.register_klass_file(klass)
-        klass.instance_variable_set(:@no_commands, false)
+        klass.instance_variable_set(:@no_commands, 0)
       end
 
       # Fire this callback whenever a method is added. Added methods are
@@ -624,8 +631,7 @@ def method_added(meth)
         # Return if it's not a public instance method
         return unless public_method_defined?(meth.to_sym)
 
-        @no_commands ||= false
-        return if @no_commands || !create_command(meth)
+        return if no_commands? || !create_command(meth)
 
         is_thor_reserved_word?(meth, :command)
         Thor::Base.register_klass_file(self)
diff --git a/lib/thor/nested_context.rb b/lib/thor/nested_context.rb
new file mode 100644
index 000000000..74f56bbdb
--- /dev/null
+++ b/lib/thor/nested_context.rb
@@ -0,0 +1,29 @@
+class Thor
+  class NestedContext
+    def initialize
+      @depth = 0
+    end
+
+    def enter
+      push
+
+      yield
+    ensure
+      pop
+    end
+
+    def entered?
+      @depth > 0
+    end
+
+    private
+
+    def push
+      @depth += 1
+    end
+
+    def pop
+      @depth -= 1
+    end
+  end
+end
diff --git a/spec/base_spec.rb b/spec/base_spec.rb
index 39b95ee67..e5a9561c2 100644
--- a/spec/base_spec.rb
+++ b/spec/base_spec.rb
@@ -49,6 +49,7 @@ def hello
     it "avoids methods being added as commands" do
       expect(MyScript.commands.keys).to include("animal")
       expect(MyScript.commands.keys).not_to include("this_is_not_a_command")
+      expect(MyScript.commands.keys).not_to include("neither_is_this")
     end
   end
 
diff --git a/spec/fixtures/script.thor b/spec/fixtures/script.thor
index 8c7e79c6c..f4bca4497 100644
--- a/spec/fixtures/script.thor
+++ b/spec/fixtures/script.thor
@@ -28,7 +28,12 @@ class MyScript < Thor
   desc "animal TYPE", "horse around"
 
   no_commands do
-    def this_is_not_a_command
+    no_commands do
+      def this_is_not_a_command
+      end
+    end
+
+    def neither_is_this
     end
   end
 
diff --git a/spec/nested_context_spec.rb b/spec/nested_context_spec.rb
new file mode 100644
index 000000000..dc8f4126d
--- /dev/null
+++ b/spec/nested_context_spec.rb
@@ -0,0 +1,20 @@
+require "helper"
+
+describe Thor::NestedContext do
+  subject(:context) { described_class.new }
+  
+  describe "#enter" do
+    it "is never empty within the entered block" do
+      context.enter do
+        context.enter {}
+
+        expect(context).to be_entered
+      end
+    end
+
+    it "is empty when outside of all blocks" do
+      context.enter { context.enter {} }
+      expect(context).not_to be_entered
+    end
+  end
+end
\ No newline at end of file