Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of no-hardcoded-env-urls linter rule #3833

Merged
merged 5 commits into from
Jul 30, 2021
Merged

Improve performance of no-hardcoded-env-urls linter rule #3833

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1276ff7
Test replacing the regex with basic string matching
anthony-c-martin Jul 27, 2021
6143e1b
Add checks for valid subdomain vs substring
jfleisher Jul 29, 2021
ea84ed4
Refactor
anthony-c-martin Jul 30, 2021
1eae3e8
Refactor 2
anthony-c-martin Jul 30, 2021
46bc88c
Third time lucky
anthony-c-martin Jul 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions docs/linter-rules/no-hardcoded-env-urls.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ The set of URL hosts to disallow may be customized using the `disallowedHosts` p
"no-hardcoded-env-urls": {
"level": "warning",
"disallowedHosts": [
"management.core.windows.net",
"gallery.azure.com",
"management.core.windows.net",
"management.azure.com",
Expand All @@ -45,17 +44,11 @@ The set of URL hosts to disallow may be customized using the `disallowedHosts` p
"login.microsoftonline.com",
"graph.windows.net",
"trafficmanager.net",
"vault.azure.net",
"datalake.azure.net",
"azuredatalakestore.net",
"azuredatalakeanalytics.net",
"vault.azure.net",
"api.loganalytics.io",
"api.loganalytics.iov1",
"asazure.windows.net",
"region.asazure.windows.net",
"api.loganalytics.iov1",
"api.loganalytics.io",
"asazure.windows.net",
"region.asazure.windows.net",
"batch.core.windows.net"
Expand Down
32 changes: 19 additions & 13 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/LinterRuleTestsBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,37 @@
using Bicep.Core.Analyzers.Linter;
using Bicep.Core.Analyzers.Linter.Rules;
using Bicep.Core.Diagnostics;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Utils;
using FluentAssertions;
using FluentAssertions.Execution;
using System;
using System.Collections.Generic;
using System.Linq;

namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests
{
public class LinterRuleTestsBase
{
virtual protected void CompileAndTest(string ruleCode, string text, int expectedDiagnosticCount)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(ruleCode, text);
errors.Should().HaveCount(expectedDiagnosticCount);
}
}
protected void CompileAndTest(string ruleCode, string text, int expectedDiagnosticCount)
=> AssertRuleCodeDiagnostics(ruleCode, text, diags => diags.Should().HaveCount(expectedDiagnosticCount));

protected IDiagnostic[] GetDiagnostics(string ruleCode, string text)
protected void AssertRuleCodeDiagnostics(string ruleCode, string text, Action<IEnumerable<IDiagnostic>> assertAction)
=> DoWithDiagnosticAnnotations(
text,
diag => diag.Code == ruleCode,
assertAction);

private void DoWithDiagnosticAnnotations(string text, Func<IDiagnostic, bool> filterFunc, Action<IEnumerable<IDiagnostic>> assertAction)
{
var compilationResult = CompilationHelper.Compile(text);
var internalRuleErrors = compilationResult.Diagnostics.Where(d => d.Code == LinterAnalyzer.FailedRuleCode).ToArray();
internalRuleErrors.Count().Should().Be(0, "There should never be linter FailedRuleCode errors");
var result = CompilationHelper.Compile(text);

result.Should().NotHaveDiagnosticsWithCodes(new [] { LinterAnalyzer.FailedRuleCode }, "There should never be linter FailedRuleCode errors");

return compilationResult.Diagnostics.OfType<IDiagnostic>().Where(d => d.Code == ruleCode).ToArray();
DiagnosticAssertions.DoWithDiagnosticAnnotations(
result.Compilation.SourceFileGrouping.EntryPoint,
result.Diagnostics.Where(filterFunc),
assertAction);
}
}
}
21 changes: 5 additions & 16 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoHardcodedEnvironmentUrlsRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,11 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Linq;
using Azure.Deployments.Core.Extensions;
using Bicep.Core.Analyzers.Linter.Rules;
using Bicep.Core.Configuration;
using Bicep.Core.Text;
using Bicep.Core.Diagnostics;
using Bicep.Core.UnitTests.Assertions;
using Bicep.Core.UnitTests.Utils;
using FluentAssertions;
using FluentAssertions.Execution;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;

// TODO: Test with different configs
namespace Bicep.Core.UnitTests.Diagnostics.LinterRuleTests
Expand Down Expand Up @@ -162,14 +153,13 @@ public void InsideExpressions(int diagnosticCount, string text)
[DataRow("subdomain1.management.azzzure.com", false)]
[DataRow("http://subdomain1.manageeeement.azure.com", false)]
[DataRow("https://subdomain1.managemeeeent.azure.com", false)]
public void DisallowedHostsRegexTest(string host, bool isMatch)
public void DisallowedHostsMatchingTest(string testString, bool isMatch)
{
var rule = new NoHardcodedEnvironmentUrlsRule();
var configHelper = new ConfigHelper(); // this ensures configuration is loaded from resources
rule.Configure(configHelper.Config);

var regex = rule.CreateDisallowedHostRegex();
Assert.AreEqual(isMatch, regex.IsMatch(host));
Assert.AreEqual(isMatch, actual: rule.DisallowedHosts.Any(host => NoHardcodedEnvironmentUrlsRule.Visitor.FindHostnameMatches(host, testString).Any()));
}

[DataTestMethod]
Expand All @@ -192,14 +182,13 @@ public void DisallowedHostsRegexTest(string host, bool isMatch)
[DataRow("https://subdomain1.management.azure.com", false)]
[DataRow("all the world is a stage, but subdomain1.management.azure.com should not be hardcoded", false)]
[DataRow("all the world is a stage, but subdomain1.schema.management.azure.com should not be hardcoded", true)]
public void ExcludedHostsRegexTest(string host, bool isMatch)
public void ExcludedHostsMatchingTest(string testString, bool isMatch)
{
var rule = new NoHardcodedEnvironmentUrlsRule();
var configHelper = new ConfigHelper(); // this ensures configuration is loaded from resources
rule.Configure(configHelper.Config);

var regex = rule.CreateExcludedHostsRegex();
Assert.AreEqual(isMatch, regex.IsMatch(host));
Assert.AreEqual(isMatch, rule.ExcludedHosts.Any(host => NoHardcodedEnvironmentUrlsRule.Visitor.FindHostnameMatches(host, testString).Any()));
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoUnusedParametersRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,18 @@ public void ParameterNameInFormattedMessage()

private void CompileAndTest(string text, params string[] unusedParams)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(NoUnusedParametersRule.Code, text);
AssertRuleCodeDiagnostics(NoUnusedParametersRule.Code, text, diags => {
if (unusedParams.Any())
{
var rule = new NoUnusedParametersRule();
string[] expectedMessages = unusedParams.Select(p => rule.GetMessage(p)).ToArray();
errors.Select(e => e.Message).Should().ContainInOrder(expectedMessages);
diags.Select(e => e.Message).Should().ContainInOrder(expectedMessages);
}
else
{
errors.Should().BeEmpty();
diags.Should().BeEmpty();
}
}
});
}

[DataRow(@"
Expand Down
10 changes: 4 additions & 6 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/NoUnusedVariablesRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,18 @@ public void VariableNameInFormattedMessage()

private void CompileAndTest(string text, params string[] unusedVars)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(NoUnusedVariablesRule.Code, text);
AssertRuleCodeDiagnostics(NoUnusedVariablesRule.Code, text, diags => {
if (unusedVars.Any())
{
var rule = new NoUnusedVariablesRule();
string[] expectedMessages = unusedVars.Select(p => rule.GetMessage(p)).ToArray();
errors.Select(e => e.Message).Should().ContainInOrder(expectedMessages);
diags.Select(e => e.Message).Should().ContainInOrder(expectedMessages);
}
else
{
errors.Should().BeEmpty();
diags.Should().BeEmpty();
}
}
});
}

[DataRow(@"
Expand Down
22 changes: 9 additions & 13 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/PreferInterpolationRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ public class PreferInterpolationRuleTests : LinterRuleTestsBase
{
private void ExpectPass(string text)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(PreferInterpolationRule.Code, text);
errors.Should().HaveCount(0, $"expecting linter rule to pass");
}
AssertRuleCodeDiagnostics(PreferInterpolationRule.Code, text, diags => {
diags.Should().HaveCount(0, $"expecting linter rule to pass");
});
}

private void ExpectDiagnosticWithFix(string text, string expectedFix)
Expand All @@ -33,15 +31,13 @@ private void ExpectDiagnosticWithFix(string text, string expectedFix)

private void ExpectDiagnosticWithFix(string text, string[] expectedFixes)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(PreferInterpolationRule.Code, text);
errors.Should().HaveCount(expectedFixes.Length, $"expecting one fix per testcase");
AssertRuleCodeDiagnostics(PreferInterpolationRule.Code, text, diags => {
diags.Should().HaveCount(expectedFixes.Length, $"expecting one fix per testcase");

errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
var a = errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.SelectMany(f => f.Replacements.SelectMany(r => r.Text));
}
diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
var a = diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.SelectMany(f => f.Replacements.SelectMany(r => r.Text));
});
}

[DataRow(@"
Expand Down
22 changes: 9 additions & 13 deletions src/Bicep.Core.UnitTests/Diagnostics/LinterRuleTests/SimplifyInterpolationRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,20 @@ public class SimplifyInterpolationRuleTests : LinterRuleTestsBase
{
private void ExpectPass(string text)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(SimplifyInterpolationRule.Code, text);
errors.Should().HaveCount(0, $"expecting linter rule to pass");
}
AssertRuleCodeDiagnostics(SimplifyInterpolationRule.Code, text, diags => {
diags.Should().HaveCount(0, $"expecting linter rule to pass");
});
}

private void ExpectDiagnosticWithFix(string text, string expectedFix)
{
using (new AssertionScope($"linter errors for this code:\n{text}\n"))
{
var errors = GetDiagnostics(SimplifyInterpolationRule.Code, text);
errors.Should().HaveCount(1, $"expected one fix per testcase");
AssertRuleCodeDiagnostics(SimplifyInterpolationRule.Code, text, diags => {
diags.Should().HaveCount(1, $"expected one fix per testcase");

errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
errors.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.First().Text.Should().Be(expectedFix);
}
diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.Should().HaveCount(1);
diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.Should().HaveCount(1);
diags.First().As<IBicepAnalyerFixableDiagnostic>().Fixes.First().Replacements.First().Text.Should().Be(expectedFix);
});
}

[DataRow(
Expand Down
Loading