From 762823ad515bae84375d957e1c507cd1f4ab32a2 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Sat, 5 Jan 2019 07:16:44 -0800 Subject: [PATCH 1/2] Fix CA1710 (IdentifiersShouldHaveCorrectSuffix) for event symbol type Fire CA1710 (Event type name should end with "EventHandler") only if event type matches event handler signature Fixes #1822 --- .../IdentifiersShouldHaveCorrectSuffix.cs | 22 ++++++++++------ ...IdentifiersShouldHaveCorrectSuffixTests.cs | 25 +++++++++++++++++++ .../Extensions/IMethodSymbolExtensions.cs | 11 ++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs b/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs index 11d959783e..84c51d988e 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs @@ -147,16 +147,22 @@ private static void AnalyzeCompilationStart(CompilationStartAnalysisContext cont } , SymbolKind.NamedType); - context.RegisterSymbolAction((saContext) => + var eventArgsType = WellKnownTypes.EventArgs(context.Compilation); + if (eventArgsType != null) { - const string eventHandlerString = "EventHandler"; - var eventSymbol = saContext.Symbol as IEventSymbol; - if (!eventSymbol.Type.Name.EndsWith(eventHandlerString, StringComparison.Ordinal)) + context.RegisterSymbolAction((saContext) => { - saContext.ReportDiagnostic(eventSymbol.CreateDiagnostic(DefaultRule, eventSymbol.Type.Name, eventHandlerString)); - } - }, - SymbolKind.Event); + const string eventHandlerString = "EventHandler"; + var eventSymbol = (IEventSymbol)saContext.Symbol; + if (!eventSymbol.Type.Name.EndsWith(eventHandlerString, StringComparison.Ordinal) && + eventSymbol.Type.TypeKind == TypeKind.Delegate && + ((INamedTypeSymbol)eventSymbol.Type).DelegateInvokeMethod?.HasEventHandlerSignature(eventArgsType) == true) + { + saContext.ReportDiagnostic(eventSymbol.CreateDiagnostic(DefaultRule, eventSymbol.Type.Name, eventHandlerString)); + } + }, + SymbolKind.Event); + } } } } diff --git a/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffixTests.cs b/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffixTests.cs index a6d2f0a2ea..72be63b886 100644 --- a/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffixTests.cs +++ b/src/Microsoft.CodeQuality.Analyzers/UnitTests/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffixTests.cs @@ -1079,6 +1079,31 @@ Inherits Attribute End Class"); } + [Fact, WorkItem(1822, "https://github.com/dotnet/roslyn-analyzers/issues/1822")] + public void CA1710_SystemAction_CSharp() + { + VerifyCSharp(@" +using System; + +public class C +{ + public event Action MyEvent; +}"); + } + + [Fact, WorkItem(1822, "https://github.com/dotnet/roslyn-analyzers/issues/1822")] + public void CA1710_CustomDelegate_CSharp() + { + VerifyCSharp(@" +using System; + +public class C +{ + public delegate void MyDelegate(int param); + public event MyDelegate MyEvent; +}"); + } + private static DiagnosticResult GetCA1710BasicResultAt(int line, int column, string symbolName, string replacementName, bool isSpecial = false) { return GetBasicResultAt( diff --git a/src/Utilities/Extensions/IMethodSymbolExtensions.cs b/src/Utilities/Extensions/IMethodSymbolExtensions.cs index 26533aba9b..bd66bfb61d 100644 --- a/src/Utilities/Extensions/IMethodSymbolExtensions.cs +++ b/src/Utilities/Extensions/IMethodSymbolExtensions.cs @@ -382,5 +382,16 @@ public static int GetParameterIndex(this IMethodSymbol methodSymbol, IParameterS throw new ArgumentException("Invalid paramater", nameof(parameterSymbol)); } + + /// + /// Returns true for void returning methods with two parameters, where + /// the first parameter is of type and the second + /// parameter inherits from or equals type. + /// + public static bool HasEventHandlerSignature(this IMethodSymbol method, INamedTypeSymbol eventArgsType) + => eventArgsType != null && + method.Parameters.Length == 2 && + method.Parameters[0].Type.SpecialType == SpecialType.System_Object && + method.Parameters[1].Type.DerivesFrom(eventArgsType, baseTypesOnly: true); } } From 0df5197b78879c0cb4d52b11bff694ff75b20489 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Mon, 7 Jan 2019 16:17:16 -0800 Subject: [PATCH 2/2] PR feedback: Only fire CA1710 for event handlers if the type is defined in source --- .../IdentifiersShouldHaveCorrectSuffix.cs | 1 + src/Utilities/Extensions/ISymbolExtensions.cs | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs b/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs index 84c51d988e..15d41e7d91 100644 --- a/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs +++ b/src/Microsoft.CodeQuality.Analyzers/Core/ApiDesignGuidelines/IdentifiersShouldHaveCorrectSuffix.cs @@ -155,6 +155,7 @@ private static void AnalyzeCompilationStart(CompilationStartAnalysisContext cont const string eventHandlerString = "EventHandler"; var eventSymbol = (IEventSymbol)saContext.Symbol; if (!eventSymbol.Type.Name.EndsWith(eventHandlerString, StringComparison.Ordinal) && + eventSymbol.Type.IsInSource() && eventSymbol.Type.TypeKind == TypeKind.Delegate && ((INamedTypeSymbol)eventSymbol.Type).DelegateInvokeMethod?.HasEventHandlerSignature(eventArgsType) == true) { diff --git a/src/Utilities/Extensions/ISymbolExtensions.cs b/src/Utilities/Extensions/ISymbolExtensions.cs index 8d29dd2313..acb79ff834 100644 --- a/src/Utilities/Extensions/ISymbolExtensions.cs +++ b/src/Utilities/Extensions/ISymbolExtensions.cs @@ -552,5 +552,13 @@ public static bool HasAttribute(this ISymbol symbol, INamedTypeSymbol attribute) { return symbol.GetAttributes().Any(attr => attr.AttributeClass.Equals(attribute)); } + + /// + /// Indicates if a symbol has at least one location in source. + /// + public static bool IsInSource(this ISymbol symbol) + { + return symbol.Locations.Any(l => l.IsInSource); + } } }