Skip to content

Commit bead6ee

Browse files
committed
Check delegate types
Fix #322
1 parent d85c8c8 commit bead6ee

9 files changed

Lines changed: 257 additions & 14 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ Roslyn analyzers for WPF.
6666
| [WPF0085](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0085.md)| Target of [XamlSetTypeConverter] should exist and have correct signature
6767
| [WPF0090](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0090.md)| Name the invoked method OnEventName
6868
| [WPF0091](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0091.md)| Name the invoked method OnEventName
69+
| [WPF0092](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0092.md)| Use correct handler type
6970
| [WPF0100](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0100.md)| Backing field for a RoutedEvent should match registered name
7071
| [WPF0101](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0101.md)| Containing type should be used as registered owner
7172
| [WPF0102](https://github.com/DotNetAnalyzers/WpfAnalyzers/tree/master/documentation/WPF0102.md)| Name of the event should match registered name

WpfAnalyzers.Test/WPF0090RegisterClassHandlerCallbackNameShouldMatchEventTests/Valid.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ public class FooControl : Control
2020
{
2121
static FooControl()
2222
{
23-
EventManager.RegisterClassHandler(typeof(TextBlock), SizeChangedEvent, new RoutedEventHandler(OnSizeChanged));
23+
EventManager.RegisterClassHandler(typeof(TextBlock), SizeChangedEvent, new SizeChangedEventHandler(OnSizeChanged));
2424
}
2525
26-
private static void OnSizeChanged(object sender, RoutedEventArgs e)
26+
private static void OnSizeChanged(object sender, SizeChangedEventArgs e)
2727
{
2828
throw new System.NotImplementedException();
2929
}
@@ -45,10 +45,10 @@ public class FooControl : Control
4545
{
4646
static FooControl()
4747
{
48-
EventManager.RegisterClassHandler(typeof(TextBlock), SizeChangedEvent, new RoutedEventHandler(OnSizeChanged), true);
48+
EventManager.RegisterClassHandler(typeof(TextBlock), SizeChangedEvent, new SizeChangedEventHandler(OnSizeChanged), true);
4949
}
5050
51-
private static void OnSizeChanged(object sender, RoutedEventArgs e)
51+
private static void OnSizeChanged(object sender, SizeChangedEventArgs e)
5252
{
5353
throw new System.NotImplementedException();
5454
}

WpfAnalyzers.Test/WPF0091AddAndRemoveHandlerCallbackNameShouldMatchEventTests/Valid.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ public class FooControl : Control
2020
{
2121
public FooControl()
2222
{
23-
this.AddHandler(SizeChangedEvent, new RoutedEventHandler(OnSizeChanged));
23+
this.AddHandler(SizeChangedEvent, new SizeChangedEventHandler(OnSizeChanged));
2424
}
2525
26-
private static void OnSizeChanged(object? sender, RoutedEventArgs e)
26+
private static void OnSizeChanged(object? sender, SizeChangedEventArgs e)
2727
{
2828
throw new System.NotImplementedException();
2929
}
@@ -45,10 +45,10 @@ public class FooControl : Control
4545
{
4646
public FooControl()
4747
{
48-
this.AddHandler(SizeChangedEvent, new RoutedEventHandler(OnSizeChanged));
48+
this.AddHandler(SizeChangedEvent, new SizeChangedEventHandler(OnSizeChanged));
4949
}
5050
51-
private static void OnSizeChanged(object? sender, RoutedEventArgs e)
51+
private static void OnSizeChanged(object? sender, SizeChangedEventArgs e)
5252
{
5353
throw new System.NotImplementedException();
5454
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
namespace WpfAnalyzers.Test.WPF0092RegisterClassHandlerDelegateType;
2+
3+
using Gu.Roslyn.Asserts;
4+
using NUnit.Framework;
5+
6+
public static class CodeFix
7+
{
8+
private static readonly RoutedEventCallbackAnalyzer Analyzer = new();
9+
private static readonly RenameMemberFix Fix = new();
10+
private static readonly ExpectedDiagnostic ExpectedDiagnostic = ExpectedDiagnostic.Create(Descriptors.WPF0092WrongDelegateType);
11+
12+
[TestCase("new Action<object, RoutedEventArgs>((sender, e) => { })")]
13+
[TestCase("(Action<object, RoutedEventArgs>)((sender, e) => { })")]
14+
public static void Message(string expression)
15+
{
16+
var code = @"
17+
namespace N;
18+
19+
using System;
20+
using System.Windows;
21+
using System.Windows.Controls;
22+
23+
public static class C
24+
{
25+
static C()
26+
{
27+
EventManager.RegisterClassHandler(
28+
typeof(PasswordBox),
29+
PasswordBox.PasswordChangedEvent,
30+
↓new Action<object, RoutedEventArgs>((sender, e) => { }));
31+
}
32+
}".AssertReplace("new Action<object, RoutedEventArgs>((sender, e) => { })", expression);
33+
RoslynAssert.Diagnostics(Analyzer, ExpectedDiagnostic.WithMessage("Use correct handler type"), code);
34+
}
35+
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
namespace WpfAnalyzers.Test.WPF0092RegisterClassHandlerDelegateType;
2+
3+
using Gu.Roslyn.Asserts;
4+
using NUnit.Framework;
5+
6+
public static class Valid
7+
{
8+
private static readonly RoutedEventCallbackAnalyzer Analyzer = new();
9+
10+
[TestCase("new RoutedEventHandler(OnPasswordChanged)")]
11+
[TestCase("new RoutedEventHandler((sender, e) => { })")]
12+
[TestCase("new RoutedEventHandler((sender, e) => OnPasswordChanged(sender, e))")]
13+
public static void RegisterClassHandlerPasswordChangedEvent(string expression)
14+
{
15+
var code = @"
16+
namespace N;
17+
18+
using System.Windows;
19+
using System.Windows.Controls;
20+
21+
public static class C
22+
{
23+
static C()
24+
{
25+
EventManager.RegisterClassHandler(
26+
typeof(PasswordBox),
27+
PasswordBox.PasswordChangedEvent,
28+
new RoutedEventHandler(OnPasswordChanged));
29+
30+
#pragma warning disable CS8321
31+
static void OnPasswordChanged(object sender, RoutedEventArgs e)
32+
{
33+
}
34+
}
35+
}".AssertReplace("new RoutedEventHandler(OnPasswordChanged)", expression);
36+
RoslynAssert.Valid(Analyzer, code);
37+
}
38+
39+
[TestCase("new KeyEventHandler(OnKeyDown)")]
40+
[TestCase("new KeyEventHandler((sender, e) => { })")]
41+
[TestCase("new KeyEventHandler((sender, e) => OnKeyDown(sender, e))")]
42+
public static void RegisterClassHandlerKeyDownEvent(string expression)
43+
{
44+
var code = @"
45+
namespace N;
46+
47+
using System.Windows;
48+
using System.Windows.Controls;
49+
using System.Windows.Input;
50+
51+
public static class C
52+
{
53+
static C()
54+
{
55+
EventManager.RegisterClassHandler(
56+
typeof(TextBox),
57+
TextBox.KeyDownEvent,
58+
new KeyEventHandler(OnKeyDown));
59+
60+
#pragma warning disable CS8321
61+
static void OnKeyDown(object sender, KeyEventArgs e)
62+
{
63+
}
64+
}
65+
}".AssertReplace("new KeyEventHandler(OnKeyDown)", expression);
66+
RoslynAssert.Valid(Analyzer, code);
67+
}
68+
69+
[Test]
70+
public static void EventDeclaration()
71+
{
72+
var code = @"
73+
namespace N;
74+
75+
using System.Windows;
76+
using System.Windows.Controls;
77+
78+
public class FooControl : Control
79+
{
80+
/// <summary>Identifies the <see cref=""ValueChanged""/> routed event.</summary>
81+
public static readonly RoutedEvent ValueChangedEvent = EventManager.RegisterRoutedEvent(
82+
nameof(ValueChanged),
83+
RoutingStrategy.Direct,
84+
typeof(RoutedEventHandler),
85+
typeof(FooControl));
86+
87+
public event RoutedEventHandler ValueChanged
88+
{
89+
add { this.AddHandler(ValueChangedEvent, value); }
90+
remove { this.RemoveHandler(ValueChangedEvent, value); }
91+
}
92+
}";
93+
RoslynAssert.Valid(Analyzer, code);
94+
}
95+
}

WpfAnalyzers.sln

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11

22
Microsoft Visual Studio Solution File, Format Version 12.00
3-
# Visual Studio Version 16
4-
VisualStudioVersion = 16.0.29201.188
3+
# Visual Studio Version 17
4+
VisualStudioVersion = 17.1.32407.343
55
MinimumVisualStudioVersion = 10.0.40219.1
66
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".sln", ".sln", "{A1A96555-15B1-4A0C-ADC1-059CEAA3F788}"
77
ProjectSection(SolutionItems) = preProject
@@ -69,6 +69,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "docs", "docs", "{BAD2A4EE-6
6969
documentation\WPF0085.md = documentation\WPF0085.md
7070
documentation\WPF0090.md = documentation\WPF0090.md
7171
documentation\WPF0091.md = documentation\WPF0091.md
72+
documentation\WPF0092.md = documentation\WPF0092.md
7273
documentation\WPF0100.md = documentation\WPF0100.md
7374
documentation\WPF0101.md = documentation\WPF0101.md
7475
documentation\WPF0102.md = documentation\WPF0102.md

WpfAnalyzers/Analyzers/RoutedEventCallbackAnalyzer.cs

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
namespace WpfAnalyzers;
22

3+
using System;
34
using System.Collections.Immutable;
45

56
using Gu.Roslyn.AnalyzerExtensions;
@@ -14,20 +15,20 @@ internal class RoutedEventCallbackAnalyzer : DiagnosticAnalyzer
1415
{
1516
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(
1617
Descriptors.WPF0090RegisterClassHandlerCallbackNameShouldMatchEvent,
17-
Descriptors.WPF0091AddAndRemoveHandlerCallbackNameShouldMatchEvent);
18+
Descriptors.WPF0091AddAndRemoveHandlerCallbackNameShouldMatchEvent,
19+
Descriptors.WPF0092WrongDelegateType);
1820

1921
public override void Initialize(AnalysisContext context)
2022
{
2123
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
2224
context.EnableConcurrentExecution();
23-
context.RegisterSyntaxNodeAction(x => Handle(x), SyntaxKind.Argument);
25+
context.RegisterSyntaxNodeAction(x => Handle(x), SyntaxKind.InvocationExpression);
2426
}
2527

2628
private static void Handle(SyntaxNodeAnalysisContext context)
2729
{
2830
if (!context.IsExcludedFromAnalysis() &&
29-
context.Node is ArgumentSyntax { Expression: ObjectCreationExpressionSyntax { } } argument &&
30-
argument.FirstAncestor<InvocationExpressionSyntax>() is { } invocation)
31+
context.Node is InvocationExpressionSyntax { } invocation)
3132
{
3233
if (EventManager.RegisterClassHandler.Match(invocation, context.SemanticModel, context.CancellationToken) is { } registerClassHandler)
3334
{
@@ -40,6 +41,14 @@ private static void Handle(SyntaxNodeAnalysisContext context)
4041
properties,
4142
expectedName));
4243
}
44+
45+
if (WrongType(registerClassHandler.EventArgument, registerClassHandler.DelegateArgument))
46+
{
47+
context.ReportDiagnostic(
48+
Diagnostic.Create(
49+
Descriptors.WPF0092WrongDelegateType,
50+
registerClassHandler.DelegateArgument.GetLocation()));
51+
}
4352
}
4453
else if (EventManager.AddHandler.Match(invocation, context.SemanticModel, context.CancellationToken) is { } addHandler)
4554
{
@@ -52,6 +61,14 @@ private static void Handle(SyntaxNodeAnalysisContext context)
5261
properties,
5362
expectedName));
5463
}
64+
65+
if (WrongType(addHandler.EventArgument, addHandler.DelegateArgument))
66+
{
67+
context.ReportDiagnostic(
68+
Diagnostic.Create(
69+
Descriptors.WPF0092WrongDelegateType,
70+
addHandler.DelegateArgument.GetLocation()));
71+
}
5572
}
5673
else if (EventManager.RemoveHandler.Match(invocation, context.SemanticModel, context.CancellationToken) is { } removeHandler)
5774
{
@@ -64,6 +81,14 @@ private static void Handle(SyntaxNodeAnalysisContext context)
6481
properties,
6582
expectedName));
6683
}
84+
85+
if (WrongType(removeHandler.EventArgument, removeHandler.DelegateArgument))
86+
{
87+
context.ReportDiagnostic(
88+
Diagnostic.Create(
89+
Descriptors.WPF0092WrongDelegateType,
90+
removeHandler.DelegateArgument.GetLocation()));
91+
}
6792
}
6893

6994
(Location Location, ImmutableDictionary<string, string?> Properties, string ExpectedName)? ShouldRename(IMethodSymbol target, ArgumentSyntax eventArgument, ArgumentSyntax callbackArg)
@@ -89,6 +114,7 @@ private static void Handle(SyntaxNodeAnalysisContext context)
89114
{
90115
return callbackArg switch
91116
{
117+
{ Expression: IdentifierNameSyntax { Identifier.ValueText: "value" } } => null,
92118
{ Expression: ObjectCreationExpressionSyntax { ArgumentList.Arguments: { Count: 1 } arguments } }
93119
=> Identifier(arguments[0].Expression),
94120
_ => Identifier(callbackArg.Expression),
@@ -105,6 +131,19 @@ private static void Handle(SyntaxNodeAnalysisContext context)
105131
};
106132
}
107133
}
134+
135+
bool WrongType(ArgumentSyntax eventArgument, ArgumentSyntax callbackArg)
136+
{
137+
if (context.SemanticModel.GetSymbolSafe(eventArgument.Expression, context.CancellationToken) is { } routedSymbol &&
138+
routedSymbol.Name.EndsWith("Event", StringComparison.Ordinal) &&
139+
routedSymbol.ContainingType.TryFindEvent(routedSymbol.Name.Substring(0, routedSymbol.Name.Length - 5), out var accessor) &&
140+
context.SemanticModel.GetType(callbackArg.Expression, context.CancellationToken) is { } actualType)
141+
{
142+
return !TypeSymbolComparer.Equal(actualType, accessor.Type);
143+
}
144+
145+
return false;
146+
}
108147
}
109148
}
110149
}

WpfAnalyzers/Descriptors.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,15 @@ internal static class Descriptors
483483
isEnabledByDefault: true,
484484
description: "Name the invoked method OnEventName.");
485485

486+
internal static readonly DiagnosticDescriptor WPF0092WrongDelegateType = Create(
487+
id: "WPF0092",
488+
title: "Use correct handler type",
489+
messageFormat: "Use correct handler type",
490+
category: AnalyzerCategory.RoutedEvent,
491+
defaultSeverity: DiagnosticSeverity.Info,
492+
isEnabledByDefault: true,
493+
description: "Use correct handler type.");
494+
486495
internal static readonly DiagnosticDescriptor WPF0100BackingFieldShouldMatchRegisteredName = Create(
487496
id: "WPF0100",
488497
title: "Backing field for a RoutedEvent should match registered name",

documentation/WPF0092.md

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# WPF0092
2+
## Use correct handler type
3+
4+
| Topic | Value
5+
| :-- | :--
6+
| Id | WPF0092
7+
| Severity | Info
8+
| Enabled | True
9+
| Category | WpfAnalyzers.RoutedEvent
10+
| Code | [RoutedEventCallbackAnalyzer](https://github.com/DotNetAnalyzers/WpfAnalyzers/blob/master/WpfAnalyzers/Analyzers/RoutedEventCallbackAnalyzer.cs)
11+
12+
13+
## Description
14+
15+
Use correct handler type.
16+
17+
## Motivation
18+
19+
Passing the wrong handler type compiles but fails at runtime.
20+
21+
```cs
22+
EventManager.RegisterClassHandler(
23+
typeof(PasswordBox),
24+
PasswordBox.PasswordChangedEvent,
25+
new Action<object, RoutedEventArgs>((sender, e) => { }));
26+
```
27+
28+
## How to fix violations
29+
30+
```cs
31+
EventManager.RegisterClassHandler(
32+
typeof(PasswordBox),
33+
PasswordBox.PasswordChangedEvent,
34+
new RoutedEventHandler((sender, e) => { }));
35+
```
36+
37+
<!-- start generated config severity -->
38+
## Configure severity
39+
40+
### Via ruleset file.
41+
42+
Configure the severity per project, for more info see [MSDN](https://msdn.microsoft.com/en-us/library/dd264949.aspx).
43+
44+
### Via #pragma directive.
45+
```C#
46+
#pragma warning disable WPF0092 // Use correct handler type
47+
Code violating the rule here
48+
#pragma warning restore WPF0092 // Use correct handler type
49+
```
50+
51+
Or put this at the top of the file to disable all instances.
52+
```C#
53+
#pragma warning disable WPF0092 // Use correct handler type
54+
```
55+
56+
### Via attribute `[SuppressMessage]`.
57+
58+
```C#
59+
[System.Diagnostics.CodeAnalysis.SuppressMessage("WpfAnalyzers.RoutedEvent",
60+
"WPF0092:Use correct handler type",
61+
Justification = "Reason...")]
62+
```
63+
<!-- end generated config severity -->

0 commit comments

Comments
 (0)