From 17839047a2ec2752449b90892dd15b88b3e800a8 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 22 Aug 2025 15:11:41 +0100 Subject: [PATCH 1/4] Avoid context enter/exit - Avoid using `Enter()` and `Exit()` operations in validation rules where possible. - Use `string` not `String`. - Remove redundant comments. - Fix typo in comment. --- .../Services/OpenApiVisitorBase.cs | 2 +- .../Validations/Rules/OpenApiContactRules.cs | 9 ++---- .../Validations/Rules/OpenApiDocumentRules.cs | 4 +-- .../Rules/OpenApiExtensionRules.cs | 6 ++-- .../Rules/OpenApiExternalDocsRules.cs | 10 ++---- .../Validations/Rules/OpenApiInfoRules.cs | 17 ++++------ .../Validations/Rules/OpenApiLicenseRules.cs | 10 ++---- .../Rules/OpenApiNonDefaultRules.cs | 32 +++++++------------ .../Rules/OpenApiOAuthFlowRules.cs | 22 ++++++------- .../Rules/OpenApiParameterRules.cs | 19 +++++------ .../Validations/Rules/OpenApiPathsRules.cs | 14 ++++---- .../Validations/Rules/OpenApiResponseRules.cs | 8 ++--- .../Rules/OpenApiResponsesRules.cs | 9 ++---- .../Validations/Rules/OpenApiSchemaRules.cs | 6 ++-- .../Validations/Rules/OpenApiServerRules.cs | 22 ++++++------- .../Validations/Rules/OpenApiTagRules.cs | 10 ++---- 16 files changed, 75 insertions(+), 125 deletions(-) diff --git a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs index 1ccfea863..7aa411b57 100644 --- a/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs +++ b/src/Microsoft.OpenApi/Services/OpenApiVisitorBase.cs @@ -22,7 +22,7 @@ public abstract class OpenApiVisitorBase public CurrentKeys CurrentKeys { get; } = new(); /// - /// Allow Rule to indicate validation error occured at a deeper context level. + /// Allow Rule to indicate validation error occurred at a deeper context level. /// /// Identifier for context public virtual void Enter(string segment) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs index 89f8f0a4b..8d32bea74 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiContactRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -18,14 +16,13 @@ public static class OpenApiContactRules new(nameof(EmailMustBeEmailFormat), (context, item) => { - context.Enter("email"); if (item is {Email: not null} && !item.Email.IsEmailAddress()) { + context.Enter("email"); context.CreateError(nameof(EmailMustBeEmailFormat), - String.Format(SRResource.Validation_StringMustBeEmailAddress, item.Email)); + string.Format(SRResource.Validation_StringMustBeEmailAddress, item.Email)); + context.Exit(); } - context.Exit(); }); - } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs index d15b0a0a0..a1f166ee3 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiDocumentRules.cs @@ -19,13 +19,13 @@ public static class OpenApiDocumentRules (context, item) => { // info - context.Enter("info"); if (item.Info == null) { + context.Enter("info"); context.CreateError(nameof(OpenApiDocumentFieldIsMissing), string.Format(SRResource.Validation_FieldIsRequired, "info", "document")); + context.Exit(); } - context.Exit(); }); /// diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs index c9df09b2e..be927c364 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExtensionRules.cs @@ -19,16 +19,16 @@ public static class OpenApiExtensibleRules new(nameof(ExtensionNameMustStartWithXDash), (context, item) => { - context.Enter("extensions"); if (item.Extensions is not null) { + context.Enter("extensions"); foreach (var extensible in item.Extensions.Keys.Where(static x => !x.StartsWith(OpenApiConstants.ExtensionFieldNamePrefix, StringComparison.OrdinalIgnoreCase))) { context.CreateError(nameof(ExtensionNameMustStartWithXDash), string.Format(SRResource.Validation_ExtensionNameMustBeginWithXDash, extensible, context.PathString)); - } + } + context.Exit(); } - context.Exit(); }); } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs index a15cd53b8..686b00c59 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiExternalDocsRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -19,15 +17,13 @@ public static class OpenApiExternalDocsRules (context, item) => { // url - context.Enter("url"); if (item.Url == null) { + context.Enter("url"); context.CreateError(nameof(UrlIsRequired), - String.Format(SRResource.Validation_FieldIsRequired, "url", "External Documentation")); + string.Format(SRResource.Validation_FieldIsRequired, "url", "External Documentation")); + context.Exit(); } - context.Exit(); }); - - // add more rule. } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs index ef7274c06..591c7695a 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiInfoRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -19,25 +17,22 @@ public static class OpenApiInfoRules (context, item) => { // title - context.Enter("title"); if (item.Title == null) { + context.Enter("title"); context.CreateError(nameof(InfoRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "title", "info")); + string.Format(SRResource.Validation_FieldIsRequired, "title", "info")); + context.Exit(); } - context.Exit(); // version - context.Enter("version"); if (item.Version == null) { + context.Enter("version"); context.CreateError(nameof(InfoRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "version", "info")); + string.Format(SRResource.Validation_FieldIsRequired, "version", "info")); + context.Exit(); } - context.Exit(); - }); - - // add more rule. } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs index c9dc7e4a6..3d2c4d49e 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiLicenseRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -18,15 +16,13 @@ public static class OpenApiLicenseRules new(nameof(LicenseRequiredFields), (context, license) => { - context.Enter("name"); if (license.Name == null) { + context.Enter("name"); context.CreateError(nameof(LicenseRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "name", "license")); + string.Format(SRResource.Validation_FieldIsRequired, "name", "license")); + context.Exit(); } - context.Exit(); }); - - // add more rules } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiNonDefaultRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiNonDefaultRules.cs index 5b3cd0a49..2bc019667 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiNonDefaultRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiNonDefaultRules.cs @@ -36,7 +36,7 @@ public static class OpenApiNonDefaultRules /// Validate the data matches with the given data type. /// public static ValidationRule ParameterMismatchedDataType => - new(nameof(ParameterMismatchedDataType), + new(nameof(ParameterMismatchedDataType), (context, parameter) => { ValidateMismatchedDataType(context, nameof(ParameterMismatchedDataType), parameter.Example, parameter.Examples, parameter.Schema); @@ -50,39 +50,33 @@ public static class OpenApiNonDefaultRules (context, schema) => { // default - context.Enter("default"); - if (schema.Default != null) { + context.Enter("default"); RuleHelpers.ValidateDataTypeMismatch(context, nameof(SchemaMismatchedDataType), schema.Default, schema); + context.Exit(); } - context.Exit(); - // example - context.Enter("example"); - if (schema.Example != null) { + context.Enter("example"); RuleHelpers.ValidateDataTypeMismatch(context, nameof(SchemaMismatchedDataType), schema.Example, schema); + context.Exit(); } - context.Exit(); - // enum - context.Enter("enum"); - if (schema.Enum != null) { + context.Enter("enum"); for (var i = 0; i < schema.Enum.Count; i++) { context.Enter(i.ToString()); RuleHelpers.ValidateDataTypeMismatch(context, nameof(SchemaMismatchedDataType), schema.Enum[i], schema); context.Exit(); } + context.Exit(); } - - context.Exit(); }); private static void ValidateMismatchedDataType(IValidationContext context, @@ -92,20 +86,17 @@ private static void ValidateMismatchedDataType(IValidationContext context, IOpenApiSchema? schema) { // example - context.Enter("example"); - if (example != null) { + context.Enter("example"); RuleHelpers.ValidateDataTypeMismatch(context, ruleName, example, schema); + context.Exit(); } - context.Exit(); - // enum - context.Enter("examples"); - if (examples != null) { + context.Enter("examples"); foreach (var key in examples.Keys.Where(k => examples[k] != null)) { context.Enter(key); @@ -114,9 +105,8 @@ private static void ValidateMismatchedDataType(IValidationContext context, context.Exit(); context.Exit(); } + context.Exit(); } - - context.Exit(); } } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs index 4f9efe831..10cd9f9a9 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiOAuthFlowRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -19,33 +17,31 @@ public static class OpenApiOAuthFlowRules (context, flow) => { // authorizationUrl - context.Enter("authorizationUrl"); if (flow.AuthorizationUrl == null) { + context.Enter("authorizationUrl"); context.CreateError(nameof(OAuthFlowRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "authorizationUrl", "OAuth Flow")); + string.Format(SRResource.Validation_FieldIsRequired, "authorizationUrl", "OAuth Flow")); + context.Exit(); } - context.Exit(); // tokenUrl - context.Enter("tokenUrl"); if (flow.TokenUrl == null) { + context.Enter("tokenUrl"); context.CreateError(nameof(OAuthFlowRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "tokenUrl", "OAuth Flow")); + string.Format(SRResource.Validation_FieldIsRequired, "tokenUrl", "OAuth Flow")); + context.Exit(); } - context.Exit(); // scopes - context.Enter("scopes"); if (flow.Scopes == null) { + context.Enter("scopes"); context.CreateError(nameof(OAuthFlowRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "scopes", "OAuth Flow")); + string.Format(SRResource.Validation_FieldIsRequired, "scopes", "OAuth Flow")); + context.Exit(); } - context.Exit(); }); - - // add more rule. } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs index 2de42e02f..a84ea3255 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiParameterRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -19,22 +17,22 @@ public static class OpenApiParameterRules (context, item) => { // name - context.Enter("name"); if (item.Name == null) { + context.Enter("name"); context.CreateError(nameof(ParameterRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "name", "parameter")); + string.Format(SRResource.Validation_FieldIsRequired, "name", "parameter")); + context.Exit(); } - context.Exit(); // in - context.Enter("in"); if (item.In == null) { + context.Enter("in"); context.CreateError(nameof(ParameterRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "in", "parameter")); + string.Format(SRResource.Validation_FieldIsRequired, "in", "parameter")); + context.Exit(); } - context.Exit(); }); /// @@ -45,15 +43,14 @@ public static class OpenApiParameterRules (context, item) => { // required - context.Enter("required"); if (item.In == ParameterLocation.Path && !item.Required) { + context.Enter("required"); context.CreateError( nameof(RequiredMustBeTrueWhenInIsPath), "\"required\" must be true when parameter location is \"path\""); + context.Exit(); } - - context.Exit(); }); /// diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs index 9f1999e4c..378c46e36 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiPathsRules.cs @@ -37,22 +37,22 @@ public static class OpenApiPathsRules /// A relative path to an individual endpoint. The field name MUST begin with a slash. /// public static ValidationRule PathMustBeUnique => - new ValidationRule(nameof(PathMustBeUnique), + new(nameof(PathMustBeUnique), (context, item) => { var hashSet = new HashSet(); foreach (var path in item.Keys) { - context.Enter(path); - var pathSignature = GetPathSignature(path); - + if (!hashSet.Add(pathSignature)) + { + context.Enter(path); context.CreateError(nameof(PathMustBeUnique), string.Format(SRResource.Validation_PathSignatureMustBeUnique, pathSignature)); - - context.Exit(); + context.Exit(); + } } }); @@ -77,7 +77,5 @@ private static string GetPathSignature(string path) return path; } - - // add more rules } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs index c454b5290..1fe301ba3 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponseRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -19,13 +17,13 @@ public static class OpenApiResponseRules (context, response) => { // description - context.Enter("description"); if (response.Description == null) { + context.Enter("description"); context.CreateError(nameof(ResponseRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "description", "response")); + string.Format(SRResource.Validation_FieldIsRequired, "description", "response")); + context.Exit(); } - context.Exit(); }); // add more rule. diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs index f1d2572ea..aa4974965 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. using System; -using System.Linq; using System.Text.RegularExpressions; namespace Microsoft.OpenApi @@ -29,7 +28,7 @@ public static partial class OpenApiResponsesRules new(nameof(ResponsesMustContainAtLeastOneResponse), (context, responses) => { - if (!responses.Keys.Any()) + if (responses.Keys.Count == 0) { context.CreateError(nameof(ResponsesMustContainAtLeastOneResponse), "Responses must contain at least one response"); @@ -45,8 +44,6 @@ public static partial class OpenApiResponsesRules { foreach (var key in responses.Keys) { - context.Enter(key); - if (!"default".Equals(key, StringComparison.OrdinalIgnoreCase) && !StatusCodeRegex #if NET8_0_OR_GREATER ().IsMatch(key) @@ -55,13 +52,13 @@ public static partial class OpenApiResponsesRules #endif ) { + context.Enter(key); context.CreateError(nameof(ResponsesMustBeIdentifiedByDefaultOrStatusCode), "Responses key must be 'default', an HTTP status code, " + "or one of the following strings representing a range of HTTP status codes: " + "'1XX', '2XX', '3XX', '4XX', '5XX' (case insensitive)"); + context.Exit(); } - - context.Exit(); } }); } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs index 81a4ab88f..70d558a13 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs @@ -44,21 +44,19 @@ public static class OpenApiSchemaRules (context, schema) => { // discriminator - context.Enter("discriminator"); - if (schema is not null && schema.Discriminator != null) { var discriminatorName = schema.Discriminator?.PropertyName; if (!ValidateChildSchemaAgainstDiscriminator(schema, discriminatorName)) { + context.Enter("discriminator"); context.CreateError(nameof(ValidateSchemaDiscriminator), string.Format(SRResource.Validation_SchemaRequiredFieldListMustContainThePropertySpecifiedInTheDiscriminator, schema is OpenApiSchemaReference { Reference: not null} schemaReference ? schemaReference.Reference.Id : string.Empty, discriminatorName)); + context.Exit(); } } - - context.Exit(); }); /// diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs index 95ea9490d..2843b9e33 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiServerRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -18,41 +16,39 @@ public static class OpenApiServerRules new(nameof(ServerRequiredFields), (context, server) => { - context.Enter("url"); if (server.Url == null) { + context.Enter("url"); context.CreateError(nameof(ServerRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "url", "server")); + string.Format(SRResource.Validation_FieldIsRequired, "url", "server")); + context.Exit(); } - context.Exit(); - context.Enter("variables"); if (server.Variables is not null) { + context.Enter("variables"); foreach (var variable in server.Variables) { context.Enter(variable.Key); ValidateServerVariableRequiredFields(context, variable.Key, variable.Value); context.Exit(); - } + } + context.Exit(); } - context.Exit(); }); - // add more rules - /// /// Validate required fields in server variable /// private static void ValidateServerVariableRequiredFields(IValidationContext context, string key, OpenApiServerVariable item) { - context.Enter("default"); if (string.IsNullOrEmpty(item.Default)) { + context.Enter("default"); context.CreateError("ServerVariableMustHaveDefaultValue", - String.Format(SRResource.Validation_FieldIsRequired, "default", key)); + string.Format(SRResource.Validation_FieldIsRequired, "default", key)); + context.Exit(); } - context.Exit(); } } } diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs index 45c9b7fda..dee4bc186 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiTagRules.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. -using System; - namespace Microsoft.OpenApi { /// @@ -18,15 +16,13 @@ public static class OpenApiTagRules new(nameof(TagRequiredFields), (context, tag) => { - context.Enter("name"); if (tag.Name == null) { + context.Enter("name"); context.CreateError(nameof(TagRequiredFields), - String.Format(SRResource.Validation_FieldIsRequired, "name", "tag")); + string.Format(SRResource.Validation_FieldIsRequired, "name", "tag")); + context.Exit(); } - context.Exit(); }); - - // add more rules } } From 7b7cea4f966ec7e89e4b275d99b0b70bef0ea782 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 22 Aug 2025 15:34:51 +0100 Subject: [PATCH 2/4] Add recommended rules Add new `OpenApiRecommendedRules` class and adds a new `GetOperationShouldNotHaveRequestBody` rule. Resolves #2454. --- .../Rules/OpenApiRecommendedRules.cs | 54 +++++ .../PublicApi/PublicApi.approved.txt | 4 + .../OpenApiRecommendedRulesTests.cs | 207 ++++++++++++++++++ 3 files changed, 265 insertions(+) create mode 100644 src/Microsoft.OpenApi/Validations/Rules/OpenApiRecommendedRules.cs create mode 100644 test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiRecommendedRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiRecommendedRules.cs new file mode 100644 index 000000000..b115b2ac1 --- /dev/null +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiRecommendedRules.cs @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +using System.Net.Http; + +namespace Microsoft.OpenApi +{ + /// + /// Additional recommended validation rules for OpenAPI. + /// + public static class OpenApiRecommendedRules + { + /// + /// A relative path to an individual endpoint. The field name MUST begin with a slash. + /// + public static ValidationRule GetOperationShouldNotHaveRequestBody => + new(nameof(GetOperationShouldNotHaveRequestBody), + (context, item) => + { + foreach (var path in item) + { + if (path.Value.Operations is not { Count: > 0 } operations) + { + continue; + } + + context.Enter(path.Key); + + foreach (var operation in operations) + { + if (!operation.Key.Equals(HttpMethod.Get)) + { + continue; + } + + if (operation.Value.RequestBody != null) + { + context.Enter(operation.Key.Method.ToLowerInvariant()); + context.Enter("requestBody"); + + context.CreateWarning( + nameof(GetOperationShouldNotHaveRequestBody), + "GET operations should not have a request body."); + + context.Exit(); + context.Exit(); + } + } + + context.Exit(); + } + }); + } +} diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt index 5733eb735..29e7ddc74 100644 --- a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt +++ b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt @@ -1031,6 +1031,10 @@ namespace Microsoft.OpenApi public OpenApiReaderException(string message, Microsoft.OpenApi.Reader.ParsingContext context) { } public OpenApiReaderException(string message, System.Exception innerException) { } } + public static class OpenApiRecommendedRules + { + public static Microsoft.OpenApi.ValidationRule GetOperationShouldNotHaveRequestBody { get; } + } public class OpenApiReferenceError : Microsoft.OpenApi.OpenApiError { public readonly Microsoft.OpenApi.BaseOpenApiReference? Reference; diff --git a/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs new file mode 100644 index 000000000..5a70a7dd0 --- /dev/null +++ b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs @@ -0,0 +1,207 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; +using Xunit; + +namespace Microsoft.OpenApi.Validations.Tests; + +public static class OpenApiRecommendedRulesTests +{ + [Fact] + public static void GetOperationWithoutRequestBodyIsValid() + { + // Arrange + var document = new OpenApiDocument + { + Components = new OpenApiComponents(), + Info = new OpenApiInfo + { + Title = "People Document", + Version = "1.0.0" + }, + Paths = [], + Workspace = new() + }; + + document.AddComponent("Person", new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = new Dictionary() + { + ["name"] = new OpenApiSchema { Type = JsonSchemaType.String }, + ["email"] = new OpenApiSchema { Type = JsonSchemaType.String, Format = "email" } + } + }); + + document.Paths.Add("people", new OpenApiPathItem + { + Operations = new Dictionary() + { + [HttpMethod.Get] = new OpenApiOperation + { + RequestBody = null, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + }, + [HttpMethod.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + }, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + } + } + }); + + var ruleSet = new ValidationRuleSet(); + ruleSet.Add(typeof(OpenApiPaths), OpenApiRecommendedRules.GetOperationShouldNotHaveRequestBody); + + // Act + + var errors = document.Validate(ruleSet); + var result = !errors.Any(); + + // Assert + Assert.True(result); + Assert.NotNull(errors); + Assert.Empty(errors); + } + + [Fact] + public static void GetOperationWithRequestBodyIsInvalid() + { + // Arrange + var document = new OpenApiDocument + { + Components = new OpenApiComponents(), + Info = new OpenApiInfo + { + Title = "People Document", + Version = "1.0.0" + }, + Paths = [], + Workspace = new() + }; + + document.AddComponent("Person", new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = new Dictionary() + { + ["name"] = new OpenApiSchema { Type = JsonSchemaType.String }, + ["email"] = new OpenApiSchema { Type = JsonSchemaType.String, Format = "email" } + } + }); + + document.Paths.Add("people", new OpenApiPathItem + { + Operations = new Dictionary() + { + [HttpMethod.Get] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + }, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + }, + [HttpMethod.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + }, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + } + } + }); + + var ruleSet = new ValidationRuleSet(); + ruleSet.Add(typeof(OpenApiPaths), OpenApiRecommendedRules.GetOperationShouldNotHaveRequestBody); + + // Act + + var errors = document.Validate(ruleSet); + var result = !errors.Any(); + + // Assert + Assert.False(result); + Assert.NotNull(errors); + var error = Assert.Single(errors); + Assert.Equal("GET operations should not have a request body.", error.Message); + Assert.Equal("#/paths/people/get/requestbody", error.Pointer); + } +} From bbdaa02308f946dbfa6f0b257a55adc825bb6718 Mon Sep 17 00:00:00 2001 From: martincostello Date: Fri, 22 Aug 2025 15:43:36 +0100 Subject: [PATCH 3/4] Address feedback Address Copilot feedback. --- .../Validations/Rules/OpenApiResponsesRules.cs | 2 +- .../Validations/OpenApiRecommendedRulesTests.cs | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs index aa4974965..4c038b2c7 100644 --- a/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs +++ b/src/Microsoft.OpenApi/Validations/Rules/OpenApiResponsesRules.cs @@ -28,7 +28,7 @@ public static partial class OpenApiResponsesRules new(nameof(ResponsesMustContainAtLeastOneResponse), (context, responses) => { - if (responses.Keys.Count == 0) + if (responses.Count == 0) { context.CreateError(nameof(ResponsesMustContainAtLeastOneResponse), "Responses must contain at least one response"); diff --git a/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs index 5a70a7dd0..e1d82326d 100644 --- a/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs +++ b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs @@ -92,7 +92,6 @@ public static void GetOperationWithoutRequestBodyIsValid() ruleSet.Add(typeof(OpenApiPaths), OpenApiRecommendedRules.GetOperationShouldNotHaveRequestBody); // Act - var errors = document.Validate(ruleSet); var result = !errors.Any(); @@ -193,15 +192,14 @@ public static void GetOperationWithRequestBodyIsInvalid() ruleSet.Add(typeof(OpenApiPaths), OpenApiRecommendedRules.GetOperationShouldNotHaveRequestBody); // Act - - var errors = document.Validate(ruleSet); - var result = !errors.Any(); + var warnings = document.Validate(ruleSet); + var result = !warnings.Any(); // Assert Assert.False(result); - Assert.NotNull(errors); - var error = Assert.Single(errors); - Assert.Equal("GET operations should not have a request body.", error.Message); - Assert.Equal("#/paths/people/get/requestbody", error.Pointer); + Assert.NotNull(warnings); + var warning = Assert.Single(warnings); + Assert.Equal("GET operations should not have a request body.", warning.Message); + Assert.Equal("#/paths/people/get/requestbody", warning.Pointer); } } From 92564f34a2a58227d79f11bd560aec9d453b6673 Mon Sep 17 00:00:00 2001 From: martincostello Date: Sat, 23 Aug 2025 10:13:56 +0100 Subject: [PATCH 4/4] Test not part of default rules Address feedback. --- .../OpenApiRecommendedRulesTests.cs | 113 ++++++++++++++++-- 1 file changed, 106 insertions(+), 7 deletions(-) diff --git a/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs index e1d82326d..fc0b0df31 100644 --- a/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs +++ b/test/Microsoft.OpenApi.Tests/Validations/OpenApiRecommendedRulesTests.cs @@ -36,7 +36,7 @@ public static void GetOperationWithoutRequestBodyIsValid() } }); - document.Paths.Add("people", new OpenApiPathItem + document.Paths.Add("/people", new OpenApiPathItem { Operations = new Dictionary() { @@ -92,13 +92,13 @@ public static void GetOperationWithoutRequestBodyIsValid() ruleSet.Add(typeof(OpenApiPaths), OpenApiRecommendedRules.GetOperationShouldNotHaveRequestBody); // Act - var errors = document.Validate(ruleSet); - var result = !errors.Any(); + var warnings = document.Validate(ruleSet); + var result = !warnings.Any(); // Assert Assert.True(result); - Assert.NotNull(errors); - Assert.Empty(errors); + Assert.NotNull(warnings); + Assert.Empty(warnings); } [Fact] @@ -127,7 +127,7 @@ public static void GetOperationWithRequestBodyIsInvalid() } }); - document.Paths.Add("people", new OpenApiPathItem + document.Paths.Add("/people", new OpenApiPathItem { Operations = new Dictionary() { @@ -200,6 +200,105 @@ public static void GetOperationWithRequestBodyIsInvalid() Assert.NotNull(warnings); var warning = Assert.Single(warnings); Assert.Equal("GET operations should not have a request body.", warning.Message); - Assert.Equal("#/paths/people/get/requestbody", warning.Pointer); + Assert.Equal("#/paths//people/get/requestBody", warning.Pointer); + } + + [Fact] + public static void GetOperationWithRequestBodyIsValidUsingDefaultRuleSet() + { + // Arrange + var document = new OpenApiDocument + { + Components = new OpenApiComponents(), + Info = new OpenApiInfo + { + Title = "People Document", + Version = "1.0.0" + }, + Paths = [], + Workspace = new() + }; + + document.AddComponent("Person", new OpenApiSchema + { + Type = JsonSchemaType.Object, + Properties = new Dictionary() + { + ["name"] = new OpenApiSchema { Type = JsonSchemaType.String }, + ["email"] = new OpenApiSchema { Type = JsonSchemaType.String, Format = "email" } + } + }); + + document.Paths.Add("/people", new OpenApiPathItem + { + Operations = new Dictionary() + { + [HttpMethod.Get] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + }, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + }, + [HttpMethod.Post] = new OpenApiOperation + { + RequestBody = new OpenApiRequestBody + { + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + }, + Responses = new() + { + ["200"] = new OpenApiResponse + { + Description = "OK", + Content = new Dictionary() + { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchemaReference("Person", document), + } + } + } + } + } + } + }); + + var ruleSet = ValidationRuleSet.GetDefaultRuleSet(); + + // Act + var warnings = document.Validate(ruleSet); + var result = !warnings.Any(); + + // Assert + Assert.True(result); + Assert.NotNull(warnings); + Assert.Empty(warnings); } }