Skip to content

Instantly share code, notes, and snippets.

@haacked
Last active May 14, 2023 18:38
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save haacked/00de560d00692b7f4859336c747af10e to your computer and use it in GitHub Desktop.
Save haacked/00de560d00692b7f4859336c747af10e to your computer and use it in GitHub Desktop.
Roslyn Analyzer to warn about access to forbidden types
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;
// CREDIT: https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeAnalysis.BannedApiAnalyzers/Core/SymbolIsBannedAnalyzer.cs
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ForbiddenTypeAnalyzer : DiagnosticAnalyzer
{
public const string DiagnosticId = nameof(ForbiddenTypeAnalyzer);
static readonly LocalizableString Description = "Restricts the set of types that may be used.";
const string Title = "Forbidden Type Analyzer";
const string MessageFormat = "Access to type {0} is forbidden.";
const string Category = "API Usage";
readonly HashSet<string> _forbiddenTypeNames;
static readonly IEnumerable<string> DefaultTypeAccessDenyList = new[]
{
"System.Console",
"System.Environment",
"System.IntPtr",
"System.Type"
};
static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
DiagnosticId,
Title,
MessageFormat,
Category,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
Description);
public ForbiddenTypeAnalyzer() : this(DefaultTypeAccessDenyList)
{
}
ForbiddenTypeAnalyzer(IEnumerable<string> forbiddenTypeNames)
{
_forbiddenTypeNames = forbiddenTypeNames.ToHashSet(StringComparer.Ordinal);
}
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
public override void Initialize(AnalysisContext compilationContext)
{
compilationContext.EnableConcurrentExecution();
compilationContext.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);
compilationContext.RegisterOperationAction(context =>
{
var type = context.Operation switch
{
IObjectCreationOperation objectCreation => objectCreation.Type,
IInvocationOperation invocationOperation => invocationOperation.TargetMethod.ContainingType,
IMemberReferenceOperation memberReference => memberReference.Member.ContainingType,
IArrayCreationOperation arrayCreation => arrayCreation.Type,
IAddressOfOperation addressOf => addressOf.Type,
IConversionOperation conversion => conversion.OperatorMethod?.ContainingType,
IUnaryOperation unary => unary.OperatorMethod?.ContainingType,
IBinaryOperation binary => binary.OperatorMethod?.ContainingType,
IIncrementOrDecrementOperation incrementOrDecrement => incrementOrDecrement.OperatorMethod?.ContainingType,
_ => throw new NotImplementedException($"Unhandled OperationKind: {context.Operation.Kind}")
};
VerifyType(context.ReportDiagnostic, type, context.Operation.Syntax);
},
OperationKind.ObjectCreation,
OperationKind.Invocation,
OperationKind.EventReference,
OperationKind.FieldReference,
OperationKind.MethodReference,
OperationKind.PropertyReference,
OperationKind.ArrayCreation,
OperationKind.AddressOf,
OperationKind.Conversion,
OperationKind.UnaryOperator,
OperationKind.BinaryOperator,
OperationKind.Increment,
OperationKind.Decrement);
}
bool VerifyType(Action<Diagnostic> reportDiagnostic, ITypeSymbol? type, SyntaxNode syntaxNode)
{
do
{
if (!VerifyTypeArguments(reportDiagnostic, type, syntaxNode, out type))
{
return false;
}
if (type is null)
{
// Type will be null for arrays and pointers.
return true;
}
var typeName = type.ToString();
if (typeName is null)
{
return true;
}
if (_forbiddenTypeNames.Contains(typeName))
{
reportDiagnostic(Diagnostic.Create(Rule, syntaxNode.GetLocation(), typeName));
return false;
}
type = type.ContainingType;
}
while (!(type is null));
return true;
}
bool VerifyTypeArguments(Action<Diagnostic> reportDiagnostic, ITypeSymbol? type, SyntaxNode syntaxNode, out ITypeSymbol? originalDefinition)
{
switch (type)
{
case INamedTypeSymbol namedTypeSymbol:
originalDefinition = namedTypeSymbol.ConstructedFrom;
foreach (var typeArgument in namedTypeSymbol.TypeArguments)
{
if (typeArgument.TypeKind != TypeKind.TypeParameter &&
typeArgument.TypeKind != TypeKind.Error &&
!VerifyType(reportDiagnostic, typeArgument, syntaxNode))
{
return false;
}
}
break;
case IArrayTypeSymbol arrayTypeSymbol:
originalDefinition = null;
return VerifyType(reportDiagnostic, arrayTypeSymbol.ElementType, syntaxNode);
case IPointerTypeSymbol pointerTypeSymbol:
originalDefinition = null;
return VerifyType(reportDiagnostic, pointerTypeSymbol.PointedAtType, syntaxNode);
default:
originalDefinition = type?.OriginalDefinition;
break;
}
return true;
}
}
// UNIT TESTS
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Scripting;
using Xunit;
public class ForbiddenTypeAnalyzerTests
{
[Theory]
[InlineData(@"if (IsRequest) {
Reply(""test"");
}
else {
var env = Environment.CommandLine;
Reply(env);
}", "System.Environment")]
[InlineData(@"if (IsRequest) {
Reply(""test"");
}
else {
var env = Environment.GetEnvironmentVariable(""test"");
Reply(env);
}", "System.Environment")]
[InlineData(@"var args = Environment.CommandLine;", "System.Environment")]
[InlineData(@"Console.WriteLine(""test"");", "System.Console")]
[InlineData(@"var ptr = new IntPtr(32);", "System.IntPtr")]
[InlineData(@"var type = (Type)SomeType;", "System.Type")]
[InlineData(@"var type = (Type)SomeType; var name = type.Name;", "System.Type")]
[InlineData(@"var type = Type.GetType(""name"");", "System.Type")]
[InlineData(@"var pointers = new IntPtr[4];", "System.IntPtr")]
public async Task ReturnsErrorsForForbiddenTypes(string code, string expectedForbiddenType)
{
var options = ScriptOptions.Default
.WithImports("System")
.WithEmitDebugInformation(true)
.WithReferences("System.Runtime.Extensions", "System.Console")
.WithAllowUnsafe(false);
var script = CSharpScript.Create<dynamic>(code, globalsType: typeof(IScriptGlobals), options: options);
var compilation = script.GetCompilation();
var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(
new ForbiddenTypeAnalyzer());
var compilationWithAnalyzers = new CompilationWithAnalyzers(
compilation,
analyzers,
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty),
CancellationToken.None);
var diagnosticResults = await compilationWithAnalyzers.GetAllDiagnosticsAsync();
var diagnostic = Assert.Single(diagnosticResults);
Assert.NotNull(diagnostic);
Assert.Equal(
$"Access to type {expectedForbiddenType} is forbidden.",
diagnostic.GetMessage());
Assert.Equal(ForbiddenTypeAnalyzer.DiagnosticId, diagnostic.Id);
}
[Fact]
public async Task DoesNotReturnsErrorsForAllowedTypes()
{
const string code = @"if (IsRequest) {
Reply(""test"");
}
else {
var rnd = new Random();
Reply(rnd.Next(1).ToString());
}";
var options = ScriptOptions.Default
.WithImports("System")
.WithEmitDebugInformation(true)
.WithReferences("System.Runtime.Extensions")
.WithAllowUnsafe(false);
var script = CSharpScript.Create<dynamic>(code, globalsType: typeof(IScriptGlobals), options: options);
var compilation = script.GetCompilation();
var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(
new ForbiddenTypeAnalyzer());
var compilationWithAnalyzers = new CompilationWithAnalyzers(
compilation,
analyzers,
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty),
CancellationToken.None);
var diagnosticResults = await compilationWithAnalyzers.GetAllDiagnosticsAsync();
Assert.Empty(diagnosticResults);
}
public interface IScriptGlobals
{
bool IsRequest { get; }
void Reply(string reply);
}
}
@jaredpar
Copy link

 public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

Nit: prefer { get; } = here as it will cache the ImmutableArray<T> vs recreating on every invocation. It's immutable so may as well take advantage of it 😄

Is the goal of this to prevent users from defining those types or consuming them? Looks like defining them but wanted to check.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jaredpar I want to prevent both really. The scenario is this, I'm allowing users to write C# scripts in a constrained environment. I want to warn if they try calling Environment.CommandLine for example. The script only gets access to types and libraries I provide.

But I would want to know if a library I allow provides them a forbidden type.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@Tyrrrz ha! I just read that post too. Usually Rider warns me about this. Just to be clear I did pass in a comparison now.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

Seems like the message format doesn't work correctly.

Screen Shot 2020-10-28 at 10 39 41 AM

@jaredpar
Copy link

@haacked Gotcha. So there are two scenarios here (stop usage, stop definition) and they have different approaches to consider.

One problem with your current approach is the following:

        var allGood = _forbiddenTypeNames.Select(
                type => context.Compilation.GetTypeByMetadataName(type))
            .All(symbol => symbol is null);

This is being done in OnCompilationStarted. That mean at the start of compilation you are essentially forcing the compiler to eagerly bind all types defined in the source as well as all the types defined in all assemblies that are referenced by the compilation. For a fully bound compilation that is a fairly cheap operation however you are essentially front loading this work to the very beginning of compilation.

That won't make much difference in command line scenarios cause we're going to do all of that work anyways, you're just changing when we do the work. In the IDE though it's a different story. The IDE is constantly partially evaluating analyzers. Basically every time you type a character the IDE says "okay well that data has changed, let's requeue the analyzers". Hence it behooves analyzers to be as lazy as possible here. This is done by registering action handlers for the types of actions you are interested in.

For the "stop them from defining the type" case you likely want to register a symbol analyzer. These are called when the compiler finishes binding types in the solution. Asking questions like "what is the full name" is cheap at this point and can easily be checked against the banned list you've defined here. Example of one is here

For the "stop them from using the type" case you likely want to register an IOperation based analyzer. Here you can break on an IInvocationOperation (akka a method call) and then you can look at the type the method is defined on, the type of the receiver, etc. ... to determine if it is in your banned set. Example of one is here

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jaredpar does the invocation operation also prevent static property access? I found an example where someone was looking at invocation and ran into problems. dotnet/roslyn#10830 (comment)

@jaredpar
Copy link

@haacked

For those you want to use IMemberReferenceOperation. That covers properties, fields, etc ... http://sourceroslyn.io/#Microsoft.CodeAnalysis/Generated/Operations.Generated.cs,00b80a1ad1684dd6,references

This though illustrates another potential issue. To forbid a type in the language you have to consider all of the ways in which it can be used. Other cases you didn't mention but wouldn't be covered are nameof or typeof.

If you want to be incredibly thorough, at the expense of being slightly less efficient, I think you may have to have a syntax receiver for named names, bind them and see if they refer to the symbols you want to ban.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

This though illustrates another potential issue. To forbid a type in the language you have to consider all of the ways in which it can be used. Other cases you didn't mention but wouldn't be covered are nameof or typeof.

Hmm, I really just want to warn if someone does one of the following:

  1. Tries to call a method or property of the type.
  2. Tries to construct the type.

I really don't care if someone declares a type of the same name or does nameof(Type). I also warn about using reflection as that's another way to work around the warnings.

I'll look into IMemberReferenceOperation and IInvocationOperation.

Out of curiosity, the IdentifierName approach seems to be working. What would circumvent it (assuming I am waring about reflection properly)?

@haacked
Copy link
Author

haacked commented Oct 28, 2020

Ah, I figured out the format message problem. The documentation is confusing. I assumed that the location would be passed as "{0}" and my additional format string would be passed to "{1}". That's not the case.

@jaredpar
Copy link

@haacked

Out of curiosity, the IdentifierName approach seems to be working. What would circumvent it (assuming I am waring about reflection properly)?

Nothing that I can think of. The issue with that approach is purely about performance. Your essentially doing a symbol lookup on every single name in the code no matter the context. Consider for a quick example a local declaration:

C local;

There are two IdentifierToken uses here: C and local. In your sample you will end up querying symblos and comparing names for each of them. This isn't wrong, it's just unnecessary in the case of local. The approaches I outlined are all about avoiding unnecessary work. The IOperation approach doesn't even do symbol lookup because you are handed the symbols. Hence from there it's just grabbing full names or comparing symbols directly (really cheap).

Overall though I doubt the IdentifierName approach would cause many issues. Given my job I tend to be hyper focused on potential perf issues cause we see all the bad things in the worst cases. 😄 In general I don't think this approach would cause issues.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jardpar thanks! Performance is important to me as well and that's really helpful.

@jaredpar
Copy link

Posting some feedback that was in the twitter thread. Can look at the banned API analyzer in roslyn-analyzers for a very thorough implementation of this

https://github.com/dotnet/roslyn-analyzers/tree/master/src/Microsoft.CodeAnalysis.BannedApiAnalyzers

@jmarolf
Copy link

jmarolf commented Oct 28, 2020

I would recommend looking at the Banned API analyzer that the roslyn team wrote and that is used throughout the .NET org

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jaredpar there doesn't seem to be an OperationKind that corresponds to IMemberReferenceOperation https://docs.microsoft.com/en-us/dotnet/api/microsoft.codeanalysis.operationkind?view=roslyn-dotnet

@jaredpar
Copy link

Yeah you'll have to list them all out individually like the banned API anaylzer does.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jmarolf oh nice! I'll check it out.

@haacked
Copy link
Author

haacked commented Oct 28, 2020

@jmarolf @jaredpar thanks for all your help so far! I updated the code sample based on the Roslyn banned API analyzer. I'm trying to strip it down to the minimum I need and there's still one part i don't understand. Check out the unit tests I created.

Only the following tests pass. The rest fail because there were no diagnostics returned.

[InlineData(@"var args = Environment.CommandLine;", "System.Environment")]
[InlineData(@"var ptr = new IntPtr(32);", "System.IntPtr")]
[InlineData(@"var type = Type.GetType(""name"");", "System.Type")]
[InlineData(@"var pointers = new IntPtr[4];", "System.IntPtr")]

I would expect the following test to fail:

[InlineData(@"Console.WriteLine(""test"");", "System.Console")]

When I step through the debugger, I notice that the callback for RegisterOperationAction is never called in this case even though I specified OperationKind.MethodReference as one of the operations to handle. Is there something obvious I'm doing wrong?

I'll keep bringing in more code from the banned analyzer, but I appreciate any help in understanding what's going on better. I'd like to understand the code I write or borrow from. 😄

@jmarolf
Copy link

jmarolf commented Oct 28, 2020

@haacked are you sure you have those symbols in your compilation? One of the main reasons we wrote Microsoft.CodeAnalysis.Testing is that people were unaware that their code samples would not actually compiler because they did not have any of the correct metadata references. Not saying that's what is happening here though. I'll need to setup a repro and try it out to see.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

@jmarolf if the symbols weren’t there, wouldn’t I get a different compilation error? I get 0 errors. I’ll put together a minimal repro to be sure.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

I have a minimal repro.

AnalyzerDemo.csproj

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <OutputType>Exe</OutputType>
        <TargetFramework>netcoreapp3.1</TargetFramework>
        <nullable>enable</nullable>
    </PropertyGroup>

    <ItemGroup>
        <PackageReference Include="Microsoft.CodeAnalysis.CSharp.Scripting" Version="3.3.1" />
        <PackageReference Include="System.Collections.Immutable" Version="1.7.1" />
    </ItemGroup>
</Project>

Program.cs

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Scripting;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Scripting;

class Program
{
    static async Task Main(string[] args)
    {
        await TestCode(@"var env = Environment.CommandLine;");
        await TestCode(@"Console.WriteLine(""test"");");
    }

    static async Task TestCode(string code)
    {
        var diagnostics = await CompileCode(code);
        Console.WriteLine($"Compiling `{code}` resulted in {diagnostics.Length} diagnostics.");
        foreach (var diagnostic in diagnostics)
        {
            Console.WriteLine(diagnostic.ToString());
        }
    }

    static async Task<ImmutableArray<Diagnostic>> CompileCode(string code)
    {
        static IEnumerable<string> GetSystemAssemblyPaths()
        {
            var assemblyPath = Path.GetDirectoryName(typeof(object).Assembly.Location)
                               ?? throw new InvalidOperationException("Could not find the assembly for object.");
            yield return Path.Combine(assemblyPath, "mscorlib.dll");
            yield return Path.Combine(assemblyPath, "System.dll");
            yield return Path.Combine(assemblyPath, "System.Core.dll");
            yield return Path.Combine(assemblyPath, "System.Console.dll");
            yield return Path.Combine(assemblyPath, "System.Runtime.dll");
            yield return Path.Combine(assemblyPath, "System.Runtime.Extensions.dll");
        }
        
        var references = GetSystemAssemblyPaths()
            .Select(path => MetadataReference.CreateFromFile(path));
        
        var options = ScriptOptions.Default
            .WithImports("System", "System.IO")
            .WithEmitDebugInformation(true)
            .WithReferences(references)
            .WithAllowUnsafe(false);
        var script = CSharpScript.Create<dynamic>(code, globalsType: typeof(IScriptGlobals), options: options);
        var compilation = script.GetCompilation();
        var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(
            new ForbiddenTypeAnalyzer());
        var compilationWithAnalyzers = new CompilationWithAnalyzers(
            compilation,
            analyzers,
            new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty),
            CancellationToken.None);

        return await compilationWithAnalyzers.GetAllDiagnosticsAsync();
    }
}

public interface IScriptGlobals
{
}

ForbiddenTypeAnalyzer.cs

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

// CREDIT: https://github.com/dotnet/roslyn-analyzers/blob/master/src/Microsoft.CodeAnalysis.BannedApiAnalyzers/Core/SymbolIsBannedAnalyzer.cs
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class ForbiddenTypeAnalyzer : DiagnosticAnalyzer
{
    public const string DiagnosticId = nameof(ForbiddenTypeAnalyzer);
    
    static readonly LocalizableString Description = "Restricts the set of types that may be used.";
    const string Title = "Forbidden Type Analyzer";
    const string MessageFormat = "Access to type {0} is forbidden.";
    const string Category = "API Usage";
    readonly HashSet<string> _forbiddenTypeNames;
    
    static readonly IEnumerable<string> DefaultTypeAccessDenyList = new[]
    {
        "System.IO.StreamWriter",
        "System.Console",
        "System.Environment",
        "System.IntPtr",
        "System.Type"
    };
    
    static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
        DiagnosticId,
        Title,
        MessageFormat,
        Category,
        DiagnosticSeverity.Warning,
        isEnabledByDefault: true, 
        Description);

    public ForbiddenTypeAnalyzer() : this(DefaultTypeAccessDenyList)
    {
    }

    ForbiddenTypeAnalyzer(IEnumerable<string> forbiddenTypeNames)
    {
        _forbiddenTypeNames = forbiddenTypeNames.ToHashSet(StringComparer.Ordinal);
    }

    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

    public override void Initialize(AnalysisContext compilationContext)
    {
        compilationContext.EnableConcurrentExecution();
        compilationContext.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze |
                                                          GeneratedCodeAnalysisFlags.ReportDiagnostics);

        compilationContext.RegisterCompilationStartAction(OnCompilationStart);
    }
    
    void OnCompilationStart(CompilationStartAnalysisContext compilationContext)
    {
        compilationContext.RegisterOperationAction(context =>
            {
                var type = context.Operation switch
                {
                    IObjectCreationOperation objectCreation => objectCreation.Type,
                    IInvocationOperation invocationOperation => invocationOperation.TargetMethod.ContainingType,
                    IMemberReferenceOperation memberReference => memberReference.Member?.ContainingType,
                    IArrayCreationOperation arrayCreation => arrayCreation.Type,
                    IAddressOfOperation addressOf => addressOf.Type,
                    IConversionOperation conversion => conversion.OperatorMethod?.ContainingType,
                    IUnaryOperation unary => unary.OperatorMethod?.ContainingType,
                    IBinaryOperation binary => binary.OperatorMethod?.ContainingType,
                    IIncrementOrDecrementOperation incrementOrDecrement => incrementOrDecrement.OperatorMethod?.ContainingType,
                    _ => throw new NotImplementedException($"Unhandled OperationKind: {context.Operation.Kind}")
                };

                VerifyType(context.ReportDiagnostic, type, context.Operation.Syntax);
            },
            OperationKind.ObjectCreation,
            OperationKind.Invocation,
            OperationKind.EventReference,
            OperationKind.FieldReference,
            OperationKind.MethodReference,
            OperationKind.PropertyReference,
            OperationKind.ArrayCreation,
            OperationKind.AddressOf,
            OperationKind.Conversion,
            OperationKind.UnaryOperator,
            OperationKind.BinaryOperator,
            OperationKind.Increment,
            OperationKind.Decrement);
    }

    bool VerifyType(Action<Diagnostic> reportDiagnostic, ITypeSymbol? type, SyntaxNode syntaxNode)
    {
        do
        {
            if (!VerifyTypeArguments(reportDiagnostic, type, syntaxNode, out type))
            {
                return false;
            }

            if (type is null)
            {
                // Type will be null for arrays and pointers.
                return true;
            }

            var typeName = type.ToString();
            if (typeName is null)
            {
                return true;
            }

            if (_forbiddenTypeNames.Contains(typeName))
            {
                reportDiagnostic(Diagnostic.Create(Rule, syntaxNode.GetLocation(), typeName));
                return false;
            }

            type = type.ContainingType;
        }
        while (!(type is null));

        return true;
    }
    
    bool VerifyTypeArguments(Action<Diagnostic> reportDiagnostic, ITypeSymbol? type, SyntaxNode syntaxNode, out ITypeSymbol? originalDefinition)
    {
        switch (type)
        {
            case INamedTypeSymbol namedTypeSymbol:
                originalDefinition = namedTypeSymbol.ConstructedFrom;
                foreach (var typeArgument in namedTypeSymbol.TypeArguments)
                {
                    if (typeArgument.TypeKind != TypeKind.TypeParameter &&
                        typeArgument.TypeKind != TypeKind.Error &&
                        !VerifyType(reportDiagnostic, typeArgument, syntaxNode))
                    {
                        return false;
                    }
                }
                break;

            case IArrayTypeSymbol arrayTypeSymbol:
                originalDefinition = null;
                return VerifyType(reportDiagnostic, arrayTypeSymbol.ElementType, syntaxNode);

            case IPointerTypeSymbol pointerTypeSymbol:
                originalDefinition = null;
                return VerifyType(reportDiagnostic, pointerTypeSymbol.PointedAtType, syntaxNode);

            default:
                originalDefinition = type?.OriginalDefinition;
                break;

        }

        return true;
    }
}

The output is

Compiling `var env = Environment.CommandLine;` resulted in 1 diagnostics.
(1,11): warning ForbiddenTypeAnalyzer: Access to type System.Environment is forbidden.
Compiling `Console.WriteLine("test");` resulted in 0 diagnostics.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

Just to be sure the code is compiling correctly, I defined a concrete ScriptGlobals class.

public class ScriptGlobals : IScriptGlobals
{
}

And then tried the following right after I create the script in Program.cs.

await script.RunAsync(new ScriptGlobals());

And I see the string "test" in the console. So the script is running fine. When I step through the debugger, the RegisterOperationAction is not called when calling await TestCode(@"Console.WriteLine(""test"");");.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

Here's a couple more cases that are strange.

var write = new StreamWriter("some-path");  // Is flagged by the analyzer
new StreamWriter("some-path");              // Is NOT flagged by the analyzer.

@jmarolf
Copy link

jmarolf commented Oct 29, 2020

new StreamWriter("some-path"); isn't valid code though right?

@haacked
Copy link
Author

haacked commented Oct 29, 2020

@jmarolf

How about

new StreamWriter("some-path").Flush();

That also doesn't trigger the analyzer.

Also, if it's not valid code, I would expect Roslyn would report it. It compiles without any diagnostics. Could it be something I'm doing must be preventing Roslyn from reporting errors?

@jmarolf
Copy link

jmarolf commented Oct 29, 2020

I forgot this was in the scripting dialect and not a top level function. @jaredpar what are the language rules for these cases?

@jmarolf
Copy link

jmarolf commented Oct 29, 2020

Well regardless, I'll just need to take some time this evening to debug through this instead of guessing.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

To clarify, I've updated my repro a tiny bit to make sure it imports System.IO and references System.Runtime.Extensions. Then I added the following two lines to Program.cs

await TestCode(@"var x = new StreamWriter(""some-path"");");
await TestCode(@"new StreamWriter(""some-path"").Flush();");

And the result is

Compiling `var x = new StreamWriter("some-path");` resulted in 0 diagnostics.
Compiling `new StreamWriter("some-path").Flush();` resulted in 0 diagnostics.

Contrary to what I said earlier (I messed up my testing). I'm not sure why my code never breaks into the RegisterOperationAction callback.

@haacked
Copy link
Author

haacked commented Oct 29, 2020

I forgot this was in the scripting dialect and not a top level function.

Yeah, I can't use C# 9 just yet. 😦

I changed the code to use the regular compilation.

var syntaxTree = CSharpSyntaxTree.ParseText(code);
        var compilation = CSharpCompilation.Create(
            "assemblyName",
            new[] { syntaxTree },
            references,
            new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary));

And then ran this...

await TestCode(@"
using System.IO;

public class Test
{
    public void DoStuff()
    {
        new StreamWriter(""test"").Flush();
    }
}
");

And got the result I expected (8,9): warning ForbiddenTypeAnalyzer: Access to type System.IO.StreamWriter is forbidden..

So it does appear to be a difference between the C# scripting dialect and C#.

@haacked
Copy link
Author

haacked commented Oct 30, 2020

Just for completeness sake, I tried this out with CSharpSymbolIsBannedAnalyzer. Here's the symbols file I used.

T:System.Console;Don't use System.Console
T:System.Environment;Don't use System.Environment
T:System.Type;Don't use System.Type
T:System.Reflection.MemberInfo;Don't use Reflection.
T:System.IO.StreamWriter;Don't use System.IO.StreamWriter.

Here are the test cases I used (same program as above but with the CSharpSymbolIsBannedAnalyzer analyzer swapped in.

await TestCode(@"new StreamWriter(""some-path"").Flush();");
await TestCode("var env = Environment.CommandLine;");
await TestCode("var type = (Type)SomeType; var name = type.Name;");
await TestCode(@"Console.WriteLine(""test"");");

And here are the results.

Compiling `new StreamWriter("some-path").Flush();` resulted in 0 diagnostics.
Compiling `var env = Environment.CommandLine;` resulted in 1 diagnostics.
(1,11): warning RS0030: The symbol 'Environment' is banned in this project: Don't use System.Environment
Compiling `var type = (Type)SomeType; var name = type.Name;` resulted in 1 diagnostics.
(1,39): warning RS0030: The symbol 'MemberInfo' is banned in this project: Don't use Reflection.
Compiling `Console.WriteLine("test");` resulted in 0 diagnostics.

It seems like there are cases where the CSharpSymbolIsBannedAnalyzer doesn't work with CSharpScript. I'm curious to understand why. Is it a bug? By design?

@haacked
Copy link
Author

haacked commented Nov 1, 2020

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