Simplify the deprecations system a bit, and introduce ways to silence deprecation warnings.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>

7 files changed, 232 insertions, 215 deletions

diff --git a/chef-config/lib/chef-config/config.rb b/chef-config/lib/chef-config/config.rb
index 82f25f4139..ed8fab9ee4 100644
--- a/chef-config/lib/chef-config/config.rb
+++ b/chef-config/lib/chef-config/config.rb
@@ -715,6 +715,11 @@ module ChefConfig
       ENV.key?("CHEF_TREAT_DEPRECATION_WARNINGS_AS_ERRORS")
     end
 
+    # Which deprecations warnings to silence. Can be set to `true` to silence
+    # all warnings, or an array of strings like either `"deprecation_type"` or
+    # `"filename.rb:lineno"`.
+    default :silence_deprecation_warnings, []
+
     # Whether the resource count should be updated for log resource
     # on running chef-client
     default :count_log_resource_updates, true
diff --git a/lib/chef/chef_class.rb b/lib/chef/chef_class.rb
index bfea51609a..95b03f1c5c 100644
--- a/lib/chef/chef_class.rb
+++ b/lib/chef/chef_class.rb
@@ -200,13 +200,14 @@ class Chef
     #
     # Emit a deprecation message.
     #
-    # @param [Symbol] type The message to send. This should refer to a class
+    # @param type [Symbol] The message to send. This should refer to a class
     #   defined in Chef::Deprecated
-    # @param message  An explicit message to display, rather than the generic one
-    #   associated with the deprecation.
-    # @param location The location. Defaults to the caller who called you (since
-    #   generally the person who triggered the check is the one that needs to be
-    #   fixed).
+    # @param message [String, nil] An explicit message to display, rather than
+    #   the generic one associated with the deprecation.
+    # @param location [String, nil] The location. Defaults to the caller who
+    #   called you (since generally the person who triggered the check is the one
+    #   that needs to be fixed).
+    # @return [void]
     #
     # @example
     #     Chef.deprecated(:my_deprecation, message: "This is deprecated!")
@@ -220,11 +221,18 @@ class Chef
       # run. If we are not yet in a run, print to `Chef::Log`.
       if run_context && run_context.events
         run_context.events.deprecation(deprecation, location)
-      else
-        Chef::Log.deprecation(deprecation, location)
+      elsif !deprecation.silenced?
+        Chef::Log.deprecation(deprecation.to_s)
       end
     end
 
+    # Log a generic deprecation warning that doesn't have a specific class in
+    # Chef::Deprecated.
+    #
+    # This should generally not be used, as the user will not be given a link
+    # to get more infomration on fixing the deprecation warning.
+    #
+    # @see #deprecated
     def log_deprecation(message, location = nil)
       location ||= Chef::Log.caller_location
       Chef.deprecated(:generic, message, location)
diff --git a/lib/chef/deprecated.rb b/lib/chef/deprecated.rb
index 904578ff0b..bba42b26a9 100644
--- a/lib/chef/deprecated.rb
+++ b/lib/chef/deprecated.rb
@@ -24,275 +24,189 @@ class Chef
     class << self
       include Chef::Mixin::ConvertToClassName
 
-      def create(type, message = nil, location = nil)
-        Chef::Deprecated.const_get(convert_to_class_name(type.to_s)).send(:new, message, location)
+      def create(type, message, location)
+        Chef::Deprecated.const_get(convert_to_class_name(type.to_s)).new(message, location)
       end
     end
 
     class Base
       BASE_URL = "https://docs.chef.io/deprecations_"
 
-      attr_accessor :message, :location
+      attr_reader :message, :location
 
       def initialize(msg = nil, location = nil)
-        @message = msg if msg
-        @location = location if location
+        @message = msg
+        @location = location
       end
 
       def link
         "Please see #{url} for further details and information on how to correct this problem."
       end
 
+      # Render the URL for the deprecation documentation page.
+      #
+      # @return [String]
       def url
-        "#{BASE_URL}#{target}"
-      end
-
-      # We know that the only time this gets called is by Chef::Log.deprecation,
-      # so special case
-      def <<(location)
-        @location = location
-      end
-
-      def inspect
-        "#{message} (CHEF-#{id})#{location}.\n#{link}"
-      end
-
-      def id
-        raise NotImplementedError, "subclasses of Chef::Deprecated::Base should define #id with a unique number"
-      end
-
-      def target
-        raise NotImplementedError, "subclasses of Chef::Deprecated::Base should define #target"
+        "#{BASE_URL}#{self.class.doc_page}"
+      end
+
+      # Render the user-visible message for this deprecation.
+      #
+      # @return [String]
+      def to_s
+        "#{message} (CHEF-#{self.class.deprecation_id})#{location}.\n#{link}"
+      end
+
+      # Check if this deprecation has been silenced.
+      #
+      # @return [Boolean]
+      def silenced?
+        # Check if all warnings have been silenced.
+        return true if Chef::Config[:silence_deprecation_warnings] == true
+        # Check if this warning has been silenced by the config.
+        return true if Chef::Config[:silence_deprecation_warnings].any? do |silence_spec|
+          # Just in case someone uses a symbol in the config by mistake.
+          silence_spec = silence_spec.to_s
+           # Check for a silence by deprecation name, or by location.
+          self.class.deprecation_key == silence_spec || location.include?(silence_spec)
+        end
+        # check if this warning has been silenced by inline comment.
+        return true if location =~ /^(.*?):(\d+):in/ && begin
+          # Don't buffer the whole file in memory, so read it one line at a time.
+          line_no = $2.to_i
+          location_file = ::File.open($1)
+          (line_no - 1).times { location_file.readline } # Read all the lines we don't care about.
+          relevant_line = location_file.readline
+          relevant_line.match?(/#.*chef:silence_deprecation($|[^:]|:#{self.class.deprecation_key})/)
+        end
+        false
+      end
+
+      class << self
+        attr_reader :deprecation_id, :doc_page
+
+        # Return the deprecation key as would be used with {Chef::Deprecated.create}.
+        #
+        # @return [String]
+        def deprecation_key
+          Chef::Mixin::ConvertToClassName.convert_to_snake_case(name, "Chef::Deprecated")
+        end
+
+        # Set the ID and documentation page path for this deprecation.
+        #
+        # Used in subclasses to set the data for each type of deprecation.
+        #
+        # @example
+        #   class MyDeprecation < Base
+        #     target 123, "my_deprecation.html"
+        #   end
+        # @param id [Integer] Deprecation ID number. This must be unique among
+        #   all deprecations.
+        # @param page [String, nil] Optional documentation page path. If not
+        #   specified, the deprecation key is used.
+        # @return [void]
+        def target(id, page = nil)
+          @deprecation_id = id
+          @doc_page = page || "#{deprecation_key}.html"
+        end
       end
     end
 
     class InternalApi < Base
-      def id
-        0
-      end
-
-      def target
-        "internal_api.html"
-      end
+      target 0
     end
 
     class JsonAutoInflate < Base
-      def id
-        1
-      end
-
-      def target
-        "json_auto_inflate.html"
-      end
+      target 1
     end
 
     class ExitCode < Base
-      def id
-        2
-      end
-
-      def target
-        "exit_code.html"
-      end
+      target 2
     end
 
     # id 3 has been deleted
 
     class Attributes < Base
-      def id
-        4
-      end
-
-      def target
-        "attributes.html"
-      end
+      target 4
     end
 
     class CustomResource < Base
-      def id
-        5
-      end
-
-      def target
-        "custom_resource_cleanups.html"
-      end
+      target 5, "custom_resource_cleanups.html"
     end
 
     class EasyInstall < Base
-      def id
-        6
-      end
-
-      def target
-        "easy_install.html"
-      end
+      target 6
     end
 
     class VerifyFile < Base
-      def id
-        7
-      end
-
-      def target
-        "verify_file.html"
-      end
+      target 7
     end
 
     class SupportsProperty < Base
-      def id
-        8
-      end
-
-      def target
-        "supports_property.html"
-      end
+      target 8
     end
 
     class ChefRest < Base
-      def id
-        9
-      end
-
-      def target
-        "chef_rest.html"
-      end
+      target 9
     end
 
     class DnfPackageAllowDowngrade < Base
-      def id
-        10
-      end
-
-      def target
-        "dnf_package_allow_downgrade.html"
-      end
+      target 10
     end
 
     class PropertyNameCollision < Base
-      def id
-        11
-      end
-
-      def target
-        "property_name_collision.html"
-      end
+      target 11
     end
 
     class LaunchdHashProperty < Base
-      def id
-        12
-      end
-
-      def target
-        "launchd_hash_property.html"
-      end
+      target 12
     end
 
     class ChefPlatformMethods < Base
-      def id
-        13
-      end
-
-      def target
-        "chef_platform_methods.html"
-      end
+      target 13
     end
 
     class RunCommand < Base
-      def id
-        14
-      end
-
-      def target
-        "run_command.html"
-      end
+      target 14
     end
 
     class PackageMisc < Base
-      def id
-        15
-      end
-
-      def target
-        "package_misc.html"
-      end
+      target 15
     end
 
     class MultiresourceMatch < Base
-      def id
-        16
-      end
-
-      def target
-        "multiresource_match.html"
-      end
+      target 16
     end
 
     class UseInlineResources < Base
-      def id
-        17
-      end
-
-      def target
-        "use_inline_resources.html"
-      end
+      target 17
     end
 
     class LocalListen < Base
-      def id
-        18
-      end
-
-      def target
-        "local_listen.html"
-      end
+      target 18
     end
 
     class NamespaceCollisions < Base
-      def id
-        19
-      end
-
-      def target
-        "namespace_collisions.html"
-      end
+      target 19
     end
 
     class DeployResource < Base
-      def id
-        21
-      end
-
-      def target
-        "deploy_resource.html"
-      end
+      target 21
     end
 
     class ErlResource < Base
-      def id
-        22
-      end
-
-      def target
-        "erl_resource.html"
-      end
+      target 22
     end
 
     class FreebsdPkgProvider < Base
-      def id
-        23
-      end
-
-      def target
-        "freebsd_pkg_provider.html"
-      end
+      target 23
     end
 
     # id 3694 was deleted
 
     # Returned when using the deprecated option on a property
     class Property < Base
-      def inspect
+      def to_s
         "#{message}\n#{location}"
       end
     end
@@ -302,8 +216,8 @@ class Chef
         "https://docs.chef.io/chef_deprecations_client.html"
       end
 
-      def inspect
-        "#{message}\nThis is a generic error message and should be updated to have a proper deprecation class. #{location}\nPlease see #{url} for an overview of Chef deprecations."
+      def to_s
+        "#{message} #{location}.\n#{link}"
       end
     end
 
diff --git a/lib/chef/formatters/base.rb b/lib/chef/formatters/base.rb
index 0b27b55048..997577aa7b 100644
--- a/lib/chef/formatters/base.rb
+++ b/lib/chef/formatters/base.rb
@@ -212,14 +212,13 @@ class Chef
         file_load_failed(path, exception)
       end
 
-      def deprecation(message, location = caller(2..2)[0])
-        out = if is_structured_deprecation?(message)
-                message.inspect
-              else
-                "#{message} at #{location}"
-              end
-
-        Chef::Log.deprecation(out)
+      # Log a deprecation warning object.
+      #
+      # @param deprecation [Chef::Deprecated::Base] Deprecation object to log.
+      #   In previous versions, this could be a string. Don't do that anymore.
+      # @param location [Object] Unused, present only for compatbility.
+      def deprecation(deprecation, _location = nil)
+        Chef::Log.deprecation(deprecation.to_s) unless deprecation.silenced?
       end
 
       def is_structured_deprecation?(deprecation)
diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb
index 0c51cc2cfb..d47ab73a30 100644
--- a/lib/chef/formatters/doc.rb
+++ b/lib/chef/formatters/doc.rb
@@ -414,19 +414,15 @@ class Chef
         end
       end
 
-      def deprecation(message, location = caller(2..2)[0])
+      # (see Base#deprecation)
+      def deprecation(deprecation, _location = nil)
         if Chef::Config[:treat_deprecation_warnings_as_errors]
           super
+        elsif !deprecation.silenced?
+          # Save non-silenced deprecations to the screen until the end.
+          deprecations[deprecation.message] ||= { url: deprecation.url, locations: Set.new }
+          deprecations[deprecation.message][:locations] << deprecation.location
         end
-
-        # Save deprecations to the screen until the end
-        if is_structured_deprecation?(message)
-          url = message.url
-          message = message.message
-        end
-
-        deprecations[message] ||= { url: url, locations: Set.new }
-        deprecations[message][:locations] << location
       end
 
       def indent
diff --git a/lib/chef/log.rb b/lib/chef/log.rb
index 10c9f0f20d..3f77158579 100644
--- a/lib/chef/log.rb
+++ b/lib/chef/log.rb
@@ -46,19 +46,21 @@ class Chef
     #
     def self.caller_location
       # Pick the first caller that is *not* part of the Chef gem, that's the
-      # thing the user wrote.
+      # thing the user wrote. Or failing that, the most recent caller.
       chef_gem_path = File.expand_path("../..", __FILE__)
-      caller(0..20).find { |c| !c.start_with?(chef_gem_path) }
+      caller(0..20).find { |c| !c.start_with?(chef_gem_path) } || caller(0..1)[0]
     end
 
-    def self.deprecation(msg = nil, location = caller(2..2)[0], &block)
-      if msg
-        msg << " at #{Array(location).join("\n")}"
-        msg = msg.join("") if msg.respond_to?(:join)
-      end
+    # Log a deprecation warning.
+    #
+    # If the treat_deprecation_warnings_as_errors config option is set, this
+    # will raise an exception instead.
+    #
+    # @param msg [String] Deprecation message to display.
+    def self.deprecation(msg, &block)
       if Chef::Config[:treat_deprecation_warnings_as_errors]
         error(msg, &block)
-        raise Chef::Exceptions::DeprecatedFeatureError.new(msg.inspect)
+        raise Chef::Exceptions::DeprecatedFeatureError.new(msg)
       else
         warn(msg, &block)
       end
diff --git a/spec/unit/chef_class_spec.rb b/spec/unit/chef_class_spec.rb
index 21987c01ab..1910d90b38 100644
--- a/spec/unit/chef_class_spec.rb
+++ b/spec/unit/chef_class_spec.rb
@@ -107,4 +107,97 @@ describe "Chef class" do
       end.to raise_error(Chef::Exceptions::InvalidEventType)
     end
   end
+
+  fdescribe "Deprecation system" do
+    context "with treat_deprecation_warnings_as_errors false" do
+      before { Chef::Config[:treat_deprecation_warnings_as_errors] = false }
+
+      it "displays a simple deprecation warning" do
+        expect(Chef::Log).to receive(:warn).with(%r{I'm a little teapot\..*?spec/unit/chef_class_spec\.rb.*?Please see}m)
+        Chef.deprecated(:generic, "I'm a little teapot.")
+      end
+
+      it "allows silencing all warnings" do
+        Chef::Config[:silence_deprecation_warnings] = true
+        expect(Chef::Log).to_not receive(:warn)
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:internal_api, "Short and stout.")
+        Chef.deprecated(:generic, "This is my handle.")
+      end
+
+      it "allows silencing specific types" do
+        Chef::Config[:silence_deprecation_warnings] = [:internal_api]
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:internal_api, "Short and stout.")
+        Chef.deprecated(:generic, "This is my handle.")
+      end
+
+      it "allows silencing specific lines" do
+        Chef::Config[:silence_deprecation_warnings] = ["chef_class_spec.rb:#{__LINE__ + 4}"]
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:generic, "Short and stout.")
+        Chef.deprecated(:internal_api, "This is my handle.")
+      end
+
+      it "allows silencing all via inline comments" do
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:generic, "Short and stout.") # chef:silence_deprecation
+        Chef.deprecated(:internal_api, "This is my handle.")
+      end
+
+      it "allows silencing specific types via inline comments" do
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:generic, "Short and stout.") # chef:silence_deprecation:generic
+        Chef.deprecated(:internal_api, "This is my handle.")
+      end
+
+      it "does not silence via inline comments when the types don't match" do
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/Short and stout/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:internal_api, "Short and stout.") # chef:silence_deprecation:generic
+        Chef.deprecated(:internal_api, "This is my handle.")
+      end
+
+      it "allows silencing all via inline comments with other stuff in the comment" do
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my handle/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:generic, "Short and stout.") # rubocop:something chef:silence_deprecation other stuff
+        Chef.deprecated(:internal_api, "This is my handle.")
+      end
+
+      it "handles multiple silence configurations at the same time" do
+        Chef::Config[:silence_deprecation_warnings] = ["exit_code", "chef_class_spec.rb:#{__LINE__ + 6}"]
+        expect(Chef::Log).to receive(:warn).with(/I'm a little teapot/).once
+        expect(Chef::Log).to receive(:warn).with(/This is my spout/).once
+        expect(Chef::Log).to receive(:warn).with(/Hear me shout/).once
+        Chef.deprecated(:generic, "I'm a little teapot.")
+        Chef.deprecated(:generic, "Short and stout.") # chef:silence_deprecation
+        Chef.deprecated(:internal_api, "This is my handle.")
+        Chef.deprecated(:internal_api, "This is my spout.")
+        Chef.deprecated(:exit_code, "When I get all steamed up.")
+        Chef.deprecated(:generic, "Hear me shout.")
+      end
+    end
+
+    context "with treat_deprecation_warnings_as_errors true" do
+      # This is already turned on globally for Chef's unit tests, but just for clarity do it here too.
+      before { Chef::Config[:treat_deprecation_warnings_as_errors] = true }
+
+      it "displays a simple deprecation error" do
+        expect(Chef::Log).to receive(:error).with(%r{I'm a little teapot\..*?spec/unit/chef_class_spec\.rb.*?Please see}m)
+        expect { Chef.deprecated(:generic, "I'm a little teapot.") }.to raise_error(/I'm a little teapot./)
+      end
+    end
+  end
 end