From 2f59791c2fa1c13ccd11597f42b56a86d7990bbf Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 15 Sep 2022 15:38:24 -0700 Subject: [PATCH 1/2] Fix edge cases of getting attribute data from syntax Fix two customer-discovered edge cases that were causing analyzer crashes. Fixes #75681 Fixes #75706 --- .../Analyzers/SyntaxExtensions.cs | 15 +++++++++++++-- .../NativeMarshallingAttributeAnalyzerTests.cs | 10 ++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs index 20400c4721369e..3fea4b0fae2e21 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs @@ -33,7 +33,17 @@ public static Location FindTypeExpressionOrNullLocation(this AttributeArgumentSy switch (attributeTarget.Identifier.Kind()) { case SyntaxKind.ReturnKeyword: - return ((IMethodSymbol)targetSymbol).GetReturnTypeAttributes().First(attributeSyntaxLocationMatches); + if (targetSymbol is IMethodSymbol method) + { + // Sometimes an attribute is put on a symbol that is nested within the containing symbol. + // For example, the ContainingSymbol for an AttributeSyntax on a local function have a ContainingSymbol of the method. + // Since this method is internal and the callers don't care about attributes on local functions, + // we just allow this method to return null in those cases. + return method.GetReturnTypeAttributes().FirstOrDefault(attributeSyntaxLocationMatches); + } + // An attribute on the return value of a delegate type's Invoke method has a ContainingSymbol of the delegate type. + // We don't care about the attributes in this case for the callers, so we'll just return null. + return null; case SyntaxKind.AssemblyKeyword: return targetSymbol.ContainingAssembly.GetAttributes().First(attributeSyntaxLocationMatches); case SyntaxKind.ModuleKeyword: @@ -43,7 +53,8 @@ public static Location FindTypeExpressionOrNullLocation(this AttributeArgumentSy } } // Sometimes an attribute is put on a symbol that is nested within the containing symbol. - // For example, the ContainingSymbol for an AttributeSyntax on a parameter have a ContainingSymbol of the method. + // For example, the ContainingSymbol for an AttributeSyntax on a parameter have a ContainingSymbol of the method + // and an AttributeSyntax on a local function have a ContainingSymbol of the containing method. // Since this method is internal and the callers don't care about attributes on parameters, we just allow // this method to return null in those cases. return targetSymbol.GetAttributes().FirstOrDefault(attributeSyntaxLocationMatches); diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/NativeMarshallingAttributeAnalyzerTests.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/NativeMarshallingAttributeAnalyzerTests.cs index 582568718722b5..5b8769e19c9ee2 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/NativeMarshallingAttributeAnalyzerTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/NativeMarshallingAttributeAnalyzerTests.cs @@ -272,8 +272,18 @@ public class X { void Foo([MarshalAs(UnmanagedType.I4)] int i) { + [return:MarshalAs(UnmanagedType.I4)] + [SkipLocalsInit] + static int Local() + { + return 0; + } } } + + [return:MarshalAs(UnmanagedType.I4)] + delegate int Y(); + """; await VerifyCS.VerifyAnalyzerAsync(source); From a4856ed1a8ed8e83f3e5eacfe2b544c1a5a34c4d Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Fri, 16 Sep 2022 12:53:05 -0700 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../LibraryImportGenerator/Analyzers/SyntaxExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs index 3fea4b0fae2e21..ac7aded42faaef 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/Analyzers/SyntaxExtensions.cs @@ -36,7 +36,7 @@ public static Location FindTypeExpressionOrNullLocation(this AttributeArgumentSy if (targetSymbol is IMethodSymbol method) { // Sometimes an attribute is put on a symbol that is nested within the containing symbol. - // For example, the ContainingSymbol for an AttributeSyntax on a local function have a ContainingSymbol of the method. + // For example, the ContainingSymbol for an AttributeSyntax on a local function has a ContainingSymbol of the method. // Since this method is internal and the callers don't care about attributes on local functions, // we just allow this method to return null in those cases. return method.GetReturnTypeAttributes().FirstOrDefault(attributeSyntaxLocationMatches); @@ -53,8 +53,8 @@ public static Location FindTypeExpressionOrNullLocation(this AttributeArgumentSy } } // Sometimes an attribute is put on a symbol that is nested within the containing symbol. - // For example, the ContainingSymbol for an AttributeSyntax on a parameter have a ContainingSymbol of the method - // and an AttributeSyntax on a local function have a ContainingSymbol of the containing method. + // For example, the ContainingSymbol for an AttributeSyntax on a parameter has a ContainingSymbol of the method + // and an AttributeSyntax on a local function has a ContainingSymbol of the containing method. // Since this method is internal and the callers don't care about attributes on parameters, we just allow // this method to return null in those cases. return targetSymbol.GetAttributes().FirstOrDefault(attributeSyntaxLocationMatches);