Merge pull request #1353 from puppetlabs/deprecate-validate-legacy

Deprecate the `validate_legacy()` function

Author: LukasAud

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