Fix OpenAPI XML code gen #60977
Fix OpenAPI XML code gen #60977
Conversation
dotnet-issue-labeler
bot
added
the
area-mvc
| @@ -189,4 +189,26 @@ public static IEnumerable<INamedTypeSymbol> GetBaseTypes(this ITypeSymbol? type) | |||
| current = current.BaseType; | |||
| } | |||
| } | |||
|
|
|||
| public static bool IsAccessibleType(this ISymbol symbol) | |||
There was a problem hiding this comment.
This helps us avoid generating XML comments for non-public types and internal properties in non-public types.
Note: this does mean that DTOs have to be defined as public types if they are defined in the same assembly as the API (see where this is called in the AssemnlyTypeSymbolVisitor above). We can consider changing this behavior to allow XML comments on internal types in the application assembly.
There was a problem hiding this comment.
Seems a bit overaggressive. I'm guessing we'll change this behavior a bit in the future.
Any reason to disallow the type entirely just because it has internal types?
| @@ -27,6 +27,9 @@ internal static string GenerateXmlCommentSupportSource(string commentsFromXmlFil | |||
| // </auto-generated> | |||
| //------------------------------------------------------------------------------ | |||
| #nullable enable | |||
| // Suppress warnings about obsolete types and members | |||
| // in generated code | |||
| #pragma warning disable CS0612, CS0618 | |||
There was a problem hiding this comment.
Suppress obsoletion warnings in generated code. If someone happens to be using an obsolete type in their API, we don't want to resurface this warning in generated code where we call typeof(ObsolteType).
| { | ||
| var parameter = comment.Parameters[i]; | ||
| var exampleLiteral = string.IsNullOrEmpty(parameter.Example) | ||
| ? "null" | ||
| : FormatStringForCode(parameter.Example!); | ||
| codeWriter.Write($"new XmlParameterComment(@\"{parameter.Name}\", @\"{parameter.Description}\", {exampleLiteral}, {(parameter.Deprecated == true ? "true" : "false")})"); | ||
| codeWriter.Write("new XmlParameterComment("); |
There was a problem hiding this comment.
Here and below we format strings in the emitted code so that we can properly escape quoted strings inside XML descriptions/summaries/etc.
| @@ -46,7 +46,7 @@ public static MemberKey FromMethodSymbol(IMethodSymbol method, Compilation compi | |||
|
|
|||
| returnType = actualReturnType.TypeKind == TypeKind.TypeParameter | |||
| ? "typeof(object)" | |||
| : $"typeof({actualReturnType.ToDisplayString(_typeKeyFormat)})"; | |||
| : $"typeof({ReplaceGenericArguments(actualReturnType.ToDisplayString(_typeKeyFormat))})"; | |||
There was a problem hiding this comment.
We need to make sure that all generics are emitted as open generics for method declarations. We already do this elsewhere in the MemberKey for the declaring type and the property type but not for methods.
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| public class DeeplyNestedLevel1 |
There was a problem hiding this comment.
Since we require all types to be public for discovery, we moved the shared types that are used by tests and the sample app to the sample app since the tests reference the sample app.
Previously, we used compile includes with internal types which doesn't work in this new model.
captainsafia
force-pushed
the
cs/openapi-xml-bugfix
branch
from
d231073 to
196bd95
Compare
| @@ -118,10 +118,12 @@ | |||
| "type": "object", | |||
| "properties": { | |||
| "title": { | |||
| "type": "string" | |||
| "type": "string", | |||
There was a problem hiding this comment.
STJ's schema generation assumes strings are nullable in an oblivious nullability context (see disable in Sample.csproj), hence the delta here.
| @@ -189,4 +189,26 @@ public static IEnumerable<INamedTypeSymbol> GetBaseTypes(this ITypeSymbol? type) | |||
| current = current.BaseType; | |||
| } | |||
| } | |||
|
|
|||
| public static bool IsAccessibleType(this ISymbol symbol) | |||
There was a problem hiding this comment.
Seems a bit overaggressive. I'm guessing we'll change this behavior a bit in the future.
Any reason to disallow the type entirely just because it has internal types?
I assume you mean internal properties? This will omit public properties in internal types. We can relax the rule a little bit and still discover XML comments on internal types on the application assembly but prohibit them on internal types in referenced files. This would strike a balance between not discovering internal types in assemblies we don't have access to while still permitting them in the application assembly. |
Fixes some of the issues in #60783.