Support banning namespaces

[Bouke]

The BannedApiAnalyzer refers to "ID string format" for reference on what symbols to ban. One of those symbols is Namespaces, but this is not supported. As this has been requested before and is also something I ran into earlier today, it seems like a good fit to add to the project.

This will ban everything under namespace A, including child namespaces like A.B and A.B.C. Having an invocation like new A.B.C.D() will yield the error "The symbol 'A' is banned in this project":

N:A

I've chosen to report on the namespace of the symbol instead of the symbol itself for clarity. Reporting the symbol would yield the message "The symbol 'D' is banned in this project" which might be confusing as it is actually any parent namespace that is banned.

This PR should be further tested other than the provided unit tests. I've tried banning System.Threading in the tests, but that didn't work. Could it be that the INamespaceSymbol implements equality other than just a name comparison? Update: this also works now, thanks to @mavasani.

[codecov]

Codecov Report

Merging #6521 (8469f59) into main (fee402d) will decrease coverage by 0.01%.
The diff coverage is 99.30%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6521      +/-   ##
==========================================
- Coverage   96.43%   96.42%   -0.01%     
==========================================
  Files        1373     1373              
  Lines      320260   320477     +217     
  Branches    10296    10302       +6     
==========================================
+ Hits       308837   309029     +192     
- Misses       8969     8985      +16     
- Partials     2454     2463       +9     

[mavasani]

This PR should be further tested other than the provided unit tests. I've tried banning System.Threading in the tests, but that didn't work. Could it be that the INamespaceSymbol implements equality other than just a name comparison?

This might be due to the fact that it might be a merged namespace symbol. See https://github.com/dotnet/roslyn/blob/98a00a37e1a66501a1a7495b89853b466668c6b8/src/Compilers/Core/Portable/Symbols/INamespaceSymbol.cs#L49-L60

I think before yield return, you may want to check if namespace's ContainingCompilation matches the compilation being analyzed and if not, iterate ConstituentNamespaces and return the one whose ContainingCompilation matches.

[Bouke]

This might be due to the fact that it might be a merged namespace symbol. See https://github.com/dotnet/roslyn/blob/98a00a37e1a66501a1a7495b89853b466668c6b8/src/Compilers/Core/Portable/Symbols/INamespaceSymbol.cs#L49-L60

Thank you for the suggestion. I've tried the code below, but it doesn't fix the problem. Did I misunderstand your suggestion, or could there be something else? Besides the compiled namespace, especially the banned namespace could be a constituent namespace. After expanding the banned namespaces the example with System.Threading works flawlessly.

I haven't went down the route of comparing ContainingCompilation; is there a reason I should be doing that?

[drewnoakes]

I've chosen to report on the namespace of the symbol instead of the symbol itself for clarity.

Will this produce warnings on the generated code when using implicit global usings?

For example, if I ban Foo then add this to my project:

<ItemGroup>
  <Using Include="Foo"/>
</ItemGroup>

Then in a file like obj\Debug\net7.0\MyProject.GlobalUsings.g.cs there'll be code like:

// <auto-generated/>
global using global::Foo;

After which I can use symbols from the banned namespace in any source file.

If we provide the warning in the generated file, that's fine. But if not, then there is the chance of accidentally bits from banned namespaces.

[Bouke]

@drewnoakes The detection of banned APIs doesn't change with this PR. Banning a namespace effectively means the same thing as banning all types within that namespace and its descendants. See also this example on what it means to ban a type. Merely referencing the namespace or a type within that namespace doesn't produce the warning. Only consuming a banned API produces the warning, e.g. invoking a method on a banned type.

[Bouke]

I've rebased my PR with the lazy defer work changes from @CyrusNajmabadi. Is there anything else I can do to get this in a shape to be merged?

[drewnoakes]

Thank you for picking this up. Looks good to me.

[mavasani]

LGTM. I can merge once you address/respond to the suggestions

[Bouke]

@mavasani thank you for the review. There's one more thing to clear up and then the PR should be good to go.

[mavasani]

Thank you @Bouke!

[jlaanstra]

@mavasani I don't think this is part of the latest 3.3.4 NuGet package? Any chance we can ship an update?

[mavasani]

@mavasani I don't think this is part of the latest 3.3.4 NuGet package? Any chance we can ship an update?

@jlaanstra That is correct, we'll work on shipping an updated package. Meanwhile, you can use the pre-release version from this feed: https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet8/NuGet/Microsoft.CodeAnalysis.BannedApiAnalyzers/overview/3.11.0-beta1.23407.1