Skip to content

Commit d852eb3

Browse files
committed
Add separate rethrow diagnostic message and remove duplicate test
Rethrows (`throw;`) now get a distinct message advising to extract the try/catch block instead of the generic ThrowHelper advice that doesn't apply. Remove duplicate ShouldFixThrowExpression test (identical to ShouldFixNullCoalesceThrowExpressionInReturn). 🤖 Co-Authored-By: Claude Code <noreply@anthropic.com>
1 parent 125de43 commit d852eb3

File tree

3 files changed

+17
-48
lines changed

3 files changed

+17
-48
lines changed

tracer/src/Datadog.Trace.Tools.Analyzers/ThrowInInlinedMethodAnalyzer/Diagnostics.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,14 @@ public static class Diagnostics
2727
defaultSeverity: DiagnosticSeverity.Warning,
2828
isEnabledByDefault: false,
2929
description: "throw statements prevent the JIT from inlining a method. Move the throw to a separate helper method marked with [MethodImpl(MethodImplOptions.NoInlining)] and [DoesNotReturn].");
30+
31+
internal static readonly DiagnosticDescriptor RethrowInAggressiveInliningRule = new(
32+
DiagnosticId,
33+
title: "Avoid throw in AggressiveInlining method",
34+
messageFormat: "Method '{0}' is marked [AggressiveInlining] but contains a rethrow statement, which prevents inlining. Extract the try/catch block to a separate non-inlined method.",
35+
category: "Performance",
36+
defaultSeverity: DiagnosticSeverity.Warning,
37+
isEnabledByDefault: false,
38+
description: "throw statements (including rethrows) prevent the JIT from inlining a method. Move the try/catch block to a separate method marked with [MethodImpl(MethodImplOptions.NoInlining)].");
3039
#pragma warning restore RS2008
3140
}

tracer/src/Datadog.Trace.Tools.Analyzers/ThrowInInlinedMethodAnalyzer/ThrowInInlinedMethodAnalyzer.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public class ThrowInInlinedMethodAnalyzer : DiagnosticAnalyzer
2525

2626
/// <inheritdoc/>
2727
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
28-
=> ImmutableArray.Create(Diagnostics.ThrowInAggressiveInliningRule);
28+
=> ImmutableArray.Create(Diagnostics.ThrowInAggressiveInliningRule, Diagnostics.RethrowInAggressiveInliningRule);
2929

3030
/// <inheritdoc/>
3131
public override void Initialize(AnalysisContext context)
@@ -56,14 +56,17 @@ private static void AnalyzeThrow(SyntaxNodeAnalysisContext context)
5656
if (HasAggressiveInlining(methodSymbol))
5757
{
5858
var memberName = GetMemberDisplayName(containingMember);
59-
var diagnostic = Diagnostic.Create(
60-
Diagnostics.ThrowInAggressiveInliningRule,
61-
throwNode.GetLocation(),
62-
memberName);
59+
var rule = IsRethrow(throwNode)
60+
? Diagnostics.RethrowInAggressiveInliningRule
61+
: Diagnostics.ThrowInAggressiveInliningRule;
62+
var diagnostic = Diagnostic.Create(rule, throwNode.GetLocation(), memberName);
6363
context.ReportDiagnostic(diagnostic);
6464
}
6565
}
6666

67+
private static bool IsRethrow(SyntaxNode throwNode)
68+
=> throwNode is ThrowStatementSyntax { Expression: null };
69+
6770
private static SyntaxNode? GetContainingMember(SyntaxNode node)
6871
{
6972
for (var current = node.Parent; current is not null; current = current.Parent)

tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ThrowInInlinedMethodAnalyzer/ThrowInInlinedMethodCodeFixProviderTests.cs

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -308,49 +308,6 @@ void TestMethod()
308308
await Verifier.VerifyCodeFixAsync(source + ThrowHelperStub, expected, fix + ThrowHelperStub);
309309
}
310310

311-
[Fact]
312-
public async Task ShouldFixThrowExpression()
313-
{
314-
// Throw expressions in null-coalescing are converted to if-statements
315-
const string source = """
316-
using System;
317-
using System.Runtime.CompilerServices;
318-
319-
class TestClass
320-
{
321-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
322-
object TestMethod(object? arg)
323-
{
324-
return arg ?? {|#0:throw new ArgumentNullException(nameof(arg))|};
325-
}
326-
}
327-
""";
328-
329-
const string fix = """
330-
using System;
331-
using System.Runtime.CompilerServices;
332-
using Datadog.Trace.Util;
333-
334-
class TestClass
335-
{
336-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
337-
object TestMethod(object? arg)
338-
{
339-
if (arg is null)
340-
{
341-
ThrowHelper.ThrowArgumentNullException(nameof(arg));
342-
}
343-
return arg;
344-
}
345-
}
346-
""";
347-
348-
var expected = new DiagnosticResult(DiagnosticId, DiagnosticSeverity.Warning)
349-
.WithLocation(0)
350-
.WithArguments("TestMethod");
351-
await Verifier.VerifyCodeFixAsync(source + ThrowHelperStub, expected, fix + ThrowHelperStub);
352-
}
353-
354311
[Fact]
355312
public async Task ShouldNotFixBareThrow()
356313
{

0 commit comments

Comments
 (0)