Skip to content

Commit 1034775

Browse files
(PUP-11518) Fix type checking with deferred functions
Using a deferred function for a typed parameter value of a class results in a compilation error stating that the type defined for the parameter does not match the “Deferred” type at compilation time. This error prevents the use of Deferred functions for the values of any typed class parameters. This change updates the type check to introspect Deferred functions for the return_value type of that Deferred function.
1 parent c0f6920 commit 1034775

File tree

3 files changed

+199
-8
lines changed

3 files changed

+199
-8
lines changed

lib/puppet/pops/functions/dispatcher.rb

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ def empty?
1919
@dispatchers.empty?
2020
end
2121

22+
def find_matching_dispatcher(args, &block)
23+
@dispatchers.find { |d| d.type.callable_with?(args, block) }
24+
end
25+
2226
# Dispatches the call to the first found signature (entry with matching type).
2327
#
2428
# @param instance [Puppet::Functions::Function] - the function to call
@@ -28,19 +32,19 @@ def empty?
2832
#
2933
# @api private
3034
def dispatch(instance, calling_scope, args, &block)
31-
found = @dispatchers.find { |d| d.type.callable_with?(args, block) }
32-
unless found
35+
36+
dispatcher = find_matching_dispatcher(args, &block)
37+
unless dispatcher
3338
args_type = Puppet::Pops::Types::TypeCalculator.singleton.infer_set(block_given? ? args + [block] : args)
3439
raise ArgumentError, Puppet::Pops::Types::TypeMismatchDescriber.describe_signatures(instance.class.name, signatures, args_type)
3540
end
36-
37-
if found.argument_mismatch_handler?
38-
msg = found.invoke(instance, calling_scope, args)
41+
if dispatcher.argument_mismatch_handler?
42+
msg = dispatcher.invoke(instance, calling_scope, args)
3943
raise ArgumentError, "'#{instance.class.name}' #{msg}"
4044
end
4145

4246
catch(:next) do
43-
found.invoke(instance, calling_scope, args, &block)
47+
dispatcher.invoke(instance, calling_scope, args, &block)
4448
end
4549
end
4650

lib/puppet/pops/types/type_mismatch_describer.rb

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,15 @@ def validate_default_parameter(subject, param_name, param_type, value, tense = :
581581
end
582582
end
583583

584+
def get_deferred_function_return_type(value)
585+
func = Puppet.lookup(:loaders).private_environment_loader.
586+
load(:function, value.name)
587+
dispatcher = func.class.dispatcher.find_matching_dispatcher(value.arguments)
588+
raise ArgumentError, "No matching arity found for #{func.class.name} with arguments #{value.arguments}" unless dispatcher
589+
dispatcher.type.return_type
590+
end
591+
private :get_deferred_function_return_type
592+
584593
# Validates that all entries in the _param_hash_ exists in the given param_struct, that their type conforms
585594
# with the corresponding param_struct element and that all required values are provided.
586595
# An error message is created for each problem found.
@@ -598,7 +607,19 @@ def describe_struct_signature(params_struct, param_hash, missing_ok = false)
598607
value = param_hash[name]
599608
value_type = elem.value_type
600609
if param_hash.include?(name)
601-
result << describe(value_type, TypeCalculator.singleton.infer_set(value), [ParameterPathElement.new(name)]) unless value_type.instance?(value)
610+
if Puppet::Pops::Types::TypeFactory.deferred.implementation_class == value.class
611+
if (df_return_type = get_deferred_function_return_type(value))
612+
result << describe(value_type, df_return_type, [ParameterPathElement.new(name)]) unless value_type.generalize.assignable?(df_return_type.generalize)
613+
else
614+
warning_text = _("Deferred function %{function_name} has no return_type, unable to guarantee value type during compilation.") %
615+
{function_name: value.name }
616+
Puppet.warn_once('deprecations',
617+
"#{value.name}_deferred_warning",
618+
warning_text)
619+
end
620+
else
621+
result << describe(value_type, TypeCalculator.singleton.infer_set(value), [ParameterPathElement.new(name)]) unless value_type.instance?(value)
622+
end
602623
else
603624
result << MissingParameter.new(nil, name) unless missing_ok || elem.key_type.is_a?(POptionalType)
604625
end

spec/unit/pops/types/type_mismatch_describer_spec.rb

Lines changed: 167 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,178 @@
11
require 'spec_helper'
22
require 'puppet/pops'
33
require 'puppet_spec/compiler'
4+
require 'puppet_spec/files'
5+
require 'puppet/loaders'
46

57
module Puppet::Pops
68
module Types
79

810
describe 'the type mismatch describer' do
9-
include PuppetSpec::Compiler
11+
include PuppetSpec::Compiler, PuppetSpec::Files
12+
13+
context 'with deferred functions' do
14+
let(:env_name) { 'spec' }
15+
let(:code_dir) { Puppet[:environmentpath] }
16+
let(:env_dir) { File.join(code_dir, env_name) }
17+
let(:env) { Puppet::Node::Environment.create(env_name.to_sym, [File.join(populated_code_dir, env_name, 'modules')]) }
18+
let(:node) { Puppet::Node.new('fooname', environment: env) }
19+
let(:populated_code_dir) do
20+
dir_contained_in(code_dir, env_name => env_content)
21+
PuppetSpec::Files.record_tmp(env_dir)
22+
code_dir
23+
end
24+
25+
let(:env_content) {
26+
{
27+
'lib' => {
28+
'puppet' => {
29+
'functions' => {
30+
'string_return.rb' => <<-RUBY.unindent,
31+
Puppet::Functions.create_function(:string_return) do
32+
dispatch :string_return do
33+
param 'String', :arg1
34+
return_type 'String'
35+
end
36+
def string_return(arg1)
37+
arg1
38+
end
39+
end
40+
RUBY
41+
'variant_return.rb' => <<-RUBY.unindent,
42+
Puppet::Functions.create_function(:variant_return) do
43+
dispatch :variant_return do
44+
param 'String', :arg1
45+
return_type 'Variant[Integer,Float]'
46+
end
47+
def variant_return(arg1)
48+
arg1
49+
end
50+
end
51+
RUBY
52+
'no_return.rb' => <<-RUBY.unindent,
53+
Puppet::Functions.create_function(:no_return) do
54+
dispatch :no_return do
55+
param 'String', :arg1
56+
end
57+
def variant_return(arg1)
58+
arg1
59+
end
60+
end
61+
RUBY
62+
}
63+
}
64+
}
65+
}
66+
}
67+
68+
before(:each) do
69+
Puppet.push_context(:loaders => Puppet::Pops::Loaders.new(env))
70+
end
71+
72+
after(:each) do
73+
Puppet.pop_context
74+
end
75+
76+
it 'will compile when the parameter type matches the function return_type' do
77+
code = <<-CODE
78+
$d = Deferred("string_return", ['/a/non/existing/path'])
79+
class testclass(String $classparam) {
80+
}
81+
class { 'testclass':
82+
classparam => $d
83+
}
84+
CODE
85+
expect { eval_and_collect_notices(code, node) }.to_not raise_error
86+
end
87+
88+
it "will compile when a Variant parameter's types matches the return type" do
89+
code = <<-CODE
90+
$d = Deferred("string_return", ['/a/non/existing/path'])
91+
class testclass(Variant[String, Float] $classparam) {
92+
}
93+
class { 'testclass':
94+
classparam => $d
95+
}
96+
CODE
97+
expect { eval_and_collect_notices(code, node) }.to_not raise_error
98+
end
99+
100+
it "will compile with a union of a Variant parameters' types and Variant return types" do
101+
code = <<-CODE
102+
$d = Deferred("variant_return", ['/a/non/existing/path'])
103+
class testclass(Variant[Any,Float] $classparam) {
104+
}
105+
class { 'testclass':
106+
classparam => $d
107+
}
108+
CODE
109+
expect { eval_and_collect_notices(code, node) }.to_not raise_error
110+
end
111+
112+
it 'will warn when there is no defined return_type for the function definition' do
113+
code = <<-CODE
114+
$d = Deferred("no_return", ['/a/non/existing/path'])
115+
class testclass(Variant[String,Boolean] $classparam) {
116+
}
117+
class { 'testclass':
118+
classparam => $d
119+
}
120+
CODE
121+
expect(Puppet).to receive(:warn_once).with(anything, anything, /.+function no_return has no return_type/).at_least(:once)
122+
expect { eval_and_collect_notices(code, node) }.to_not raise_error
123+
end
124+
125+
it 'will report a mismatch between a deferred function return type and class parameter value' do
126+
code = <<-CODE
127+
$d = Deferred("string_return", ['/a/non/existing/path'])
128+
class testclass(Integer $classparam) {
129+
}
130+
class { 'testclass':
131+
classparam => $d
132+
}
133+
CODE
134+
expect { eval_and_collect_notices(code, node) }.to raise_error(Puppet::Error, /.+'classparam' expects an Integer value, got String/)
135+
end
136+
137+
it 'will report an argument error when no matching arity is found' do
138+
code = <<-CODE
139+
$d = Deferred("string_return", ['/a/non/existing/path', 'second-invalid-arg'])
140+
class testclass(Integer $classparam) {
141+
}
142+
class { 'testclass':
143+
classparam => $d
144+
}
145+
CODE
146+
expect { eval_and_collect_notices(code,node) }.to raise_error(Puppet::Error, /.+ No matching arity found for string_return/)
147+
end
148+
149+
it 'will error with no matching Variant class parameters and return_type' do
150+
code = <<-CODE
151+
$d = Deferred("string_return", ['/a/non/existing/path'])
152+
class testclass(Variant[Integer,Float] $classparam) {
153+
}
154+
class { 'testclass':
155+
classparam => $d
156+
}
157+
CODE
158+
expect { eval_and_collect_notices(code,node) }.to raise_error(Puppet::Error, /.+'classparam' expects a value of type Integer or Float, got String/)
159+
end
160+
161+
# This test exposes a shortcoming in the #message function for Puppet::Pops::Type::TypeMismatch
162+
# where the `actual` is not introspected for the list of Variant types, so the error message
163+
# shows that the list of expected types does not match Variant, instead of a list of actual types.
164+
it 'will error with no matching Variant class parameters and Variant return_type' do
165+
code = <<-CODE
166+
$d = Deferred("variant_return", ['/a/non/existing/path'])
167+
class testclass(Variant[String,Boolean] $classparam) {
168+
}
169+
class { 'testclass':
170+
classparam => $d
171+
}
172+
CODE
173+
expect { eval_and_collect_notices(code, node) }.to raise_error(Puppet::Error, /.+'classparam' expects a value of type String or Boolean, got Variant/)
174+
end
175+
end
10176

11177
it 'will report a mismatch between a hash and a struct with details' do
12178
code = <<-CODE

0 commit comments

Comments
 (0)