Skip to content

Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK #3077

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 25, 2022
Merged

Change analyzer versions such that the repo can be built with .NET 7 RC2 SDK #3077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04801f1
Change analyzer versions such that the repo can be built with .NET 7 …
vitek-karas Oct 19, 2022
d91427b
Remove CheckAttributeInstantiation method since is no longer necessary
tlakollo Oct 25, 2022
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
1 change: 1 addition & 0 deletions eng/Analyzers.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
<ItemGroup Condition="'$(RunAnalyzers)' == 'true'">
<PackageReference Include="Microsoft.DotNet.CodeAnalysis" Version="$(MicrosoftDotNetCodeAnalysisVersion)" PrivateAssets="all" IsImplicitlyDefined="true" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="$(MicrosoftCodeAnalysisCSharpCodeStyleVersion)" PrivateAssets="all" />
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="$(MicrosoftCodeAnalysisNetAnalyzersVersion)" PrivateAssets="all" />
<!-- <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.205" PrivateAssets="all" /> -->
</ItemGroup>

Expand Down
3 changes: 2 additions & 1 deletion eng/Versions.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@
<MicrosoftDotNetApiCompatVersion>8.0.0-beta.22473.1</MicrosoftDotNetApiCompatVersion>
<MicrosoftDotNetCodeAnalysisVersion>6.0.0-beta.21271.1</MicrosoftDotNetCodeAnalysisVersion>
<MicrosoftCodeAnalysisCSharpCodeStyleVersion>3.10.0-2.final</MicrosoftCodeAnalysisCSharpCodeStyleVersion>
<MicrosoftCodeAnalysisVersion>4.4.0-1.final</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersion>4.5.0-1.22517.9</MicrosoftCodeAnalysisVersion>
<MicrosoftNetCompilersToolsetVersion>$(MicrosoftCodeAnalysisVersion)</MicrosoftNetCompilersToolsetVersion>
<MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>1.0.1-beta1.*</MicrosoftCodeAnalysisCSharpAnalyzerTestingXunitVersion>
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>3.3.2</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
<MicrosoftCodeAnalysisNetAnalyzersVersion>7.0.0-preview1.22513.1</MicrosoftCodeAnalysisNetAnalyzersVersion>
<MicrosoftILVerificationVersion>7.0.0-preview.7.22375.6</MicrosoftILVerificationVersion>
<!-- This controls the version of the cecil package, or the version of cecil in the project graph
when we build the cecil submodule. The reference assembly package will depend on this version of cecil.
Expand Down
2 changes: 1 addition & 1 deletion eng/ilasm.ilproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
project. -->

<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
<TargetFramework>net7.0</TargetFramework>
</PropertyGroup>

<Target Name="CopyILAsmTool" DependsOnTargets="ResolveIlasmToolPaths" Condition="'$(MonoBuild)' == ''">
Expand Down
32 changes: 0 additions & 32 deletions src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,11 @@ public override void Initialize (AnalysisContext context)
if (methodSymbol.IsStaticConstructor () && methodSymbol.HasAttribute (RequiresAttributeName))
ReportRequiresOnStaticCtorDiagnostic (symbolAnalysisContext, methodSymbol);
CheckMatchingAttributesInOverrides (symbolAnalysisContext, methodSymbol);
CheckAttributeInstantiation (symbolAnalysisContext, methodSymbol);
foreach (var typeParameter in methodSymbol.TypeParameters)
CheckAttributeInstantiation (symbolAnalysisContext, typeParameter);

}, SymbolKind.Method);

context.RegisterSymbolAction (symbolAnalysisContext => {
var typeSymbol = (INamedTypeSymbol) symbolAnalysisContext.Symbol;
CheckMatchingAttributesInInterfaces (symbolAnalysisContext, typeSymbol);
CheckAttributeInstantiation (symbolAnalysisContext, typeSymbol);
foreach (var typeParameter in typeSymbol.TypeParameters)
CheckAttributeInstantiation (symbolAnalysisContext, typeParameter);

}, SymbolKind.NamedType);


Expand All @@ -68,24 +60,15 @@ public override void Initialize (AnalysisContext context)
if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Property)) {
CheckMatchingAttributesInOverrides (symbolAnalysisContext, propertySymbol);
}

CheckAttributeInstantiation (symbolAnalysisContext, propertySymbol);
}, SymbolKind.Property);

context.RegisterSymbolAction (symbolAnalysisContext => {
var eventSymbol = (IEventSymbol) symbolAnalysisContext.Symbol;
if (AnalyzerDiagnosticTargets.HasFlag (DiagnosticTargets.Event)) {
CheckMatchingAttributesInOverrides (symbolAnalysisContext, eventSymbol);
}

CheckAttributeInstantiation (symbolAnalysisContext, eventSymbol);
}, SymbolKind.Event);

context.RegisterSymbolAction (symbolAnalysisContext => {
var fieldSymbol = (IFieldSymbol) symbolAnalysisContext.Symbol;
CheckAttributeInstantiation (symbolAnalysisContext, fieldSymbol);
}, SymbolKind.Field);

context.RegisterOperationAction (operationContext => {
var methodInvocation = (IInvocationOperation) operationContext.Operation;
CheckCalledMember (operationContext, methodInvocation.TargetMethod, incompatibleMembers);
Expand Down Expand Up @@ -205,21 +188,6 @@ public override void Initialize (AnalysisContext context)
foreach (var extraSymbolAction in ExtraSymbolActions)
context.RegisterSymbolAction (extraSymbolAction.Action, extraSymbolAction.SymbolKind);

void CheckAttributeInstantiation (
Copy link
Member Author

Choose a reason for hiding this comment

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

Why is it OK to remove this?
Where is the place we perform this check now? (and why did it change with a new analyzer package version)

Copy link
Contributor

Choose a reason for hiding this comment

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

There is new callback calls in attributes, for at least one scenario (I didn't test all scenarios) instead of going through a type argument it went through an object creation, basically when it sees the use of the attribute it created a callback for object creation and then we have a CheckMemberCall that verifies that it has requires producing the warning. My guess is that it was a limitation of the analyzer that we had to work around, just like the global attribute and at somepoint it got fixed

Copy link
Member Author

Choose a reason for hiding this comment

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

OK - I guess if all the tests are passing, it's at least not completely broken

SymbolAnalysisContext symbolAnalysisContext,
ISymbol symbol)
{
if (symbol.IsInRequiresScope (RequiresAttributeName))
return;

foreach (var attr in symbol.GetAttributes ()) {
if (attr.AttributeConstructor?.DoesMemberRequire (RequiresAttributeName, out var requiresAttribute) == true) {
symbolAnalysisContext.ReportDiagnostic (Diagnostic.Create (RequiresDiagnosticRule,
symbol.Locations[0], attr.AttributeConstructor.GetDisplayName (), GetMessageFromAttribute (requiresAttribute), GetUrlFromAttribute (requiresAttribute)));
}
}
}

void CheckCalledMember (
OperationAnalysisContext operationContext,
ISymbol member,
Expand Down
2 changes: 1 addition & 1 deletion test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ public class C
[RequiresAssemblyFiles("Calls Wrapper()")]
Action M2()
{
[global::System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute("Calls C.M1()")] void Wrapper () => M1();
[RequiresAssemblyFiles("Calls C.M1()")] void Wrapper () => M1();
return Wrapper;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/ILLink.RoslynAnalyzer.Tests/RequiresDynamicCodeAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ public class C
[RequiresDynamicCode("Calls Wrapper()")]
Action M2()
{
[global::System.Diagnostics.CodeAnalysis.RequiresDynamicCodeAttribute("Calls C.M1()")] void Wrapper () => M1();
[RequiresDynamicCode("Calls C.M1()")] void Wrapper () => M1();
return Wrapper;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ public class C
[RequiresUnreferencedCode("Calls Wrapper()")]
Action M2()
{
[global::System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute("Calls C.M1()")] void Wrapper () => M1();
[RequiresUnreferencedCode("Calls C.M1()")] void Wrapper () => M1();
return Wrapper;
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/ILLink.RoslynAnalyzer.Tests/UnconditionalSuppressMessageCodeFixTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ public class C

Action M2()
{
[global::System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute(""Trimming"", ""IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code"", Justification = ""<Pending>"")] void Wrapper () => M1();
[UnconditionalSuppressMessage(""Trimming"", ""IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code"", Justification = ""<Pending>"")] void Wrapper () => M1();
return Wrapper;
}
}";
Expand Down
10 changes: 10 additions & 0 deletions test/Mono.Linker.Tests/TestCasesRunner/AssemblyChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public void Verify ()
return s.FullName;
}), StringComparer.Ordinal);

// Workaround for compiler injected attribute to describe the language version
linkedMembers.Remove ("System.Void Microsoft.CodeAnalysis.EmbeddedAttribute::.ctor()");
linkedMembers.Remove ("System.Int32 System.Runtime.CompilerServices.RefSafetyRulesAttribute::Version");
linkedMembers.Remove ("System.Void System.Runtime.CompilerServices.RefSafetyRulesAttribute::.ctor(System.Int32)");

var membersToAssert = originalAssembly.MainModule.Types;
foreach (var originalMember in membersToAssert) {
if (originalMember is TypeDefinition td) {
Expand Down Expand Up @@ -91,6 +96,10 @@ protected virtual void VerifyModule (ModuleDefinition original, ModuleDefinition

protected virtual void VerifyTypeDefinition (TypeDefinition original, TypeDefinition linked)
{
// Workaround for compiler injected attribute to describe the language version
verifiedGeneratedTypes.Add ("Microsoft.CodeAnalysis.EmbeddedAttribute");
verifiedGeneratedTypes.Add ("System.Runtime.CompilerServices.RefSafetyRulesAttribute");

if (linked != null && verifiedGeneratedTypes.Contains (linked.FullName))
return;

Expand Down Expand Up @@ -839,6 +848,7 @@ protected virtual IEnumerable<string> FilterLinkedAttributes (ICustomAttributePr
case "System.Runtime.CompilerServices.RuntimeCompatibilityAttribute":
case "System.Runtime.CompilerServices.CompilerGeneratedAttribute":
case "System.Runtime.CompilerServices.IsReadOnlyAttribute":
case "System.Runtime.CompilerServices.RefSafetyRulesAttribute":
continue;

// When mcs is used to compile the test cases, backing fields end up with this attribute on them
Expand Down