From 5d0367147ed7cb15c98b7fd6266707c0b544dc1b Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Tue, 20 May 2025 10:44:15 -0700 Subject: [PATCH 1/6] fix and add test for invalid parameter type --- .../BuildParamsCommandTests.cs | 26 +++++++++++++++++++ .../Emit/EmitLimitationCalculator.cs | 10 +++++++ 2 files changed, 36 insertions(+) diff --git a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs index 66b6fc1be1d..c3f6530338f 100644 --- a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs @@ -640,5 +640,31 @@ param intParam int File.Exists(Path.Combine(bicepOutputPath, outputFile)).Should().Be(true, f); } } + + [TestMethod] + public async Task BuildParams_Extends_InvalidType_ThrowsError() + { + var outputPath = FileHelper.GetUniqueTestOutputPath(TestContext); + FileHelper.SaveResultFile(TestContext, "main.bicep", "param tag string", outputPath); + FileHelper.SaveResultFile(TestContext, "base.bicepparam", @" + using none + param tag = false + ", outputPath); + var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicepparam", @" + using 'main.bicep' + extends 'base.bicepparam' + ", outputPath); + + var expectedOutputFile = FileHelper.GetResultFilePath(TestContext, "main.json", outputPath); + File.Exists(expectedOutputFile).Should().BeFalse(); + + var (output, error, result) = await Bicep(["build-params", inputFile]); + + File.Exists(expectedOutputFile).Should().BeFalse(); + + output.Should().BeEmpty(); + error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"false\"."); + result.Should().Be(1); + } } } diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index 43bf65a5485..a8564c4c304 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -11,6 +11,7 @@ using Bicep.Core.Navigation; using Bicep.Core.Semantics; using Bicep.Core.Semantics.Metadata; +using Bicep.Core.SourceGraph; using Bicep.Core.Syntax; using Bicep.Core.Syntax.Visitors; using Bicep.Core.Text; @@ -664,6 +665,15 @@ private static ImmutableDictionary sourceFile is BicepFile); + var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); + + if (paramDefinitionFromBicepFile.Value.TypeReference.Type != symbol.Type) + { + diagnostics.WriteMultiple(DiagnosticBuilder.ForPosition(symbol.NameSource).ParameterTypeMismatch(symbol.Name, paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)); + referencedValueHasError = true; } if (referencedValueHasError) From 41644827a90767a72f9fdb5cae41577f9e3dadb5 Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Tue, 20 May 2025 11:52:31 -0700 Subject: [PATCH 2/6] adding test for multiple invalid parameter types --- .../BuildParamsCommandTests.cs | 40 ++++++++++++++++++- .../Emit/EmitLimitationCalculator.cs | 2 +- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs index c3f6530338f..9975cb9dcc6 100644 --- a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs @@ -645,7 +645,9 @@ param intParam int public async Task BuildParams_Extends_InvalidType_ThrowsError() { var outputPath = FileHelper.GetUniqueTestOutputPath(TestContext); - FileHelper.SaveResultFile(TestContext, "main.bicep", "param tag string", outputPath); + FileHelper.SaveResultFile(TestContext, "main.bicep", @" + param tag string + ", outputPath); FileHelper.SaveResultFile(TestContext, "base.bicepparam", @" using none param tag = false @@ -663,7 +665,41 @@ public async Task BuildParams_Extends_InvalidType_ThrowsError() File.Exists(expectedOutputFile).Should().BeFalse(); output.Should().BeEmpty(); - error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"false\"."); + error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"bool\"."); + result.Should().Be(1); + } + + [TestMethod] + public async Task BuildParams_Extends_Multiple_InvalidType_ThrowsMultipleErrors() + { + var outputPath = FileHelper.GetUniqueTestOutputPath(TestContext); + FileHelper.SaveResultFile(TestContext, "main.bicep", @" + param myString string + param myInt int + param myBool bool + ", outputPath); + FileHelper.SaveResultFile(TestContext, "base.bicepparam", @" + using none + param myInt = '42' + param myString = {} + param myBool = 'true' + ", outputPath); + var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicepparam", @" + using './main.bicep' + extends 'base.bicepparam' + ", outputPath); + + var expectedOutputFile = FileHelper.GetResultFilePath(TestContext, "main.json", outputPath); + File.Exists(expectedOutputFile).Should().BeFalse(); + + var (output, error, result) = await Bicep(["build-params", inputFile]); + + File.Exists(expectedOutputFile).Should().BeFalse(); + + output.Should().BeEmpty(); + error.Should().Contain("Error BCP260: The parameter \"myInt\" expects a value of type \"int\" but the provided value is of type \"string\"."); + error.Should().Contain("Error BCP260: The parameter \"myString\" expects a value of type \"string\" but the provided value is of type \"'object'\"."); + error.Should().Contain("Error BCP260: The parameter \"myBool\" expects a value of type \"bool\" but the provided value is of type \"string\"."); result.Should().Be(1); } } diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index a8564c4c304..4280417423c 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -665,8 +665,8 @@ private static ImmutableDictionary sourceFile is BicepFile); var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); From bdbdcd1c75d30a82ff38414693821b5b1ccad007 Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Tue, 20 May 2025 17:55:58 -0700 Subject: [PATCH 3/6] fix parameter type mismatch diagnostics --- .../BuildParamsCommandTests.cs | 10 +++++----- .../Emit/EmitLimitationCalculator.cs | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs index 9975cb9dcc6..356071eb0ba 100644 --- a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs @@ -650,7 +650,7 @@ param tag string ", outputPath); FileHelper.SaveResultFile(TestContext, "base.bicepparam", @" using none - param tag = false + param tag = 42 ", outputPath); var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicepparam", @" using 'main.bicep' @@ -665,7 +665,7 @@ param tag string File.Exists(expectedOutputFile).Should().BeFalse(); output.Should().BeEmpty(); - error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"bool\"."); + error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"int\"."); result.Should().Be(1); } @@ -682,7 +682,7 @@ param myBool bool using none param myInt = '42' param myString = {} - param myBool = 'true' + param myBool = 42 ", outputPath); var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicepparam", @" using './main.bicep' @@ -698,8 +698,8 @@ param myBool bool output.Should().BeEmpty(); error.Should().Contain("Error BCP260: The parameter \"myInt\" expects a value of type \"int\" but the provided value is of type \"string\"."); - error.Should().Contain("Error BCP260: The parameter \"myString\" expects a value of type \"string\" but the provided value is of type \"'object'\"."); - error.Should().Contain("Error BCP260: The parameter \"myBool\" expects a value of type \"bool\" but the provided value is of type \"string\"."); + error.Should().Contain("Error BCP260: The parameter \"myString\" expects a value of type \"string\" but the provided value is of type \"object\"."); + error.Should().Contain("Error BCP260: The parameter \"myBool\" expects a value of type \"bool\" but the provided value is of type \"int\"."); result.Should().Be(1); } } diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index 4280417423c..f92bd510bcb 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -670,9 +670,24 @@ private static ImmutableDictionary sourceFile is BicepFile); var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); - if (paramDefinitionFromBicepFile.Value.TypeReference.Type != symbol.Type) + if (!TypeValidator.AreTypesAssignable(paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)) { - diagnostics.WriteMultiple(DiagnosticBuilder.ForPosition(symbol.NameSource).ParameterTypeMismatch(symbol.Name, paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)); + var symbolType = symbol.Type switch + { + IntegerType or IntegerLiteralType => TypeFactory.CreateIntegerType(), + StringType => TypeFactory.CreateStringType(), + ObjectType => new ObjectType(LanguageConstants.ObjectType, TypeSymbolValidationFlags.Default, [], new TypeProperty(LanguageConstants.Any)), + ArrayType => TypeFactory.CreateArrayType(), + _ => symbol.Type.TypeKind switch + { + TypeKind.StringLiteral => TypeFactory.CreateStringType(), + TypeKind.BooleanLiteral => TypeFactory.CreateBooleanType(), + TypeKind.IntegerLiteral => TypeFactory.CreateIntegerType(), + _ => symbol.Type + }, + }; + + diagnostics.WriteMultiple(DiagnosticBuilder.ForPosition(symbol.NameSource).ParameterTypeMismatch(symbol.Name, paramDefinitionFromBicepFile.Value.TypeReference.Type, symbolType)); referencedValueHasError = true; } From f9f69c2d40a26ebc1ad2f62c1c22d5a7c792e4e7 Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Wed, 21 May 2025 08:39:30 -0700 Subject: [PATCH 4/6] refactor error messages for parameter type mismatches --- .../BuildParamsCommandTests.cs | 10 ++++----- .../ScenarioTests.cs | 6 +++--- .../Emit/EmitLimitationCalculator.cs | 21 +------------------ 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs index 356071eb0ba..07fec39c690 100644 --- a/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs +++ b/src/Bicep.Cli.IntegrationTests/BuildParamsCommandTests.cs @@ -665,7 +665,7 @@ param tag string File.Exists(expectedOutputFile).Should().BeFalse(); output.Should().BeEmpty(); - error.Should().Contain("Error BCP260: The parameter \"tag\" expects a value of type \"string\" but the provided value is of type \"int\"."); + error.Should().Contain("Error BCP033: Expected a value of type \"string\" but the provided value is of type \"42\"."); result.Should().Be(1); } @@ -682,7 +682,7 @@ param myBool bool using none param myInt = '42' param myString = {} - param myBool = 42 + param myBool = [] ", outputPath); var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicepparam", @" using './main.bicep' @@ -697,9 +697,9 @@ param myBool bool File.Exists(expectedOutputFile).Should().BeFalse(); output.Should().BeEmpty(); - error.Should().Contain("Error BCP260: The parameter \"myInt\" expects a value of type \"int\" but the provided value is of type \"string\"."); - error.Should().Contain("Error BCP260: The parameter \"myString\" expects a value of type \"string\" but the provided value is of type \"object\"."); - error.Should().Contain("Error BCP260: The parameter \"myBool\" expects a value of type \"bool\" but the provided value is of type \"int\"."); + error.Should().Contain("Error BCP033: Expected a value of type \"int\" but the provided value is of type \"'42'\"."); + error.Should().Contain("Error BCP033: Expected a value of type \"string\" but the provided value is of type \"object\"."); + error.Should().Contain("Error BCP033: Expected a value of type \"bool\" but the provided value is of type \"\"."); result.Should().Be(1); } } diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs index 5cc339ad11e..9ef28474f55 100644 --- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs +++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs @@ -7014,7 +7014,7 @@ public void DependsOn_order_is_deterministic_for_hierarchical_resource_symbolic_ } tenantId: '00000000-0000-0000-0000-000000000000' } - + resource secret 'secrets' = { name: 'secret' properties: {} @@ -7031,7 +7031,7 @@ public void DependsOn_order_is_deterministic_for_hierarchical_resource_symbolic_ } tenantId: '00000000-0000-0000-0000-000000000000' } - + resource secret 'secrets' = { name: 'secret' properties: {} @@ -7067,7 +7067,7 @@ public void DependsOn_on_existing_resource_triggers_languageVersion_2() module empty 'empty.bicep' = { name: 'foo' } - + resource sa 'Microsoft.Storage/storageAccounts@2023-05-01' existing = { name: 'storage' dependsOn: [ diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index f92bd510bcb..7377794f83c 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -670,26 +670,7 @@ private static ImmutableDictionary sourceFile is BicepFile); var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); - if (!TypeValidator.AreTypesAssignable(paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)) - { - var symbolType = symbol.Type switch - { - IntegerType or IntegerLiteralType => TypeFactory.CreateIntegerType(), - StringType => TypeFactory.CreateStringType(), - ObjectType => new ObjectType(LanguageConstants.ObjectType, TypeSymbolValidationFlags.Default, [], new TypeProperty(LanguageConstants.Any)), - ArrayType => TypeFactory.CreateArrayType(), - _ => symbol.Type.TypeKind switch - { - TypeKind.StringLiteral => TypeFactory.CreateStringType(), - TypeKind.BooleanLiteral => TypeFactory.CreateBooleanType(), - TypeKind.IntegerLiteral => TypeFactory.CreateIntegerType(), - _ => symbol.Type - }, - }; - - diagnostics.WriteMultiple(DiagnosticBuilder.ForPosition(symbol.NameSource).ParameterTypeMismatch(symbol.Name, paramDefinitionFromBicepFile.Value.TypeReference.Type, symbolType)); - referencedValueHasError = true; - } + TypeValidator.NarrowTypeAndCollectDiagnostics(model.TypeManager, model.Binder, model.SourceFile.ParsingErrorLookup, diagnostics, symbol.DeclaringSyntax, paramDefinitionFromBicepFile.Value.TypeReference.Type); if (referencedValueHasError) { From 34ab9601b7d22e76446c2263dfbfb060ac760fce Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Wed, 21 May 2025 08:50:34 -0700 Subject: [PATCH 5/6] fix: handle null bicep file in parameter definition lookup --- src/Bicep.Core/Emit/EmitLimitationCalculator.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index 7377794f83c..5e26105281a 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -667,10 +667,13 @@ private static ImmutableDictionary sourceFile is BicepFile); - var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); + var bicepFile = model.SourceFileGrouping.SourceFiles.FirstOrDefault(sourceFile => sourceFile is BicepFile); + if (bicepFile is not null) + { + var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); - TypeValidator.NarrowTypeAndCollectDiagnostics(model.TypeManager, model.Binder, model.SourceFile.ParsingErrorLookup, diagnostics, symbol.DeclaringSyntax, paramDefinitionFromBicepFile.Value.TypeReference.Type); + TypeValidator.NarrowTypeAndCollectDiagnostics(model.TypeManager, model.Binder, model.SourceFile.ParsingErrorLookup, diagnostics, symbol.DeclaringSyntax, paramDefinitionFromBicepFile.Value.TypeReference.Type); + } if (referencedValueHasError) { From ddc892ced62859e6109bbb435838e50514de59f9 Mon Sep 17 00:00:00 2001 From: Engin Polat Date: Wed, 21 May 2025 09:39:22 -0700 Subject: [PATCH 6/6] fix: handle potential null reference in parameter definition lookup --- src/Bicep.Core/Emit/EmitLimitationCalculator.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs index 5e26105281a..7c3155d0ccf 100644 --- a/src/Bicep.Core/Emit/EmitLimitationCalculator.cs +++ b/src/Bicep.Core/Emit/EmitLimitationCalculator.cs @@ -667,12 +667,16 @@ private static ImmutableDictionary sourceFile is BicepFile); if (bicepFile is not null) { - var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.First(e => e.Key == symbol.Name); + var paramDefinitionFromBicepFile = model.ModelLookup.GetSemanticModel(bicepFile).Parameters.FirstOrDefault(e => e.Key == symbol.Name); - TypeValidator.NarrowTypeAndCollectDiagnostics(model.TypeManager, model.Binder, model.SourceFile.ParsingErrorLookup, diagnostics, symbol.DeclaringSyntax, paramDefinitionFromBicepFile.Value.TypeReference.Type); + if (paramDefinitionFromBicepFile.Value is not null && !TypeValidator.AreTypesAssignable(paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)) + { + diagnostics.WriteMultiple(DiagnosticBuilder.ForPosition(symbol.NameSource).ExpectedValueTypeMismatch(false, paramDefinitionFromBicepFile.Value.TypeReference.Type, symbol.Type)); + } } if (referencedValueHasError)