Skip to content

Commit c3bd4e4

Browse files
author
duncanp
committed
[SONARMSBRU-201] Change ruleset Include action to "Warning"
Updated tests
1 parent 60fd6dd commit c3bd4e4

File tree

4 files changed

+42
-37
lines changed

4 files changed

+42
-37
lines changed

‎SonarQube.MSBuild.Tasks/MergeRulesets.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class MergeRuleSets : Task
2424
private static readonly XName IncludeElementName = "Include";
2525
private static readonly XName PathAttributeName = "Path";
2626
private static readonly XName ActionAttributeName = "Action";
27-
private const string DefaultActionValue = "Default";
27+
private const string IncludeActionValue = "Warning";
2828

2929
#region Input properties
3030

@@ -128,24 +128,26 @@ private string TryResolveIncludedRuleset(string includePath)
128128

129129
private void EnsureIncludeExists(XDocument ruleset, string includePath)
130130
{
131-
if (IncludeExists(ruleset, includePath))
131+
XElement includeElement = FindExistingInclude(ruleset, includePath);
132+
if (includeElement != null)
132133
{
133134
this.Log.LogMessage(MessageImportance.Low, Resources.MergeRulesets_RulesetAlreadyIncluded, includePath);
134135
}
135136
else
136137
{
137138
this.Log.LogMessage(MessageImportance.Low, Resources.MergeRuleset_IncludingRuleset, includePath);
138-
XElement newInclude = new XElement(IncludeElementName);
139-
newInclude.SetAttributeValue(PathAttributeName, includePath);
140-
newInclude.SetAttributeValue(ActionAttributeName, DefaultActionValue);
141-
142-
ruleset.Root.AddFirst(newInclude);
139+
includeElement = new XElement(IncludeElementName);
140+
includeElement.SetAttributeValue(PathAttributeName, includePath);
141+
ruleset.Root.AddFirst(includeElement);
143142
}
143+
144+
// Ensure the include (new or existing) has the desired Action value
145+
includeElement.SetAttributeValue(ActionAttributeName, IncludeActionValue);
144146
}
145147

146-
private static bool IncludeExists(XDocument ruleset, string includePath)
148+
private static XElement FindExistingInclude(XDocument ruleset, string includePath)
147149
{
148-
return ruleset.Descendants(IncludeElementName).Any(e =>
150+
return ruleset.Descendants(IncludeElementName).FirstOrDefault(e =>
149151
{
150152
XAttribute attr = e.Attribute(PathAttributeName);
151153
return attr != null && string.Equals(includePath, attr.Value, StringComparison.OrdinalIgnoreCase);

‎Tests/SonarQube.MSBuild.Tasks.IntegrationTests/TargetsTests/RoslynTargetsTests.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ public void Roslyn_Settings_ValidSetup_MergeWithExistingSettings_AbsolutePath()
9494
BuildAssertions.AssertWarningsAreNotTreatedAsErrors(result);
9595

9696
string mergedRulesetFilePath = AssertMergedResultFileExists(result, sourceRulesetFilePath);
97-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRulesetFilePath, "c:\\existing.ruleset");
98-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRulesetFilePath, "c:\\existing.ruleset", RuleSetAssertions.DefaultActionValue);
97+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRulesetFilePath, "c:\\existing.ruleset");
9998
}
10099

101100
[TestMethod]
@@ -138,8 +137,7 @@ public void Roslyn_Settings_ValidSetup_MergeWithExistingSettings_RelativePath()
138137
AssertExpectedAdditionalFiles(result, GetDummySonarLintXmlFilePath());
139138

140139
string mergedRulesetFilePath = AssertMergedResultFileExists(result, sourceRulesetFilePath);
141-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRulesetFilePath, rulesetFilePath);
142-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRulesetFilePath, rulesetFilePath, RuleSetAssertions.DefaultActionValue);
140+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRulesetFilePath, rulesetFilePath);
143141
}
144142

145143
[TestMethod]

‎Tests/SonarQube.MSBuild.Tasks.UnitTests/MergeRuleSetsTests.cs‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,24 +63,19 @@ public void MergeRulesets_IncludeRulesets_AbsolutePaths()
6363
// 1. No included rulesets
6464
string primaryRuleset = this.CreateValidRuleset("no-added-includes.ruleset.txt");
6565
mergedRuleset = ExecuteAndCheckSuccess(projectDir, primaryRuleset);
66-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset /* no included files*/);
66+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset /* no included files*/);
6767

6868
// 2. Adding includes
6969
primaryRuleset = this.CreateValidRuleset("added-includes1.ruleset.txt");
7070
mergedRuleset = ExecuteAndCheckSuccess(projectDir, primaryRuleset, "c:\\include1.ruleset");
71-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset, "c:\\include1.ruleset");
72-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRuleset, "c:\\include1.ruleset", RuleSetAssertions.DefaultActionValue);
71+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset, "c:\\include1.ruleset");
7372

7473
// 3. Adding an include that already exists
7574
primaryRuleset = this.CreateValidRuleset("added-includes.existing.ruleset.txt");
7675
string rulesetWithExistingInclude = ExecuteAndCheckSuccess(projectDir, primaryRuleset, "c:\\include1.ruleset", "c:\\include2.ruleset"); // create a file with includes
7776

7877
mergedRuleset = ExecuteAndCheckSuccess(projectDir, rulesetWithExistingInclude, "c:\\include1.ruleset", "c:\\INCLUDE2.RULESET", "c:\\include3.ruleset"); // add the same includes again with one extra
79-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset, "c:\\include1.ruleset", "c:\\include2.ruleset", "c:\\include3.ruleset");
80-
81-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRuleset, "c:\\include1.ruleset", RuleSetAssertions.DefaultActionValue);
82-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRuleset, "c:\\include2.ruleset", RuleSetAssertions.DefaultActionValue);
83-
RuleSetAssertions.AssertExpectedIncludeAction(mergedRuleset, "c:\\include3.ruleset", RuleSetAssertions.DefaultActionValue);
78+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset, "c:\\include1.ruleset", "c:\\include2.ruleset", "c:\\include3.ruleset");
8479
}
8580

8681
[TestMethod]
@@ -94,20 +89,20 @@ public void MergeRulesets_IncludeRulesets_RelativePaths()
9489
// 1. Relative ruleset path that can be resolved -> included
9590
string primaryRuleset = this.CreateValidRuleset("found.relative.ruleset.txt");
9691
mergedRuleset = ExecuteAndCheckSuccess(projectDir, primaryRuleset, "relative.ruleset");
97-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset, absoluteRulesetPath);
92+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset, absoluteRulesetPath);
9893

9994
// 1. Relative ruleset path that can be resolved -> included
10095
primaryRuleset = this.CreateValidRuleset("found.relative2.ruleset.txt");
10196
mergedRuleset = ExecuteAndCheckSuccess(projectDir, primaryRuleset,
10297
".\\relative.ruleset", // should be resolved correctly...
10398
".\\.\\relative.ruleset"); // ... but only added once.
10499

105-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset, absoluteRulesetPath);
100+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset, absoluteRulesetPath);
106101

107102
// 2. Relative ruleset path that cannot be resolved -> not included
108103
primaryRuleset = this.CreateValidRuleset("not.found.relative.ruleset.txt");
109104
mergedRuleset = ExecuteAndCheckSuccess(projectDir, primaryRuleset, "not.found\\relative.ruleset");
110-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset /* ruleset should not have been resolved */);
105+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset /* ruleset should not have been resolved */);
111106
}
112107

113108
[TestMethod]
@@ -122,7 +117,7 @@ public void MergeRulesets_EmptyRuleset()
122117
string mergedRuleset = ExecuteAndCheckSuccess(testDir, primaryRuleset, "c:\\foo\\added.ruleset");
123118

124119
// Assert
125-
RuleSetAssertions.AssertExpectedIncludeFiles(mergedRuleset, "c:\\foo\\added.ruleset");
120+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset, "c:\\foo\\added.ruleset");
126121
}
127122

128123

@@ -134,18 +129,29 @@ public void MergeRulesets_ExistingInclude()
134129
string primaryRuleset = TestUtils.CreateTextFile(testDir, "existing.ruleset.txt",
135130
@"<?xml version=""1.0"" encoding=""utf-8""?>
136131
<RuleSet Name=""RulesetName"" ToolsVersion=""14.0"">
137-
<Include Path=""d:\include1.ruleset"" Action=""Error"" />
132+
<Include Path=""d:\error.include.ruleset"" Action=""Error"" />
133+
<Include Path=""d:\info.include.ruleset"" Action=""Info"" />
134+
<Include Path=""d:\warning.include.ruleset"" Action=""Warning"" />
135+
<Include Path=""d:\default.include.ruleset"" Action=""Default"" />
138136
<Rules AnalyzerId=""My.Analyzer"" RuleNamespace=""My.Analyzers"">
139137
<Rule Id=""Rule002"" Action=""Error"" />
140138
</Rules>
141139
</RuleSet>
142140
");
143141
// Act
144-
ExecuteAndCheckSuccess(testDir, primaryRuleset, "d:\\include1.ruleset");
142+
string mergedRuleset = ExecuteAndCheckSuccess(testDir, primaryRuleset,
143+
"d:\\error.include.ruleset",
144+
"d:\\info.include.ruleset",
145+
"d:\\warning.include.ruleset",
146+
"d:\\default.include.ruleset");
145147

146148
// Assert
147-
RuleSetAssertions.AssertExpectedIncludeFiles(primaryRuleset, "d:\\include1.ruleset");
148-
RuleSetAssertions.AssertExpectedIncludeAction(primaryRuleset, "d:\\include1.ruleset", ErrorActionValue); // action value should still be Error
149+
RuleSetAssertions.AssertExpectedIncludeFilesAndDefaultAction(mergedRuleset,
150+
// Action value should be "Warning" for all included rulesets
151+
"d:\\error.include.ruleset",
152+
"d:\\info.include.ruleset",
153+
"d:\\warning.include.ruleset",
154+
"d:\\default.include.ruleset");
149155
}
150156

151157
#endregion

‎Tests/TestUtilities/RulesetAssertions.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,30 +15,29 @@ namespace TestUtilities
1515
{
1616
public static class RuleSetAssertions
1717
{
18-
public const string DefaultActionValue = "Default";
18+
public const string WarningActionValue = "Warning";
1919

2020
private static readonly XName IncludeElementName = "Include";
2121
private static readonly XName PathAttrName = "Path";
2222
private static readonly XName ActionAttrName = "Action";
2323

24-
public static void AssertExpectedIncludeFiles(string rulesetFilePath, params string[] expectedIncludePaths)
24+
public static void AssertExpectedIncludeFilesAndDefaultAction(string rulesetFilePath, params string[] expectedIncludePaths)
2525
{
2626
XDocument doc = XDocument.Load(rulesetFilePath);
2727
IEnumerable<XElement> includeElements = doc.Descendants(IncludeElementName);
2828
foreach (string expected in expectedIncludePaths)
2929
{
30-
AssertSingleIncludeExists(includeElements, expected);
30+
XElement includeElement = AssertSingleIncludeExists(includeElements, expected);
31+
32+
// We expect the Include Action to always be "Warning"
33+
AssertExpectedIncludeAction(includeElement, WarningActionValue);
3134
}
3235

3336
Assert.AreEqual(expectedIncludePaths.Length, includeElements.Count(), "Unexpected number of Includes");
3437
}
3538

36-
public static void AssertExpectedIncludeAction(string rulesetFilePath, string includePath, string expectedAction)
39+
private static void AssertExpectedIncludeAction(XElement includeElement, string expectedAction)
3740
{
38-
XDocument doc = XDocument.Load(rulesetFilePath);
39-
IEnumerable<XElement> includeElements = doc.Descendants(IncludeElementName);
40-
XElement includeElement = AssertSingleIncludeExists(includeElements, includePath);
41-
4241
XAttribute actionAttr = includeElement.Attribute(ActionAttrName);
4342

4443
Assert.IsNotNull(actionAttr, "Include element does not have an Action attribute: {0}", includeElement);

0 commit comments

Comments
 (0)