From 57a44c02302e3fcd3e3c19e163a8cd9a1179dd88 Mon Sep 17 00:00:00 2001 From: Anthony Martin <38542602+anthony-c-martin@users.noreply.github.com> Date: Thu, 18 Jul 2024 14:32:39 -0400 Subject: [PATCH] Fix deploy pane when deploying `.bicep` files directly (#14587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As part of the local deployment implementation, I apparently completely removed the ability for the deployment pane to deploy a non-local `.bicep` file 🤦. This PR fixes that. The changes in this PR: * Fixes deployment through the deploy pane if the user is referencing a `.bicep` file directly. * As part of testing this, I noticed we're actually missing validation for a `using` statement referencing a broken file. I've updated this to bring it in line with behavior for a `module` statement referencing a broken file. * Fixes the issue where a cryptic error appears in local-deploy mode if the `.bicep` file is invalid. * Fixes an issue where we're not logging the artifact reference properly in error messages (missing a `.ToString()` implementation). ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/14587) --- .../CompileTimeImportTests.cs | 1 + .../EvaluationTests.cs | 11 +++- .../ParameterFileTests.cs | 16 +++++ .../parameters.diagnostics.bicepparam | 1 + src/Bicep.Core/Registry/ArtifactReference.cs | 2 + src/Bicep.Core/Semantics/SemanticModel.cs | 23 ++++++- .../Handlers/GetDeploymentDataHandler.cs | 14 +++-- .../src/bicepparam.test.ts | 3 +- .../src/panes/deploy/app/components/App.tsx | 62 ++++++++++--------- .../app/components/hooks/useMessageHandler.ts | 8 +-- 10 files changed, 97 insertions(+), 44 deletions(-) diff --git a/src/Bicep.Core.IntegrationTests/CompileTimeImportTests.cs b/src/Bicep.Core.IntegrationTests/CompileTimeImportTests.cs index 06be07c6298..15d717ad3ed 100644 --- a/src/Bicep.Core.IntegrationTests/CompileTimeImportTests.cs +++ b/src/Bicep.Core.IntegrationTests/CompileTimeImportTests.cs @@ -2007,6 +2007,7 @@ INVALID FILE { ("BCP104", DiagnosticLevel.Error, "The referenced module has errors."), ("BCP104", DiagnosticLevel.Error, "The referenced module has errors."), + ("BCP104", DiagnosticLevel.Error, "The referenced module has errors."), }); } diff --git a/src/Bicep.Core.IntegrationTests/EvaluationTests.cs b/src/Bicep.Core.IntegrationTests/EvaluationTests.cs index 77cc02fb04c..6e5b33793f8 100644 --- a/src/Bicep.Core.IntegrationTests/EvaluationTests.cs +++ b/src/Bicep.Core.IntegrationTests/EvaluationTests.cs @@ -918,9 +918,14 @@ param bar string } "; - var (parameters, diag, comp) = CompilationHelper.CompileParams(("parameters.bicepparam", bicepparamText), ("main.bicep", bicepTemplateText)); - - var result = CompilationHelper.Compile(("main.bicep", bicepTemplateText), ("module.bicep", bicepModuleText)); + var (parameters, diag, comp) = CompilationHelper.CompileParams( + ("parameters.bicepparam", bicepparamText), + ("main.bicep", bicepTemplateText), + ("module.bicep", bicepModuleText)); + + var result = CompilationHelper.Compile( + ("main.bicep", bicepTemplateText), + ("module.bicep", bicepModuleText)); var evaluated = TemplateEvaluator.Evaluate(result.Template, parameters, config => config with { diff --git a/src/Bicep.Core.IntegrationTests/ParameterFileTests.cs b/src/Bicep.Core.IntegrationTests/ParameterFileTests.cs index 764a86bcd80..f533d4f384b 100644 --- a/src/Bicep.Core.IntegrationTests/ParameterFileTests.cs +++ b/src/Bicep.Core.IntegrationTests/ParameterFileTests.cs @@ -222,4 +222,20 @@ param optionalBecauseNullable string? result.Should().NotHaveAnyDiagnostics(); } + + [TestMethod] + public void Error_is_displayed_for_file_reference_with_errors() + { + var result = CompilationHelper.CompileParams( +("parameters.bicepparam", """ +using 'main.bicep' +"""), ("main.bicep", """ +invalid file +""")); + + result.Should().HaveDiagnostics(new[] + { + ("BCP104", DiagnosticLevel.Error, "The referenced module has errors."), + }); + } } diff --git a/src/Bicep.Core.Samples/Files/baselines_bicepparam/Invalid_Parameters/parameters.diagnostics.bicepparam b/src/Bicep.Core.Samples/Files/baselines_bicepparam/Invalid_Parameters/parameters.diagnostics.bicepparam index 3ad0435910a..b3aa87fa23b 100644 --- a/src/Bicep.Core.Samples/Files/baselines_bicepparam/Invalid_Parameters/parameters.diagnostics.bicepparam +++ b/src/Bicep.Core.Samples/Files/baselines_bicepparam/Invalid_Parameters/parameters.diagnostics.bicepparam @@ -1,5 +1,6 @@ using './main.bicep' //@[06:20) [BCP258 (Error)] The following parameters are declared in the Bicep file but are missing an assignment in the params file: "additionalMetadata", "decoratedString", "description", "description2", "emptyMetadata", "myBool", "myInt", "myString", "password", "secretObject", "someArray", "someParameter", "storageName", "storageSku", "stringLiteral". (CodeDescription: none) |'./main.bicep'| +//@[06:20) [BCP104 (Error)] The referenced module has errors. (CodeDescription: none) |'./main.bicep'| param para1 = 'value //@[00:20) [BCP259 (Error)] The parameter "para1" is assigned in the params file without being declared in the Bicep file. (CodeDescription: none) |param para1 = 'value| diff --git a/src/Bicep.Core/Registry/ArtifactReference.cs b/src/Bicep.Core/Registry/ArtifactReference.cs index 26706af78c6..58618796862 100644 --- a/src/Bicep.Core/Registry/ArtifactReference.cs +++ b/src/Bicep.Core/Registry/ArtifactReference.cs @@ -35,5 +35,7 @@ protected ArtifactReference(string scheme, Uri parentModuleUri) /// Gets a value indicating whether this reference points to an external artifact. /// public abstract bool IsExternal { get; } + + public override string ToString() => FullyQualifiedReference; } } diff --git a/src/Bicep.Core/Semantics/SemanticModel.cs b/src/Bicep.Core/Semantics/SemanticModel.cs index f3155fd4f99..821456d6a3e 100644 --- a/src/Bicep.Core/Semantics/SemanticModel.cs +++ b/src/Bicep.Core/Semantics/SemanticModel.cs @@ -503,7 +503,28 @@ private IEnumerable GetAdditionalParamsSemanticDiagnostics() // get diagnostics relating to missing parameter assignments or declarations GatherParameterMismatchDiagnostics(semanticModel) // get diagnostics relating to type mismatch of params between Bicep and params files - .Concat(GatherTypeMismatchDiagnostics()); + .Concat(GatherTypeMismatchDiagnostics()) + // get diagnostics on whether the module referenced in the using statement is valid + .Concat(GatherUsingModelInvalidDiagnostics(semanticModel)); + } + + private IEnumerable GatherUsingModelInvalidDiagnostics(ISemanticModel usingModel) + { + // emit diagnostic only if there is a using statement + var usingSyntax = this.Root.UsingDeclarationSyntax; + + if (usingSyntax is null || + usingSyntax.Path is NoneLiteralSyntax) + { + yield break; + } + + if (usingModel.HasErrors()) + { + yield return usingModel is ArmTemplateSemanticModel + ? DiagnosticBuilder.ForPosition(usingSyntax.Path).ReferencedArmTemplateHasErrors() + : DiagnosticBuilder.ForPosition(usingSyntax.Path).ReferencedModuleHasErrors(); + } } private IEnumerable GatherParameterMismatchDiagnostics(ISemanticModel usingModel) diff --git a/src/Bicep.LangServer/Handlers/GetDeploymentDataHandler.cs b/src/Bicep.LangServer/Handlers/GetDeploymentDataHandler.cs index db72d166d04..7966655ccc5 100644 --- a/src/Bicep.LangServer/Handlers/GetDeploymentDataHandler.cs +++ b/src/Bicep.LangServer/Handlers/GetDeploymentDataHandler.cs @@ -49,7 +49,7 @@ public async Task Handle(GetDeploymentDataRequest req if (result.Parameters is null || result.Template?.Template is null) { - return new(ErrorMessage: $"Bicep compilation failed. The Bicep parameters file contains errors.", LocalDeployEnabled: localDeployEnabled); + return new(ErrorMessage: $"Compilation failed. The Bicep parameters file contains errors.", LocalDeployEnabled: localDeployEnabled); } paramsFile = result.Parameters; @@ -57,12 +57,18 @@ public async Task Handle(GetDeploymentDataRequest req if (!semanticModel.Root.TryGetBicepFileSemanticModelViaUsing().IsSuccess(out var usingModel)) { - return new(ErrorMessage: $"Bicep compilation failed. The Bicep parameters file contains errors.", LocalDeployEnabled: localDeployEnabled); + return new(ErrorMessage: $"Compilation failed. Failed to find a file referenced via 'using'.", LocalDeployEnabled: localDeployEnabled); } } - else + else if (semanticModel.Root.FileKind == BicepSourceFileKind.BicepFile) { - return new(ErrorMessage: $"Bicep compilation failed. The Bicep file contains errors.", LocalDeployEnabled: localDeployEnabled); + var result = context.Compilation.Emitter.Template(); + templateFile = result.Template; + + if (result.Template is null) + { + return new(ErrorMessage: $"Compilation failed. The Bicep file contains errors.", LocalDeployEnabled: localDeployEnabled); + } } return new(TemplateJson: templateFile, ParametersJson: paramsFile, LocalDeployEnabled: localDeployEnabled); diff --git a/src/Bicep.MSBuild.E2eTests/src/bicepparam.test.ts b/src/Bicep.MSBuild.E2eTests/src/bicepparam.test.ts index 14004ae2c69..c454e0b832f 100644 --- a/src/Bicep.MSBuild.E2eTests/src/bicepparam.test.ts +++ b/src/Bicep.MSBuild.E2eTests/src/bicepparam.test.ts @@ -53,8 +53,9 @@ describe("msbuild", () => { asserts.expectLinesInLog(result.stdout, [ "1 Warning(s)", - "3 Error(s)", + "4 Error(s)", 'main.bicepparam(1,7): error BCP258: The following parameters are declared in the Bicep file but are missing an assignment in the params file: "extraneous"', + "main.bicepparam(1,7): error BCP104: The referenced module has errors.", 'main.bicepparam(3,1): error BCP259: The parameter "missing" is assigned in the params file without being declared in the Bicep file.', 'main.bicep(1,7): warning no-unused-params: Parameter "extraneous" is declared but never used. [https://aka.ms/bicep/linter/no-unused-params]', 'main.bicep(3,27): error BCP033: Expected a value of type "object" but the provided value is of type "\'\'"', diff --git a/src/vscode-bicep/src/panes/deploy/app/components/App.tsx b/src/vscode-bicep/src/panes/deploy/app/components/App.tsx index 6a7cb1cb98f..ad1ffc54f0b 100644 --- a/src/vscode-bicep/src/panes/deploy/app/components/App.tsx +++ b/src/vscode-bicep/src/panes/deploy/app/components/App.tsx @@ -64,13 +64,9 @@ export const App: FC = () => { ); } - if (messages.messageState.localDeployEnabled && !messages.paramsMetadata.sourceFilePath?.endsWith('.bicepparam')) { - return ( -
- Local Deployment is only currently supported for .bicepparam files. Relaunch this pane for a .bicepparam file. -
- ); - } + const showLocalDeployControls = messages.messageState.localDeployEnabled && + // if there's an error, this'll cause sourceFilePath to be empty - we still want to show the controls to display the error + (errorMessage || messages.paramsMetadata.sourceFilePath?.endsWith('.bicepparam')); return (
@@ -123,30 +119,36 @@ export const App: FC = () => { Local Deployment is an experimental feature. - - - - - {errorMessage &&
- - {errorMessage} -
} -
- Deploy + {showLocalDeployControls && <> + + + + {errorMessage &&
+ + {errorMessage} +
} +
+ Deploy +
+ {localDeployRunning && } + + + {!localDeployRunning && messages.localDeployResult && <> + + + + } + } + {!showLocalDeployControls && <> +
+ Local Deployment is only currently supported for .bicepparam files. Relaunch this pane for a .bicepparam file.
- {localDeployRunning && } - - - {!localDeployRunning && messages.localDeployResult && <> - - - } }
diff --git a/src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts b/src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts index fa64ab8715d..32467defa50 100644 --- a/src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts +++ b/src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts @@ -64,12 +64,10 @@ export function useMessageHandler(props: UseMessageHandlerProps) { initialized: true, localDeployEnabled: message.localDeployEnabled, }); - if (!message.templateJson) { + + if (message.errorMessage || !message.templateJson) { setTemplateMetadata(undefined); - setErrorMessage( - message.errorMessage ?? - "An error occurred building the deployment object.", - ); + setErrorMessage(message.errorMessage ?? "An error occurred compiling the Bicep file."); return; }