Deprecate the validate_legacy() function

`validate_legacy()` was added to help migrating from the legacy
validation functions to the new Puppet data types which where not 100%
backward compatible with the previous validation (e.g. `'42'` was a
valid integer according to `validate_integer()` but indeed a String and
not an Integer when using data types.

The legacy validation functions have been removed from stdlib, so
`validate_legacy()` will now fail if it tries to run them.  But as
they produced deprecation warning, they are supposed to have already
been fixed.  For the sake of security, instead of removing
`validate_legacy()` now, deprecate it so that it is strictly equivalent
to validating using a data type, but also emits a warning.

Author: smortex

lib/puppet/functions/validate_legacy.rb

@@ -1,13 +1,14 @@
 # frozen_string_literal: true
 
 # @summary
-#   Validate a value against both the target_type (new) and the previous_validation function (old).
+#   **Deprecated:** Validate a value against both the target_type (new).
 Puppet::Functions.create_function(:validate_legacy) do
-  # The function checks a value against both the target_type (new) and the previous_validation function (old).
+  # The function checks a value against both the target_type (new).
   # @param scope
   #   The main value that will be passed to the method
   # @param target_type
   # @param function_name
+  #   Unused
   # @param value
   # @param args
   #   Any additional values that are to be passed to the method
@@ -25,6 +26,7 @@
   #   The main value that will be passed to the method
   # @param type_string
   # @param function_name
+  #   Unused
   # @param value
   # @param args Any additional values that are to be passed to the method
   # @return Legacy validation method
@@ -49,33 +51,17 @@ def validate_legacy_s(scope, type_string, *args)
     validate_legacy(scope, t, *args)
   end
 
-  def validate_legacy(scope, target_type, function_name, value, *prev_args)
+  def validate_legacy(_scope, target_type, _function_name, value, *_prev_args)
+    call_function('deprecation', 'validate_legacy', 'This method is deprecated, please use Puppet data types to validate parameters')
     if assert_type(target_type, value)
-      if previous_validation(scope, function_name, value, *prev_args)
-        # Silently passes
-      else
-        Puppet.notice("Accepting previously invalid value for target type '#{target_type}'")
-      end
+      # "Silently" passes
     else
       inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
-      error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("validate_legacy(#{function_name})", target_type, inferred_type)
-      if previous_validation(scope, function_name, value, *prev_args)
-        call_function('deprecation', 'validate_legacy', error_msg)
-      else
-        call_function('fail', error_msg)
-      end
+      error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("validate_legacy(#{target_type}, ...)", target_type, inferred_type)
+      call_function('fail', error_msg)
     end
   end
 
-  def previous_validation(scope, function_name, value, *prev_args)
-    # Call the previous validation function and catch any errors. Return true if no errors are thrown.
-
-    scope.send("function_#{function_name}".to_s, [value, *prev_args])
-    true
-  rescue Puppet::ParseError
-    false
-  end
-
   def assert_type(type, value)
     Puppet::Pops::Types::TypeCalculator.instance?(type, value)
   end

spec/functions/validate_legacy_spec.rb

@@ -7,49 +7,37 @@
     it { is_expected.not_to eq(nil) }
     it { is_expected.to run.with_params.and_raise_error(ArgumentError) }
 
-    describe 'when passing the type assertion and passing the previous validation' do
-      it 'passes without notice' do
-        expect(scope).to receive(:function_validate_foo).with([5]).once
+    describe 'when passing the type assertion' do
+      it 'passes with a deprecation warning' do
+        expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once
+        expect(scope).to receive(:function_validate_foo).never
         expect(Puppet).to receive(:notice).never
         is_expected.to run.with_params('Integer', 'validate_foo', 5)
       end
     end
 
-    describe 'when passing the type assertion and failing the previous validation' do
-      it 'passes with a notice about newly accepted value' do
-        expect(scope).to receive(:function_validate_foo).with([5]).and_raise(Puppet::ParseError, 'foo').once
-        expect(Puppet).to receive(:notice).with(include('Accepting previously invalid value for target type'))
-        is_expected.to run.with_params('Integer', 'validate_foo', 5)
-      end
-    end
-
-    describe 'when failing the type assertion and passing the previous validation' do
-      it 'passes with a deprecation message' do
-        expect(scope).to receive(:function_validate_foo).with(['5']).once
-        expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('Integer')).once
-        is_expected.to run.with_params('Integer', 'validate_foo', '5')
-      end
-    end
-
-    describe 'when failing the type assertion and failing the previous validation' do
+    describe 'when failing the type assertion' do
       it 'fails with a helpful message' do
-        expect(scope).to receive(:function_validate_foo).with(['5']).and_raise(Puppet::ParseError, 'foo').once
-        expect(subject.func).to receive(:call_function).with('fail', include('Integer')).once
+        expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once
+        expect(scope).to receive(:function_validate_foo).never
+        expect(subject.func).to receive(:call_function).with('fail', 'validate_legacy(Integer, ...) expects an Integer value, got String').once
         is_expected.to run.with_params('Integer', 'validate_foo', '5')
       end
     end
 
     describe 'when passing in undef' do
       it 'works' do
-        expect(scope).to receive(:function_validate_foo).with([:undef]).once
+        expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once
+        expect(scope).to receive(:function_validate_foo).never
         expect(Puppet).to receive(:notice).never
         is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef)
       end
     end
 
     describe 'when passing in multiple arguments' do
       it 'passes with a deprecation message' do
-        expect(scope).to receive(:function_validate_foo).with([:undef, 1, 'foo']).once
+        expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once
+        expect(scope).to receive(:function_validate_foo).never
         expect(Puppet).to receive(:notice).never
         is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef, 1, 'foo')
       end