Only report SA1206 once on a given identifier

sharwell · sharwell · commit 0d967013dbc9 · 2020-09-03T23:58:54.000-07:00
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1206CodeFixProviderUnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1206CodeFixProviderUnitTests.cs
@@ -116,7 +116,6 @@ public async Task VerifyKeywordReorderingInMethodDeclarationAsync()
             DiagnosticResult[] expected =
             {
                 Diagnostic().WithLocation(1, 38).WithArguments("static", "new"),
-                Diagnostic().WithLocation(1, 45).WithArguments("public", "static"),
                 Diagnostic().WithLocation(1, 45).WithArguments("public", "new"),
             };
             await VerifyCSharpFixAsync(testCode, expected, fixedTestCode, CancellationToken.None).ConfigureAwait(false);
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1206UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/OrderingRules/SA1206UnitTests.cs
@@ -40,7 +40,6 @@ public async Task TestKeywordsWithExpressionBodiedMemberAndAsyncKeywordAsync()
             DiagnosticResult[] expected = new[]
             {
                 Diagnostic().WithLocation(3, 11).WithArguments("static", "async"),
-                Diagnostic().WithLocation(3, 18).WithArguments("public", "static"),
                 Diagnostic().WithLocation(3, 18).WithArguments("public", "async"),
             };
 
@@ -75,7 +74,6 @@ static public void ThirdMethod() {}
                 Diagnostic().WithLocation(7, 32).WithArguments("protected", "async"),
                 Diagnostic().WithLocation(8, 16).WithArguments("static", "new"),
                 Diagnostic().WithLocation(9, 9).WithArguments("static", "new"),
-                Diagnostic().WithLocation(9, 16).WithArguments("public", "static"),
                 Diagnostic().WithLocation(9, 16).WithArguments("public", "new"),
                 Diagnostic().WithLocation(10, 13).WithArguments("public", "virtual"),
                 Diagnostic().WithLocation(11, 12).WithArguments("public", "extern"),
@@ -160,7 +158,6 @@ public async Task TestKeywordsWithOperatorDeclarationsAsync()
                 Diagnostic().WithLocation(3, 12).WithArguments("public", "extern"),
                 Diagnostic().WithLocation(3, 19).WithArguments("static", "extern"),
                 Diagnostic().WithLocation(4, 12).WithArguments("static", "extern"),
-                Diagnostic().WithLocation(4, 19).WithArguments("public", "static"),
                 Diagnostic().WithLocation(4, 19).WithArguments("public", "extern"),
                 Diagnostic().WithLocation(5, 19).WithArguments("static", "extern"),
             };
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1206DeclarationKeywordsMustFollowOrder.cs b/StyleCop.Analyzers/StyleCop.Analyzers/OrderingRules/SA1206DeclarationKeywordsMustFollowOrder.cs
@@ -95,14 +95,26 @@ private static void CheckModifiersOrderAndReportDiagnostics(SyntaxNodeAnalysisCo
             {
                 var currentModifierType = GetModifierType(modifier);
 
+                bool reportPreviousModifier = false;
+                bool reportPreviousOtherModifier = false;
                 if (CompareModifiersType(currentModifierType, previousModifierType) < 0)
                 {
-                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, previousModifier.ValueText));
+                    reportPreviousModifier = true;
                 }
 
                 if (AccessOrStaticModifierNotFollowingOtherModifier(currentModifierType, previousModifierType) && otherModifiersAppearEarlier)
                 {
-                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, previousOtherModifier.ValueText));
+                    reportPreviousOtherModifier = true;
+                }
+
+                if (reportPreviousModifier || reportPreviousOtherModifier)
+                {
+                    // Note: Only report one diagnostic per modifier. If both diagnostics apply, report the diagnostic
+                    // relative to the earlier modifier.
+                    var reportedModifier = reportPreviousModifier && (!reportPreviousOtherModifier || previousModifier.SpanStart < previousOtherModifier.SpanStart)
+                        ? previousModifier.ValueText
+                        : previousOtherModifier.ValueText;
+                    context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, reportedModifier));
                 }
 
                 if (!otherModifiersAppearEarlier && currentModifierType == ModifierType.Other)

Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,6 @@ public async Task VerifyKeywordReorderingInMethodDeclarationAsync()`
`116`	`116`	`DiagnosticResult[] expected =`
`117`	`117`	`{`
`118`	`118`	`Diagnostic().WithLocation(1, 38).WithArguments("static", "new"),`
`119`		`- Diagnostic().WithLocation(1, 45).WithArguments("public", "static"),`
`120`	`119`	`Diagnostic().WithLocation(1, 45).WithArguments("public", "new"),`
`121`	`120`	`};`
`122`	`121`	`await VerifyCSharpFixAsync(testCode, expected, fixedTestCode, CancellationToken.None).ConfigureAwait(false);`
Original file line number	Diff line number	Diff line change
`@@ -95,14 +95,26 @@ private static void CheckModifiersOrderAndReportDiagnostics(SyntaxNodeAnalysisCo`
`95`	`95`	`{`
`96`	`96`	`var currentModifierType = GetModifierType(modifier);`
`97`	`97`
	`98`	`+ bool reportPreviousModifier = false;`
	`99`	`+ bool reportPreviousOtherModifier = false;`
`98`	`100`	`if (CompareModifiersType(currentModifierType, previousModifierType) < 0)`
`99`	`101`	`{`
`100`		`- context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, previousModifier.ValueText));`
	`102`	`+ reportPreviousModifier = true;`
`101`	`103`	`}`
`102`	`104`
`103`	`105`	`if (AccessOrStaticModifierNotFollowingOtherModifier(currentModifierType, previousModifierType) && otherModifiersAppearEarlier)`
`104`	`106`	`{`
`105`		`- context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, previousOtherModifier.ValueText));`
	`107`	`+ reportPreviousOtherModifier = true;`
	`108`	`+ }`
	`109`	`+`
	`110`	`+ if (reportPreviousModifier \|\| reportPreviousOtherModifier)`
	`111`	`+ {`
	`112`	`+ // Note: Only report one diagnostic per modifier. If both diagnostics apply, report the diagnostic`
	`113`	`+ // relative to the earlier modifier.`
	`114`	`+ var reportedModifier = reportPreviousModifier && (!reportPreviousOtherModifier \|\| previousModifier.SpanStart < previousOtherModifier.SpanStart)`
	`115`	`+ ? previousModifier.ValueText`
	`116`	`+ : previousOtherModifier.ValueText;`
	`117`	`+ context.ReportDiagnostic(Diagnostic.Create(Descriptor, modifier.GetLocation(), modifier.ValueText, reportedModifier));`
`106`	`118`	`}`
`107`	`119`
`108`	`120`	`if (!otherModifiersAppearEarlier && currentModifierType == ModifierType.Other)`