Skip to content

Commit

Permalink
Bicep lint command (#10819)
Browse files Browse the repository at this point in the history 
# Contributing a Pull Request

If you haven't already, read the full [contribution
guide](https://github.com/Azure/bicep/blob/main/CONTRIBUTING.md). The
guide may have changed since the last time you read it, so please
double-check. Once you are done and ready to submit your PR, run through
the relevant checklist below.

## Contributing a feature

* [x] I have opened a new issue for the proposal, or commented on an
existing one, and ensured that the Bicep maintainers are good with the
design of the feature being implemented
* [x] I have included "Fixes #{issue_number}" in the PR description, so
GitHub can link to the issue and close it when the PR is merged
* [ ] I have appropriate test coverage of my new feature

Fixes #2811

Added new command `lint` that will return an error exit code when there
are any warnings or errors when building the bicep files. The command
has a flag `--ignore-warnings` to only react on errors, not sure if this
is needed though since you could use the `build` command instead.

I also added tests, copied and refactored from `build` command tests, I
could use some feedback if the tests cover enough or there are some
missing tests that should be added (that's why I didn't check the test
checkbox above).

## Checklist/discussion points
- [ ] Do we want the `--ignore-warnings` flag? Does it serve a purpose?
- [ ] Are there any more tests needed?
- [ ] #2811 describes different exit codes depending on warnings/errors,
is this wanted?
- [ ] Are there any additional flags that are needed for the command?

###### Microsoft Reviewers: [Open in
CodeFlow](https://portal.fabricbot.ms/api/codeflow?pullrequest=https://github.com/Azure/bicep/pull/10819)

---------

Co-authored-by: Anthony Martin <[email protected]>
  • Loading branch information
@anthony-c-martin
Johan Lindqvist and anthony-c-martin authored Aug 10, 2023
1 parent 3319ec7 commit 6c3a71d
Show file tree
Hide file tree
Showing 11 changed files with 418 additions and 20 deletions.
235 changes: 235 additions & 0 deletions src/Bicep.Cli.IntegrationTests/LintCommandTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Bicep.Core;
using Bicep.Core.Configuration;
using Bicep.Core.Registry;
using Bicep.Core.Samples;
using Bicep.Core.UnitTests;
using Bicep.Core.UnitTests.Mock;
using Bicep.Core.UnitTests.Registry;
using Bicep.Core.UnitTests.Utils;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.CodeAnalysis.Sarif;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.WindowsAzure.ResourceStack.Common.Json;
using Moq;
using Newtonsoft.Json.Linq;

namespace Bicep.Cli.IntegrationTests;

[TestClass]
public class LintCommandTests : TestBase
{
[NotNull]
public TestContext? TestContext { get; set; }

[TestMethod]
public async Task Help_should_output_lint_usage_information()
{
var (output, error, result) = await Bicep("--help");

result.Should().Be(0);
error.Should().BeEmpty();
output.Should().Contain("""
bicep lint [options] <file>
Lints a .bicep file.
Arguments:
<file> The input file
Options:
--no-restore Skips restoring external modules.
--diagnostics-format <format> Sets the format with which diagnostics are displayed. Valid values are ( Default | Sarif ).
Examples:
bicep lint file.bicep
bicep lint file.bicep --no-restore
bicep lint file.bicep --diagnostics-format sarif
""");
}

[TestMethod]
public async Task Lint_ZeroFiles_ShouldFail_WithExpectedErrorMessage()
{
var (output, error, result) = await Bicep("lint");

using (new AssertionScope())
{
result.Should().Be(1);
output.Should().BeEmpty();

error.Should().NotBeEmpty();
error.Should().Contain($"The input file path was not specified");
}
}

[TestMethod]
public async Task Lint_NonBicepFiles_ShouldFail_WithExpectedErrorMessage()
{
var (output, error, result) = await Bicep("lint", "/dev/zero");

using (new AssertionScope())
{
result.Should().Be(1);
output.Should().BeEmpty();

error.Should().NotBeEmpty();
error.Should().Contain($@"The specified input ""/dev/zero"" was not recognized as a Bicep file. Bicep files must use the {LanguageConstants.LanguageFileExtension} extension.");
}
}

[DataTestMethod]
[DynamicData(nameof(GetValidDataSetsWithoutWarnings), DynamicDataSourceType.Method, DynamicDataDisplayNameDeclaringType = typeof(DataSet), DynamicDataDisplayName = nameof(DataSet.GetDisplayName))]
public async Task Lint_Valid_SingleFile_WithTemplateSpecReference_ShouldSucceed(DataSet dataSet)
{
var outputDirectory = dataSet.SaveFilesToTestDirectory(TestContext);
var clientFactory = dataSet.CreateMockRegistryClients().Object;
var templateSpecRepositoryFactory = dataSet.CreateMockTemplateSpecRepositoryFactory(TestContext);
await dataSet.PublishModulesToRegistryAsync(clientFactory);
var bicepFilePath = Path.Combine(outputDirectory, DataSet.TestFileMain);

var settings = new InvocationSettings(new(TestContext, RegistryEnabled: dataSet.HasExternalModules), clientFactory, templateSpecRepositoryFactory);
var (output, error, result) = await Bicep(settings, "lint", bicepFilePath);

using (new AssertionScope())
{
result.Should().Be(0);
output.Should().BeEmpty();
AssertNoErrors(error);
}
}

[TestMethod]
public async Task Lint_Valid_SingleFile_WithDigestReference_ShouldSucceed()
{
var registry = "example.com";
var registryUri = new Uri("https://" + registry);
var repository = "hello/there";

var client = new MockRegistryBlobClient();

var clientFactory = StrictMock.Of<IContainerRegistryClientFactory>();
clientFactory.Setup(m => m.CreateAuthenticatedBlobClient(It.IsAny<RootConfiguration>(), registryUri, repository)).Returns(client);

var settings = new InvocationSettings(new(TestContext, RegistryEnabled: true), clientFactory.Object, BicepTestConstants.TemplateSpecRepositoryFactory);

var tempDirectory = FileHelper.GetUniqueTestOutputPath(TestContext);
Directory.CreateDirectory(tempDirectory);

var publishedBicepFilePath = Path.Combine(tempDirectory, "published.bicep");
File.WriteAllText(publishedBicepFilePath, string.Empty);

var (publishOutput, publishError, publishResult) = await Bicep(settings, "publish", publishedBicepFilePath, "--target", $"br:{registry}/{repository}:v1");
using (new AssertionScope())
{
publishResult.Should().Be(0);
publishOutput.Should().BeEmpty();
publishError.Should().BeEmpty();
}

client.Blobs.Should().HaveCount(2);
client.Manifests.Should().HaveCount(1);
client.ManifestTags.Should().HaveCount(1);

string digest = client.Manifests.Single().Key;

var bicep = $@"
module empty 'br:{registry}/{repository}@{digest}' = {{
name: 'empty'
}}
";

var bicepFilePath = Path.Combine(tempDirectory, "built.bicep");
File.WriteAllText(bicepFilePath, bicep);

var (output, error, result) = await Bicep(settings, "lint", bicepFilePath);
using (new AssertionScope())
{
result.Should().Be(0);
output.Should().BeEmpty();
error.Should().BeEmpty();
}
}

[DataTestMethod]
[DynamicData(nameof(GetInvalidDataSets), DynamicDataSourceType.Method, DynamicDataDisplayNameDeclaringType = typeof(DataSet), DynamicDataDisplayName = nameof(DataSet.GetDisplayName))]
public async Task Lint_Invalid_SingleFile_ShouldFail_WithExpectedErrorMessage(DataSet dataSet)
{
var outputDirectory = dataSet.SaveFilesToTestDirectory(TestContext);
var bicepFilePath = Path.Combine(outputDirectory, DataSet.TestFileMain);
var defaultSettings = CreateDefaultSettings();
var diagnostics = await GetAllDiagnostics(bicepFilePath, defaultSettings.ClientFactory, defaultSettings.TemplateSpecRepositoryFactory);

var (output, error, result) = await Bicep("lint", bicepFilePath);

using (new AssertionScope())
{
result.Should().Be(1);
output.Should().BeEmpty();
error.Should().ContainAll(diagnostics);
}
}

[TestMethod]
public async Task Lint_WithEmptyBicepConfig_ShouldProduceConfigurationError()
{
string testOutputPath = FileHelper.GetUniqueTestOutputPath(TestContext);
var inputFile = FileHelper.SaveResultFile(TestContext, "main.bicep", DataSets.Empty.Bicep, testOutputPath);
var configurationPath = FileHelper.SaveResultFile(TestContext, "bicepconfig.json", string.Empty, testOutputPath);

var (output, error, result) = await Bicep("lint", inputFile);

result.Should().Be(1);
output.Should().BeEmpty();
error.Should().StartWith($"{inputFile}(1,1) : Error BCP271: Failed to parse the contents of the Bicep configuration file \"{configurationPath}\" as valid JSON: \"The input does not contain any JSON tokens. Expected the input to start with a valid JSON token, when isFinalBlock is true. LineNumber: 0 | BytePositionInLine: 0.\".");
}

[TestMethod]
public async Task Lint_with_warnings_should_log_warnings_and_return_0_exit_code()
{
var inputFile = FileHelper.SaveResultFile(this.TestContext, "main.bicep", $@"
@minValue(1)
@maxValue(50)
param notUsedParam int = 3
");

var (output, error, result) = await Bicep("lint", inputFile);

result.Should().Be(0);
output.Should().BeEmpty();
error.Should().StartWith($"{inputFile}(4,7) : Warning no-unused-params: Parameter \"notUsedParam\" is declared but never used. [https://aka.ms/bicep/linter/no-unused-params]");
}

[TestMethod]
public async Task Lint_with_sarif_diagnostics_format_should_output_valid_sarif()
{
var inputFile = FileHelper.SaveResultFile(this.TestContext, "main.bicep", @"param storageAccountName string = 'test'");

var (output, error, result) = await Bicep("lint", inputFile, "--diagnostics-format", "sarif");

result.Should().Be(0);
output.Should().BeEmpty();
var errorLog = error.FromJson<SarifLog>();
errorLog.Runs[0].Results[0].RuleId.Should().Be("no-unused-params");
errorLog.Runs[0].Results[0].Message.Text.Should().Contain("is declared but never used");
}

private static IEnumerable<object[]> GetValidDataSetsWithoutWarnings() => DataSets
.AllDataSets
.Where(ds => ds.IsValid)
.Where(ds => ds.Name is "Functions_LF" or "ModulesWithScopes_LF")
.ToDynamicTestData();

private static IEnumerable<object[]> GetInvalidDataSets() => DataSets
.AllDataSets
.Where(ds => ds.IsValid == false)
.ToDynamicTestData();
}
17 changes: 2 additions & 15 deletions src/Bicep.Cli/Arguments/BuildArguments.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using Bicep.Cli.Helpers;
using Bicep.Core.FileSystem;
using System;
using System.IO;
Expand Down Expand Up @@ -58,7 +59,7 @@ public BuildArguments(string[] args) : base(Constants.Command.Build)
{
throw new CommandLineException($"The --diagnostics-format parameter cannot be specified twice");
}
DiagnosticsFormat = ToDiagnosticsFormat(args[i + 1]);
DiagnosticsFormat = ArgumentHelper.ToDiagnosticsFormat(args[i + 1]);
i++;
break;

Expand Down Expand Up @@ -112,20 +113,6 @@ public BuildArguments(string[] args) : base(Constants.Command.Build)
}
}

private static DiagnosticsFormat ToDiagnosticsFormat(string? format)
{
if(format is null || (format is not null && format.Equals("default", StringComparison.OrdinalIgnoreCase)))
{
return Arguments.DiagnosticsFormat.Default;
}
else if(format is not null && format.Equals("sarif", StringComparison.OrdinalIgnoreCase))
{
return Arguments.DiagnosticsFormat.Sarif;
}

throw new ArgumentException($"Unrecognized diagnostics format {format}");
}

public bool OutputToStdOut { get; }

public string InputFile { get; }
Expand Down
65 changes: 65 additions & 0 deletions src/Bicep.Cli/Arguments/LintArguments.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.IO;
using Bicep.Cli.Helpers;
using Bicep.Core.FileSystem;

namespace Bicep.Cli.Arguments;

public class LintArguments : ArgumentsBase
{
public LintArguments(string[] args)
: base(Constants.Command.Lint)
{
for (var i = 0; i < args.Length; i++)
{
switch (args[i].ToLowerInvariant())
{
case "--no-restore":
NoRestore = true;
break;

case "--diagnostics-format":
if (args.Length == i + 1)
{
throw new CommandLineException($"The --diagnostics-format parameter expects an argument");
}
if (DiagnosticsFormat is not null)
{
throw new CommandLineException($"The --diagnostics-format parameter cannot be specified twice");
}
DiagnosticsFormat = ArgumentHelper.ToDiagnosticsFormat(args[i + 1]);
i++;
break;
default:
if (args[i].StartsWith("--"))
{
throw new CommandLineException($"Unrecognized parameter \"{args[i]}\"");
}
if (InputFile is not null)
{
throw new CommandLineException($"The input file path cannot be specified multiple times");
}
InputFile = args[i];
break;
}
}

if (InputFile is null)
{
throw new CommandLineException($"The input file path was not specified");
}

if (DiagnosticsFormat is null)
{
DiagnosticsFormat = Arguments.DiagnosticsFormat.Default;
}
}

public string InputFile { get; }

public DiagnosticsFormat? DiagnosticsFormat { get; }

public bool NoRestore { get; }
}
Loading

0 comments on commit 6c3a71d

Please sign in to comment.