diff --git a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs index 3b0c75f..e27492b 100644 --- a/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs +++ b/samples/ErrorProne.Samples/CoreAnalyzers/DemoEventSource.cs @@ -28,4 +28,11 @@ 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; } + + // 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 345aa89..1141b2d 100644 --- a/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs +++ b/src/ErrorProne.NET.CoreAnalyzers.Tests/CoreAnalyzers/EventSourceAnalyzerTests.cs @@ -95,6 +95,69 @@ 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() + { + // 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() + { + 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..e667602 100644 --- a/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs +++ b/src/ErrorProne.NET.CoreAnalyzers/EventSourceAnalysis/EventSourceAnalyzer.cs @@ -286,12 +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." - if (!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.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); } @@ -314,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!;