Skip to content

Commit 9c44901

Browse files
SteveL-MSFTSteveL-MSFT
committed
refactor and address clippy/copilot
1 parent bbec365 commit 9c44901

File tree

5 files changed

+137
-99
lines changed

5 files changed

+137
-99
lines changed

‎dsc/tests/dsc_user_functions.tests.ps1‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ resources:
7474
@{ expression = "[reference('foo/bar')]"; errorText = "The 'reference()' function is not available in user-defined functions" }
7575
@{ expression = "[utcNow()]"; errorText = "The 'utcNow()' function can only be used as a parameter default" }
7676
@{ expression = "[variables('myVar')]"; errorText = "The 'variables()' function is not available in user-defined functions" }
77-
@{ expression = "[MyFunction.OtherFunction()]"; errorText = "Unknown user function: MyFunction.OtherFunctio" }
78-
@{ expression = "[MyFunction.BadFunction()]"; errorText = "Unknown user function: MyFunction.BadFunction" }
77+
@{ expression = "[MyFunction.OtherFunction()]"; errorText = "Unknown user function 'MyFunction.OtherFunction'" }
78+
@{ expression = "[MyFunction.BadFunction()]"; errorText = "Unknown user function 'MyFunction.BadFunction'" }
7979
) {
8080
param($expression, $errorText)
8181

@@ -130,4 +130,25 @@ resources:
130130
$LASTEXITCODE | Should -Be 2 -Because (Get-Content $testdrive/error.log | Out-String)
131131
(Get-Content $testdrive/error.log -Raw) | Should -BeLike "*Parameter 'BadParam' not found in context*" -Because (Get-Content $testdrive/error.log | Out-String)
132132
}
133+
134+
It 'user function with wrong output type fails' {
135+
$configYaml = @"
136+
`$schema: https://aka.ms/dsc/schemas/v3/bundled/config/document.json
137+
functions:
138+
- namespace: MyFunction
139+
members:
140+
BadFunction:
141+
output:
142+
type: int
143+
value: "'this is a string'"
144+
resources:
145+
- name: test
146+
type: Microsoft.DSC.Debug/Echo
147+
properties:
148+
output: "[MyFunction.BadFunction()]"
149+
"@
150+
dsc -l trace config get -i $configYaml 2>$testdrive/error.log | Out-Null
151+
$LASTEXITCODE | Should -Be 2 -Because (Get-Content $testdrive/error.log | Out-String)
152+
(Get-Content $testdrive/error.log -Raw) | Should -BeLike "*Output of user function 'MyFunction.BadFunction' returned an integer, but was expected to be of type 'int'*" -Because (Get-Content $testdrive/error.log | Out-String)
153+
}
133154
}

‎dsc_lib/locales/en-us.toml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,16 @@ invalidArgType = "All arguments must either be arrays or objects"
479479
description = "Returns a deterministic unique string from the given strings"
480480
invoked = "uniqueString function"
481481

482+
[functions.userFunction]
483+
expectedNoParamters = "User function '%{name}' does not accept parameters"
484+
unknownUserFunction = "Unknown user function '%{name}'"
485+
wrongParamCount = "User function '%{name}' expects %{expected} parameters, but %{got} were provided"
486+
outputNotString = "Output of user function '%{name}' returned a string, but was expected to be of type '%{expected_type}'"
487+
outputNotInteger = "Output of user function '%{name}' returned an integer, but was expected to be of type '%{expected_type}'"
488+
outputNotBoolean = "Output of user function '%{name}' returned a boolean, but was expected to be of type '%{expected_type}'"
489+
outputNotArray = "Output of user function '%{name}' returned an array, but was expected to be of type '%{expected_type}'"
490+
outputNotObject = "Output of user function '%{name}' returned an object, but was expected to be of type '%{expected_type}'"
491+
482492
[functions.utcNow]
483493
description = "Returns the current UTC time"
484494
invoked = "utcNow function"

‎dsc_lib/src/configure/config_doc.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rust_i18n::t;
66
use schemars::{JsonSchema, json_schema};
77
use serde::{Deserialize, Serialize};
88
use serde_json::{Map, Value};
9-
use std::collections::HashMap;
9+
use std::{collections::HashMap, fmt::Display};
1010

1111
use crate::{dscerror::DscError, schemas::DscRepoSchema};
1212

@@ -188,6 +188,21 @@ pub enum DataType {
188188
Array,
189189
}
190190

191+
impl Display for DataType {
192+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
193+
let type_str = match self {
194+
DataType::String => "string",
195+
DataType::SecureString => "secureString",
196+
DataType::Int => "int",
197+
DataType::Bool => "bool",
198+
DataType::Object => "object",
199+
DataType::SecureObject => "secureObject",
200+
DataType::Array => "array",
201+
};
202+
write!(f, "{type_str}")
203+
}
204+
}
205+
191206
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)]
192207
pub enum CopyMode {
193208
#[serde(rename = "serial")]

‎dsc_lib/src/configure/mod.rs‎

Lines changed: 48 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ impl Configurator {
760760
} else {
761761
default_value.clone()
762762
};
763-
Configurator::validate_parameter_type(name, &value, &parameter.parameter_type)?;
763+
validate_parameter_type(name, &value, &parameter.parameter_type)?;
764764
self.context.parameters.insert(name.clone(), (value, parameter.parameter_type.clone()));
765765
}
766766
}
@@ -784,7 +784,7 @@ impl Configurator {
784784
// TODO: additional array constraints
785785
// TODO: object constraints
786786

787-
Configurator::validate_parameter_type(&name, &value, &constraint.parameter_type)?;
787+
validate_parameter_type(&name, &value, &constraint.parameter_type)?;
788788
if constraint.parameter_type == DataType::SecureString || constraint.parameter_type == DataType::SecureObject {
789789
info!("{}", t!("configure.mod.setSecureParameter", name = name));
790790
} else {
@@ -836,7 +836,7 @@ impl Configurator {
836836
return Err(DscError::Validation(t!("configure.mod.userFunctionAlreadyDefined", name = function_name, namespace = user_function.namespace).to_string()));
837837
}
838838
debug!("{}", t!("configure.mod.addingUserFunction", name = format!("{}.{}", user_function.namespace, function_name)));
839-
self.context.user_functions.insert(format!("{}.{}", user_function.namespace.to_lowercase(), function_name.to_lowercase()), function_definition.clone());
839+
self.context.user_functions.insert(format!("{}.{}", user_function.namespace, function_name), function_definition.clone());
840840
}
841841
}
842842
Ok(())
@@ -866,38 +866,6 @@ impl Configurator {
866866
}
867867
}
868868

869-
fn validate_parameter_type(name: &str, value: &Value, parameter_type: &DataType) -> Result<(), DscError> {
870-
match parameter_type {
871-
DataType::String | DataType::SecureString => {
872-
if !value.is_string() {
873-
return Err(DscError::Validation(t!("configure.mod.parameterNotString", name = name).to_string()));
874-
}
875-
},
876-
DataType::Int => {
877-
if !value.is_i64() {
878-
return Err(DscError::Validation(t!("configure.mod.parameterNotInteger", name = name).to_string()));
879-
}
880-
},
881-
DataType::Bool => {
882-
if !value.is_boolean() {
883-
return Err(DscError::Validation(t!("configure.mod.parameterNotBoolean", name = name).to_string()));
884-
}
885-
},
886-
DataType::Array => {
887-
if !value.is_array() {
888-
return Err(DscError::Validation(t!("configure.mod.parameterNotArray", name = name).to_string()));
889-
}
890-
},
891-
DataType::Object | DataType::SecureObject => {
892-
if !value.is_object() {
893-
return Err(DscError::Validation(t!("configure.mod.parameterNotObject", name = name).to_string()));
894-
}
895-
},
896-
}
897-
898-
Ok(())
899-
}
900-
901869
fn validate_config(&mut self) -> Result<(), DscError> {
902870
let mut config: Configuration = serde_json::from_str(self.json.as_str())?;
903871
check_security_context(config.metadata.as_ref())?;
@@ -1011,6 +979,51 @@ impl Configurator {
1011979
}
1012980
}
1013981

982+
/// Validate that a parameter value matches the expected type.
983+
///
984+
/// # Arguments
985+
/// * `name` - The name of the parameter.
986+
/// * `value` - The value of the parameter.
987+
/// * `parameter_type` - The expected type of the parameter.
988+
///
989+
/// # Returns
990+
/// * `Result<(), DscError>` - Ok if the value matches the expected type, Err otherwise.
991+
///
992+
/// # Errors
993+
/// This function will return an error if the value does not match the expected type.
994+
///
995+
pub fn validate_parameter_type(name: &str, value: &Value, parameter_type: &DataType) -> Result<(), DscError> {
996+
match parameter_type {
997+
DataType::String | DataType::SecureString => {
998+
if !value.is_string() {
999+
return Err(DscError::Validation(t!("configure.mod.parameterNotString", name = name).to_string()));
1000+
}
1001+
},
1002+
DataType::Int => {
1003+
if !value.is_i64() {
1004+
return Err(DscError::Validation(t!("configure.mod.parameterNotInteger", name = name).to_string()));
1005+
}
1006+
},
1007+
DataType::Bool => {
1008+
if !value.is_boolean() {
1009+
return Err(DscError::Validation(t!("configure.mod.parameterNotBoolean", name = name).to_string()));
1010+
}
1011+
},
1012+
DataType::Array => {
1013+
if !value.is_array() {
1014+
return Err(DscError::Validation(t!("configure.mod.parameterNotArray", name = name).to_string()));
1015+
}
1016+
},
1017+
DataType::Object | DataType::SecureObject => {
1018+
if !value.is_object() {
1019+
return Err(DscError::Validation(t!("configure.mod.parameterNotObject", name = name).to_string()));
1020+
}
1021+
},
1022+
}
1023+
1024+
Ok(())
1025+
}
1026+
10141027
fn get_failure_from_error(err: &DscError) -> Option<Failure> {
10151028
match err {
10161029
DscError::CommandExit(_resource, exit_code, reason) => {

‎dsc_lib/src/functions/user_function.rs‎

Lines changed: 40 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,28 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
use crate::configure::{config_doc::{DataType, UserFunctionDefinition}, context::ProcessMode};
4+
use crate::configure::{{config_doc::{DataType, UserFunctionDefinition}, context::ProcessMode}, validate_parameter_type};
55
use crate::dscerror::DscError;
66
use crate::functions::Context;
77
use crate::parser::Statement;
8+
use rust_i18n::t;
89
use serde_json::Value;
910

11+
/// Invoke a user-defined function by name with the provided arguments and context.
12+
///
13+
/// # Arguments
14+
/// * `name` - The name of the user-defined function to invoke.
15+
/// * `args` - The arguments to pass to the user-defined function.
16+
/// * `context` - The current execution context.
17+
///
18+
/// # Returns
19+
/// * `Result<Value, DscError>` - The result of the function invocation or an error.
20+
///
21+
/// # Errors
22+
/// * `DscError::Parser` - If the function is not found, parameters are invalid, or output type is incorrect.
23+
///
1024
pub fn invoke_user_function(name: &str, args: &[Value], context: &Context) -> Result<Value, DscError> {
11-
if let Some(function_definition) = context.user_functions.get(name.to_lowercase().as_str()) {
25+
if let Some(function_definition) = context.user_functions.get(name) {
1226
validate_parameters(name, function_definition, args)?;
1327
let mut user_context = context.clone();
1428
user_context.process_mode = ProcessMode::UserFunction;
@@ -18,7 +32,7 @@ pub fn invoke_user_function(name: &str, args: &[Value], context: &Context) -> Re
1832
user_context.user_functions.clear();
1933
for (i, arg) in args.iter().enumerate() {
2034
let Some(params) = &function_definition.parameters else {
21-
return Err(DscError::Parser(format!("Function '{}' does not accept any parameters, but {} were provided", name, args.len())));
35+
return Err(DscError::Parser(t!("functions.userFunction.expectedNoParamters", name = name).to_string()));
2236
};
2337
user_context.parameters.insert(params[i].name.clone(), (arg.clone(), params[i].r#type.clone()));
2438
}
@@ -27,85 +41,50 @@ pub fn invoke_user_function(name: &str, args: &[Value], context: &Context) -> Re
2741
validate_output_type(name, function_definition, &result)?;
2842
Ok(result)
2943
} else {
30-
Err(DscError::Parser(format!("Unknown user function: {}", name)))
44+
Err(DscError::Parser(t!("functions.userFunction.unknownUserFunction", name = name).to_string()))
3145
}
3246
}
3347

3448
fn validate_parameters(name: &str, function_definition: &UserFunctionDefinition, args: &[Value]) -> Result<(), DscError> {
3549
if let Some(expected_params) = &function_definition.parameters {
3650
if args.len() != expected_params.len() {
37-
return Err(DscError::Parser(format!("Function '{}' expects {} parameters, but {} were provided", name, expected_params.len(), args.len())));
51+
return Err(DscError::Parser(t!("functions.userFunction.wrongParamCount", name = name, expected = expected_params.len(), got = args.len()).to_string()));
3852
}
39-
for (i, (arg, expected_param)) in args.iter().zip(expected_params).enumerate() {
40-
match expected_param.r#type {
41-
DataType::String => {
42-
if !arg.is_string() {
43-
return Err(DscError::Parser(format!("Parameter {} of function '{}' expects a string", i + 1, name)));
44-
}
45-
}
46-
DataType::Int => {
47-
if !arg.is_number() {
48-
return Err(DscError::Parser(format!("Parameter {} of function '{}' expects a number", i + 1, name)));
49-
}
50-
}
51-
DataType::Bool => {
52-
if !arg.is_boolean() {
53-
return Err(DscError::Parser(format!("Parameter {} of function '{}' expects a boolean", i + 1, name)));
54-
}
55-
}
56-
DataType::Array => {
57-
if !arg.is_array() {
58-
return Err(DscError::Parser(format!("Parameter {} of function '{}' expects an array", i + 1, name)));
59-
}
60-
}
61-
DataType::Object => {
62-
if !arg.is_object() {
63-
return Err(DscError::Parser(format!("Parameter {} of function '{}' expects an object", i + 1, name)));
64-
}
65-
}
66-
_ => {
67-
return Err(DscError::Parser(format!("Function '{}' has an unsupported parameter type: {:?}", name, expected_param.r#type)));
68-
}
69-
}
70-
}
71-
} else {
72-
if !args.is_empty() {
73-
return Err(DscError::Parser(format!("Function '{}' does not accept any parameters, but {} were provided", name, args.len())));
53+
for (arg, expected_param) in args.iter().zip(expected_params) {
54+
validate_parameter_type(name, arg, &expected_param.r#type)?;
7455
}
7556
}
7657
Ok(())
7758
}
7859

79-
fn validate_output_type(name: &str, function_definition: &UserFunctionDefinition, result: &Value) -> Result<(), DscError> {
60+
fn validate_output_type(name: &str, function_definition: &UserFunctionDefinition, output: &Value) -> Result<(), DscError> {
8061
match function_definition.output.r#type {
81-
DataType::String => {
82-
if !result.is_string() {
83-
return Err(DscError::Parser(format!("Function '{}' is expected to return a string", name)));
62+
DataType::String | DataType::SecureString => {
63+
if !output.is_string() {
64+
return Err(DscError::Validation(t!("functions.userFunction.outputNotString", name = name, expected_type = function_definition.output.r#type.to_string()).to_string()));
8465
}
85-
}
66+
},
8667
DataType::Int => {
87-
if !result.is_number() {
88-
return Err(DscError::Parser(format!("Function '{}' is expected to return a number", name)));
68+
if !output.is_i64() {
69+
return Err(DscError::Validation(t!("functions.userFunction.outputNotInteger", name = name, expected_type = function_definition.output.r#type.to_string()).to_string()));
8970
}
90-
}
71+
},
9172
DataType::Bool => {
92-
if !result.is_boolean() {
93-
return Err(DscError::Parser(format!("Function '{}' is expected to return a boolean", name)));
73+
if !output.is_boolean() {
74+
return Err(DscError::Validation(t!("functions.userFunction.outputNotBoolean", name = name, expected_type = function_definition.output.r#type.to_string()).to_string()));
9475
}
95-
}
76+
},
9677
DataType::Array => {
97-
if !result.is_array() {
98-
return Err(DscError::Parser(format!("Function '{}' is expected to return an array", name)));
78+
if !output.is_array() {
79+
return Err(DscError::Validation(t!("functions.userFunction.outputNotArray", name = name, expected_type = function_definition.output.r#type.to_string()).to_string()));
9980
}
100-
}
101-
DataType::Object => {
102-
if !result.is_object() {
103-
return Err(DscError::Parser(format!("Function '{}' is expected to return an object", name)));
81+
},
82+
DataType::Object | DataType::SecureObject => {
83+
if !output.is_object() {
84+
return Err(DscError::Validation(t!("functions.userFunction.outputNotObject", name = name, expected_type = function_definition.output.r#type.to_string()).to_string()));
10485
}
105-
}
106-
_ => {
107-
return Err(DscError::Parser(format!("Function '{}' has an unsupported output type: {:?}", name, function_definition.output.r#type)));
108-
}
86+
},
10987
}
88+
11089
Ok(())
11190
}

0 commit comments

Comments
 (0)