Skip to content

Commit f58989b

Browse files
Merge pull request #8905 from tvpartytonight/PUP-11518
(PUP-11518) Fix type checking with deferred functions
2 parents c0f6920 + 1034775 commit f58989b

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)