SonarLintXmlReader refactoring (#6924)

SonarSource · Mar 21, 2023 · 082afb7 · 082afb7
1 parent d335c96
commit 082afb7
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 77 deletions.
diff --git a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarAnalysisContextBase.cs b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarAnalysisContextBase.cs
@@ -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()));
 
     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));
 }
diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXml.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXml.cs
@@ -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"/>.

diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/SonarLintXmlReader.cs
@@ -19,8 +19,6 @@
  */
 
 using System.IO;
-using System.Text;
-using System.Xml;
 using System.Xml.Serialization;
 using Microsoft.CodeAnalysis.Text;
 
@@ -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) =>
-        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) =>
+        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)
     {
@@ -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;
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SonarLintXmlReaderTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/SonarLintXmlReaderTest.cs
@@ -32,7 +32,7 @@ public class SonarLintXmlReaderTest
     [DataRow(LanguageNames.VisualBasic, "vbnet")]
     public void SonarLintXmlReader_WhenAllValuesAreSet_ExpectedValues(string language, string xmlLanguageName)
     {
-        var sut = CreateSonarLintXmlReader($"ResourceTests\\SonarLintXml\\All_Properties_{xmlLanguageName}\\SonarLint.xml");
+        var sut = CreateSonarLintXmlReader(@$"ResourceTests\SonarLintXml\All_Properties_{xmlLanguageName}\SonarLint.xml");
         sut.IgnoreHeaderComments(language).Should().BeTrue();
         sut.AnalyzeGeneratedCode(language).Should().BeFalse();
         AssertArrayContent(sut.Exclusions, nameof(sut.Exclusions));
@@ -60,7 +60,7 @@ static void AssertArrayContent(string[] array, string folder)
     [TestMethod]
     public void SonarLintXmlReader_PartiallyMissingProperties_ExpectedAndDefaultValues()
     {
-        var sut = CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Partially_missing_properties\\SonarLint.xml");
+        var sut = CreateSonarLintXmlReader(@"ResourceTests\SonarLintXml\Partially_missing_properties\SonarLint.xml");
         sut.IgnoreHeaderComments(LanguageNames.CSharp).Should().BeFalse();
         sut.AnalyzeGeneratedCode(LanguageNames.CSharp).Should().BeTrue();
         AssertArrayContent(sut.Exclusions, nameof(sut.Exclusions));
@@ -75,13 +75,17 @@ public void SonarLintXmlReader_PartiallyMissingProperties_ExpectedAndDefaultValu
     [TestMethod]
     public void SonarLintXmlReader_PropertiesCSharpTrueVBNetFalse_ExpectedValues()
     {
-        var sut = CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\PropertiesCSharpTrueVbnetFalse\\SonarLint.xml");
+        var sut = CreateSonarLintXmlReader(@"ResourceTests\SonarLintXml\PropertiesCSharpTrueVbnetFalse\SonarLint.xml");
         sut.IgnoreHeaderComments(LanguageNames.CSharp).Should().BeTrue();
         sut.IgnoreHeaderComments(LanguageNames.VisualBasic).Should().BeFalse();
         sut.AnalyzeGeneratedCode(LanguageNames.CSharp).Should().BeTrue();
         sut.AnalyzeGeneratedCode(LanguageNames.VisualBasic).Should().BeFalse();
     }
 
+    [TestMethod]
+    public void SonarLintXmlReader_DuplicatedProperties_DoesNotFail() =>
+        ((Action)(() => CreateSonarLintXmlReader(@"ResourceTests\SonarLintXml\Duplicated_Properties\SonarLint.xml"))).Should().NotThrow();
+
     [DataTestMethod]
     [DataRow("")]
     [DataRow("this is not an xml")]
@@ -91,11 +95,11 @@ public void SonarLintXmlReader_PropertiesCSharpTrueVBNetFalse_ExpectedValues()
 
     [TestMethod]
     public void SonarLintXmlReader_MissingProperties_DefaultBehaviour() =>
-        CheckSonarLintXmlReaderDefaultValues(CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Missing_properties\\SonarLint.xml"));
+        CheckSonarLintXmlReaderDefaultValues(CreateSonarLintXmlReader(@"ResourceTests\SonarLintXml\Missing_properties\SonarLint.xml"));
 
     [TestMethod]
     public void SonarLintXmlReader_WithIncorrectValueType_DefaultBehaviour() =>
-        CheckSonarLintXmlReaderDefaultValues(CreateSonarLintXmlReader("ResourceTests\\SonarLintXml\\Incorrect_value_type\\SonarLint.xml"));
+        CheckSonarLintXmlReaderDefaultValues(CreateSonarLintXmlReader(@"ResourceTests\SonarLintXml\Incorrect_value_type\SonarLint.xml"));
 
     [TestMethod]
     public void SonarLintXmlReader_CheckEmpty_DefaultBehaviour() =>
@@ -104,7 +108,7 @@ public void SonarLintXmlReader_PropertiesCSharpTrueVBNetFalse_ExpectedValues()
     [TestMethod]
     public void SonarLintXmlReader_LanguageDoesNotExist_Throws()
     {
-        var sut = CreateSonarLintXmlReader($"ResourceTests\\SonarLintXml\\All_Properties_cs\\SonarLint.xml");
+        var sut = CreateSonarLintXmlReader(@$"ResourceTests\SonarLintXml\All_Properties_cs\SonarLint.xml");
         sut.Invoking(x => x.IgnoreHeaderComments(LanguageNames.FSharp)).Should().Throw<UnexpectedLanguageException>().WithMessage("Unexpected language: F#");
         sut.Invoking(x => x.AnalyzeGeneratedCode(LanguageNames.FSharp)).Should().Throw<UnexpectedLanguageException>().WithMessage("Unexpected language: F#");
     }

diff --git a/...sts/SonarAnalyzer.UnitTest/ResourceTests/SonarLintXml/Duplicated_Properties/SonarLint.xml b/...sts/SonarAnalyzer.UnitTest/ResourceTests/SonarLintXml/Duplicated_Properties/SonarLint.xml
@@ -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>