RavenDB-26506 - allow variable argument in ConfigureAwait by grisha-kotler · Pull Request #7 · ravendb/raven.codeanalysis

grisha-kotler · 2026-05-16T20:49:06Z

Previously the analyzer only accepted ConfigureAwait(false) — any other argument, including a field or variable, would trigger RDB0002. This forced callers with intentionally dynamic behavior to suppress the warning via #pragma warning disable.

Change

Flip the check from "argument must be false" to "argument must not be explicit true". Any non-literal argument (field, variable, property, expression) is now trusted and passes without a warning.

Allowed after this change

.ConfigureAwait(false) ✓
.ConfigureAwait(continueOnCapturedContext: false) ✓
.ConfigureAwait(_continueOnCapturedContext) ✓ ← new

Robustness

Roslyn analyzers run continuously on incomplete syntax trees while the developer is typing. If a developer pauses mid-expression after typing await myTask.ConfigureAwait(, the argument list is temporarily empty.
The original .Single() call would throw an InvalidOperationException on an empty collection. Roslyn catches analyzer crashes gracefully, but it creates unnecessary noise in the IDE error logs.
Changed to .FirstOrDefault() with a null guard — if no argument exists yet, the analyzer silently returns without reporting a diagnostic. The compiler already shows a red squiggle for incomplete code, so there is
no need for the analyzer to add further noise.

Still warned

.ConfigureAwait(true) ✗
missing .ConfigureAwait(...) entirely ✗

Test

Added TestMethod6_VariableArgument covering the field-variable case.

…nalyzer

Copilot

Pull request overview

Updates the ConfigureAwait analyzer to accept non-literal (variable/field/expression) arguments and to be more resilient during live-editing/incomplete syntax, reducing unnecessary diagnostics and IDE analyzer noise.

Changes:

Loosen ConfigureAwait(...) validation to only flag explicit ConfigureAwait(true).
Replace .Single() with a safer first-argument lookup to avoid exceptions on incomplete argument lists.
Add a unit test covering variable/field argument usage.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
Raven.CodeAnalysis/ConfigureAwait/ConfigureAwaitAnalyzer.cs	Adjusts argument inspection logic for `ConfigureAwait` and attempts to improve robustness for incomplete code.
Raven.CodeAnalysis.Test/ConfigureAwaitTests.cs	Adds a new test ensuring variable/field arguments do not trigger the diagnostic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

grisha-kotler · 2026-05-18T19:44:11Z

+                if (argument != null)
+                {
+                    var configureAwaitIsExplicitTrue = argument.IsKind(SyntaxKind.TrueLiteralExpression);
+                    if (configureAwaitIsExplicitTrue == false)
+                        return;
+                }


grisha-kotler · 2026-05-18T19:44:06Z

+                // safely get the first argument, or null if the user hasn't typed it yet
+                var argument = invocationExpressionSyntax.ArgumentList.Arguments.FirstOrDefault()?.Expression;
+                if (argument != null)
+                {
+                    var configureAwaitIsExplicitTrue = argument.IsKind(SyntaxKind.TrueLiteralExpression);
+                    if (configureAwaitIsExplicitTrue == false)
+                        return;
+                }
+            }


grisha-kotler · 2026-05-18T19:44:01Z

+        [TestMethod]
+        public void TestMethod6_VariableArgument()
+        {
+            const string input = @"
+class C
+{
+	private Task SthAsync() { return null; }
+	private bool _continueOnCapturedContext;
+
+	async Task M()
+    {
+		await SthAsync().ConfigureAwait(_continueOnCapturedContext);
+    }
+}";
+
+            VerifyCSharpDiagnostic(input);
+        }
+
+
+        protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()


ppekrol

Small, well-scoped change with sound intent — allowing non-literal ConfigureAwait arguments matches the ticket, and swapping .Single() for .FirstOrDefault() avoids a crash on incomplete syntax during typing. A few things to address before merge (inline). Highlights:

Indentation: all new lines use spaces; both files use tabs everywhere else. Most visible issue and likely an editor-config slip.
Style: if (configureAwaitIsExplicitTrue == false) → if (!...). The intermediate variable can be inlined.
Description vs. behaviour: the PR says incomplete-code (empty argument list) silently returns, but the code falls through to ReportDiagnostic. Either add an early return when argument == null or update the description.
Tests: robustness fix (FirstOrDefault) is untested — a test for zero-arg / incomplete ConfigureAwait(...) would lock it in.

Nothing structural; happy to approve once the above are addressed.

ppekrol · 2026-05-18T08:45:19Z

+                // safely get the first argument, or null if the user hasn't typed it yet
+                var argument = invocationExpressionSyntax.ArgumentList.Arguments.FirstOrDefault()?.Expression;
+                if (argument != null)
+                {
+                    var configureAwaitIsExplicitTrue = argument.IsKind(SyntaxKind.TrueLiteralExpression);
+                    if (configureAwaitIsExplicitTrue == false)
+                        return;
+                }
+            }


Indentation: these 9 lines use spaces (4/8/12), but the rest of the file uses tabs (you can see it in cat -A: surrounding lines start with ^I^I^I, new lines start with spaces). Please re-indent to tabs so the file stays consistent — otherwise mixed whitespace will bite the next person who runs a formatter.

ppekrol · 2026-05-18T08:45:19Z

-			}
+                // safely get the first argument, or null if the user hasn't typed it yet
+                var argument = invocationExpressionSyntax.ArgumentList.Arguments.FirstOrDefault()?.Expression;
+                if (argument != null)


Behaviour doesn't match the PR description. The description says: "if no argument exists yet, the analyzer silently returns without reporting a diagnostic."

But when Arguments is empty (incomplete typing), argument is null, this if is skipped, and control falls through to ReportDiagnostic at line 39 — so the diagnostic still fires on top of the compiler's red squiggle.

If the intent is genuinely to suppress on incomplete code, add an early return:

if (argument == null) return; // incomplete code — let the compiler handle it

If the intent is just to stop crashing, update the description so it doesn't claim suppression that isn't there.

ppekrol · 2026-05-18T08:45:19Z

+                    var configureAwaitIsExplicitTrue = argument.IsKind(SyntaxKind.TrueLiteralExpression);
+                    if (configureAwaitIsExplicitTrue == false)
+                        return;


== false is non-idiomatic in C# and the temp variable is used exactly once. Suggest collapsing to:

if (!argument.IsKind(SyntaxKind.TrueLiteralExpression)) return;

Matches the terseness of the surrounding analyzer code.

ppekrol · 2026-05-18T08:45:19Z

-				if (configureAwaitIsFalse)
-					return;
-			}
+                // safely get the first argument, or null if the user hasn't typed it yet


Nit: "safely" is filler — the value of this comment is the why (incomplete syntax during typing would have made .Single() throw). Something like // argument list may be empty during typing — Single() would throw is closer to the load-bearing context. Optional.

ppekrol · 2026-05-18T08:45:19Z

+        [TestMethod]
+        public void TestMethod6_VariableArgument()
+        {
+            const string input = @"
+class C
+{
+	private Task SthAsync() { return null; }
+	private bool _continueOnCapturedContext;
+
+	async Task M()
+    {
+		await SthAsync().ConfigureAwait(_continueOnCapturedContext);
+    }
+}";
+
+            VerifyCSharpDiagnostic(input);
+        }


Indentation: the new test method body uses 8-space indent (and 4-space for the attribute), but TestMethod1–TestMethod5 above all use tabs. Please re-indent to match the rest of the file.

ppekrol · 2026-05-18T08:45:19Z

+
+

Two blank lines between the new test and the protected override below — drop one to match the single-blank-line spacing used elsewhere in the file.

ppekrol · 2026-05-18T08:45:19Z


-		protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer()
+        [TestMethod]
+        public void TestMethod6_VariableArgument()


Test coverage gap: the PR description enumerates field / variable / property / expression as newly-allowed arguments, but only the field case is exercised. The analyzer treats them identically, so one test is technically sufficient — however, the robustness fix (.FirstOrDefault() replacing .Single()) has no test at all. A case like await x.ConfigureAwait() (zero args) or an incomplete invocation would lock in the no-crash guarantee and prevent a future revert to .Single() from silently regressing. Worth adding.

ppekrol

Remember to bump version: d21cfb5

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

 			if (awaitedExpression?.Name.Identifier.Text == "ConfigureAwait")
 			{
-				var configureAwaitIsFalse = invocationExpressionSyntax.ArgumentList.Arguments.Single().Expression.IsKind(SyntaxKind.FalseLiteralExpression);
-				if (configureAwaitIsFalse)
+				// argument list may be empty during typing — FirstOrDefault avoids a throw from Single()
+				var argument = invocationExpressionSyntax.ArgumentList.Arguments.FirstOrDefault()?.Expression;
+				if (argument == null || !argument.IsKind(SyntaxKind.TrueLiteralExpression))
 					return;


 			var awaitExpressionSyntax = (AwaitExpressionSyntax)context.Node;
 			var invocationExpressionSyntax = awaitExpressionSyntax.Expression as InvocationExpressionSyntax;
 			var awaitedExpression = invocationExpressionSyntax?.Expression as MemberAccessExpressionSyntax;

 			if (awaitedExpression?.Name.Identifier.Text == "ConfigureAwait")
 			{


RavenDB-26506 - allow variable argument in ConfigureAwait

d6c6ef6

grisha-kotler mentioned this pull request May 16, 2026

RavenDB-26506 - Run all backup work on the dedicated backup thread to respect its BelowNormal priority ravendb/ravendb#22720

Merged

26 tasks

RavenDB-26506 - guard against empty argument list in ConfigureAwait a…

e13cf61

…nalyzer

ppekrol requested a review from Copilot May 17, 2026 20:49

Copilot started reviewing on behalf of ppekrol May 17, 2026 20:50 View session

Copilot AI reviewed May 17, 2026

View reviewed changes

ppekrol reviewed May 18, 2026

View reviewed changes

RavenDB-26506 - address PR comments

a712d52

grisha-kotler requested a review from ppekrol May 18, 2026 19:44

ppekrol requested a review from Copilot May 18, 2026 20:21

Copilot started reviewing on behalf of ppekrol May 18, 2026 20:21 View session

ppekrol approved these changes May 18, 2026

View reviewed changes

Copilot AI reviewed May 18, 2026

View reviewed changes

RavenDB-26506 - bump version

4c20260

ppekrol merged commit e9265f5 into ravendb:master May 19, 2026
1 check passed

Conversation

grisha-kotler commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

Allowed after this change

Robustness

Still warned

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppekrol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppekrol left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

grisha-kotler commented May 16, 2026 •

edited

Loading