From b1722a8678c95c4cdfcc2b29b033db5241fb510c Mon Sep 17 00:00:00 2001 From: Veronica Giaudrone Date: Thu, 2 Feb 2017 11:44:22 -0800 Subject: [PATCH 1/2] Adding rule to warn about boolean type properties --- .../Properties/Resources.Designer.cs | 9 +++ .../AutoRest.Core/Properties/Resources.resx | 3 + .../Model/ServiceDefinition.cs | 1 + .../BooleanPropertyNotRecommended.cs | 58 +++++++++++++++++++ 4 files changed, 71 insertions(+) create mode 100644 src/modeler/AutoRest.Swagger/Validation/BooleanPropertyNotRecommended.cs diff --git a/src/core/AutoRest.Core/Properties/Resources.Designer.cs b/src/core/AutoRest.Core/Properties/Resources.Designer.cs index 6ac11daaf8..d284f0529b 100644 --- a/src/core/AutoRest.Core/Properties/Resources.Designer.cs +++ b/src/core/AutoRest.Core/Properties/Resources.Designer.cs @@ -95,6 +95,15 @@ public static string BodyWithType { } } + /// + /// Looks up a localized string similar to Booleans are not descriptive and make them hard to use. Instead use string enums with allowed set of values defined: '{0}'.. + /// + public static string BooleanPropertyNotRecommended { + get { + return ResourceManager.GetString("BooleanPropertyNotRecommended", resourceCulture); + } + } + /// /// Looks up a localized string similar to Errors found during Swagger document validation.. /// diff --git a/src/core/AutoRest.Core/Properties/Resources.resx b/src/core/AutoRest.Core/Properties/Resources.resx index 607c5edf10..4e557e204f 100644 --- a/src/core/AutoRest.Core/Properties/Resources.resx +++ b/src/core/AutoRest.Core/Properties/Resources.resx @@ -285,4 +285,7 @@ When property is modeled as "readOnly": true then x-ms-mutability extension can only have "read" value. When property is modeled as "readOnly": false then applying x-ms-mutability extension with only "read" value is not allowed. Extension contains invalid values: '{0}'. + + Booleans are not descriptive and make them hard to use. Instead use string enums with allowed set of values defined: '{0}'. + \ No newline at end of file diff --git a/src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs b/src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs index 80f183e5a0..6b6c4d0a87 100644 --- a/src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs +++ b/src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs @@ -91,6 +91,7 @@ public ServiceDefinition() /// /// Key is the object serviceTypeName and the value is swagger definition. /// + [Rule(typeof(BooleanPropertyNotRecommended))] public Dictionary Definitions { get; set; } /// diff --git a/src/modeler/AutoRest.Swagger/Validation/BooleanPropertyNotRecommended.cs b/src/modeler/AutoRest.Swagger/Validation/BooleanPropertyNotRecommended.cs new file mode 100644 index 0000000000..8be6e94195 --- /dev/null +++ b/src/modeler/AutoRest.Swagger/Validation/BooleanPropertyNotRecommended.cs @@ -0,0 +1,58 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using AutoRest.Core.Logging; +using AutoRest.Core.Properties; +using AutoRest.Core.Validation; +using System.Collections.Generic; +using AutoRest.Swagger.Model; + +namespace AutoRest.Swagger.Validation +{ + /// + /// Flags properties of boolean type as they are not recommended, unless it's the only option. + /// + public class BooleanPropertyNotRecommended : TypedRule> + { + /// + /// The template message for this Rule. + /// + /// + /// This may contain placeholders '{0}' for parameterized messages. + /// + public override string MessageTemplate => Resources.BooleanPropertyNotRecommended; + + /// + /// The severity of this message (ie, debug/info/warning/error/fatal, etc) + /// + public override Category Severity => Category.Warning; + + /// + /// Validates whether properties of type boolean exist. + /// + /// Operation Definition to validate + /// true if there are propeties of type boolean, false otherwise. + public override bool IsValid(Dictionary definitions, RuleContext context, out object[] formatParameters) + { + formatParameters = null; + List booleanProperties = new List(); + foreach (string key in definitions.Keys) + { + Schema definitionSchema = definitions.GetValueOrNull(key); + if (definitionSchema.Properties != null) + { + foreach (var property in definitionSchema.Properties) + { + if (property.Value.Type.ToString().ToLower().Equals("boolean")) + { + booleanProperties.Add(key + "/" + property.Key); + + } + } + } + } + formatParameters = new[] { string.Join(", ", booleanProperties.ToArray()) }; + return (booleanProperties.Count == 0); + } + } +} \ No newline at end of file From 1361647cfd278468dd26a1ecf701f72cc5913884 Mon Sep 17 00:00:00 2001 From: Veronica Giaudrone Date: Wed, 8 Feb 2017 15:09:08 -0800 Subject: [PATCH 2/2] Adding test case for rule --- .../Validation/boolean-properties.json | 61 +++++++++++++++++++ .../SwaggerModelerValidationTests.cs | 8 +++ 2 files changed, 69 insertions(+) create mode 100644 src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/boolean-properties.json diff --git a/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/boolean-properties.json b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/boolean-properties.json new file mode 100644 index 0000000000..d480dd9fc5 --- /dev/null +++ b/src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/boolean-properties.json @@ -0,0 +1,61 @@ +{ + "swagger": "2.0", + "info": { + "title": "Boolean properties not recommended in models", + "description": "Some documentation.", + "version": "2017-02-08" + }, + "host": "management.azure.com", + "schemes": [ + "https" + ], + "basePath": "/", + "produces": [ + "application/json" + ], + "consumes": [ + "application/json" + ], + "paths": { + "/foo": { + "post": { + "operationId": "PostFoo", + "summary": "Foo path", + "description": "Foo operation", + "responses": { + "default": { + "description": "Unexpected error" + } + } + } + } + }, + "definitions": { + "Test1": { + "description": "Property for foo path 1", + "properties": { + "nameAvailable": { + "readOnly": true, + "type": "boolean", + "description": "Gets a boolean value that indicates whether the name is available for you to use. If true, the name is available. If false, the name has already been taken or invalid and cannot be used." + } + } + }, + "parameters": { + "SubscriptionIdParameter": { + "name": "subscriptionId", + "in": "path", + "required": true, + "type": "string", + "description": "test subscription id" + }, + "ApiVersion": { + "name": "api-version", + "in": "path", + "required": true, + "type": "string", + "description": "test api version" + } + } + } +} \ No newline at end of file diff --git a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs index ea01776566..0461a098e5 100644 --- a/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs +++ b/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs @@ -72,6 +72,14 @@ public void AvoidMsdnReferencesValidation() messages.AssertOnlyValidationMessage(typeof(AvoidMsdnReferences), 4); } + [Fact] + public void BooleanPropertiesValidation() + { + var messages = ValidateSwagger(Path.Combine("Swagger", "Validation", "boolean-properties.json")); + + messages.AssertOnlyValidationMessage(typeof(BooleanPropertyNotRecommended)); + } + [Fact] public void DefaultValueInEnumValidation() {