Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IDP-1222] Add Roslyn rule to flag response type mismatch between annotation and method return #6

Merged
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9ca9546
Initial commit of analyzers
Apr 29, 2024
4408baa
Working flow for roslyn
Apr 29, 2024
662da58
Merge branch 'main' into feature/idp-1222_roslyn_rules_for_annotation…
heqianwang Apr 30, 2024
3e2caa0
Adding tests for analyser
heqianwang Apr 30, 2024
c91516b
Changes for analyzer tests
heqianwang Apr 30, 2024
4ce1916
working test
heqianwang Apr 30, 2024
3b109f7
Working tests
heqianwang May 1, 2024
3b86423
Passing all test cases except when double tagged
heqianwang May 1, 2024
2acb901
All tests passing
heqianwang May 1, 2024
6cf1ba0
Flagged based on annotation instead of method name
heqianwang May 1, 2024
8268c67
Refactor code and add more result codes
heqianwang May 1, 2024
1597349
Remove unused code and comments
heqianwang May 1, 2024
6d3da21
Update csproj name
heqianwang May 1, 2024
1e0d457
Remove no-build on pack
heqianwang May 1, 2024
0dfea4c
Added changes
heqianwang May 1, 2024
8ab81c0
fix test
heqianwang May 1, 2024
85c3791
fix spec
heqianwang May 1, 2024
2a1fda5
build for .net8.0 too
heqianwang May 1, 2024
2c72317
Correct according to PR feedback
heqianwang May 1, 2024
0306622
use Internals
heqianwang May 1, 2024
31c09cb
Add help uri
heqianwang May 1, 2024
dbb121b
Have a isValid check before registering symbols
heqianwang May 2, 2024
fcc4255
Refactor according to feedback
heqianwang May 2, 2024
2406673
Refactor according to PR
heqianwang May 2, 2024
42a010b
Upgrade xunit
heqianwang May 2, 2024
5281c63
Spectral changes
heqianwang May 2, 2024
3ec78bb
use latest xunit
heqianwang May 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Build.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Process {
Exec { & dotnet build -c Release }
Exec { & dotnet test -c Release --no-build --results-directory "$outputDir" --no-restore -l "trx" -l "console;verbosity=detailed" }
Exec { & Compare-GeneratedAndExpectedFiles -generatedFilePath $generatedFilePath -expectedFilePath $expectedFilePath }
Exec { & dotnet pack -c Release --no-build -o "$outputDir" }
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
Exec { & dotnet pack -c Release -o "$outputDir" }

if (($null -ne $env:NUGET_SOURCE ) -and ($null -ne $env:NUGET_API_KEY)) {
Exec { & dotnet nuget push "$nupkgsPath" -s $env:NUGET_SOURCE -k $env:NUGET_API_KEY }
Expand Down
21 changes: 20 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ The library offers an opinionated configuration of OpenAPI document generation a

As such, we provide the following features:

OpenAPI Spec generation filters:
- Display OperationId in SwaggerUI
- Extract Type schema from TypedResult endpoint response types
- Extract Type schema from TypedResult endpoint response types
- (Optional) Fallback to use controller name as OperationId when there is no OperationId explicitly defined for the endpoint.

Roslyn Analyzers to help validate usage typed responses:
- Rule to catch mismatches between endpoint response annotations and Typed Responses

## Getting started

Install the package Workleap.Extensions.OpenAPI in your .NET API project. Then you may use the following method to register the required service. Here is a code snippet on how to register this and to enable the operationId fallback feature in your application.
Expand Down Expand Up @@ -96,7 +100,22 @@ public Ok<ProblemDetails> TypedResultWithProducesResponseTypeAnnotation()
}
```

## Analyzer Rules

### `WLOAS001`: Mismatch between annotation return type and endpoint return type.
heqianwang marked this conversation as resolved.
Show resolved Hide resolved

This rule validates the return type indicated by the endpoint annotations against the Typed response values indicated by the endpoint. Here is an example:

```cs
[HttpGet]
[Route("/example")]
[ProducesResponseType(typeof(string), StatusCodes.Status200OK)] // This would be marked with a warning
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
[ProducesResponseType(typeof(TypedResultExample), StatusCodes.Status200OK)] // This would be valid
public Ok<TypedResultExample> TypedResultWithSwaggerResponseAnnotation()
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
{
return TypedResults.Ok(new TypedResultExample());
}
```

### Limitations

Expand Down
6 changes: 5 additions & 1 deletion src/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<PackageProjectUrl>https://github.com/gsoft-inc/wl-extensions-openapi</PackageProjectUrl>
<PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
<PublishRepositoryUrl>true</PublishRepositoryUrl>
<LangVersion>11</LangVersion>
<LangVersion>12</LangVersion>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<AnalysisLevel>latest-all</AnalysisLevel>
Expand Down Expand Up @@ -44,4 +44,8 @@
<Using Include="Xunit" />
<Using Include="Xunit.Abstractions" />
</ItemGroup>

<PropertyGroup>
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
<RestoreAdditionalProjectSources>https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json</RestoreAdditionalProjectSources>
</PropertyGroup>
</Project>
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,294 @@
using Microsoft.CodeAnalysis;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis.Diagnostics;
using Workleap.Extensions.OpenAPI.Analyzers.Internals;

namespace Workleap.Extensions.OpenAPI.Analyzer;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class CompareTypedResultWithAnnotationAnalyzer : DiagnosticAnalyzer
{
internal static readonly DiagnosticDescriptor AnnotationMustMatchTypedResult = new(
id: RuleIdentifiers.MismatchResponseTypeWithAnnotation,
title: "Mismatch between annotation return type and endpoint return type",
messageFormat: "Mismatch between annotation return type and endpoint return type",
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
category: RuleCategories.Design,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
helpLinkUri: RuleIdentifiers.HelpUri);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(AnnotationMustMatchTypedResult);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);

context.RegisterCompilationStartAction(compilationContext =>
{
var analyzerContext = new AnalyzerContext(compilationContext.Compilation);
if (analyzerContext.IsValid)
{
compilationContext.RegisterSymbolAction(analyzerContext.ValidateEndpointResponseType, SymbolKind.Method);
}
});
}

private sealed class AnalyzerContext(Compilation compilation)
{
private INamedTypeSymbol? TaskOfTSymbol { get; } = compilation.GetTypeByMetadataName("System.Threading.Tasks.Task`1");
private INamedTypeSymbol? ValueTaskOfTSymbol { get; } = compilation.GetTypeByMetadataName("System.Threading.Tasks.ValueTask`1");
private INamedTypeSymbol? ProducesResponseSymbol { get; } = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute");
private INamedTypeSymbol? ProducesResponseOfTSymbol { get; } = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute`1");
private INamedTypeSymbol? SwaggerResponseSymbol { get; } = compilation.GetTypeByMetadataName("Swashbuckle.AspNetCore.Annotations.SwaggerResponseAttribute");

private INamedTypeSymbol? ResultSymbol { get; } = compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.IResult");

public bool IsValid => this.ProducesResponseSymbol is not null && this.ResultSymbol is not null;

public INamedTypeSymbol?[] ResultTaskOfTSymbol { get; } =
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
[
compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpResults.Results`2"),
compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpResults.Results`3"),
compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpResults.Results`4"),
compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpResults.Results`5"),
compilation.GetTypeByMetadataName("Microsoft.AspNetCore.Http.HttpResults.Results`6"),
];

private readonly Dictionary<ITypeSymbol, int> _resultsToStatusCodeMap = InitializeHttpResultStatusCodeMap(compilation);
private readonly Dictionary<int, ITypeSymbol> _statusCodeToResultsMap = InitializeStatusCodeMapHttpResultMap(compilation);

private static Dictionary<ITypeSymbol, int> InitializeHttpResultStatusCodeMap(Compilation compilation)
{
var dictionary = new Dictionary<ITypeSymbol, int>(SymbolEqualityComparer.Default);
Add("Microsoft.AspNetCore.Http.HttpResults.Ok", 200);
Add("Microsoft.AspNetCore.Http.HttpResults.Ok`1", 200);
Add("Microsoft.AspNetCore.Http.HttpResults.Created", 201);
Add("Microsoft.AspNetCore.Http.HttpResults.Created`1", 201);
Add("Microsoft.AspNetCore.Http.HttpResults.CreatedAtRoute", 201);
Add("Microsoft.AspNetCore.Http.HttpResults.CreatedAtRoute`1", 201);
Add("Microsoft.AspNetCore.Http.HttpResults.Accepted", 202);
Add("Microsoft.AspNetCore.Http.HttpResults.Accepted`1", 202);
Add("Microsoft.AspNetCore.Http.HttpResults.AcceptedAtRoute", 202);
Add("Microsoft.AspNetCore.Http.HttpResults.AcceptedAtRoute`1", 202);
Add("Microsoft.AspNetCore.Http.HttpResults.NoContent", 204);
Add("Microsoft.AspNetCore.Http.HttpResults.BadRequest", 400);
Add("Microsoft.AspNetCore.Http.HttpResults.BadRequest`1", 400);
Add("Microsoft.AspNetCore.Http.HttpResults.UnauthorizedHttpResult", 401);
Add("Microsoft.AspNetCore.Http.HttpResults.NotFound", 404);
Add("Microsoft.AspNetCore.Http.HttpResults.NotFound`1", 404);
Add("Microsoft.AspNetCore.Http.HttpResults.Conflict", 409);
Add("Microsoft.AspNetCore.Http.HttpResults.Conflict`1", 409);
Add("Microsoft.AspNetCore.Http.HttpResults.UnprocessableEntity", 422);
Add("Microsoft.AspNetCore.Http.HttpResults.UnprocessableEntity`T", 422);
// Will be Supported in .NET 9
Add("Microsoft.AspNetCore.Http.HttpResults.InternalServerError", 500);
Add("Microsoft.AspNetCore.Http.HttpResults.InternalServerError`1", 500);
// Workleap's definition of the InternalServerError type result for other .NET versions
Add("Workleap.Extensions.OpenAPI.TypedResult.InternalServerError", 500);
Add("Workleap.Extensions.OpenAPI.TypedResult.InternalServerError`1", 500);

return dictionary;

void Add(string metadata, int statusCode)
{
var type = compilation.GetTypeByMetadataName(metadata);
if (type is not null)
{
dictionary.Add(type, statusCode);
}
}
}

private static Dictionary<int, ITypeSymbol> InitializeStatusCodeMapHttpResultMap(Compilation compilation)
{
var dictionary = new Dictionary<int, ITypeSymbol>();
Add(200, "Microsoft.AspNetCore.Http.HttpResults.Ok");
Add(201, "Microsoft.AspNetCore.Http.HttpResults.Created");
Add(202, "Microsoft.AspNetCore.Http.HttpResults.Accepted");
Add(204, "Microsoft.AspNetCore.Http.HttpResults.NoContent");
Add(400, "Microsoft.AspNetCore.Http.HttpResults.BadRequest");
Add(401, "Microsoft.AspNetCore.Http.HttpResults.UnauthorizedHttpResult");
Add(404, "Microsoft.AspNetCore.Http.HttpResults.NotFound");
Add(409, "Microsoft.AspNetCore.Http.HttpResults.Conflict");
Add(422, "Microsoft.AspNetCore.Http.HttpResults.UnprocessableEntity");
Add(500, "Microsoft.AspNetCore.Http.HttpResults.InternalServerError");
Add(500, "Workleap.Extensions.OpenAPI.TypedResult.InternalServerError");

return dictionary;

void Add(int statusCode, string metadata)
{
var type = compilation.GetTypeByMetadataName(metadata);
if (type is not null)
{
dictionary.Add(statusCode, type);
}
}
}

public void ValidateEndpointResponseType(SymbolAnalysisContext context)
{
if (context.Symbol.GetAttributes().Length == 0)
{
return;
}

var methodSymbol = (IMethodSymbol)context.Symbol;

var returnType = methodSymbol.ReturnType;

if (returnType is INamedTypeSymbol typedReturnType)
{
if (this.ResultSymbol is null)
{
return;
}

typedReturnType = this.UnwrapTypeFromTask(typedReturnType);

if (Implements(typedReturnType, this.ResultSymbol))
{
var methodSignatureStatusCodeToTypeMap = this.GetMethodReturnStatusCodeToTypeMap(typedReturnType);
heqianwang marked this conversation as resolved.
Show resolved Hide resolved

foreach (var attribute in methodSymbol.GetAttributes())
{
this.ValidateAnnotationWithTypedResult(context, attribute, methodSignatureStatusCodeToTypeMap);
}
}
}
}

private INamedTypeSymbol UnwrapTypeFromTask(INamedTypeSymbol typedReturnType)
{
// Check if the return type is of Task<> or ValueOfTask<>. If yes, then keep the inner type.
if (SymbolEqualityComparer.Default.Equals(typedReturnType.ConstructedFrom, this.TaskOfTSymbol) ||
SymbolEqualityComparer.Default.Equals(typedReturnType.ConstructedFrom, this.ValueTaskOfTSymbol))
{
var subType = typedReturnType.TypeArguments[0];
if (subType is INamedTypeSymbol namedSubType)
{
typedReturnType = namedSubType;
}
}

return typedReturnType;
}

private void ValidateAnnotationWithTypedResult(SymbolAnalysisContext context, AttributeData attribute,
Dictionary<int, ITypeSymbol> methodSignatureStatusCodeToTypeMap)
{
if (attribute.AttributeClass == null || attribute.ConstructorArguments.Length == 0)
{
return;
}

if (attribute.AttributeClass.Equals(this.ProducesResponseSymbol, SymbolEqualityComparer.Default))
{
if (attribute.ConstructorArguments.Length == 1)
{
if (attribute.ConstructorArguments[0].Value is int statusCodeValue)
{
var type = this._statusCodeToResultsMap[statusCodeValue];
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
ValidateAnnotationForTypeMismatch(context, methodSignatureStatusCodeToTypeMap, statusCodeValue, type, attribute);
}
}
else if (attribute.ConstructorArguments[1].Value is int statusCodeValue && attribute.ConstructorArguments[0].Value is ITypeSymbol type)
{
ValidateAnnotationForTypeMismatch(context, methodSignatureStatusCodeToTypeMap, statusCodeValue, type, attribute);
}
}

else if (attribute.AttributeClass.ConstructedFrom.Equals(this.ProducesResponseOfTSymbol, SymbolEqualityComparer.Default))
{
if (attribute.ConstructorArguments[0].Value is int statusCodeValue)
{
ValidateAnnotationForTypeMismatch(context, methodSignatureStatusCodeToTypeMap, statusCodeValue, attribute.AttributeClass.TypeArguments[0], attribute);
}
}

else if (attribute.AttributeClass.ConstructedFrom.Equals(this.SwaggerResponseSymbol, SymbolEqualityComparer.Default))
{
if (attribute.ConstructorArguments.Length > 2 && attribute.ConstructorArguments[0].Value is int statusCodeValue && attribute.ConstructorArguments[2].Value is ITypeSymbol type)
{
ValidateAnnotationForTypeMismatch(context, methodSignatureStatusCodeToTypeMap, statusCodeValue, type, attribute);
}
}
}

private Dictionary<int, ITypeSymbol> GetMethodReturnStatusCodeToTypeMap(ITypeSymbol methodReturnSignature)
{
var methodReturnSignatures = this.GetReturnStatusAndTypeFromMethodReturn(methodReturnSignature);
var methodSignatureStatusCodeToTypeMap = new Dictionary<int, ITypeSymbol>();
foreach (var returnValues in methodReturnSignatures)
heqianwang marked this conversation as resolved.
Show resolved Hide resolved
{
if (!methodSignatureStatusCodeToTypeMap.ContainsKey(returnValues.statusCode))
{
methodSignatureStatusCodeToTypeMap.Add(returnValues.statusCode, returnValues.symbol);
}
}

return methodSignatureStatusCodeToTypeMap;
}

private IEnumerable<(int statusCode, ITypeSymbol symbol)> GetReturnStatusAndTypeFromMethodReturn(ITypeSymbol typeSymbol)
{
if (typeSymbol is not INamedTypeSymbol namedTypeSymbol)
{
return Enumerable.Empty<(int, ITypeSymbol)>();
}

// Task<>
if (SymbolEqualityComparer.Default.Equals(namedTypeSymbol.ConstructedFrom, this.TaskOfTSymbol))
{
return this.GetReturnStatusAndTypeFromMethodReturn(namedTypeSymbol.TypeArguments[0]);
}

if (SymbolEqualityComparer.Default.Equals(namedTypeSymbol.ConstructedFrom, this.ValueTaskOfTSymbol))
{
return this.GetReturnStatusAndTypeFromMethodReturn(namedTypeSymbol.TypeArguments[0]);
}

// Result<OK, NotFound>
if (this.ResultTaskOfTSymbol.Any(symbol => SymbolEqualityComparer.Default.Equals(namedTypeSymbol.ConstructedFrom, symbol)))
{
return namedTypeSymbol.TypeArguments.SelectMany(this.GetReturnStatusAndTypeFromMethodReturn);
}

if (this._resultsToStatusCodeMap.TryGetValue(namedTypeSymbol.ConstructedFrom, out var statusCode))
{
// If there is a type, then return the type, otherwise return IResult type
return [(statusCode, namedTypeSymbol.TypeArguments.Length == 0 ? namedTypeSymbol : namedTypeSymbol.TypeArguments[0])];
}

return Enumerable.Empty<(int, ITypeSymbol)>();
}

private static void ValidateAnnotationForTypeMismatch(SymbolAnalysisContext context,
Dictionary<int, ITypeSymbol> methodSignatureStatusCodeToTypeMap, int statusCodeValue, ITypeSymbol type, AttributeData attribute)
{
if (attribute.ApplicationSyntaxReference is null)
{
return;
}

var attributeLocation = attribute.ApplicationSyntaxReference.GetSyntax(context.CancellationToken).GetLocation();
if (methodSignatureStatusCodeToTypeMap.TryGetValue(statusCodeValue, out var mappedType))
{
if (!SymbolEqualityComparer.Default.Equals(mappedType, type))
{
context.ReportDiagnostic(AnnotationMustMatchTypedResult, attributeLocation);
}
}
else
{
context.ReportDiagnostic(AnnotationMustMatchTypedResult, attributeLocation);
}
}

private static bool Implements(ITypeSymbol interfaceSymbol, ITypeSymbol comparedType)
{
return SymbolEqualityComparer.Default.Equals(interfaceSymbol, comparedType) || interfaceSymbol.AllInterfaces.Any(i => SymbolEqualityComparer.Default.Equals(comparedType, i));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

namespace Workleap.Extensions.OpenAPI.Analyzers.Internals;

internal static class RoslynExtensions
{
public static void ReportDiagnostic(this SymbolAnalysisContext context, DiagnosticDescriptor diagnosticDescriptor, Location location)
{
context.ReportDiagnostic(Diagnostic.Create(diagnosticDescriptor, location));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace Workleap.Extensions.OpenAPI.Analyzers.Internals;

internal static class RuleCategories
{
// Same categories used by Microsoft
public const string Design = "Design";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
namespace Workleap.Extensions.OpenAPI.Analyzers.Internals;

internal static class RuleIdentifiers
{
public const string HelpUri = "https://github.com/gsoft-inc/wl-extensions-openapi";

// DO NOT change the identifier of existing rules.
// Projects can customize the severity level of analysis rules using a .editorconfig file.
public const string MismatchResponseTypeWithAnnotation = "WLOAS001";
}
Loading