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.

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

SonarLintXmlReader refactoring #6924

Merged
merged 17 commits into from Mar 21, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -125,25 +125,8 @@ public bool HasMatchingScope(DiagnosticDescriptor descriptor)
// If ProjectType is not 'Unknown' it means we are in S4NET context and all files are analyzed.
// If ProjectType is 'Unknown' then we are in SonarLint or NuGet context and we need to check if the file has been excluded from analysis through SonarLint.xml.
ProjectConfiguration().ProjectType == ProjectType.Unknown
&& FileInclusionCache.GetValue(Compilation, _ => new()) is var cache
&& !cache.GetOrAdd(filePath, _ => IsFileIncluded(sonarLintXml, filePath));
&& !FileInclusionCache.GetValue(Compilation, _ => new()).GetOrAdd(filePath, _ => sonarLintXml.IsFileIncluded(filePath, IsTestProject()));
cristian-ambrosini-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

private ImmutableHashSet<string> CreateUnchangedFilesHashSet() =>
ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase, ProjectConfiguration().AnalysisConfig?.UnchangedFiles() ?? Array.Empty<string>());

private bool IsFileIncluded(SonarLintXmlReader sonarLintXml, string filePath) =>
IsTestProject()
? IsFileIncluded(sonarLintXml.TestInclusions, sonarLintXml.TestExclusions, sonarLintXml.GlobalTestExclusions, filePath)
: IsFileIncluded(sonarLintXml.Inclusions, sonarLintXml.Exclusions, sonarLintXml.GlobalExclusions, filePath);

private static bool IsFileIncluded(string[] inclusions, string[] exclusions, string[] globalExclusions, string filePath) =>
IsIncluded(inclusions, filePath)
&& !IsExcluded(exclusions, filePath)
&& !IsExcluded(globalExclusions, filePath);

private static bool IsIncluded(string[] inclusions, string filePath) =>
inclusions is { Length: 0 } || inclusions.Any(x => WildcardPatternMatcher.IsMatch(x, filePath, true));

private static bool IsExcluded(string[] exclusions, string filePath) =>
exclusions.Any(x => WildcardPatternMatcher.IsMatch(x, filePath, false));
}
2 changes: 1 addition & 1 deletion analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXml.cs
Expand Up @@ -23,7 +23,7 @@
namespace SonarAnalyzer.Helpers;

/// <summary>
/// DTO to represent the SonarLint.xml for our analyzers.
/// Data class to represent the SonarLint.xml for our analyzers.
/// </summary>
/// <remarks>
/// This class should not be used in this codebase. To get SonarLint.xml properties, use <see cref="SonarLintXmlReader"/>.
Expand Down
112 changes: 60 additions & 52 deletions analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
Expand Up @@ -19,8 +19,6 @@
*/

using System.IO;
using System.Text;
using System.Xml;
using System.Xml.Serialization;
using Microsoft.CodeAnalysis.Text;

Expand All @@ -29,52 +27,78 @@ namespace SonarAnalyzer.Helpers;
public class SonarLintXmlReader
{
public static readonly SonarLintXmlReader Empty = new(null);
private readonly bool ignoreHeaderCommentsCS;
private readonly bool ignoreHeaderCommentsVB;
private readonly bool analyzeGeneratedCodeCS;
private readonly bool analyzeGeneratedCodeVB;

public string[] Exclusions { get; }
public string[] Inclusions { get; }
public string[] GlobalExclusions { get; }
public string[] TestExclusions { get; }
public string[] TestInclusions { get; }
public string[] GlobalTestExclusions { get; }
public List<SonarLintXmlRule> ParametrizedRules { get; }

public SonarLintXmlReader(SourceText sonarLintXmlText)
{
var sonarLintXml = ParseContent(sonarLintXmlText);
var settings = sonarLintXml.Settings?.GroupBy(x => x.Key).ToDictionary(x => x.Key, x => x.First().Value) ?? new Dictionary<string, string>();
Exclusions = ReadArray("sonar.exclusions");
Inclusions = ReadArray("sonar.inclusions");
GlobalExclusions = ReadArray("sonar.global.exclusions");
TestExclusions = ReadArray("sonar.test.exclusions");
TestInclusions = ReadArray("sonar.test.inclusions");
GlobalTestExclusions = ReadArray("sonar.global.test.exclusions");
ParametrizedRules = ReadRuleParameters();
ignoreHeaderCommentsCS = ReadBoolean("sonar.cs.ignoreHeaderComments");
ignoreHeaderCommentsVB = ReadBoolean("sonar.vbnet.ignoreHeaderComments");
analyzeGeneratedCodeCS = ReadBoolean("sonar.cs.analyzeGeneratedCode");
analyzeGeneratedCodeVB = ReadBoolean("sonar.vbnet.analyzeGeneratedCode");

string[] ReadArray(string key) =>
settings.GetValueOrDefault(key) is { } value && !string.IsNullOrEmpty(value)
? value.Split(',')
: Array.Empty<string>();

bool ReadBoolean(string key) =>
bool.TryParse(settings.GetValueOrDefault(key), out var value) && value;

List<SonarLintXmlRule> ReadRuleParameters() =>
sonarLintXml.Rules?.Where(x => x.Parameters.Any()).ToList() ?? new();
}

private readonly SonarLintXml sonarLintXml;

private bool? ignoreHeaderCommentsCS;
private bool? ignoreHeaderCommentsVB;
public bool IgnoreHeaderComments(string language) =>
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
language switch
{
LanguageNames.CSharp => ignoreHeaderCommentsCS ??= ReadBoolean(ReadSettingsProperty("sonar.cs.ignoreHeaderComments")),
LanguageNames.VisualBasic => ignoreHeaderCommentsVB ??= ReadBoolean(ReadSettingsProperty("sonar.vbnet.ignoreHeaderComments")),
_ => throw new UnexpectedLanguageException(language)
};
language switch
{
LanguageNames.CSharp => ignoreHeaderCommentsCS,
LanguageNames.VisualBasic => ignoreHeaderCommentsVB,
_ => throw new UnexpectedLanguageException(language)
};

private bool? analyzeGeneratedCodeCS;
private bool? analyzeGeneratedCodeVB;
public bool AnalyzeGeneratedCode(string language) =>
language switch
{
LanguageNames.CSharp => analyzeGeneratedCodeCS ??= ReadBoolean(ReadSettingsProperty("sonar.cs.analyzeGeneratedCode")),
LanguageNames.VisualBasic => analyzeGeneratedCodeVB ??= ReadBoolean(ReadSettingsProperty("sonar.vbnet.analyzeGeneratedCode")),
LanguageNames.CSharp => analyzeGeneratedCodeCS,
LanguageNames.VisualBasic => analyzeGeneratedCodeVB,
_ => throw new UnexpectedLanguageException(language)
};

private string[] exclusions;
public string[] Exclusions => exclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.exclusions"));
public bool IsFileIncluded(string filePath, bool isTestProject) =>
isTestProject
? IsFileIncluded(TestInclusions, TestExclusions, GlobalTestExclusions, filePath)
: IsFileIncluded(Inclusions, Exclusions, GlobalExclusions, filePath);

private string[] inclusions;
public string[] Inclusions => inclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.inclusions"));
private static bool IsFileIncluded(string[] inclusions, string[] exclusions, string[] globalExclusions, string filePath) =>
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
IsIncluded(inclusions, filePath)
&& !IsExcluded(exclusions, filePath)
&& !IsExcluded(globalExclusions, filePath);

private string[] globalExclusions;
public string[] GlobalExclusions => globalExclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.global.exclusions"));
private static bool IsIncluded(string[] inclusions, string filePath) =>
inclusions.Length == 0 || inclusions.Any(x => WildcardPatternMatcher.IsMatch(x, filePath, true));

private string[] testExclusions;
public string[] TestExclusions => testExclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.test.exclusions"));

private string[] testInclusions;
public string[] TestInclusions => testInclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.test.inclusions"));

private string[] globalTestExclusions;
public string[] GlobalTestExclusions => globalTestExclusions ??= ReadCommaSeparatedArray(ReadSettingsProperty("sonar.global.test.exclusions"));

private List<SonarLintXmlRule> parametrizedRules;
public List<SonarLintXmlRule> ParametrizedRules => parametrizedRules ??= ReadRuleParameters();

public SonarLintXmlReader(SourceText sonarLintXml) =>
this.sonarLintXml = sonarLintXml == null ? SonarLintXml.Empty : ParseContent(sonarLintXml);
private static bool IsExcluded(string[] exclusions, string filePath) =>
exclusions.Any(x => WildcardPatternMatcher.IsMatch(x, filePath, false));

private static SonarLintXml ParseContent(SourceText sonarLintXml)
{
Expand All @@ -89,20 +113,4 @@ private static SonarLintXml ParseContent(SourceText sonarLintXml)
return SonarLintXml.Empty;
}
}

private List<SonarLintXmlRule> ReadRuleParameters() =>
sonarLintXml is { Rules: { } rules }
? rules.Where(x => x.Parameters.Any()).ToList()
: new();

private string ReadSettingsProperty(string property) =>
sonarLintXml is { Settings: { } settings }
? settings.Where(x => x.Key.Equals(property)).Select(x => x.Value).FirstOrDefault()
: null;

private static string[] ReadCommaSeparatedArray(string str) =>
string.IsNullOrEmpty(str) ? Array.Empty<string>() : str.Split(',');

private static bool ReadBoolean(string str, bool defaultValue = false) =>
bool.TryParse(str, out var propertyValue) ? propertyValue : defaultValue;
}
Expand Up @@ -82,6 +82,22 @@ public void SonarLintXmlReader_PropertiesCSharpTrueVBNetFalse_ExpectedValues()
sut.AnalyzeGeneratedCode(LanguageNames.VisualBasic).Should().BeFalse();
}

[TestMethod]
public void SonarLintXmlReader_DuplicatedProperties_DoesNotFail()
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throwing is asserted via lambdas.

Suggested change
{
public void SonarLintXmlReader_DuplicatedProperties_DoesNotFail() =>
((Action)(() => CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Duplicated_Properties\\SonarLint.xml"))).Should().NotThrow();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly prefer @"asdf\asdf" over double \\ - this applies everywhere. The \\ is cumbersome to work with, copy paste and use in general.

try
{
_ = CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Duplicated_Properties\\SonarLint.xml");
}
catch (ArgumentException ex)
{
if (ex.Message.Contains("An item with the same key has already been added."))
{
Assert.Fail("Expected no exception, but got: " + ex.Message);
}
}
}

[DataTestMethod]
[DataRow("")]
[DataRow("this is not an xml")]
Expand Down
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="UTF-8"?>
<AnalysisInput>
<Settings>
<Setting>
<Key>sonar.cs.ignoreHeaderComments</Key>
<Value>true</Value>
</Setting>
<Setting>
<Key>sonar.cs.ignoreHeaderComments</Key>
<Value>true</Value>
</Setting>
<Setting>
<Key>sonar.cs.analyzeGeneratedCode</Key>
<Value>true</Value>
</Setting>
<Setting>
<Key>sonar.cs.analyzeGeneratedCode</Key>
<Value>false</Value>
</Setting>
<Setting>
<Key>sonar.exclusions</Key>
<Value>Fake/Exclusions/**/*,Fake/Exclusions/Second*/**/*</Value>
</Setting>
<Setting>
<Key>sonar.exclusions</Key>
<Value>Fake/Inclusions/**/*</Value>
</Setting>
</Settings>
</AnalysisInput>