Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SemanticModel.IsAccessible returning true for inaccessible protected member #43696

Open
JohanLarsson opened this issue Apr 26, 2020 · 5 comments

Comments

@JohanLarsson
Copy link

JohanLarsson commented Apr 26, 2020

namespace N
{
    class Base
    {
        protected void M1() { }
    }

    class C : Base
    {
        class Nested
        {
            void M2()
            {
                // we check if Base.M1 is accessible here 
                typeof(Base).GetMethod("M1");
            }
        }
    }
}

[jcouv update:] Thanks @jnm2 for clarifying the repro (below)

@JohanLarsson
Copy link
Author

@Joe4evr
Copy link

Joe4evr commented Apr 26, 2020

This is not a bug, there was an entire conversation in the latest .NET Design Meeting about this exact topic. It comes down to the fact that there's a difference between what reflection considers accessible and what the rest of the .NET world considers it. In reflection, Public (which is used by default) is only what is actually declared public, and everything else is NonPublic, including protected. In other tooling public and protected are both considered accessible to outside assemblies, albeit one requires deriving from a type.

@jnm2
Copy link
Contributor

jnm2 commented Apr 26, 2020

@Joe4evr Then how do you check whether or not replacing "M1" with nameof(Base.M1) will fail to compile, if not via SemanticModel.IsAccessible?

This isn't reflection and it doesn't even involve IL at this level, only C# syntax rules. I'm not sure the .NET design meeting was about this.

@jnm2
Copy link
Contributor

jnm2 commented Apr 26, 2020

Repro code btw:

Click to expand
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp3.1</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="3.5.0" />
  </ItemGroup>

</Project>
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using System;
using System.IO;
using System.Linq;

public static class Program
{
    public static void Main()
    {
        var syntaxTree = CSharpSyntaxTree.ParseText(@"
class Base
{
    protected void M1() { }
}

class C : Base
{
    class Nested
    {
        void M2()
        {
            typeof(Base).GetMethod(""M1"");
        }
    }
}");

        var compilation = CSharpCompilation.Create(
            "Repro",
            new[] { syntaxTree },
            new[] { MetadataReference.CreateFromFile(typeof(object).Assembly.Location) },
            new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));

        var emitResult = compilation.Emit(Stream.Null);
        if (!emitResult.Success) throw new NotImplementedException("Repro must not have compilation errors.");

        var stringLiteralSyntax = syntaxTree.GetRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single();

        var model = compilation.GetSemanticModel(syntaxTree, ignoreAccessibility: false);
        var baseM1Symbol = compilation.Assembly.GlobalNamespace.GetTypeMembers("Base").Single().GetMembers("M1").Single();

        // Should print False but prints True
        Console.WriteLine(model.IsAccessible(stringLiteralSyntax.SpanStart, baseM1Symbol));
    }
}

@Joe4evr
Copy link

Joe4evr commented Apr 26, 2020

Ah, I see what's going on now. OP probably should've been a bit more clear about the intention, rather than posting only a snippet of code with no elaboration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants