Skip to content

Commit

Permalink
Fix deploy pane when deploying .bicep files directly (#14587)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
anthony-c-martin authored Jul 18, 2024
1 parent f1c7212 commit 57a44c0
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 44 deletions.
1 change: 1 addition & 0 deletions src/Bicep.Core.IntegrationTests/CompileTimeImportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
});
}

Expand Down
11 changes: 8 additions & 3 deletions src/Bicep.Core.IntegrationTests/EvaluationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
16 changes: 16 additions & 0 deletions src/Bicep.Core.IntegrationTests/ParameterFileTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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."),
});
}
}
Original file line number Diff line number Diff line change
@@ -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|
Expand Down
2 changes: 2 additions & 0 deletions src/Bicep.Core/Registry/ArtifactReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,7 @@ protected ArtifactReference(string scheme, Uri parentModuleUri)
/// Gets a value indicating whether this reference points to an external artifact.
/// </summary>
public abstract bool IsExternal { get; }

public override string ToString() => FullyQualifiedReference;
}
}
23 changes: 22 additions & 1 deletion src/Bicep.Core/Semantics/SemanticModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,28 @@ private IEnumerable<IDiagnostic> 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<IDiagnostic> 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<IDiagnostic> GatherParameterMismatchDiagnostics(ISemanticModel usingModel)
Expand Down
14 changes: 10 additions & 4 deletions src/Bicep.LangServer/Handlers/GetDeploymentDataHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,26 @@ public async Task<GetDeploymentDataResponse> 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;
templateFile = result.Template.Template;

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);
Expand Down
3 changes: 2 additions & 1 deletion src/Bicep.MSBuild.E2eTests/src/bicepparam.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "\'\'"',
Expand Down
62 changes: 32 additions & 30 deletions src/vscode-bicep/src/panes/deploy/app/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,9 @@ export const App: FC = () => {
);
}

if (messages.messageState.localDeployEnabled && !messages.paramsMetadata.sourceFilePath?.endsWith('.bicepparam')) {
return (
<div className="alert-error">
Local Deployment is only currently supported for .bicepparam files. Relaunch this pane for a .bicepparam file.
</div>
);
}
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 (
<main id="webview-body">
Expand Down Expand Up @@ -123,30 +119,36 @@ export const App: FC = () => {
Local Deployment is an experimental feature.
</div>
</FormSection>

<ParametersInputView
parameters={messages.paramsMetadata}
template={messages.templateMetadata}
disabled={localDeployRunning}
onValueChange={setParamValue}
onEnableEditing={handleEnableParamEditing}
onPickParametersFile={messages.pickParamsFile} />

<FormSection title="Actions">
{errorMessage && <div className="alert-error">
<span className="codicon codicon-error" />
{errorMessage}
</div>}
<div className="controls">
<VSCodeButton onClick={handleLocalDeployClick} disabled={localDeployRunning}>Deploy</VSCodeButton>
{showLocalDeployControls && <>
<ParametersInputView
parameters={messages.paramsMetadata}
template={messages.templateMetadata}
disabled={localDeployRunning}
onValueChange={setParamValue}
onEnableEditing={handleEnableParamEditing}
onPickParametersFile={messages.pickParamsFile} />

<FormSection title="Actions">
{errorMessage && <div className="alert-error">
<span className="codicon codicon-error" />
{errorMessage}
</div>}
<div className="controls">
<VSCodeButton onClick={handleLocalDeployClick} disabled={localDeployRunning}>Deploy</VSCodeButton>
</div>
{localDeployRunning && <VSCodeProgressRing></VSCodeProgressRing>}
</FormSection>

{!localDeployRunning && messages.localDeployResult && <>
<LocalDeployResult result={messages.localDeployResult} />
<LocalDeployOperations result={messages.localDeployResult} />
<LocalDeployOutputs result={messages.localDeployResult} />
</>}
</>}
{!showLocalDeployControls && <>
<div className="alert-error">
Local Deployment is only currently supported for .bicepparam files. Relaunch this pane for a .bicepparam file.
</div>
{localDeployRunning && <VSCodeProgressRing></VSCodeProgressRing>}
</FormSection>

{!localDeployRunning && messages.localDeployResult && <>
<LocalDeployResult result={messages.localDeployResult} />
<LocalDeployOperations result={messages.localDeployResult} />
<LocalDeployOutputs result={messages.localDeployResult} />
</>}
</>}
</main>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.");

Check warning on line 70 in src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts

View workflow job for this annotation

GitHub Actions / Test: VSCode Extension (windows-latest, false)

Replace `message.errorMessage·??·"An·error·occurred·compiling·the·Bicep·file."` with `⏎············message.errorMessage·??⏎··············"An·error·occurred·compiling·the·Bicep·file.",⏎··········`

Check warning on line 70 in src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts

View workflow job for this annotation

GitHub Actions / Test: VSCode Extension (ubuntu-latest, false)

Replace `message.errorMessage·??·"An·error·occurred·compiling·the·Bicep·file."` with `⏎············message.errorMessage·??⏎··············"An·error·occurred·compiling·the·Bicep·file.",⏎··········`

Check warning on line 70 in src/vscode-bicep/src/panes/deploy/app/components/hooks/useMessageHandler.ts

View workflow job for this annotation

GitHub Actions / Test: VSCode Extension (macos-latest, false)

Replace `message.errorMessage·??·"An·error·occurred·compiling·the·Bicep·file."` with `⏎············message.errorMessage·??⏎··············"An·error·occurred·compiling·the·Bicep·file.",⏎··········`
return;
}

Expand Down

0 comments on commit 57a44c0

Please sign in to comment.