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 1 commit
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
6 changes: 6 additions & 0 deletions src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ public override void Initialize (AnalysisContext context)
CheckCalledMember (operationContext, eventRef.Member, dangerousPatterns);
}, OperationKind.EventReference);

context.RegisterOperationAction (operationContext => {
var delegateCreation = (IDelegateCreationOperation) operationContext.Operation;
var target = (IMethodReferenceOperation) delegateCreation.Target;
tlakollo marked this conversation as resolved.
Show resolved Hide resolved
CheckCalledMember (operationContext, target.Member, dangerousPatterns);
}, OperationKind.DelegateCreation);

static void CheckCalledMember (
OperationAnalysisContext operationContext,
ISymbol member,
Expand Down
6 changes: 6 additions & 0 deletions src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ 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;
var target = (IMethodReferenceOperation) delegateCreation.Target;
tlakollo marked this conversation as resolved.
Show resolved Hide resolved
CheckMethodOrCtorCall (operationContext, target.Method);
}, 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 @@ -413,5 +413,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--", ""));
}
}
}