Skip to content

Commit

Permalink
Warn on RUC annotated ctors invoked through new() constraint in ana…
Browse files Browse the repository at this point in the history 
…lyzer (#2254)

* Add support in RUC analyzer for new() constraint on generics

* Support `new()` constraint on types

* Use SyntaxNodeAnalysisContext for constructor constraints

* Lint

* Update justification
  • Loading branch information
@mateoatr
mateoatr authored Sep 8, 2021
1 parent a9888c2 commit 4f28a65
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 8 deletions.
13 changes: 13 additions & 0 deletions src/ILLink.RoslynAnalyzer/ISymbolExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ internal static bool HasAttribute (this ISymbol symbol, string attributeName)
return false;
}

internal static bool TryGetAttribute (this ISymbol member, string attributeName, [NotNullWhen (returnValue: true)] out AttributeData? attribute)
{
attribute = null;
foreach (var attr in member.GetAttributes ()) {
if (attr.AttributeClass is { } attrClass && attrClass.HasName (attributeName)) {
attribute = attr;
return true;
}
}

return false;
}

internal static bool TryGetOverriddenMember (this ISymbol? symbol, [NotNullWhen (returnValue: true)] out ISymbol? overridenMember)
{
overridenMember = symbol switch {
Expand Down
10 changes: 8 additions & 2 deletions src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using ILLink.Shared;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

Expand All @@ -27,6 +28,8 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer

private protected virtual ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)> ExtraOperationActions { get; } = ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)>.Empty;

private protected virtual ImmutableArray<(Action<SyntaxNodeAnalysisContext> Action, SyntaxKind[] SyntaxKind)> ExtraSyntaxNodeActions { get; } = ImmutableArray<(Action<SyntaxNodeAnalysisContext> Action, SyntaxKind[] SyntaxKind)>.Empty;

public override void Initialize (AnalysisContext context)
{
context.EnableConcurrentExecution ();
Expand All @@ -35,8 +38,8 @@ public override void Initialize (AnalysisContext context)
var compilation = context.Compilation;
if (!IsAnalyzerEnabled (context.Options, compilation))
return;
var incompatibleMembers = GetSpecialIncompatibleMembers (compilation);

var incompatibleMembers = GetSpecialIncompatibleMembers (compilation);
context.RegisterSymbolAction (symbolAnalysisContext => {
var methodSymbol = (IMethodSymbol) symbolAnalysisContext.Symbol;
CheckMatchingAttributesInOverrides (symbolAnalysisContext, methodSymbol);
Expand Down Expand Up @@ -119,6 +122,9 @@ public override void Initialize (AnalysisContext context)
foreach (var extraOperationAction in ExtraOperationActions)
context.RegisterOperationAction (extraOperationAction.Action, extraOperationAction.OperationKind);

foreach (var extraSyntaxNodeAction in ExtraSyntaxNodeActions)
context.RegisterSyntaxNodeAction (extraSyntaxNodeAction.Action, extraSyntaxNodeAction.SyntaxKind);

void CheckStaticConstructors (OperationAnalysisContext operationContext,
ImmutableArray<IMethodSymbol> staticConstructors)
{
Expand Down Expand Up @@ -263,7 +269,7 @@ private void ReportMismatchInAttributesDiagnostic (SymbolAnalysisContext symbolA

protected abstract string GetMessageFromAttribute (AttributeData requiresAttribute);

private string GetUrlFromAttribute (AttributeData? requiresAttribute)
public static string GetUrlFromAttribute (AttributeData? requiresAttribute)
{
var url = requiresAttribute?.NamedArguments.FirstOrDefault (na => na.Key == "Url").Value.Value?.ToString ();
return MessageFormat.FormatRequiresAttributeUrlArg (url);
Expand Down
56 changes: 56 additions & 0 deletions src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

using System;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using ILLink.Shared;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;

namespace ILLink.RoslynAnalyzer
Expand All @@ -31,6 +34,56 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
operationContext.Operation.Syntax.GetLocation ()));
};

[SuppressMessage ("MicrosoftCodeAnalysisPerformance", "RS1008",
Justification = "Storing per-compilation data inside a diagnostic analyzer might cause stale compilations to remain alive." +
"This action is registered through a compilation start action, so that instances that register this syntax" +
" node action will not outlive a compilation's lifetime, avoiding the possibility of the locals stored in" +
" this function to cause for any stale compilations to remain in memory.")]
static readonly Action<SyntaxNodeAnalysisContext> s_constructorConstraint = syntaxNodeAnalysisContext => {
var model = syntaxNodeAnalysisContext.SemanticModel;
if (syntaxNodeAnalysisContext.ContainingSymbol is not ISymbol containingSymbol || containingSymbol.HasAttribute (RequiresUnreferencedCodeAttribute))
return;

GenericNameSyntax genericNameSyntaxNode = (GenericNameSyntax) syntaxNodeAnalysisContext.Node;
var typeParams = ImmutableArray<ITypeParameterSymbol>.Empty;
var typeArgs = ImmutableArray<ITypeSymbol>.Empty;
switch (model.GetSymbolInfo (genericNameSyntaxNode).Symbol) {
case INamedTypeSymbol typeSymbol:
typeParams = typeSymbol.TypeParameters;
typeArgs = typeSymbol.TypeArguments;
break;

case IMethodSymbol methodSymbol:
typeParams = methodSymbol.TypeParameters;
typeArgs = methodSymbol.TypeArguments;
break;

default:
return;
}

for (int i = 0; i < typeParams.Length; i++) {
var typeParam = typeParams[i];
var typeArg = typeArgs[i];
if (!typeParam.HasConstructorConstraint)
continue;

var typeArgCtors = ((INamedTypeSymbol) typeArg).InstanceConstructors;
foreach (var instanceCtor in typeArgCtors) {
if (instanceCtor.Arity > 0)
continue;

if (instanceCtor.TryGetAttribute (RequiresUnreferencedCodeAttribute, out var requiresUnreferencedCodeAttribute)) {
syntaxNodeAnalysisContext.ReportDiagnostic (Diagnostic.Create (s_requiresUnreferencedCodeRule,
syntaxNodeAnalysisContext.Node.GetLocation (),
containingSymbol.GetDisplayName (),
(string) requiresUnreferencedCodeAttribute.ConstructorArguments[0].Value!,
GetUrlFromAttribute (requiresUnreferencedCodeAttribute)));
}
}
}
};

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics =>
ImmutableArray.Create (s_dynamicTypeInvocationRule, s_requiresUnreferencedCodeRule, s_requiresUnreferencedCodeAttributeMismatch);

Expand All @@ -50,6 +103,9 @@ protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation
private protected override ImmutableArray<(Action<OperationAnalysisContext> Action, OperationKind[] OperationKind)> ExtraOperationActions =>
ImmutableArray.Create ((s_dynamicTypeInvocation, new OperationKind[] { OperationKind.DynamicInvocation }));

private protected override ImmutableArray<(Action<SyntaxNodeAnalysisContext> Action, SyntaxKind[] SyntaxKind)> ExtraSyntaxNodeActions =>
ImmutableArray.Create ((s_constructorConstraint, new SyntaxKind[] { SyntaxKind.GenericName }));

protected override bool VerifyAttributeArguments (AttributeData attribute) =>
attribute.ConstructorArguments.Length >= 1 && attribute.ConstructorArguments[0] is { Type: { SpecialType: SpecialType.System_String } } ctorArg;

Expand Down
34 changes: 28 additions & 6 deletions test/Mono.Linker.Tests.Cases/RequiresCapability/RequiresUnreferencedCodeCapability.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ public static void Main ()
AccessThroughPInvoke.Test ();
OnEventMethod.Test ();
AccessThroughNewConstraint.Test ();
AccessThroughNewConstraint.TestNewConstraintOnTypeParameter ();
AccessThroughNewConstraint.TestNewConstraintOnTypeParameterOfStaticType ();
AccessThroughLdToken.Test ();
RequiresOnClass.Test ();
}
Expand Down Expand Up @@ -770,19 +772,39 @@ public static void Test ()

class AccessThroughNewConstraint
{
class NewConstrainTestType
class NewConstraintTestType
{
[RequiresUnreferencedCode ("Message for --NewConstrainTestType.ctor--")]
public NewConstrainTestType () { }
[RequiresUnreferencedCode ("Message for --NewConstraintTestType.ctor--")]
public NewConstraintTestType () { }
}

static void GenericMethod<T> () where T : new() { }

// https://github.com/mono/linker/issues/2117
[ExpectedWarning ("IL2026", "--NewConstrainTestType.ctor--", GlobalAnalysisOnly = true)]
[ExpectedWarning ("IL2026", "--NewConstraintTestType.ctor--")]
public static void Test ()
{
GenericMethod<NewConstrainTestType> ();
GenericMethod<NewConstraintTestType> ();
}

static class NewConstraintOnTypeParameterOfStaticType<T> where T : new()
{
public static void DoNothing () { }
}

class NewConstaintOnTypeParameter<T> where T : new()
{
}

[ExpectedWarning ("IL2026", "--NewConstraintTestType.ctor--")]
public static void TestNewConstraintOnTypeParameter ()
{
_ = new NewConstaintOnTypeParameter<NewConstraintTestType> ();
}

[ExpectedWarning ("IL2026", "--NewConstraintTestType.ctor--")]
public static void TestNewConstraintOnTypeParameterOfStaticType ()
{
NewConstraintOnTypeParameterOfStaticType<NewConstraintTestType>.DoNothing ();
}
}

Expand Down

0 comments on commit 4f28a65

Please sign in to comment.