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

Fix delegate conversion on analyzers #1964

Merged
merged 4 commits into from
Apr 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,18 @@ public override void Initialize (AnalysisContext context)
CheckCalledMember (operationContext, eventRef.Member, dangerousPatterns);
}, OperationKind.EventReference);

context.RegisterOperationAction (operationContext => {
var delegateCreation = (IDelegateCreationOperation) operationContext.Operation;
IMethodSymbol methodSymbol;
if (delegateCreation.Target is IMethodReferenceOperation methodRef)
methodSymbol = methodRef.Method;
else if (delegateCreation.Target is IAnonymousFunctionOperation lambda)
methodSymbol = lambda.Symbol;
else
return;
CheckCalledMember (operationContext, methodSymbol, dangerousPatterns);
}, OperationKind.DelegateCreation);

static void CheckCalledMember (
OperationAnalysisContext operationContext,
ISymbol member,
Expand Down
12 changes: 12 additions & 0 deletions src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,18 @@ public override void Initialize (AnalysisContext context)
CheckMethodOrCtorCall (operationContext, prop.SetMethod);
}, OperationKind.PropertyReference);

context.RegisterOperationAction (operationContext => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff makes me think we need to share more between the two analyzers. They look like they're doing pretty much exactly the stuff.

var delegateCreation = (IDelegateCreationOperation) operationContext.Operation;
IMethodSymbol methodSymbol;
if (delegateCreation.Target is IMethodReferenceOperation methodRef)
methodSymbol = methodRef.Method;
else if (delegateCreation.Target is IAnonymousFunctionOperation lambda)
methodSymbol = lambda.Symbol;
else
return;
CheckMethodOrCtorCall (operationContext, methodSymbol);
}, OperationKind.DelegateCreation);

static void CheckStaticConstructors (OperationAnalysisContext operationContext,
ImmutableArray<IMethodSymbol> constructors)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,5 +352,52 @@ public void M()

return VerifyRequiresAssemblyFilesAnalyzer (src);
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with the tests. Can we start to share stuff here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I open #1986 to track the refactoring work in a separate PR since I think it would have a lot of feedback.

public Task LazyDelegateWithRequiresAssemblyFiles ()
{
const string src = @"
using System;
using System.Diagnostics.CodeAnalysis;
class C
{
public static Lazy<C> _default = new Lazy<C>(InitC);
public static C Default => _default.Value;

[RequiresAssemblyFiles]
public static C InitC() {
C cObject = new C();
return cObject;
}
}";

return VerifyRequiresAssemblyFilesAnalyzer (src,
// (6,50): warning IL3002: Using member 'C.InitC()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.
VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (6, 50, 6, 55).WithArguments ("C.InitC()", "", ""));
}

[Fact]
public Task ActionDelegateWithRequiresAssemblyFiles ()
{
const string src = @"
using System;
using System.Diagnostics.CodeAnalysis;
class C
{
[RequiresAssemblyFiles]
public static void M1() { }
public static void M2()
{
Action a = M1;
Action b = () => M1();
}
}";

return VerifyRequiresAssemblyFilesAnalyzer (src,
// (10,20): warning IL3002: Using member 'C.M1()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.
VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (10, 20, 10, 22).WithArguments ("C.M1()", "", ""),
// (11,26): warning IL3002: Using member 'C.M1()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.
VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (11, 26, 11, 30).WithArguments ("C.M1()", "", ""));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ static Task VerifyRequiresUnreferencedCodeCodeFix (
string source,
string fixedSource,
DiagnosticResult[] baselineExpected,
DiagnosticResult[] fixedExpected)
DiagnosticResult[] fixedExpected,
int? numberOfIterations = null)
{
const string rucDef = @"
#nullable enable
Expand All @@ -50,6 +51,10 @@ public sealed class RequiresUnreferencedCodeAttribute : Attribute
("/.editorconfig", SourceText.From (@$"
is_global = true
build_property.{MSBuildPropertyOptionNames.EnableTrimAnalyzer} = true")));
if (numberOfIterations != null) {
test.NumberOfIncrementalIterations = numberOfIterations;
test.NumberOfFixAllIterations = numberOfIterations;
}
test.FixedState.ExpectedDiagnostics.AddRange (fixedExpected);
return test.RunAsync ();
}
Expand Down Expand Up @@ -220,6 +225,7 @@ public class C
[RequiresUnreferencedCodeAttribute(""message"")]
public int M1() => 0;

[RequiresUnreferencedCode(""Calls Wrapper"")]
Action M2()
{
[global::System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute(""Calls M1"")] void Wrapper () => M1();
Expand All @@ -234,7 +240,11 @@ Action M2()
// /0/Test0.cs(12,28): warning IL2026: Using method 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message.
VerifyCS.Diagnostic().WithSpan(12, 28, 12, 32).WithArguments("C.M1()", "message", "")
},
fixedExpected: Array.Empty<DiagnosticResult> ());
fixedExpected: Array.Empty<DiagnosticResult> (),
// The default iterations for the codefix is the number of diagnostics (1 in this case)
// but since the codefixer introduces a new diagnostic in the first iteration, it needs
// to run twice, so we need to set the number of iterations to 2.
numberOfIterations: 2);
}

[Fact]
Expand Down Expand Up @@ -413,5 +423,52 @@ static void TestTypeIsBeforeFieldInit ()
VerifyCS.Diagnostic ().WithSpan (8, 29, 8, 47).WithArguments ("C.TypeIsBeforeFieldInit.AnnotatedMethod()", "Message from --TypeIsBeforeFieldInit.AnnotatedMethod--", "")
);
}

[Fact]
public Task LazyDelegateWithRequiresUnreferencedCode ()
{
const string src = @"
using System;
using System.Diagnostics.CodeAnalysis;
class C
{
public static Lazy<C> _default = new Lazy<C>(InitC);
public static C Default => _default.Value;

[RequiresUnreferencedCode (""Message from --C.InitC--"")]
public static C InitC() {
C cObject = new C();
return cObject;
}
}";

return VerifyRequiresUnreferencedCodeAnalyzer (src,
// (6,50): warning IL2026: Using method 'C.InitC()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Message from --C.InitC--.
VerifyCS.Diagnostic ().WithSpan (6, 50, 6, 55).WithArguments ("C.InitC()", "Message from --C.InitC--", ""));
}

[Fact]
public Task ActionDelegateWithRequiresAssemblyFiles ()
{
const string src = @"
using System;
using System.Diagnostics.CodeAnalysis;
class C
{
[RequiresUnreferencedCode (""Message from --C.M1--"")]
public static void M1() { }
public static void M2()
{
Action a = M1;
Action b = () => M1();
}
}";

return VerifyRequiresUnreferencedCodeAnalyzer (src,
// (10,20): warning IL2026: Using method 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Message from --C.M1--.
VerifyCS.Diagnostic ().WithSpan (10, 20, 10, 22).WithArguments ("C.M1()", "Message from --C.M1--", ""),
// (11,26): warning IL2026: Using method 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Message from --C.M1--.
VerifyCS.Diagnostic ().WithSpan (11, 26, 11, 30).WithArguments ("C.M1()", "Message from --C.M1--", ""));
}
}
}