Deprecate automatic method_missing and Chef::Resource lookup

- Declare all resource DSL as methods on Chef::DSL::Resources
- Declare all definition DSL as methods on Chef::DSL::Definitions

10 files changed, 240 insertions, 85 deletions

diff --git a/lib/chef/dsl/definitions.rb b/lib/chef/dsl/definitions.rb
new file mode 100644
index 0000000000..1358f67720
--- /dev/null
+++ b/lib/chef/dsl/definitions.rb
@@ -0,0 +1,44 @@
+class Chef
+  module DSL
+    #
+    # Module containing a method for each declared definition
+    #
+    # Depends on declare_resource(name, created_at, &block)
+    #
+    # @api private
+    #
+    module Definitions
+      def self.add_definition(dsl_name)
+        module_eval <<-EOM, __FILE__, __LINE__+1
+          def #{dsl_name}(*args, &block)
+            evaluate_resource_definition(#{dsl_name.inspect}, *args, &block)
+          end
+        EOM
+      end
+
+      # @api private
+      def has_resource_definition?(name)
+        run_context.definitions.has_key?(name)
+      end
+
+      # Processes the arguments and block as a resource definition.
+      #
+      # @api private
+      def evaluate_resource_definition(definition_name, *args, &block)
+
+        # This dupes the high level object, but we still need to dup the params
+        new_def = run_context.definitions[definition_name].dup
+
+        new_def.params = new_def.params.dup
+        new_def.node = run_context.node
+        # This sets up the parameter overrides
+        new_def.instance_eval(&block) if block
+
+        new_recipe = Chef::Recipe.new(cookbook_name, recipe_name, run_context)
+        new_recipe.params = new_def.params
+        new_recipe.params[:name] = args[0]
+        new_recipe.instance_eval(&new_def.recipe)
+      end
+    end
+  end
+end
diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index c22f053292..e17e008432 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -21,6 +21,8 @@ require 'chef/mixin/convert_to_class_name'
 require 'chef/exceptions'
 require 'chef/resource_builder'
 require 'chef/mixin/shell_out'
+require 'chef/dsl/resources'
+require 'chef/dsl/definitions'
 
 class Chef
   module DSL
@@ -31,49 +33,62 @@ class Chef
     module Recipe
 
       include Chef::Mixin::ShellOut
-      include Chef::Mixin::ConvertToClassName
 
+      # method_missing must live for backcompat purposes until Chef 13.
       def method_missing(method_symbol, *args, &block)
-        # If we have a definition that matches, we want to use that instead.  This should
-        # let you do some really crazy over-riding of "native" types, if you really want
-        # to.
-        if has_resource_definition?(method_symbol)
-          evaluate_resource_definition(method_symbol, *args, &block)
-        elsif have_resource_class_for?(method_symbol)
-          # Otherwise, we're rocking the regular resource call route.
-          declare_resource(method_symbol, args[0], caller[0], &block)
-        else
-          begin
-            super
-          rescue NoMethodError
-            raise NoMethodError, "No resource or method named `#{method_symbol}' for #{describe_self_for_error}"
-          rescue NameError
-            raise NameError, "No resource, method, or local variable named `#{method_symbol}' for #{describe_self_for_error}"
-          end
+        #
+        # If there is already DSL for this, someone must have called
+        # method_missing manually. Not a fan. Not. A. Fan.
+        #
+        if respond_to?(method_symbol)
+          Chef::Log.warn("Calling method_missing(#{method_symbol.inspect}) directly is deprecated in Chef 12 and will be removed in Chef 13.")
+          Chef::Log.warn("Use public_send() or send() instead.")
+          return send(method_symbol, *args, &block)
         end
-      end
-
-      def has_resource_definition?(name)
-        run_context.definitions.has_key?(name)
-      end
-
-      # Processes the arguments and block as a resource definition.
-      def evaluate_resource_definition(definition_name, *args, &block)
 
-        # This dupes the high level object, but we still need to dup the params
-        new_def = run_context.definitions[definition_name].dup
+        #
+        # If a definition exists, then Chef::DSL::Definitions.add_definition was
+        # never called.  DEPRECATED.
+        #
+        if run_context.definitions.has_key?(method_symbol.to_sym)
+          Chef::Log.warn("Definition #{method_symbol} (#{run_context.definitions[method_symbol.to_sym]}) was added to the run_context without calling Chef::DSL::Definitions.add_definition(#{method_symbol.to_sym.inspect}).  This will become required in Chef 13.")
+          Chef::DSL::Definitions.add_definition(method_symbol)
+          return send(method_symbol, *args, &block)
+        end
 
-        new_def.params = new_def.params.dup
-        new_def.node = run_context.node
-        # This sets up the parameter overrides
-        new_def.instance_eval(&block) if block
+        #
+        # See if the resource exists anyway.  If the user had set
+        # Chef::Resource::Blah = <resource>, a deprecation warning will be
+        # emitted and the DSL method 'blah' will be added to the DSL.
+        #
+        resource_class = Chef::ResourceResolver.new(run_context ? run_context.node : nil, method_symbol).resolve
+        if resource_class
+          #
+          # If the DSL method was *not* added, this is the case where the
+          # matching class implements 'provides?' and matches resources that it
+          # never declared "provides" for (which means we would never have
+          # created DSL).  Anything where we don't create DSL is deprecated.
+          #
+          if !respond_to?(method_symbol)
+            Chef::Log.warn("#{resource_class} is marked as providing DSL #{method_symbol}, but provides #{method_symbol.inspect} was never called!")
+            Chef::Log.warn("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
+            Chef::DSL::Resources.add_resource_dsl(method_symbol)
+          end
+          return send(method_symbol, *args, &block)
+        end
 
-        new_recipe = Chef::Recipe.new(cookbook_name, recipe_name, run_context)
-        new_recipe.params = new_def.params
-        new_recipe.params[:name] = args[0]
-        new_recipe.instance_eval(&new_def.recipe)
+        begin
+          super
+        rescue NoMethodError
+          raise NoMethodError, "No resource or method named `#{method_symbol}' for #{describe_self_for_error}"
+        rescue NameError
+          raise NameError, "No resource, method, or local variable named `#{method_symbol}' for #{describe_self_for_error}"
+        end
       end
 
+      include Chef::DSL::Resources
+      include Chef::DSL::Definitions
+
       #
       # Instantiates a resource (via #build_resource), then adds it to the
       # resource collection. Note that resource classes are looked up directly,
diff --git a/lib/chef/dsl/resources.rb b/lib/chef/dsl/resources.rb
new file mode 100644
index 0000000000..dfa0788879
--- /dev/null
+++ b/lib/chef/dsl/resources.rb
@@ -0,0 +1,19 @@
+class Chef
+  module DSL
+    #
+    # Module containing a method for each globally declared Resource
+    #
+    # Depends on declare_resource(name, created_at, &block)
+    #
+    # @api private
+    module Resources
+      def self.add_resource_dsl(dsl_name)
+        module_eval <<-EOM, __FILE__, __LINE__+1
+          def #{dsl_name}(name, created_at=nil, &block)
+            declare_resource(#{dsl_name.inspect}, name, created_at || caller[0], &block)
+          end
+        EOM
+      end
+    end
+  end
+end
diff --git a/lib/chef/mixin/provides.rb b/lib/chef/mixin/provides.rb
index bc25af62b2..d71097ca4b 100644
--- a/lib/chef/mixin/provides.rb
+++ b/lib/chef/mixin/provides.rb
@@ -23,7 +23,8 @@ class Chef
         node_map.set(short_name, true, opts, &block)
       end
 
-      # provides a node on the resource (early binding)
+      # Check whether this resource provides the resource_name DSL for the given
+      # node
       def provides?(node, resource_name)
         resource_name = resource_name.resource_name if resource_name.is_a?(Chef::Resource)
         node_map.get(node, resource_name)
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 3d319617d1..ef0b59e4dc 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -22,6 +22,7 @@ require 'chef/dsl/platform_introspection'
 require 'chef/dsl/data_query'
 require 'chef/dsl/registry_helper'
 require 'chef/dsl/reboot_pending'
+require 'chef/dsl/resources'
 require 'chef/mixin/convert_to_class_name'
 require 'chef/guard_interpreter/resource_guard_interpreter'
 require 'chef/resource/conditional'
@@ -820,7 +821,10 @@ class Chef
     #
     # @return [String] The DSL name of this resource.
     def self.dsl_name
-      convert_to_snake_case(name, 'Chef::Resource')
+      if name
+        name = self.name.split('::')[-1]
+        convert_to_snake_case(name)
+      end
     end
 
     #
@@ -969,6 +973,19 @@ class Chef
       end
     end
 
+    # Cause each subclass to register itself with the DSL
+    def self.inherited(subclass)
+      super
+      if subclass.dsl_name
+        subclass.provides subclass.dsl_name.to_sym
+      end
+    end
+
+    def self.provides(name, *args, &block)
+      super
+      Chef::DSL::Resources.add_resource_dsl(name)
+    end
+
     # Helper for #notifies
     def validate_resource_spec!(resource_spec)
       run_context.resource_collection.validate_lookup_spec!(resource_spec)
@@ -1099,25 +1116,23 @@ class Chef
     # === Returns
     # <Chef::Resource>:: returns the proper Chef::Resource class
     def self.resource_for_node(short_name, node)
-      require 'chef/resource_resolver'
       klass = Chef::ResourceResolver.new(node, short_name).resolve
       raise Chef::Exceptions::NoSuchResourceType.new(short_name, node) if klass.nil?
       klass
     end
 
+    #
     # Returns the class of a Chef::Resource based on the short name
+    # Only returns the *canonical* class with the given name, not the one that
+    # would be picked by the ResourceResolver.
+    #
     # ==== Parameters
     # short_name<Symbol>:: short_name of the resource (ie :directory)
     #
     # === Returns
     # <Chef::Resource>:: returns the proper Chef::Resource class
     def self.resource_matching_short_name(short_name)
-      begin
-        rname = convert_to_class_name(short_name.to_s)
-        Chef::Resource.const_get(rname)
-      rescue NameError
-        nil
-      end
+      Chef::ResourceResolver.new(Chef::Node.new, short_name).resolve
     end
 
     private
@@ -1135,3 +1150,5 @@ class Chef
     end
   end
 end
+
+require 'chef/resource_resolver'
diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb
index ce72e98028..f702db5c96 100644
--- a/lib/chef/resource/lwrp_base.rb
+++ b/lib/chef/resource/lwrp_base.rb
@@ -35,32 +35,64 @@ class Chef
 
       # Evaluates the LWRP resource file and instantiates a new Resource class.
       def self.build_from_file(cookbook_name, filename, run_context)
-        resource_class = nil
-        rname = filename_to_qualified_string(cookbook_name, filename)
-
-        class_name = convert_to_class_name(rname)
-        if Resource.const_defined?(class_name, false)
-          Chef::Log.info("#{class_name} light-weight resource is already initialized -- Skipping loading #{filename}!")
-          Chef::Log.debug("Overriding already defined LWRPs is not supported anymore starting with Chef 12.")
-          resource_class = Resource.const_get(class_name)
-        else
-          resource_class = Class.new(self)
+        resource_name = filename_to_qualified_string(cookbook_name, filename)
+
+        node = run_context ? run_context.node : Chef::Node.new
+        existing_class = Chef::ResourceResolver.new(node, resource_name).resolve
+
+        if existing_class
+          Chef::Log.info("Skipping LWRP resource #{filename} from cookbook #{cookbook_name}.  #{existing_class} already defined.")
+          Chef::Log.debug("Overriding resources with LWRPs is not supported anymore starting with Chef 12.")
+          return existing_class
+        end
 
-          Chef::Resource.const_set(class_name, resource_class)
-          resource_class.resource_name = rname
-          resource_class.run_context = run_context
-          resource_class.class_from_file(filename)
+        # We load the class first to give it a chance to set its own name
+        resource_class = Class.new(self)
+        resource_class.resource_name = resource_name
+        resource_class.run_context = run_context
+        resource_class.class_from_file(filename)
 
-          Chef::Log.debug("Loaded contents of #{filename} into a resource named #{rname} defined in Chef::Resource::#{class_name}")
+        # Respect resource_name set inside the LWRP
+        resource_class.instance_eval do
+          define_method(:to_s) do
+            "LWRP #{resource_name} from cookbook #{cookbook_name}"
+          end
+          define_method(:inspect) { to_s }
         end
 
+        Chef::Log.debug("Loaded contents of #{filename} into #{resource_class}")
+
+        create_deprecated_class_in_chef_resource(resource_class)
+
         resource_class
       end
 
-      # Set the resource name for this LWRP
+      def self.create_deprecated_class_in_chef_resource(resource_class)
+        # Create a class in Chef::Resource::MyResource with deprecation
+        # warnings if you try to instantiate or inherit it (for Chef 12 compatibility)
+        class_name = convert_to_class_name(resource_class.resource_name)
+        if Chef::Resource.const_defined?(class_name, false)
+          Chef::Log.debug "#{class_name} already exists!  Cannot create deprecation class #{resource_class}"
+        else
+          Chef::Resource.const_set(class_name, Class.new(resource_class) do
+            self.resource_name = resource_class.resource_name
+            self.run_context = resource_class.run_context
+            define_method(:initialize) do |*args, &block|
+              Chef::Log.warn("Using an LWRP by its name (#{class_name}) directly is no longer supported in Chef 12 and will be removed.")
+              super(*args, &block)
+            end
+            define_singleton_method(:inherited) do |*args, &block|
+              Chef::Log.warn("Using an LWRP by its name (#{class_name}) directly is no longer supported in Chef 12 and will be removed.")
+              super(*args, &block)
+            end
+          end)
+        end
+        Chef::Resource.const_get(class_name)
+      end
+
       def self.resource_name(arg = NULL_ARG)
         if arg.equal?(NULL_ARG)
-          @resource_name
+          @resource_name || dsl_name
         else
           @resource_name = arg
         end
@@ -127,7 +159,7 @@ class Chef
       end
 
       def self.node
-        run_context.node
+        run_context ? run_context.node : nil
       end
 
       def self.lazy(&block)
diff --git a/lib/chef/resource_definition.rb b/lib/chef/resource_definition.rb
index 9d6844129c..cffabb6786 100644
--- a/lib/chef/resource_definition.rb
+++ b/lib/chef/resource_definition.rb
@@ -50,6 +50,7 @@ class Chef
       else
         raise ArgumentError, "You must pass a block to a definition."
       end
+      Chef::DSL::Definitions.add_definition(name)
       true
     end
 
diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb
index ff9d7aeb9f..ac6b3d8839 100644
--- a/lib/chef/resource_resolver.rb
+++ b/lib/chef/resource_resolver.rb
@@ -18,9 +18,11 @@
 
 require 'chef/exceptions'
 require 'chef/platform/resource_priority_map'
+require 'chef/mixin/convert_to_class_name'
 
 class Chef
   class ResourceResolver
+    include Chef::Mixin::ConvertToClassName
 
     attr_reader :node
     attr_reader :resource
@@ -37,23 +39,6 @@ class Chef
     end
 
     def resolve
-      maybe_dynamic_resource_resolution(resource) ||
-        maybe_chef_platform_lookup(resource)
-    end
-
-    # this cut looks at if the resource can handle the resource type on the node
-    def enabled_handlers
-      @enabled_handlers ||=
-        resources.select do |klass|
-          klass.provides?(node, resource)
-        end.sort {|a,b| a.to_s <=> b.to_s }
-      @enabled_handlers
-    end
-
-    private
-
-    # try dynamically finding a resource based on querying the resources to see what they support
-    def maybe_dynamic_resource_resolution(resource)
       # log this so we know what resources will work for the generic resource on the node (early cut)
       Chef::Log.debug "resources for generic #{resource} resource enabled on node include: #{enabled_handlers}"
 
@@ -61,7 +46,7 @@ class Chef
       # enabled on the node to handle the why-run use case.
       handlers = enabled_handlers
 
-      if handlers.count >= 2
+      if handlers.size >= 2
         # this magic stack ranks the resources by where they appear in the resource_priority_map
         priority_list = [ get_priority_array(node, resource) ].flatten.compact
         handlers = handlers.sort_by { |x| i = priority_list.index x; i.nil? ? Float::INFINITY : i }
@@ -70,7 +55,7 @@ class Chef
           # entry for this resource is missing -- we should probably raise here and force resolution of the ambiguity.
           Chef::Log.warn "Ambiguous resource precedence: #{handlers}, please use Chef.set_resource_priority_array to provide determinism"
         end
-        handlers = [ handlers.first ]
+        handlers = handlers[0..0]
       end
 
       Chef::Log.debug "resources that survived replacement include: #{handlers}"
@@ -84,9 +69,48 @@ class Chef
       handlers[0]
     end
 
-    # try the old static lookup of resources by mangling name to resource klass
-    def maybe_chef_platform_lookup(resource)
-      Chef::Resource.resource_matching_short_name(resource)
+    # this cut looks at if the resource can handle the resource type on the node
+    def enabled_handlers
+      @enabled_handlers ||= begin
+        enabled_handlers = resources.select do |klass|
+          klass.provides?(node, resource)
+        end.sort {|a,b| a.to_s <=> b.to_s }
+
+        # Add Chef::Resource::X as a possibility if it is not a handler already
+        check_for_deprecated_chef_resource_class(enabled_handlers)
+
+        enabled_handlers
+      end
+    end
+
+    private
+
+    #
+    # Checks if the Chef::Resource::* class corresponding to the DSL name
+    # exists, emits a deprecation warning, marks it as providing the given
+    # short name and adds it to the DSL.  This is used for method_missing
+    # deprecation and ResourceResolver checking, when people have created
+    # anonymous classes and assigned them to Chef::Resource::X.
+    #
+    # Returns the matched class, if it exists.
+    #
+    # @api private
+    def check_for_deprecated_chef_resource_class(enabled_handlers)
+      # If Chef::Resource::MyResource exists, but was not set, it won't have a
+      # DSL name.  Add the DSL method and warn about this pattern.
+      class_name = convert_to_class_name(resource.to_s)
+      if Chef::Resource.const_defined?(class_name)
+        # If Chef::Resource::X already exists, and is *not* already marked as
+        # providing this resource, mark it as providing the resource and add it
+        # to the list of handlers for next time.
+        resource_class = Chef::Resource.const_get(class_name)
+        if resource_class <= Chef::Resource && !enabled_handlers.include?(resource_class)
+          Chef::Log.warn("Class #{resource_class} was created with Class.new and assigned directly to a constant (#{resource_class.name} = <class>) rather than being created directly (class #{resource_class.name} < <superclass>).")
+          Chef::Log.warn("This will no longer work in Chef 13: you can either declare your class directly (in any namespace), or specify 'provides #{resource.to_sym.inspect}' in the class definition.")
+          resource_class.provides resource.to_sym
+          enabled_handlers << resource_class
+        end
+      end
     end
 
     # dep injection hooks
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index ec39174da6..08273f3872 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -67,6 +67,8 @@ describe "LWRP" do
 
       Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file|
         expect(Chef::Log).to receive(:info).with(/Skipping/)
+        expect(Chef::Log).to receive(:debug).with(/enabled on node/)
+        expect(Chef::Log).to receive(:debug).with(/survived replacement/)
         expect(Chef::Log).to receive(:debug).with(/anymore/)
         Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil)
       end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 7442f4477e..9c4fb49497 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -83,7 +83,7 @@ describe Chef::Recipe do
       it "should require a name argument" do
         expect {
           recipe.cat
-        }.to raise_error(ArgumentError, "You must supply a name when declaring a cat resource")
+        }.to raise_error(ArgumentError)
       end
 
       it "should allow regular errors (not NameErrors) to pass unchanged" do