Skip to content

Commit

Permalink
Refactor according to PR
Browse files Browse the repository at this point in the history
  • Loading branch information
heqianwang committed May 2, 2024
1 parent fcc4255 commit 2406673
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 74 deletions.
22 changes: 19 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,23 @@ public Ok<ProblemDetails> TypedResultWithProducesResponseTypeAnnotation()
}
```

## Analyzer Rules
## INcluded Roslyn analyzers

## Included Roslyn analyzers

| Rule ID | Category | Severity | Description |
|---------|----------|----------|--------------------------------------------------------------------|
| WLOAS001 | Design | Warning | Mismatch between annotation return type and endpoint return type. |

To modify the severity of one of these diagnostic rules, you can use a `.editorconfig` file. For example:

```ini
## Disable analyzer for test files
[**Tests*/**.cs]
dotnet_diagnostic.WLOAS001.severity = none
```

To learn more about configuring or suppressing code analysis warnings, refer to [this documentation](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings).

### `WLOAS001`: Mismatch between annotation return type and endpoint return type.

Expand All @@ -109,9 +125,9 @@ This rule validates the return type indicated by the endpoint annotations agains
```cs
[HttpGet]
[Route("/example")]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)] // This would be marked with a warning
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)] // This would be marked with a warning given typeof(string) is different from typeof(TypedResultExample)
[ProducesResponseType(typeof(TypedResultExample), StatusCodes.Status200OK)] // This would be valid
public Ok<TypedResultExample> TypedResultWithSwaggerResponseAnnotation()
public Ok<TypedResultExample> TypedResultExample()
{
return TypedResults.Ok(new TypedResultExample());
}
Expand Down
1 change: 1 addition & 0 deletions src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
<Using Include="Xunit.Abstractions" />
</ItemGroup>

<!-- Use a separate feed to have access to the public newer version of Analyzers test. https://github.com/dotnet/roslyn/issues/61979 -->
<PropertyGroup>
<RestoreAdditionalProjectSources>https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json</RestoreAdditionalProjectSources>
</PropertyGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ public void ValidateEndpointResponseType(SymbolAnalysisContext context)
var typedReturnType = this.GetReturnTypeSymbol(methodSymbol);
if (typedReturnType != null && this.IsImplementingIResult(typedReturnType))
{
var methodSignatureStatusCodeToTypeMap = this.GetMethodReturnStatusCodeToTypeMap(typedReturnType);
var methodSignatureStatusCodeToType = this.GetMethodReturnStatusCodeToType(typedReturnType);

foreach (var attribute in methodSymbol.GetAttributes())
{
this.ValidateAnnotationWithTypedResult(context, attribute, methodSignatureStatusCodeToTypeMap);
this.ValidateAnnotationWithTypedResult(context, attribute, methodSignatureStatusCodeToType);
}
}
}
Expand Down Expand Up @@ -175,7 +175,7 @@ private INamedTypeSymbol UnwrapTypeFromTask(INamedTypeSymbol typedReturnType)
}

private void ValidateAnnotationWithTypedResult(SymbolAnalysisContext context, AttributeData attribute,
Dictionary<int, ITypeSymbol> methodSignatureStatusCodeToTypeMap)
Dictionary<int, List<ITypeSymbol>> methodSignatureStatusCodeToTypeMap)
{
if (attribute.AttributeClass == null || attribute.ConstructorArguments.Length == 0)
{
Expand All @@ -189,8 +189,10 @@ private INamedTypeSymbol UnwrapTypeFromTask(INamedTypeSymbol typedReturnType)
{
if (attribute.ConstructorArguments[0].Value is int statusCodeValue)
{
var type = this._statusCodeToResultsMap[statusCodeValue];
ValidateAnnotationForTypeMismatch(attribute, statusCodeValue, type, methodSignatureStatusCodeToTypeMap, context);
if (this._statusCodeToResultsMap.TryGetValue(statusCodeValue, out var type))
{
ValidateAnnotationForTypeMismatch(attribute, statusCodeValue, type, methodSignatureStatusCodeToTypeMap, context);
}
}
}
else if (attribute.ConstructorArguments[1].Value is int statusCodeValue && attribute.ConstructorArguments[0].Value is ITypeSymbol type)
Expand All @@ -217,15 +219,20 @@ private INamedTypeSymbol UnwrapTypeFromTask(INamedTypeSymbol typedReturnType)
}
}

private Dictionary<int, ITypeSymbol> GetMethodReturnStatusCodeToTypeMap(ITypeSymbol methodSymbol)
// Result<Ok<type>, Notfound>
private Dictionary<int, List<ITypeSymbol>> GetMethodReturnStatusCodeToType(ITypeSymbol methodSymbol)
{
var methodReturnSignatures = this.ExtractStatusCodeAndResultFromMethodReturn(methodSymbol);
var methodSignatureStatusCodeToTypeMap = new Dictionary<int, ITypeSymbol>();
var methodSignatureStatusCodeToTypeMap = new Dictionary<int, List<ITypeSymbol>>();
foreach (var returnValues in methodReturnSignatures)
{
if (!methodSignatureStatusCodeToTypeMap.ContainsKey(returnValues.statusCode))
if (methodSignatureStatusCodeToTypeMap.TryGetValue(returnValues.statusCode, out var returnTypeSymbols))
{
methodSignatureStatusCodeToTypeMap.Add(returnValues.statusCode, returnValues.symbol);
returnTypeSymbols.Add(returnValues.symbol);
}
else
{
methodSignatureStatusCodeToTypeMap.Add(returnValues.statusCode, [returnValues.symbol]);
}
}

Expand Down Expand Up @@ -255,22 +262,20 @@ private IEnumerable<(int statusCode, ITypeSymbol symbol)> ExtractStatusCodeAndRe
}

private static void ValidateAnnotationForTypeMismatch(AttributeData attribute, int statusCodeFromAnnotation, ITypeSymbol typeFromAnnotation,
Dictionary<int, ITypeSymbol> methodReturnStatusCodeTypes, SymbolAnalysisContext context)
Dictionary<int, List<ITypeSymbol>> methodReturnStatusCodeTypes, SymbolAnalysisContext context)
{
if (attribute.ApplicationSyntaxReference is null)
{
return;
}

var attributeLocation = attribute.ApplicationSyntaxReference.GetSyntax(context.CancellationToken).GetLocation();
if (methodReturnStatusCodeTypes.TryGetValue(statusCodeFromAnnotation, out var mappedType))
if (!methodReturnStatusCodeTypes.TryGetValue(statusCodeFromAnnotation, out var mappedType))
{
if (!SymbolEqualityComparer.Default.Equals(mappedType, typeFromAnnotation))
{
context.ReportDiagnostic(AnnotationMustMatchTypedResult, attributeLocation);
}
return;
}
else

if (!mappedType.Any(type => SymbolEqualityComparer.Default.Equals(type, typeFromAnnotation)))
{
context.ReportDiagnostic(AnnotationMustMatchTypedResult, attributeLocation);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc;
using Swashbuckle.AspNetCore.Annotations;

namespace WebApi.OpenAPI.SystemTest.OperationId;
Expand All @@ -9,11 +8,9 @@ namespace WebApi.OpenAPI.SystemTest.OperationId;
public class OperationIdController : ControllerBase
{
[HttpGet("/explicitOperationIdInName", Name = "GetExplicitOperationIdInName")]
[ProducesResponseType<int>(StatusCodes.Status200OK)]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
public Ok<string> GetExplicitOperationIdInName()
public IActionResult GetExplicitOperationIdInName()
{
return TypedResults.Ok("Hello World");
return this.Ok();
}

[HttpGet]
Expand Down
1 change: 0 additions & 1 deletion src/tests/WebApi.OpenAPI.SystemTest/custom.spectral.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ extends: [https://raw.githubusercontent.com/gsoft-inc/wl-api-guidelines/main/.sp
rules:
properties-must-have-a-type: false
application-json-response-must-not-be-type-string: false
operation-operationId: false
must-return-content-types: false
10 changes: 0 additions & 10 deletions src/tests/WebApi.OpenAPI.SystemTest/openapi-v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,6 @@ paths:
responses:
'200':
description: Success
content:
text/plain:
schema:
type: string
application/json:
schema:
type: string
text/json:
schema:
type: string
/explicitOperationIdInSwagger:
get:
tags:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ public class BaseAnalyzerTest<TAnalyzer> : CSharpAnalyzerTest<TAnalyzer, XUnitVe
where TAnalyzer : DiagnosticAnalyzer, new()
{
private const string GlobalUsingList = """
global using System;
global using System.Collections.Generic;
global using System.IO;
global using System.Linq;
global using System.Threading;
global using System.Threading.Tasks;
global using Microsoft.AspNetCore.Http.HttpResults;
global using Microsoft.AspNetCore.Http;
global using Microsoft.AspNetCore.Mvc;
global using Swashbuckle.AspNetCore.Annotations;
global using Workleap.Extensions.OpenAPI.TypedResult;
""";
global using System;
global using System.Collections.Generic;
global using System.IO;
global using System.Linq;
global using System.Threading;
global using System.Threading.Tasks;
global using Microsoft.AspNetCore.Http.HttpResults;
global using Microsoft.AspNetCore.Http;
global using Microsoft.AspNetCore.Mvc;
global using Swashbuckle.AspNetCore.Annotations;
global using Workleap.Extensions.OpenAPI.TypedResult;
""";

private const string SourceFileName = "Program.cs";

Expand All @@ -42,11 +42,13 @@ protected BaseAnalyzerTest()
this.TestState.AdditionalReferences.Add(typeof(InternalServerError).Assembly);
}

// Specify compilation options as DLL and that unsafe code is not allowed.
protected override CompilationOptions CreateCompilationOptions()
{
return new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary, allowUnsafe: false);
}

// Specify the language version and that diagnostics should be reported for missing documentation.
protected override ParseOptions CreateParseOptions()
{
return new CSharpParseOptions(LanguageVersion.CSharp11, DocumentationMode.Diagnose);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public async Task Given_NoAnnotationIActionResult_When_Analyze_Then_No_Diagnosti
public class AnalyzersController : ControllerBase
{
[HttpGet]
public IActionResult GetExplicitOperationIdInName() => throw null;
public IActionResult GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -27,7 +27,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
public IActionResult GetExplicitOperationIdInName() => throw null;
public IActionResult GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -42,7 +42,7 @@ public async Task Given_NoAnnotationTypedResult_When_Analyze_Then_No_Diagnostic(
public class AnalyzersController : ControllerBase
{
[HttpGet]
public Ok<string> GetExplicitOperationIdInName() => throw null;
public Ok<string> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -59,7 +59,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public Results<Ok<String>, NotFound> GetExplicitOperationIdInName() => throw null;
public Results<Ok<String>, NotFound> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -75,7 +75,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[SwaggerResponse(StatusCodes.Status200OK, "Returns string", typeof(string), "application/json")]
public Ok<string> GetExplicitOperationIdInName() => throw null;
public Ok<string> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -91,7 +91,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
public Ok<string> GetExplicitOperationIdInName() => throw null;
public Ok<string> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -108,7 +108,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public async Task<Results<Ok<String>, NotFound>> GetExplicitOperationIdInName() => throw null;
public async Task<Results<Ok<String>, NotFound>> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -124,8 +124,8 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
[{|WLOAS001:ProducesResponseType(StatusCodes.Status404NotFound)|}]
public Ok<string> GetExplicitOperationIdInName() => throw null;
[ProducesResponseType(StatusCodes.Status404NotFound)]
public Ok<string> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -141,7 +141,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[{|WLOAS001:ProducesResponseType(typeof(string), StatusCodes.Status200OK)|}]
public Ok<int> GetExplicitOperationIdInName() => throw null;
public Ok<int> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -158,7 +158,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[{|WLOAS001:ProducesResponseType(typeof(string), StatusCodes.Status200OK)|}]
[ProducesResponseType(typeof(int), StatusCodes.Status200OK)]
public Ok<int> GetExplicitOperationIdInName() => throw null;
public Ok<int> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -174,7 +174,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[{|WLOAS001:ProducesResponseType<string>(StatusCodes.Status200OK)|}]
public Ok<int> GetExplicitOperationIdInName() => throw null;
public Ok<int> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -190,7 +190,7 @@ public class AnalyzersController : ControllerBase
{
[HttpGet]
[{|WLOAS001:SwaggerResponse(StatusCodes.Status200OK, "Returns TypedResult", typeof(string))|}]
public Ok<int> GetExplicitOperationIdInName() => throw null;
public Ok<int> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -207,7 +207,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[{|WLOAS001:ProducesResponseType(typeof(string), StatusCodes.Status200OK)|}]
[ProducesResponseType(StatusCodes.Status404NotFound)]
public async Task<Results<Ok<int>, NotFound>> GetExplicitOperationIdInName() => throw null;
public async Task<Results<Ok<int>, NotFound>> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -224,7 +224,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[{|WLOAS001:ProducesResponseType(typeof(string), StatusCodes.Status200OK)|}]
[{|WLOAS001:ProducesResponseType(StatusCodes.Status404NotFound)|}]
public async Task<Results<Ok<int>, NotFound<int>>> GetExplicitOperationIdInName() => throw null;
public async Task<Results<Ok<int>, NotFound<int>>> GetSampleEndpoint() => throw null;
}
""";

Expand All @@ -241,7 +241,7 @@ public class AnalyzersController : ControllerBase
[HttpGet]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)]
[ProducesResponseType(StatusCodes.Status500InternalServerError)]
public async Task<Results<Ok<string>, InternalServerError>> GetExplicitOperationIdInName() => throw null;
public async Task<Results<Ok<string>, InternalServerError>> GetSampleEndpoint() => throw null;
}
""";

Expand Down
Loading

0 comments on commit 2406673

Please sign in to comment.