From cc6f65f920ac4b5002f27bb5dfbfddaaecb3dd45 Mon Sep 17 00:00:00 2001 From: luissena Date: Fri, 20 Mar 2026 19:26:04 -0300 Subject: [PATCH] Support conditional expressions in API1000/API1001 analyzers The API convention analyzers (API1000 and API1001) did not inspect conditional (ternary) expressions, so undocumented status codes in returns like `return id == 0 ? NotFound() : Ok()` went undetected. This change extends `InspectReturnOperation` in `ActualApiResponseMetadataFactory` to recurse into both branches of `IConditionalOperation`, matching the existing pattern used for `ISwitchExpressionOperation`. The `ISwitchExpressionArmOperation?` parameter was generalized to `IOperation?` to support passing branch values from both switch arms and conditional expressions through the same code path. Fixes #33105 --- .../src/ActualApiResponseMetadataFactory.cs | 14 +- .../ActualApiResponseMetadataFactoryTest.cs | 58 ++++++++ .../ApiConventionAnalyzerIntegrationTest.cs | 124 +++++++++++++++++- ...xpressionReturnsUndocumentedStatusCodes.cs | 15 +++ ...ontroller_IfStatusCodesCannotBeInferred.cs | 15 --- 5 files changed, 206 insertions(+), 20 deletions(-) create mode 100644 src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes.cs delete mode 100644 src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred.cs diff --git a/src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs b/src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs index df7efae24a66..bbcbf0c25b6a 100644 --- a/src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs +++ b/src/Mvc/Mvc.Api.Analyzers/src/ActualApiResponseMetadataFactory.cs @@ -59,9 +59,9 @@ void AnalyzeResponseExpression(IReturnOperation returnOperation) internal static ActualApiResponseMetadata?[] InspectReturnOperation( in ApiControllerSymbolCache symbolCache, IReturnOperation returnOperation, - ISwitchExpressionArmOperation? armOperation = null) + IOperation? overrideReturnedValue = null) { - var returnedValue = armOperation?.Value ?? returnOperation.ReturnedValue; + var returnedValue = overrideReturnedValue ?? returnOperation.ReturnedValue; var defaultStatusCodeAttributeSymbol = symbolCache.DefaultStatusCodeAttribute; if (returnedValue is null || returnedValue is IInvalidOperation) @@ -106,13 +106,21 @@ void AnalyzeResponseExpression(IReturnOperation returnOperation) for (var i = 0; i < switchExpression.Arms.Length; i++) { var arm = switchExpression.Arms[i]; - var armMetadata = InspectReturnOperation(symbolCache, returnOperation, arm); + var armMetadata = InspectReturnOperation(symbolCache, returnOperation, arm.Value); metadata.AddRange(armMetadata); } return metadata.ToArray(); } + if (returnedValue is IConditionalOperation conditionalOperation) + { + var metadata = new List(); + metadata.AddRange(InspectReturnOperation(symbolCache, returnOperation, conditionalOperation.WhenTrue)); + metadata.AddRange(InspectReturnOperation(symbolCache, returnOperation, conditionalOperation.WhenFalse)); + return metadata.ToArray(); + } + ITypeSymbol? returnType = null; switch (returnedValue) { diff --git a/src/Mvc/Mvc.Api.Analyzers/test/ActualApiResponseMetadataFactoryTest.cs b/src/Mvc/Mvc.Api.Analyzers/test/ActualApiResponseMetadataFactoryTest.cs index 3685a0d329d0..fd6b54e5b251 100644 --- a/src/Mvc/Mvc.Api.Analyzers/test/ActualApiResponseMetadataFactoryTest.cs +++ b/src/Mvc/Mvc.Api.Analyzers/test/ActualApiResponseMetadataFactoryTest.cs @@ -87,6 +87,64 @@ public IActionResult Get(int id) Assert.Null(metadata); } + [Fact] + public async Task InspectReturnExpression_RecursesIntoConditionalExpression() + { + // Arrange & Act + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + [ApiController] + public class TestController : ControllerBase + { + public IActionResult Get(int id) + { + return id == 0 ? NotFound() : Ok(); + } + } +}"; + var actualResponseMetadata = await RunInspectReturnStatementSyntax(source, "TestController"); + + // Assert + Assert.Equal(2, actualResponseMetadata.Length); + Assert.NotNull(actualResponseMetadata[0]); + Assert.Equal(404, actualResponseMetadata[0].Value.StatusCode); + Assert.NotNull(actualResponseMetadata[1]); + Assert.Equal(200, actualResponseMetadata[1].Value.StatusCode); + } + + [Fact] + public async Task InspectReturnExpression_RecursesIntoNestedConditionalExpression() + { + // Arrange & Act + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + [ApiController] + public class TestController : ControllerBase + { + public IActionResult Get(int id) + { + return id == 0 ? NotFound() : id == 1 ? BadRequest() : Ok(); + } + } +}"; + var actualResponseMetadata = await RunInspectReturnStatementSyntax(source, "TestController"); + + // Assert + Assert.Equal(3, actualResponseMetadata.Length); + Assert.NotNull(actualResponseMetadata[0]); + Assert.Equal(404, actualResponseMetadata[0].Value.StatusCode); + Assert.NotNull(actualResponseMetadata[1]); + Assert.Equal(400, actualResponseMetadata[1].Value.StatusCode); + Assert.NotNull(actualResponseMetadata[2]); + Assert.Equal(200, actualResponseMetadata[2].Value.StatusCode); + } + [Theory] [InlineData(ReturnOperationTestVariant.Default)] [InlineData(ReturnOperationTestVariant.SwitchExpression)] diff --git a/src/Mvc/Mvc.Api.Analyzers/test/ApiConventionAnalyzerIntegrationTest.cs b/src/Mvc/Mvc.Api.Analyzers/test/ApiConventionAnalyzerIntegrationTest.cs index 234fe1511734..c5e7f17598b1 100644 --- a/src/Mvc/Mvc.Api.Analyzers/test/ApiConventionAnalyzerIntegrationTest.cs +++ b/src/Mvc/Mvc.Api.Analyzers/test/ApiConventionAnalyzerIntegrationTest.cs @@ -29,8 +29,17 @@ public Task NoDiagnosticsAreReturned_ForOkResultReturningAction() => RunNoDiagnosticsAreReturned(); [Fact] - public Task NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred() - => RunNoDiagnosticsAreReturned(); + public async Task DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes() + { + // Arrange + var testSource = MvcTestSource.Read(GetType().Name, "DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes"); + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Contains(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } [Fact] public Task NoDiagnosticsAreReturned_ForReturnStatementsInLambdas() @@ -171,6 +180,117 @@ public IActionResult Get(bool b) Assert.Contains(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); } + [Fact] + public async Task DiagnosticsAreReturned_ForActionsReturnedFromConditionalExpression() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Test +{ + [ApiController] + public class Foo : ControllerBase + { + [ProducesResponseType(typeof(string), 200)] + public IActionResult Get(int id) + { + return id == 0 ? NotFound() : Ok(); + } + } +}"; + var testSource = TestSource.Read(source); + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Contains(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } + + [Fact] + public async Task DiagnosticsAreReturned_ForActionsReturnedFromNestedConditionalExpression() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Test +{ + [ApiController] + public class Foo : ControllerBase + { + [ProducesResponseType(typeof(string), 200)] + public IActionResult Get(int id) + { + return id == 0 ? NotFound() : id == 1 ? BadRequest() : Ok(); + } + } +}"; + var testSource = TestSource.Read(source); + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Contains(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } + + [Fact] + public async Task NoDiagnosticsAreReturned_ForConditionalExpressionWithAllDocumentedStatusCodes() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Test +{ + [ApiController] + public class Foo : ControllerBase + { + [ProducesResponseType(200)] + [ProducesResponseType(404)] + public IActionResult Get(int id) + { + return id == 0 ? NotFound() : Ok(); + } + } +}"; + var testSource = TestSource.Read(source); + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.DoesNotContain(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } + + [Fact] + public async Task DiagnosticsAreReturned_ForExpressionBodiedMemberWithConditionalExpression() + { + // Arrange + var source = @" +using Microsoft.AspNetCore.Mvc; + +namespace Test +{ + [ApiController] + public class Foo : ControllerBase + { + [ProducesResponseType(typeof(string), 200)] + public IActionResult Get(int id) + => id == 0 ? NotFound() : Ok(); + } +}"; + var testSource = TestSource.Read(source); + + // Act + var result = await Executor.GetDiagnosticsAsync(testSource.Source); + + // Assert + Assert.Contains(result, d => d.Id == ApiDiagnosticDescriptors.API1000_ActionReturnsUndocumentedStatusCode.Id); + } + [Fact] public async Task DiagnosticsAreReturned_ForActionsReturnedFromSwitchStatement() { diff --git a/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes.cs b/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes.cs new file mode 100644 index 000000000000..94cf1f750488 --- /dev/null +++ b/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.AspNetCore.Mvc.Api.Analyzers +{ + [ApiController] + public class DiagnosticsAreReturned_ForApiController_IfConditionalExpressionReturnsUndocumentedStatusCodes : ControllerBase + { + [ProducesResponseType(201)] + public IActionResult Method(int id) + { + return id == 0 ? /*MM*/(IActionResult)NotFound() : Ok(); + } + } +} diff --git a/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred.cs b/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred.cs deleted file mode 100644 index 21c134fd3911..000000000000 --- a/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/ApiConventionAnalyzerIntegrationTest/NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace Microsoft.AspNetCore.Mvc.Api.Analyzers -{ - [ApiController] - public class NoDiagnosticsAreReturned_ForApiController_IfStatusCodesCannotBeInferred : ControllerBase - { - [ProducesResponseType(201)] - public IActionResult Method(int id) - { - return id == 0 ? (IActionResult)NotFound() : Ok(); - } - } -}