Enable supplemental params validation with build-params command
#11975
Conversation
…build-params-overload
|
Test this change out locally with the following install scripts (Action run 6386146837) VSCode
Azure CLI
|
Test Results 132 files ±0 132 suites ±0 4h 0m 29s ⏱️ + 11m 51s For more details on these failures, see this check. Results for commit 430d71f. ± Comparison against base commit c43ca5f. ♻️ This comment has been updated with latest results. |
| { | ||
| JTokenType.String => SyntaxFactory.CreateStringLiteral(value.ToString(CultureInfo.InvariantCulture)), | ||
| JTokenType.Integer => SyntaxFactory.CreatePositiveOrNegativeInteger(value.Value<long>()), | ||
| // Floats are currently not supported in Bicep, so fall back to the default behavior of "any" |
There was a problem hiding this comment.
We will likely have to change this behavior since we need to assign types that are as accurate as possible for correct type validation.
This behavior is similar to that of ConvertJsonToBicepType from SystemNamesapceType so I am thinking the breaking change (with the introduction of floats) would be in all locations where we are going from JSON to Bicep
Alternatively, we could also just block usage of floats to avoid breaking change at least in .bicepparam files
| var fileContents = await File.ReadAllTextAsync(paramsInputPath); | ||
| var sourceFile = SourceFileFactory.CreateBicepParamFile(PathHelper.FilePathToFileUrl(paramsInputPath), fileContents); | ||
|
|
||
| var newProgramSyntax = CallbackRewriter.Rewrite(sourceFile.ProgramSyntax, syntax => |
There was a problem hiding this comment.
Since we're modifying the user-provided file, what happens to the locations in the TextSpan objects on the generated syntax nodes? If the compiler decides to emit a diagnostic whose span involves one of the generated nodes, are we going to emit misleading locations?
This is likely out of scope for this PR, but I'm wondering if we need to add a concept of an "external" location for syntax nodes for scenarios such as this. (Plus semantics on how an external location interacts with regular locations when we do TextSpan calculations in the compiler.)
There was a problem hiding this comment.
The issue with line numbers is something we've discussed during parameters sync meeting multiple times. The solution two for question one in design gist somewhat attempts to solve this issue. However, we've decided to continue with current implementation as it is lower cost and errors are "good enough" for users to debug.
Adding an "external" location sounds like a great idea. I've linked this comment to the gist so we can track this in future discussions
…build-params-overload
|
Which milestone is this a part of? We are looking to use both bicepparams and inline parameters with our DevOps pipelines. |
This'll be available in the Azure CLI & Azure PowerShell upcoming releases on 11/14: |
Enable
build-paramscommand to use values from aBICEP_PARAMETERS_OVERRIDESenvironment variable to update the.bicepparamfileThis work is to support the Inline overrides use case for az cli
Microsoft Reviewers: Open in CodeFlow