roslyn: Compilation.GetTypeByMetadataName returns null even though the other ObsoleteAttribute is internal

Version Used: Visual Studio 16.9.2 (hosting analyzers), 3.9.0 NuGet packages (repro below)

Roslyn should be able to see that the internal System.ObsoleteAttribute declared by System.DirectoryServices is not accessible to the compilation. (System.DirectoryServices does not declare its internals visible to my project.) GetTypeByMetadataName should return the only public one.

The compilation succeeds, so there is a well-known way to resolve this conflict if it’s even a conflict.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using System;
using System.IO;

var compilation = CSharpCompilation.Create(
    assemblyName: null,
    syntaxTrees: new[] { CSharpSyntaxTree.ParseText("[System.Obsolete] class C { }") },
    references: new[]
    {
        // These both get included in projects that target net5.0-windows
        MetadataReference.CreateFromFile(@"C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\5.0.0\ref\net5.0\System.Runtime.dll"),
        MetadataReference.CreateFromFile(@"C:\Program Files\dotnet\packs\Microsoft.WindowsDesktop.App.Ref\5.0.0\ref\net5.0\System.DirectoryServices.dll"),
    },
    new(OutputKind.DynamicallyLinkedLibrary));

Console.WriteLine("Compilation success: " + compilation.Emit(Stream.Null).Success);

var type = compilation.GetTypeByMetadataName(typeof(ObsoleteAttribute).FullName!);
Console.WriteLine("GetTypeByMetadataName returns null: " + (type is null));

Prints:

Compilation success: True
GetTypeByMetadataName returns null: True

In case this is “by design”

This is a pit of failure for people writing our own analyzers and source generators. For example, my analyzer stopped ignoring obsolete members as soon as it happened to be used in a net5.0-windows project. If there is some reason that this behavior can’t be fixed, analyzer writers should get a warning if we try to use it. I think leaving things in the current pitfall-inducing state should not be seen as acceptable.

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 2
  • Comments: 24 (24 by maintainers)

Most upvoted comments

I’m not certain deprecation is the right approach for all scenarios where this API is used. It might be better to start with an analyzer in Microsoft.CodeAnalysis.Analyzers that encourages analyzer authors to migrate. Of course all of this depends on the shape of the new API, which still hasn’t been decided.

I would obsolete because:

  1. the existing api is a pit of fail.
  2. teh new api is a drop-in replacement.

No good comes from using the existing api. At best, it gives a false sense of security, but is a ticking timebomb. And, when someone runs into thsi, they’ll have to then go and try to find out what to do. Obsolete (warning is fine with me) immediately tells people about the problem, points them immediately to teh right solution, and future proofs their code to issues they may not even know they have.

We could certainly also deliver this with an easy code-fixer that will make this change to their solution.

@jaredpar FWIW< we already have teh replacement API we use since the compiler one isn’t viable. I think we could jsut take a PR to move that into roslyn, with an obsolete attribute on the existing API that points to the new one.