Adding "clean" Swagger validation test (#1254)

* Fixing minor validation errors

- Fixing description rule to allow references to define a description
- Fixing AutoRest logging output to avoid throwing NRE when an error is logged without an exception
- Changing the message that gets output for validation errors to make it more useful

* Adding a validation test that validates a clean spec

- This verifies that rules are not unintentionally being returned for specs that don't have the issue. If a rule is added that causes this test to fail, this spec should be updated so that it passes those rules (and verify that the rule isn't being triggered inadvertently)

src/core/AutoRest.Core/AutoRest.cs

@@ -57,7 +57,7 @@ public static void Generate(Settings settings)
 
                 if (messages.Any(entry => entry.Severity >= settings.ValidationLevel))
                 {
-                    throw ErrorManager.CreateError(Resources.CodeGenerationError);
+                    throw ErrorManager.CreateError(null, Resources.ErrorGeneratingClientModel, "Errors found during Swagger validation");
                 }
             }
             catch (Exception exception)

src/core/AutoRest.Core/Logging/ErrorManager.cs

@@ -30,7 +30,7 @@ public static CodeGenerationException CreateError(Exception exception, string me
             }
 
             var errors =
-                Logger.Entries.Where(e => e.Severity == LogEntrySeverity.Error).Select(e => e.Exception).ToList();
+                Logger.Entries.Where(e => e.Severity == LogEntrySeverity.Error).Select(e => e.Exception).Where(e => e != null).ToList();
             Logger.Entries.Add(new LogEntry(LogEntrySeverity.Fatal, FormatMessageString(message, args))
             {
                 Exception = exception

src/modeler/AutoRest.Swagger.Tests/Swagger/Validation/clean-complex-spec.json

@@ -0,0 +1,206 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "version": "1.0.0",
+    "title": "Swagger Petstore",
+    "license": {
+      "name": "MIT"
+    }
+  },
+  "host": "petstore.swagger.io",
+  "basePath": "/v1",
+  "schemes": [
+    "http"
+  ],
+  "consumes": [
+    "application/json"
+  ],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/pets": {
+      "get": {
+        "summary": "List all pets",
+        "operationId": "listPets",
+        "tags": [
+          "pets"
+        ],
+        "parameters": [
+          {
+            "name": "limit",
+            "in": "query",
+            "description": "How many items to return at one time (max 100)",
+            "required": false,
+            "type": "integer",
+            "format": "int32"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "An paged array of pets",
+            "headers": {
+              "x-next": {
+                "type": "string",
+                "description": "A link to the next page of responses"
+              }
+            },
+            "schema": {
+              "$ref": "#/definitions/Pets"
+            }
+          },
+          "default": {
+            "description": "unexpected error",
+            "schema": {
+              "$ref": "#/definitions/Error"
+            }
+          }
+        }
+      },
+      "post": {
+        "summary": "Create a pet",
+        "operationId": "createPets",
+        "tags": [
+          "pets"
+        ],
+        "responses": {
+          "201": {
+            "description": "Null response"
+          },
+          "default": {
+            "description": "unexpected error",
+            "schema": {
+              "$ref": "#/definitions/Error"
+            }
+          }
+        }
+      }
+    },
+    "/pets/{petId}": {
+      "get": {
+        "summary": "Info for a specific pet",
+        "operationId": "showPetById",
+        "tags": [
+          "pets"
+        ],
+        "parameters": [
+          {
+            "name": "petId",
+            "in": "path",
+            "required": true,
+            "description": "The id of the pet to retrieve",
+            "type": "string"
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Expected response to a valid request",
+            "schema": {
+              "$ref": "#/definitions/Pets"
+            }
+          },
+          "default": {
+            "description": "unexpected error",
+            "schema": {
+              "$ref": "#/definitions/Error"
+            }
+          }
+        }
+      }
+    },
+    "/foo": {
+      "get": {
+        "operationId": "Foo_Get",
+        "responses": {
+          "default": {
+            "$ref": "#/responses/FooResponse"
+          }
+        }
+      },
+      "post": {
+        "operationId": "Foo_Post",
+        "parameters": [
+          {
+            "in": "body",
+            "name": "fooPost",
+            "schema": {
+              "type": "object",
+              "description": "A foo object"
+            },
+            "description": "Foo body parameter"
+          },
+          {
+            "$ref": "#/parameters/FooQueryParam"
+          }
+        ],
+        "responses": {
+          "default": {
+            "$ref": "#/responses/FooResponse"
+          }
+        }
+      }
+    }
+  },
+  "parameters": {
+    "FooQueryParam": {
+      "in": "query",
+      "name": "FooQueryParam",
+      "description": "Query parameter for Foo operation",
+      "type": "string"
+    }
+  },
+  "responses": {
+    "FooResponse": {
+      "description": "Response for Foo"
+    }
+  },
+  "definitions": {
+    "Pet": {
+      "required": [
+        "id",
+        "name"
+      ],
+      "properties": {
+        "id": {
+          "type": "integer",
+          "format": "int64",
+          "description": "The pet id"
+        },
+        "name": {
+          "type": "string",
+          "description": "The pet name"
+        },
+        "tag": {
+          "type": "string",
+          "description": "The pet tag"
+        }
+      },
+      "description": "A pet"
+    },
+    "Pets": {
+      "type": "array",
+      "items": {
+        "$ref": "#/definitions/Pet"
+      },
+      "description": "A set of pets"
+    },
+    "Error": {
+      "required": [
+        "code",
+        "message"
+      ],
+      "properties": {
+        "code": {
+          "type": "integer",
+          "format": "int32",
+          "description": "The code of the error"
+        },
+        "message": {
+          "type": "string",
+          "description": "The message of the error"
+        }
+      },
+      "description": "An error result"
+    }
+  }
+}

src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs

@@ -40,6 +40,16 @@ private IEnumerable<ValidationMessage> ValidateSwagger(string input)
             return messages;
         }
 
+        /// <summary>
+        /// Verifies that a clean Swagger file does not result in any validation errors
+        /// </summary>
+        [Fact]
+        public void CleanFileValidation()
+        {
+            var messages = ValidateSwagger(Path.Combine("Swagger", "Validation", "clean-complex-spec.json"));
+            Assert.Empty(messages.Where(m => m.Severity >= LogEntrySeverity.Warning));
+        }
+
         [Fact]
         public void MissingDescriptionValidation()
         {

src/modeler/AutoRest.Swagger/Validation/DescriptionRequired.cs

@@ -14,7 +14,7 @@ public class DescriptionRequired : TypedRule<SwaggerObject>
         /// <param name="entity"></param>
         /// <returns></returns>
         public override bool IsValid(SwaggerObject entity)
-            => entity == null || entity.Description != null || string.IsNullOrEmpty(entity.Reference);
+            => entity == null || entity.Description != null || !string.IsNullOrEmpty(entity.Reference);
 
         public override ValidationExceptionName Exception => ValidationExceptionName.DescriptionRequired;
     }