diff --git a/src/Bicep.Core.UnitTests/BicepTestConstants.cs b/src/Bicep.Core.UnitTests/BicepTestConstants.cs index f26ee2d67c9..ad28e80bb4d 100644 --- a/src/Bicep.Core.UnitTests/BicepTestConstants.cs +++ b/src/Bicep.Core.UnitTests/BicepTestConstants.cs @@ -67,10 +67,10 @@ public static class BicepTestConstants public static readonly ITemplateSpecRepositoryFactory TemplateSpecRepositoryFactory = StrictMock.Of().Object; // Linter rules added to this list will be automatically disabled for most tests. - public static readonly string[] NonStableAnalyzerRules = [UseRecentApiVersionRule.Code, UseRecentModuleVersionsRule.Code, NoHardcodedOutputsRule.Code]; + public static readonly string[] NonStableAnalyzerRules = [UseRecentApiVersionRule.Code, UseRecentModuleVersionsRule.Code, NoHardcodedOutputsRule.Code, UseParameterDescriptionsRule.Code]; // Rules that are currently skipped due to configuration for ProgramsShouldProduceExpectedDiagnostics - public static readonly string[] TestAnalyzersToSkip = [UseRecentApiVersionRule.Code, UseRecentModuleVersionsRule.Code, NoHardcodedLocationRule.Code, ExplicitValuesForLocationParamsRule.Code, NoLocationExprOutsideParamsRule.Code, NoModuleNameRule.Code, NoHardcodedOutputsRule.Code]; + public static readonly string[] TestAnalyzersToSkip = [UseRecentApiVersionRule.Code, UseRecentModuleVersionsRule.Code, NoHardcodedLocationRule.Code, ExplicitValuesForLocationParamsRule.Code, NoLocationExprOutsideParamsRule.Code, NoModuleNameRule.Code, NoHardcodedOutputsRule.Code, UseParameterDescriptionsRule.Code]; public static readonly RootConfiguration BuiltInConfigurationWithAllAnalyzersDisabled = IConfigurationManager.GetBuiltInConfiguration().WithAllAnalyzersDisabled(); public static readonly RootConfiguration BuiltInConfigurationWithStableAnalyzers = IConfigurationManager.GetBuiltInConfiguration().WithAllAnalyzers().WithAnalyzersDisabled(NonStableAnalyzerRules); diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/UseParameterDescriptionsRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/UseParameterDescriptionsRuleTests.cs new file mode 100644 index 00000000000..10817f2aa36 --- /dev/null +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/UseParameterDescriptionsRuleTests.cs @@ -0,0 +1,134 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Bicep.Core.Analyzers.Linter.Rules; +using Bicep.Core.Configuration; +using Bicep.Core.Extensions; +using Bicep.Core.UnitTests.Assertions; +using Bicep.Core.UnitTests.Utils; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests; + +[TestClass] +public class UseParameterDescriptionsRuleTests : LinterRuleTestsBase +{ + private static readonly Options RuleOptions = new(ConfigurationPatch: EnableRule); + + private static RootConfiguration EnableRule(RootConfiguration configuration) => + configuration.WithAnalyzersConfiguration( + configuration.Analyzers.SetValue($"core.rules.{UseParameterDescriptionsRule.Code}.level", "warning")); + + private void AssertDiagnostics(string inputFile, int expectedCount = 1) + => AssertLinterRuleDiagnostics(UseParameterDescriptionsRule.Code, inputFile, expectedCount, RuleOptions); + + private void AssertDiagnostics(string inputFile, string[] expectedMessages) + => AssertLinterRuleDiagnostics(UseParameterDescriptionsRule.Code, inputFile, expectedMessages, RuleOptions); + + private void AssertNoDiagnostics(string inputFile, OnCompileErrors onCompileErrors = OnCompileErrors.IncludeErrors) + => AssertLinterRuleDiagnostics( + UseParameterDescriptionsRule.Code, + inputFile, + [], + RuleOptions with + { + OnCompileErrors = onCompileErrors, + IncludePosition = IncludePosition.None, + }); + + [TestMethod] + public void Rule_defaults_to_off() + { + var result = CompilationHelper.Compile(""" + param input string + """); + + result.ExcludingDiagnostics("no-unused-params").Should().NotHaveAnyDiagnostics(); + } + + [TestMethod] + public void Parameters_without_descriptions_are_reported() + { + AssertDiagnostics( + """ + param first string + + @secure() + param second string + """, + [ + """[1] Parameter "first" must have a non-empty description.""", + """[4] Parameter "second" must have a non-empty description.""", + ]); + } + + [DataRow(""" + @description('Parameter description.') + param input string + """)] + [DataRow(""" + @sys.description('Parameter description.') + param input string + """)] + [DataTestMethod] + public void Non_empty_descriptions_are_accepted(string text) + { + AssertNoDiagnostics(text); + } + + [DataRow(""" + @description('') + param input string + """)] + [DataRow(""" + @description(' ') + param input string + """)] + [DataRow(""" + @sys.description('') + param input string + """)] + [DataRow(""" + @sys.description(''' + + ''') + param input string + """)] + [DataTestMethod] + public void Empty_and_whitespace_descriptions_are_reported(string text) + { + AssertDiagnostics(text); + } + + [TestMethod] + public void Metadata_description_does_not_satisfy_the_rule() + { + AssertDiagnostics( + """ + @metadata({ description: 'Metadata description.' }) + param input string + """, + ["""[2] Parameter "input" must have a non-empty description."""]); + } + + [DataRow(""" + @description('Variable description.') + var value = 'value' + """)] + [DataRow(""" + @description('Output description.') + output result string = 'value' + """)] + [DataTestMethod] + public void Descriptions_on_other_declarations_are_ignored(string text) + { + AssertNoDiagnostics(text); + } + + [TestMethod] + public void Malformed_parameter_without_a_name_is_ignored() + { + AssertNoDiagnostics("param", OnCompileErrors.Ignore); + } +} diff --git a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/WhatIfShortCircuitingRuleTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/WhatIfShortCircuitingRuleTests.cs index d2285a903ca..64f18c50a60 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/WhatIfShortCircuitingRuleTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/WhatIfShortCircuitingRuleTests.cs @@ -247,8 +247,8 @@ param condition bool } """), ("mod3.bicep", """ - param name string - + param name string + resource vnet 'Microsoft.Network/virtualNetworks@2024-07-01' = { name: name } diff --git a/src/Bicep.Core/Analyzers/Linter/Rules/UseParameterDescriptionsRule.cs b/src/Bicep.Core/Analyzers/Linter/Rules/UseParameterDescriptionsRule.cs new file mode 100644 index 00000000000..b8d390a8952 --- /dev/null +++ b/src/Bicep.Core/Analyzers/Linter/Rules/UseParameterDescriptionsRule.cs @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Bicep.Core.Diagnostics; +using Bicep.Core.Semantics; +using Bicep.Core.Semantics.Namespaces; + +namespace Bicep.Core.Analyzers.Linter.Rules; + +public sealed class UseParameterDescriptionsRule : LinterRuleBase +{ + public new const string Code = "use-parameter-descriptions"; + + public UseParameterDescriptionsRule() : base( + code: Code, + description: CoreResources.UseParameterDescriptionsRuleDescription, + LinterRuleCategory.BestPractice, + overrideCategoryDefaultDiagnosticLevel: DiagnosticLevel.Off) + { } + + public override string FormatMessage(params object[] values) + => string.Format(CoreResources.UseParameterDescriptionsRuleMessageFormat, values); + + public override IEnumerable AnalyzeInternal(SemanticModel model, DiagnosticLevel diagnosticLevel) + { + foreach (var parameter in model.Root.ParameterDeclarations.Where(parameter => parameter.NameSource.IsValid)) + { + var descriptionDecorator = parameter.TryGetDecorator( + model, + SystemNamespaceType.BuiltInName, + LanguageConstants.MetadataDescriptionPropertyName); + + if (descriptionDecorator is null) + { + yield return CreateDiagnosticForSpan(diagnosticLevel, parameter.NameSource.Span, parameter.Name); + continue; + } + + if (DescriptionHelper.TryGetFromDecorator(model, parameter.DeclaringParameter) is { } description && + string.IsNullOrWhiteSpace(description)) + { + yield return CreateDiagnosticForSpan( + diagnosticLevel, + descriptionDecorator.Arguments.First().Expression.Span, + parameter.Name); + } + } + } +} diff --git a/src/Bicep.Core/CoreResources.Designer.cs b/src/Bicep.Core/CoreResources.Designer.cs index 320f14eaef7..8ac5109a57c 100644 --- a/src/Bicep.Core/CoreResources.Designer.cs +++ b/src/Bicep.Core/CoreResources.Designer.cs @@ -185,7 +185,7 @@ internal static string ExperimentalFeatureNames_OciEnabled { return ResourceManager.GetString("ExperimentalFeatureNames_OciEnabled", resourceCulture); } } - + /// /// Looks up a localized string similar to Resource info code generation. /// @@ -951,6 +951,24 @@ internal static string UseParentPropertyRule_MessageFormat { } } + /// + /// Looks up a localized string similar to Parameters should have non-empty descriptions.. + /// + internal static string UseParameterDescriptionsRuleDescription { + get { + return ResourceManager.GetString("UseParameterDescriptionsRule_Description", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Parameter "{0}" must have a non-empty description.. + /// + internal static string UseParameterDescriptionsRuleMessageFormat { + get { + return ResourceManager.GetString("UseParameterDescriptionsRule_MessageFormat", resourceCulture); + } + } + /// /// Looks up a localized string similar to Acceptable versions: {0}. /// diff --git a/src/Bicep.Core/CoreResources.resx b/src/Bicep.Core/CoreResources.resx index 03f39b56a7c..de27249ab42 100644 --- a/src/Bicep.Core/CoreResources.resx +++ b/src/Bicep.Core/CoreResources.resx @@ -381,6 +381,13 @@ Use parent property + + Parameters should have non-empty descriptions. + + + Parameter "{0}" must have a non-empty description. + {0} is the parameter name + Property '{0}' expects a secure value, but the value provided may not be secure. {0} property name diff --git a/src/vscode-bicep/schemas/bicepconfig.schema.json b/src/vscode-bicep/schemas/bicepconfig.schema.json index af1492c80a5..504741e193c 100644 --- a/src/vscode-bicep/schemas/bicepconfig.schema.json +++ b/src/vscode-bicep/schemas/bicepconfig.schema.json @@ -765,6 +765,16 @@ } ] }, + "use-parameter-descriptions": { + "allOf": [ + { + "description": "Parameters should have non-empty descriptions. Defaults to 'Off'. See https://aka.ms/bicep/linter-diagnostics#use-parameter-descriptions" + }, + { + "$ref": "#/definitions/rule-def-level-off" + } + ] + }, "use-safe-access": { "allOf": [ {