From 169f540ee10a05608640b01ac7b74be04d00be7b Mon Sep 17 00:00:00 2001 From: Sergey Teplyakov Date: Wed, 10 Jun 2026 07:59:29 -0700 Subject: [PATCH 1/3] Fix ERP042 false positive on properties EventSource implicit event-method detection treated property setters as event methods because they are non-static, non-virtual and void-returning. Restrict detection to ordinary methods so properties no longer trigger ERP042. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CoreAnalyzers/DemoEventSource.cs | 4 +++ .../CoreAnalyzers/EventSourceAnalyzerTests.cs | 25 +++++++++++++++++++ .../EventSourceAnalyzer.cs | 6 +++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs index 3b0c75f..1229e73 100644 --- a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs +++ b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs @@ -28,4 +28,8 @@ public void MyEvent5(string str) WriteEvent(5, str, 42); } + // Properties should NOT be treated as event methods (ERP042 must not warn here). + private int SomeValue { get; set; } + + private string Name { get; set; } } \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs index 345aa89..15ae844 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs @@ -95,6 +95,31 @@ public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource await VerifyCS.VerifyAsync(code); } + [Test] + public async Task No_Warn_On_Properties() + { + string code = @" +[System.Diagnostics.Tracing.EventSource(Name = ""Demo"")] +public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource +{ + private int AutoProperty { get; set; } + + private string ExpressionProperty => string.Empty; + + private int _field; + private int FullProperty + { + get => _field; + set => _field = value; + } + + [System.Diagnostics.Tracing.Event(1)] + public void AppStarted(string message) => WriteEvent(1, message); +}"; + + await VerifyCS.VerifyAsync(code); + } + [Test] public async Task Warn_On_Count_Mismatch_Core() { diff --git a/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs b/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs index ee4ee30..7570e96 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs @@ -286,11 +286,13 @@ private void AnalyzeMethodBody(OperationAnalysisContext context) if (eventAttribute == null) { // From the docs: "Any instance, non-virtual, void returning method defined in an event source class is by default an ETW event method." - if (!method.IsStatic && !method.IsVirtual && method.ReturnsVoid && !method.IsConstructor() && method.MethodKind != MethodKind.Destructor) + // Only ordinary methods can be event methods. Property/event accessors, operators, etc. must be ignored even + // though some of them (like property setters) are non-static, non-virtual and void-returning. + if (method.MethodKind == MethodKind.Ordinary && !method.IsStatic && !method.IsVirtual && method.ReturnsVoid && !method.IsConstructor() && method.MethodKind != MethodKind.Destructor) { // In this case the Id is inferred. // "Implicitly: by the ordinal number of the method in the class (thus the first method in the class is 1, second 2 …)" - var methods = method.ContainingType.GetMembers().OfType().Where(m => !m.IsStatic && !m.IsVirtual && m.ReturnsVoid).ToList(); + var methods = method.ContainingType.GetMembers().OfType().Where(m => m.MethodKind == MethodKind.Ordinary && !m.IsStatic && !m.IsVirtual && m.ReturnsVoid).ToList(); var index = methods.IndexOf(method) + 1; return new EventMethodInfo(EventId: index, MethodSymbol: method); From 683bb622643b902fb0da7e664a88556bee6b73ba Mon Sep 17 00:00:00 2001 From: Sergey Teplyakov Date: Wed, 10 Jun 2026 08:09:43 -0700 Subject: [PATCH 2/3] Address PR review: simplify event-method check and fix NonEvent ordinal shift - Extract IsImplicitEventMethodCandidate; MethodKind.Ordinary already excludes constructors/destructors so drop the redundant checks. - Exclude [NonEvent] methods from the ordinal list so they no longer shift the inferred implicit event ID. - Add regression test for the NonEvent ordinal shift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CoreAnalyzers/EventSourceAnalyzerTests.cs | 19 +++++++++++++ .../EventSourceAnalyzer.cs | 28 +++++++++++++++---- 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs index 15ae844..54ae413 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs @@ -95,6 +95,25 @@ public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource await VerifyCS.VerifyAsync(code); } + [Test] + public async Task No_Warn_When_NonEvent_Method_Precedes_Implicit_Event() + { + // A [NonEvent] method must not shift the inferred ordinal id of the implicit event method. + // Here the implicit event method is the 2nd ordinary method but the 1st actual event, so its id is 1 + // and WriteEvent(1, ...) must match. + string code = @" +[System.Diagnostics.Tracing.EventSource(Name = ""Demo"")] +public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource +{ + [System.Diagnostics.Tracing.NonEvent] + public void Helper() {} + + public void AppStarted(string message) => WriteEvent(1, message); +}"; + + await VerifyCS.VerifyAsync(code); + } + [Test] public async Task No_Warn_On_Properties() { diff --git a/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs b/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs index 7570e96..e667602 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs @@ -286,14 +286,15 @@ private void AnalyzeMethodBody(OperationAnalysisContext context) if (eventAttribute == null) { // From the docs: "Any instance, non-virtual, void returning method defined in an event source class is by default an ETW event method." - // Only ordinary methods can be event methods. Property/event accessors, operators, etc. must be ignored even - // though some of them (like property setters) are non-static, non-virtual and void-returning. - if (method.MethodKind == MethodKind.Ordinary && !method.IsStatic && !method.IsVirtual && method.ReturnsVoid && !method.IsConstructor() && method.MethodKind != MethodKind.Destructor) + if (IsImplicitEventMethodCandidate(method, compilation)) { // In this case the Id is inferred. // "Implicitly: by the ordinal number of the method in the class (thus the first method in the class is 1, second 2 …)" - var methods = method.ContainingType.GetMembers().OfType().Where(m => m.MethodKind == MethodKind.Ordinary && !m.IsStatic && !m.IsVirtual && m.ReturnsVoid).ToList(); - + // The ordinal list must use the same eligibility rules so that [NonEvent] (and other non-event) methods + // don't shift the inferred index of real event methods. + var methods = method.ContainingType.GetMembers().OfType() + .Where(m => IsImplicitEventMethodCandidate(m, compilation)).ToList(); + var index = methods.IndexOf(method) + 1; return new EventMethodInfo(EventId: index, MethodSymbol: method); } @@ -316,6 +317,23 @@ private void AnalyzeMethodBody(OperationAnalysisContext context) return new EventMethodInfo(eventId, method); } + /// + /// Returns true if the method can be an implicit ETW event method (i.e. one without an explicit 'Event' attribute). + /// + private static bool IsImplicitEventMethodCandidate(IMethodSymbol method, Compilation compilation) + { + // Only ordinary methods can be event methods. Property/event accessors, operators, constructors, destructors, etc. + // must be ignored even though some of them (like property setters) are non-static, non-virtual and void-returning. + // 'MethodKind.Ordinary' already excludes constructors and destructors. + if (method.MethodKind != MethodKind.Ordinary || method.IsStatic || method.IsVirtual || !method.ReturnsVoid) + { + return false; + } + + // Methods marked with [NonEvent] are explicitly excluded from being events. + return !method.GetAttributes().Any(a => a.AttributeClass?.IsClrType(compilation, typeof(NonEventAttribute)) == true); + } + private static IOperation GetExpectedEventId(IInvocationOperation invocation) { return null!; From 7384aca433f7c6799458b1d5b1a91b8ad5766243 Mon Sep 17 00:00:00 2001 From: Sergey Teplyakov Date: Wed, 10 Jun 2026 08:15:45 -0700 Subject: [PATCH 3/3] Add explicit non-void method coverage for ERP042 Add a dedicated No_Warn_On_Non_Void_Method unit test and a non-void method in the DemoEventSource sample to document that only void-returning methods are implicit event methods. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CoreAnalyzers/DemoEventSource.cs | 3 +++ .../CoreAnalyzers/EventSourceAnalyzerTests.cs | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs index 1229e73..e27492b 100644 --- a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs +++ b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs @@ -32,4 +32,7 @@ public void MyEvent5(string str) private int SomeValue { get; set; } private string Name { get; set; } + + // Non-void returning methods are not implicit event methods (ERP042 must not warn here). + private int Compute() => 42; } \ No newline at end of file diff --git a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs index 54ae413..1141b2d 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs @@ -95,6 +95,25 @@ public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource await VerifyCS.VerifyAsync(code); } + [Test] + public async Task No_Warn_On_Non_Void_Method() + { + // Only void-returning methods are implicit event methods, so non-void methods must not be flagged. + string code = @" +[System.Diagnostics.Tracing.EventSource(Name = ""Demo"")] +public sealed class DemoEventSource : System.Diagnostics.Tracing.EventSource +{ + private int Compute() => 42; + + private string GetName() { return string.Empty; } + + [System.Diagnostics.Tracing.Event(1)] + public void AppStarted(string message) => WriteEvent(1, message); +}"; + + await VerifyCS.VerifyAsync(code); + } + [Test] public async Task No_Warn_When_NonEvent_Method_Precedes_Implicit_Event() {