Skip to content

Commit

Permalink
Add implicit nullability to optional module params (#11887)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeskew committed Sep 19, 2023
1 parent 668b5e2 commit 7524201
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 8 deletions.
35 changes: 31 additions & 4 deletions src/Bicep.Core.IntegrationTests/ScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4881,11 +4881,11 @@ public void Test_Issue11437()
{
var result = CompilationHelper.CompileParams(
("parameters.bicepparam", """
using 'main.bicep'
using 'main.bicep'

param foo = 'asdf'
param foo = 'asdf'
"""),
param foo = 'asdf'
param foo = 'asdf'
"""),
("main.bicep", """param foo string"""));

result.Should().HaveDiagnostics(new[]
Expand Down Expand Up @@ -5076,4 +5076,31 @@ public void Test_Issue11742()
// ("BCP333", DiagnosticLevel.Warning, "The provided value (whose length will always be less than or equal to 8) is too short to assign to a target for which the minimum allowable length is 36."),
// });
}

// https://github.com/Azure/bicep/issues/11883
[TestMethod]
public void Test_Issue11883()
{
var result = CompilationHelper.Compile(
("main.bicep", """
param nullable string?
param withNestedNullable {
property: string?
}

module mod 'mod.bicep' = {
name: 'mod'
params: {
withDefault: nullable
nullable: withNestedNullable.?property
}
}
"""),
("mod.bicep", """
param withDefault string = 'default'
param nullable string?
"""));

result.Should().NotHaveAnyDiagnostics();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"detail": "stringParamA",
"documentation": {
"kind": "markdown",
"value": "Type: `string` \nWrite-only property \n"
"value": "Type: `null | string` \nWrite-only property \n"
},
"deprecated": false,
"preselect": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"detail": "stringParamA",
"documentation": {
"kind": "markdown",
"value": "Type: `string` \nWrite-only property \n"
"value": "Type: `null | string` \nWrite-only property \n"
},
"deprecated": false,
"preselect": false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ module secureModule1 'moduleb.bicep' = {
//@[023:052) [simplify-interpolation (Warning)] Remove unnecessary string interpolation. (CodeDescription: bicep core(https://aka.ms/bicep/linter/simplify-interpolation)) |'${kv.getSecret('mySecret')}'|
//@[026:050) [BCP180 (Error)] Function "getSecret" is not valid at this location. It can only be used when directly assigning to a module parameter with a secure decorator. (CodeDescription: none) |kv.getSecret('mySecret')|
secureObjectParam: kv.getSecret('mySecret')
//@[023:047) [BCP036 (Error)] The property "secureObjectParam" expected a value of type "object" but the provided value is of type "string". (CodeDescription: none) |kv.getSecret('mySecret')|
//@[023:047) [BCP036 (Error)] The property "secureObjectParam" expected a value of type "null | object" but the provided value is of type "string". (CodeDescription: none) |kv.getSecret('mySecret')|
secureStringParam2: '${kv.getSecret('mySecret')}'
//@[004:022) [BCP037 (Error)] The property "secureStringParam2" is not allowed on objects of type "params". No other properties are allowed. (CodeDescription: none) |secureStringParam2|
//@[024:053) [simplify-interpolation (Warning)] Remove unnecessary string interpolation. (CodeDescription: bicep core(https://aka.ms/bicep/linter/simplify-interpolation)) |'${kv.getSecret('mySecret')}'|
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/FunctionPlacementValidatorVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private void VerifyModuleSecureParameterFunctionPlacement(FunctionCallSyntaxBase
var (_, levelUpSymbol) = syntaxRecorder.Skip(1).SkipWhile(x => x.syntax is TernaryOperationSyntax).FirstOrDefault();
if (!(elementsRecorder.TryPeek(out var head) && head == VisitedElement.ModuleParams)
|| levelUpSymbol is not PropertySymbol propertySymbol
|| !propertySymbol.Type.ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure))
|| !(TypeHelper.TryRemoveNullability(propertySymbol.Type) ?? propertySymbol.Type).ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure))
{
diagnosticWriter.Write(DiagnosticBuilder.ForPosition(syntax).FunctionOnlyValidInModuleSecureParameterAssignment(functionSymbol.Name));
}
Expand Down
7 changes: 7 additions & 0 deletions src/Bicep.Core/TypeSystem/DeclaredTypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,13 @@ private TypeSymbol GetDeclaredModuleType(ModuleDeclarationSyntax module)
}

var flags = parameter.IsRequired ? TypePropertyFlags.Required | TypePropertyFlags.WriteOnly : TypePropertyFlags.WriteOnly;

// add implicit nullability for optional parameters
if (!parameter.IsRequired)
{
type = TypeHelper.CreateTypeUnion(type, LanguageConstants.Null);
}

parameters.Add(new TypeProperty(parameter.Name, type, flags, parameter.Description));
}

Expand Down

0 comments on commit 7524201

Please sign in to comment.