Skip to content

Commit

Permalink
Add a new SuppressMessageAttribute target scope to suppress diagnosti…
Browse files Browse the repository at this point in the history 
…cs in a namespace and all its descendant symbols

Fixes #486

This SuppressMessageAttribute feature has been requested for a very long time and still gets frequent customer requests to date.

The current "Namespace" target scope for SuppressMessageAttribute suppresses diagnostics only in nodes directly contained within the namespace, but in none of it's child symbols. This makes it very tedious to suppress certain analyzer diagnostics in entire namespace as one needs to add suppressions to individual types/namespaces within the namespace. This also creates a maintenance nightmare.

This PR adds a new target scope "NamespaceAndChildren" which suppresses diagnostics on the corresponding namespace and all its descendant symbols/members. Better naming suggestions for this scope name are welcome!
  • Loading branch information
@mavasani
mavasani committed Nov 10, 2018
1 parent e63f327 commit 677d520
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 19 deletions.
71 changes: 56 additions & 15 deletions src/Compilers/Core/Portable/DiagnosticAnalyzer/SuppressMessageAttributeState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,16 @@ internal partial class SuppressMessageAttributeState
{ "namespace", TargetScope.Namespace },
{ "resource", TargetScope.Resource },
{ "type", TargetScope.Type },
{ "member", TargetScope.Member }
{ "member", TargetScope.Member },
{ "namespaceandchildren", TargetScope.NamespaceAndChildren }
};

private static bool TryGetTargetScope(SuppressMessageInfo info, out TargetScope scope)
{
string scopeString = info.Scope != null ? info.Scope.ToLowerInvariant() : null;
return s_suppressMessageScopeTypes.TryGetValue(scopeString, out scope);
}

private readonly Compilation _compilation;
private GlobalSuppressions _lazyGlobalSuppressions;
private readonly ConcurrentDictionary<ISymbol, ImmutableDictionary<string, SuppressMessageInfo>> _localSuppressionsBySymbol;
Expand Down Expand Up @@ -56,14 +63,31 @@ public bool HasCompilationWideSuppression(string id, out SuppressMessageInfo inf
return _compilationWideSuppressions.TryGetValue(id, out info);
}

public bool HasGlobalSymbolSuppression(ISymbol symbol, string id, out SuppressMessageInfo info)
public bool HasGlobalSymbolSuppression(ISymbol symbol, string id, bool isImmediatelyContainingSymbol, out SuppressMessageInfo info)
{
Debug.Assert(symbol != null);
Dictionary<string, SuppressMessageInfo> suppressions;
if (_globalSymbolSuppressions.TryGetValue(symbol, out suppressions) &&
suppressions.TryGetValue(id, out info))
{
return true;
if (symbol.Kind != SymbolKind.Namespace)
{
return true;
}

if (TryGetTargetScope(info, out TargetScope targetScope))
{
switch (targetScope)
{
case TargetScope.Namespace:
// Special case: Only suppress syntax diagnostics in namespace declarations if the namespace is the closest containing symbol.
// In other words, only apply suppression to the immediately containing namespace declaration and not to its children or parents.
return isImmediatelyContainingSymbol;

case TargetScope.NamespaceAndChildren:
return true;
}
}
}

info = default(SuppressMessageInfo);
Expand Down Expand Up @@ -118,7 +142,7 @@ private bool IsDiagnosticSuppressed(string id, Location location, out SuppressMe

info = default(SuppressMessageInfo);

if (IsDiagnosticGloballySuppressed(id, symbolOpt: null, info: out info))
if (IsDiagnosticGloballySuppressed(id, symbolOpt: null, isImmediatelyContainingSymbol: false, info: out info))
{
return true;
}
Expand All @@ -140,11 +164,9 @@ private bool IsDiagnosticSuppressed(string id, Location location, out SuppressMe
{
if (symbol.Kind == SymbolKind.Namespace)
{
// Special case: Only suppress syntax diagnostics in namespace declarations if the namespace is the closest containing symbol.
// In other words, only apply suppression to the immediately containing namespace declaration and not to its children or parents.
return inImmediatelyContainingSymbol && IsDiagnosticGloballySuppressed(id, symbol, out info);
return hasNamespaceSuppression((INamespaceSymbol)symbol, inImmediatelyContainingSymbol);
}
else if (IsDiagnosticLocallySuppressed(id, symbol, out info) || IsDiagnosticGloballySuppressed(id, symbol, out info))
else if (IsDiagnosticLocallySuppressed(id, symbol, out info) || IsDiagnosticGloballySuppressed(id, symbol, inImmediatelyContainingSymbol, out info))
{
return true;
}
Expand All @@ -155,13 +177,30 @@ private bool IsDiagnosticSuppressed(string id, Location location, out SuppressMe
}

return false;

bool hasNamespaceSuppression(INamespaceSymbol namespaceSymbol, bool inImmediatelyContainingSymbol)
{
do
{
if (IsDiagnosticGloballySuppressed(id, namespaceSymbol, inImmediatelyContainingSymbol, out _))
{
return true;
}

namespaceSymbol = namespaceSymbol.ContainingNamespace;
inImmediatelyContainingSymbol = false;
}
while (namespaceSymbol != null);

return false;
}
}

private bool IsDiagnosticGloballySuppressed(string id, ISymbol symbolOpt, out SuppressMessageInfo info)
private bool IsDiagnosticGloballySuppressed(string id, ISymbol symbolOpt, bool isImmediatelyContainingSymbol, out SuppressMessageInfo info)
{
this.DecodeGlobalSuppressMessageAttributes();
return _lazyGlobalSuppressions.HasCompilationWideSuppression(id, out info) ||
symbolOpt != null && _lazyGlobalSuppressions.HasGlobalSymbolSuppression(symbolOpt, id, out info);
symbolOpt != null && _lazyGlobalSuppressions.HasGlobalSymbolSuppression(symbolOpt, id, isImmediatelyContainingSymbol, out info);
}

private bool IsDiagnosticLocallySuppressed(string id, ISymbol symbol, out SuppressMessageInfo info)
Expand Down Expand Up @@ -251,10 +290,7 @@ private static void DecodeGlobalSuppressMessageAttributes(Compilation compilatio
continue;
}

string scopeString = info.Scope != null ? info.Scope.ToLowerInvariant() : null;
TargetScope scope;

if (s_suppressMessageScopeTypes.TryGetValue(scopeString, out scope))
if (TryGetTargetScope(info, out TargetScope scope))
{
if ((scope == TargetScope.Module || scope == TargetScope.None) && info.Target == null)
{
Expand Down Expand Up @@ -294,6 +330,10 @@ internal static IEnumerable<ISymbol> ResolveTargetSymbols(Compilation compilatio
new TargetSymbolResolver(compilation, scope, target).Resolve(results);
return results;
}

case TargetScope.NamespaceAndChildren:
return ResolveTargetSymbols(compilation, target, TargetScope.Namespace);

default:
return SpecializedCollections.EmptyEnumerable<ISymbol>();
}
Expand Down Expand Up @@ -341,7 +381,8 @@ internal enum TargetScope
Namespace,
Resource,
Type,
Member
Member,
NamespaceAndChildren
}
}
}
138 changes: 134 additions & 4 deletions src/Test/Utilities/Portable/Diagnostics/SuppressMessageAttributeTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,101 @@ namespace N4
Diagnostic("Declaration", "N3"));
}

[Fact, WorkItem(486, "https://github.com/dotnet/roslyn/issues/486")]
public async Task GlobalSuppressionOnNamespaces_NamespaceAndChildren()
{
await VerifyCSharpAsync(@"
using System.Diagnostics.CodeAnalysis;
[assembly: SuppressMessage(""Test"", ""Declaration"", Scope=""NamespaceAndChildren"", Target=""N.N1"")]
[module: SuppressMessage(""Test"", ""Declaration"", Scope=""NamespaceAndChildren"", Target=""N4"")]
namespace N
{
namespace N1
{
namespace N2.N3
{
}
}
}
namespace N4
{
namespace N5
{
}
}
namespace N.N1.N6.N7
{
}
",
new[] { new WarningOnNamePrefixDeclarationAnalyzer("N") },
Diagnostic("Declaration", "N"));
}

[Fact, WorkItem(486, "https://github.com/dotnet/roslyn/issues/486")]
public async Task GlobalSuppressionOnTypesAndNamespaces_NamespaceAndChildren()
{
await VerifyCSharpAsync(@"
using System.Diagnostics.CodeAnalysis;
[assembly: SuppressMessage(""Test"", ""Declaration"", Scope=""NamespaceAndChildren"", Target=""N.N1.N2"")]
[module: SuppressMessage(""Test"", ""Declaration"", Scope=""NamespaceAndChildren"", Target=""N4"")]
[module: SuppressMessage(""Test"", ""Declaration"", Scope=""Type"", Target=""C2"")]
namespace N
{
namespace N1
{
class C1
{
}
namespace N2.N3
{
class C2
{
}
class C3
{
class C4
{
}
}
}
}
}
namespace N4
{
namespace N5
{
class C5
{
}
}
class C6
{
}
}
namespace N.N1.N2.N7
{
class C7
{
}
}
",
new[] { new WarningOnNamePrefixDeclarationAnalyzer("N"), new WarningOnNamePrefixDeclarationAnalyzer("C") },
Diagnostic("Declaration", "N"),
Diagnostic("Declaration", "N1"),
Diagnostic("Declaration", "C1"));
}

[Fact]
public async Task GlobalSuppressionOnTypes()
{
Expand Down Expand Up @@ -350,6 +445,23 @@ class C {}
Diagnostic("Token", "}").WithLocation(9, 1));
}

[Fact, WorkItem(486, "https://github.com/dotnet/roslyn/issues/486")]
public async Task SuppressSyntaxDiagnosticsOnNamespaceAndChildDeclarationCSharp()
{
await VerifyTokenDiagnosticsCSharpAsync(@"
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""Test"", ""Token"", Scope=""NamespaceAndChildren"", Target=""A.B"")]
namespace A
[|{
namespace B
{
class C {}
}
}|]
",
Diagnostic("Token", "{").WithLocation(4, 1),
Diagnostic("Token", "}").WithLocation(9, 1));
}

[Fact]
public async Task SuppressSyntaxDiagnosticsOnNamespaceDeclarationBasic()
{
Expand All @@ -370,11 +482,29 @@ End Namespace
Diagnostic("Token", "End").WithLocation(8, 1));
}

[Fact]
public async Task DontSuppressSyntaxDiagnosticsInRootNamespaceBasic()
[Fact, WorkItem(486, "https://github.com/dotnet/roslyn/issues/486")]
public async Task SuppressSyntaxDiagnosticsOnNamespaceAndChildrenDeclarationBasic()
{
await VerifyBasicAsync(@"
<module: System.Diagnostics.SuppressMessage(""Test"", ""Comment"", Scope:=""Namespace"", Target:=""RootNamespace"")>
await VerifyTokenDiagnosticsBasicAsync(@"
<assembly: System.Diagnostics.CodeAnalysis.SuppressMessage(""Test"", ""Token"", Scope:=""NamespaceAndChildren"", Target:=""A.B"")>
Namespace [|A
Namespace B
Class C
End Class
End Namespace
End|] Namespace
",
Diagnostic("Token", "A").WithLocation(3, 11),
Diagnostic("Token", "End").WithLocation(8, 1));
}

[Theory, WorkItem(486, "https://github.com/dotnet/roslyn/issues/486")]
[InlineData("Namespace")]
[InlineData("NamespaceAndChildren")]
public async Task DontSuppressSyntaxDiagnosticsInRootNamespaceBasic(string scope)
{
await VerifyBasicAsync($@"
<module: System.Diagnostics.SuppressMessage(""Test"", ""Comment"", Scope:=""{scope}"", Target:=""RootNamespace"")>
' In root namespace
",
rootNamespace: "RootNamespace",
Expand Down

0 comments on commit 677d520

Please sign in to comment.