From 7a61266453f937bad25af292b602dc5348ff18c4 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 20 Jul 2022 08:40:43 -0700 Subject: Fix syntax node scope and nullable warnings (#2900) The syntax node scope should be the innermost node, otherwise GetSymbolInfo will often return null. As an example, GetSymbolInfo will return null for an ArgumentSyntax node, but the underlying expression will likely resolve into a symbol. Also adds proper null checks in case any of the providers return null. --- src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs | 19 +++++++---- .../RequiresAssemblyFilesCodeFixProvider.cs | 2 +- .../RequiresDynamicCodeCodeFixProvider.cs | 2 +- .../RequiresUnreferencedCodeCodeFixProvider.cs | 2 +- .../UnconditionalSuppressMessageCodeFixProvider.cs | 2 +- .../RequiresUnreferencedCodeAnalyzerTests.cs | 37 ++++++++++++++++++++++ 6 files changed, 53 insertions(+), 11 deletions(-) diff --git a/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs b/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs index 17353d26e..909dd3a65 100644 --- a/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs +++ b/src/ILLink.CodeFix/BaseAttributeCodeFixProvider.cs @@ -32,17 +32,22 @@ namespace ILLink.CodeFix protected async Task BaseRegisterCodeFixesAsync (CodeFixContext context) { var document = context.Document; - var root = await document.GetSyntaxRootAsync (context.CancellationToken).ConfigureAwait (false); + if (await document.GetSyntaxRootAsync (context.CancellationToken).ConfigureAwait (false) is not { } root) + return; var diagnostic = context.Diagnostics.First (); - SyntaxNode targetNode = root!.FindNode (diagnostic.Location.SourceSpan); + SyntaxNode targetNode = root.FindNode (diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); if (FindAttributableParent (targetNode, AttributableParentTargets) is not SyntaxNode attributableNode) return; - var model = await document.GetSemanticModelAsync (context.CancellationToken).ConfigureAwait (false); - var targetSymbol = model!.GetSymbolInfo (targetNode).Symbol!; + if (await document.GetSemanticModelAsync (context.CancellationToken).ConfigureAwait (false) is not { } model) + return; + if (model.GetSymbolInfo (targetNode).Symbol is not { } targetSymbol) + return; + if (model.Compilation.GetTypeByMetadataName (FullyQualifiedAttributeName) is not { } attributeSymbol) + return; + // N.B. May be null for FieldDeclaration, since field declarations can declare multiple variables + var attributableSymbol = model.GetDeclaredSymbol (attributableNode); - var attributableSymbol = model!.GetDeclaredSymbol (attributableNode)!; - var attributeSymbol = model!.Compilation.GetTypeByMetadataName (FullyQualifiedAttributeName)!; var attributeArguments = GetAttributeArguments (attributableSymbol, targetSymbol, SyntaxGenerator.GetGenerator (document), diagnostic); var codeFixTitle = CodeFixTitle.ToString (); @@ -104,7 +109,7 @@ namespace ILLink.CodeFix } protected abstract SyntaxNode[] GetAttributeArguments ( - ISymbol attributableSymbol, + ISymbol? attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic); diff --git a/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs b/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs index ea734e746..1bf420c05 100644 --- a/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs +++ b/src/ILLink.CodeFix/RequiresAssemblyFilesCodeFixProvider.cs @@ -33,7 +33,7 @@ namespace ILLink.CodeFix public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context); - protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) + protected override SyntaxNode[] GetAttributeArguments (ISymbol? attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) { var symbolDisplayName = targetSymbol.GetDisplayName (); if (string.IsNullOrEmpty (symbolDisplayName) || HasPublicAccessibility (attributableSymbol)) diff --git a/src/ILLink.CodeFix/RequiresDynamicCodeCodeFixProvider.cs b/src/ILLink.CodeFix/RequiresDynamicCodeCodeFixProvider.cs index cab57d7e6..b0e5ef9ec 100644 --- a/src/ILLink.CodeFix/RequiresDynamicCodeCodeFixProvider.cs +++ b/src/ILLink.CodeFix/RequiresDynamicCodeCodeFixProvider.cs @@ -30,7 +30,7 @@ namespace ILLink.CodeFix public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context); - protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) + protected override SyntaxNode[] GetAttributeArguments (ISymbol? attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) { var symbolDisplayName = targetSymbol.GetDisplayName (); if (string.IsNullOrEmpty (symbolDisplayName) || HasPublicAccessibility (attributableSymbol)) diff --git a/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs b/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs index 12671a8e7..578fdff03 100644 --- a/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs +++ b/src/ILLink.CodeFix/RequiresUnreferencedCodeCodeFixProvider.cs @@ -30,7 +30,7 @@ namespace ILLink.CodeFix public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context); - protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) + protected override SyntaxNode[] GetAttributeArguments (ISymbol? attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) { var symbolDisplayName = targetSymbol.GetDisplayName (); if (string.IsNullOrEmpty (symbolDisplayName) || HasPublicAccessibility (attributableSymbol)) diff --git a/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs b/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs index 25cd9d54f..c40aeba0f 100644 --- a/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs +++ b/src/ILLink.CodeFix/UnconditionalSuppressMessageCodeFixProvider.cs @@ -37,7 +37,7 @@ namespace ILLink.CodeFix public sealed override Task RegisterCodeFixesAsync (CodeFixContext context) => BaseRegisterCodeFixesAsync (context); - protected override SyntaxNode[] GetAttributeArguments (ISymbol attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) + protected override SyntaxNode[] GetAttributeArguments (ISymbol? attributableSymbol, ISymbol targetSymbol, SyntaxGenerator syntaxGenerator, Diagnostic diagnostic) { // Category of the attribute var ruleCategory = syntaxGenerator.AttributeArgument ( diff --git a/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs b/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs index f810da20e..f758fa8e0 100644 --- a/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs +++ b/test/ILLink.RoslynAnalyzer.Tests/RequiresUnreferencedCodeAnalyzerTests.cs @@ -54,6 +54,43 @@ build_property.{MSBuildPropertyOptionNames.EnableTrimAnalyzer} = true"))); } + [Fact] + public async Task WarningInArgument () + { + var test = """ +using System.Diagnostics.CodeAnalysis; +public class C +{ + [RequiresUnreferencedCode("message")] + public int M1() => 0; + public void M2(int x) + { + } + public void M3() => M2(M1()); +} +"""; + var fixtest = """ +using System.Diagnostics.CodeAnalysis; +public class C +{ + [RequiresUnreferencedCode("message")] + public int M1() => 0; + public void M2(int x) + { + } + + [RequiresUnreferencedCode()] + public void M3() => M2(M1()); +} +"""; + await VerifyRequiresUnreferencedCodeCodeFix (test, fixtest, new[] { + // /0/Test0.cs(9,25): warning IL2026: Using member 'C.M1()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. message. + VerifyCS.Diagnostic(DiagnosticId.RequiresUnreferencedCode).WithSpan(9, 25, 9, 29).WithArguments("C.M1()", " message.", ""), + }, new[] { + // /0/Test0.cs(10,6): error CS7036: There is no argument given that corresponds to the required formal parameter 'message' of 'RequiresUnreferencedCodeAttribute.RequiresUnreferencedCodeAttribute(string)' + DiagnosticResult.CompilerError("CS7036").WithSpan(10, 6, 10, 32).WithArguments("message", "System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute.RequiresUnreferencedCodeAttribute(string)"), + }); + } [Fact] public async Task SimpleDiagnosticFix () -- cgit v1.2.3