Adding Swagger file linting infrastructure and rules

Documentation/README.md

@@ -19,6 +19,7 @@
     - Modelers
 5. [Building AutoRest](building-code.md)
 6. [Writing Tests](writing-tests.md)
+6. [Writing Swagger Validation Rules](writing-validation-rules.md)
 7. Contributing to the code
 
 [Swagger2.0]:https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md

Documentation/writing-validation-rules.md

@@ -0,0 +1,49 @@
+# Writing Swagger Validation Rules
+
+## Architecture
+In the AutoRest pipeline, Swagger files get deserialized into intermediate classes, which are then used to create the language-independent client model. Our validation logic is performed on these deserialization classes to allow logic written in C# to be used to check the object representation of the Swagger spec.
+
+The root Swagger spec is deserialized into a [`ServiceDefinition`](../src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs) object. The validation step recursively traverses this object tree and applies the validation rules that apply to each property and consolidate the messages from all rules. Validation rules are associated with a property by decorating it with a `RuleAttribute`. This `RuleAttribute` will be passed the value for that property and determines if that value satisfies the rule or not. Multiple `RuleAttribute` attributes can be applied to the same property, and any rules that fail will be part of the output.
+
+## Steps for writing a rule (see [instructions below](#instructions))
+1. Define a canonical name that represents the rule and a message that should be shown to the user explaining the validation failure
+2. Determine if your rule is an `Info`, a `Warning` or an `Error`
+3. Implement the logic that validates this rule against a given object
+4. Define where this validation rule gets applied in the object tree
+5. Write a test that verifies that this rule correctly validates Swagger specs
+
+## Instructions
+### 1. Add the rule name and message
+- The name of your validation rule should be added to the end of the `ValidationExceptionName` enum
+- Messages are added to the [`AutoRest.Core.Properties.Resource` resx](../src/core/AutoRest.Core/Properties/Resources.resx).
+
+### 2. Specify the severity of your validation rule
+- Add a mapping that associates your message with the rule name in [`ValidationExceptionConstants`](../src/core/AutoRest.Core/Validation/ValidationExceptionConstants.cs) in either the `Info`, `Warning` or `Error` sections.
+
+### 3. Add a `Rule` subclass that implements your validation rule logic
+- Create a subclass of the `Rule` class, and override the `bool IsValid(object entity)` method.
+- For more complex rules (including getting type information in `IsValid()`, see the [Complex rules](#complex-rules) section below.
+
+### 4. Decorate the appropriate Swagger model property that your rule applies to
+- Add a `[Rule(typeof({MyRule})]` attribute above the property that should satisfy the rule. Replace `{MyRule}` with the subclass that you implemented.
+- The `typeof()` is necessary because C# doesn't support generics in attributes.
+
+### 5. Add a test to `SwaggerModelerValidationTests` that validates your validation rule 
+- Add an incorrect Swagger file to the [`Swagger/Validation/`](../src/modeler/AutoRest.Swagger.Tests/Swagger/Validation) folder that should trigger your validation rule.
+- Add a test case to [`SwaggerModelerValidationTests.cs`](../src/modeler/AutoRest.Swagger.Tests/SwaggerModelerValidationTests.cs) that asserts that the validation message returned for the Swagger file is  
+
+## Complex rules
+### Typed rules
+The `IsValid()` method of the `Rule` class only passes an object with no type information. You can have your rule subclass work on a typed model class by inheriting from the `TypedRule<T>` class instead. By replacing `T` with a model class, your override of `IsValid()` will use `T` as the type for the `entity` parameter.
+
+### Message interpolation (e.g. `"'foo' is not a valid MIME type for consumes"`)
+Simple rules can simply override the `bool IsValid(object entity)` method when subclassing `Rule` and return true or false, depending on if the object satisfies the rule. However, some messages are more useful if they provide the incorrect value as part of the message.
+
+Rules can override a different `IsValid` overload (`IsValid(object enitity, out object[] formatParameters)`. Any objects returned in `formatParameters` will be passed on to `string.Format()` along with the message associated with the rule. When writing the message, use standard `string.Format()` conventions to define where replacements go (e.g. `"'{0}' is not a valid MIME type for consumes"`).
+
+### Collection rules
+Sometimes, a rule should apply to every item in a list or dictionary, but it cannot be applied to the class definition (since the same class can be used in multiple places in the `ServiceDefinition` tree).
+
+An example of this is the `AnonymousTypesDiscouraged` rule. The purpose of this rule is to have schemas defined in the `definitions` section of the Swagger file instead of in the parameter that it will be used for. It validates the `Schema` class, but it cannot be applied to all instances of this class, because the `definitions` section also uses the `Schema` class.
+
+Since we want to apply this rule to parameters in an operation, we can decorate the `Parameters` property of the [`OperationResponse`](../src/modeler/AutoRest.Swagger/Model/Operation.cs) class with the `CollectionRule` attribute. When the object tree is traversed to apply validation rules, each item in the collection will be validated against the `AnonymousParameterTypes` logic.

src/core/AutoRest.Core/AutoRest.cs

@@ -7,6 +7,9 @@
 using AutoRest.Core.Extensibility;
 using AutoRest.Core.Logging;
 using AutoRest.Core.Properties;
+using AutoRest.Core.Validation;
+using System.Collections.Generic;
+using System.Linq;
 
 namespace AutoRest.Core
 {
@@ -22,7 +25,7 @@ public static string Version
         {
             get
             {
-                FileVersionInfo fvi = FileVersionInfo.GetVersionInfo((typeof (Settings)).Assembly.Location);
+                FileVersionInfo fvi = FileVersionInfo.GetVersionInfo((typeof(Settings)).Assembly.Location);
                 return fvi.FileVersion;
             }
         }
@@ -40,10 +43,22 @@ public static void Generate(Settings settings)
             Logger.Entries.Clear();
             Logger.LogInfo(Resources.AutoRestCore, Version);
             Modeler modeler = ExtensionsLoader.GetModeler(settings);
-            ServiceClient serviceClient;
+            ServiceClient serviceClient = null;
+
             try
             {
-                serviceClient = modeler.Build();
+                IEnumerable<ValidationMessage> messages = new List<ValidationMessage>();
+                serviceClient = modeler.Build(out messages);
+
+                foreach (var message in messages)
+                {
+                    Logger.Entries.Add(new LogEntry(message.Severity, message.ToString()));
+                }
+
+                if (messages.Any(entry => entry.Severity >= settings.ValidationLevel))
+                {
+                    throw ErrorManager.CreateError(Resources.CodeGenerationError);
+                }
             }
             catch (Exception exception)
             {

src/core/AutoRest.Core/Extensibility/ExtensionsLoader.cs

@@ -38,32 +38,39 @@ public static CodeGenerator GetCodeGenerator(Settings settings)
             if (string.IsNullOrEmpty(settings.CodeGenerator))
             {
                 throw new ArgumentException(
-                    string.Format(CultureInfo.InvariantCulture, 
+                    string.Format(CultureInfo.InvariantCulture,
                         Resources.ParameterValueIsMissing, "CodeGenerator"));
             }
 
             CodeGenerator codeGenerator = null;
 
-            string configurationFile = GetConfigurationFileContent(settings);
-
-            if (configurationFile != null)
+            if (string.Equals("None", settings.CodeGenerator, StringComparison.OrdinalIgnoreCase))
             {
-                try
+                codeGenerator = new NoOpCodeGenerator(settings);
+            }
+            else
+            {
+                string configurationFile = GetConfigurationFileContent(settings);
+
+                if (configurationFile != null)
                 {
-                    var config = JsonConvert.DeserializeObject<AutoRestConfiguration>(configurationFile);
-                    codeGenerator = LoadTypeFromAssembly<CodeGenerator>(config.CodeGenerators, settings.CodeGenerator,
-                        settings);
-                    codeGenerator.PopulateSettings(settings.CustomSettings);
+                    try
+                    {
+                        var config = JsonConvert.DeserializeObject<AutoRestConfiguration>(configurationFile);
+                        codeGenerator = LoadTypeFromAssembly<CodeGenerator>(config.CodeGenerators, settings.CodeGenerator,
+                            settings);
+                        codeGenerator.PopulateSettings(settings.CustomSettings);
+                    }
+                    catch (Exception ex)
+                    {
+                        throw ErrorManager.CreateError(ex, Resources.ErrorParsingConfig);
+                    }
                 }
-                catch (Exception ex)
+                else
                 {
-                    throw ErrorManager.CreateError(ex, Resources.ErrorParsingConfig);
+                    throw ErrorManager.CreateError(Resources.ConfigurationFileNotFound);
                 }
             }
-            else
-            {
-                throw ErrorManager.CreateError(Resources.ConfigurationFileNotFound);
-            }
             Logger.LogInfo(Resources.GeneratorInitialized,
                 settings.CodeGenerator,
                 codeGenerator.GetType().Assembly.GetName().Version);
@@ -137,7 +144,7 @@ public static string GetConfigurationFileContent(Settings settings)
 
             if (!settings.FileSystem.FileExists(path))
             {
-                path = Path.Combine(Path.GetDirectoryName(Assembly.GetAssembly(typeof (Settings)).Location),
+                path = Path.Combine(Path.GetDirectoryName(Assembly.GetAssembly(typeof(Settings)).Location),
                     ConfigurationFileName);
             }
 
@@ -177,10 +184,10 @@ public static T LoadTypeFromAssembly<T>(IDictionary<string, AutoRestProviderConf
                     {
                         loadedAssembly = Assembly.Load(assemblyName);
                     }
-                    catch(FileNotFoundException)
+                    catch (FileNotFoundException)
                     {
                         loadedAssembly = Assembly.LoadFrom(assemblyName + ".dll");
-                        if(loadedAssembly == null)
+                        if (loadedAssembly == null)
                         {
                             throw;
                         }
@@ -191,8 +198,8 @@ public static T LoadTypeFromAssembly<T>(IDictionary<string, AutoRestProviderConf
                                      t.Name == typeName ||
                                      t.FullName == typeName);
 
-                    instance = (T) loadedType.GetConstructor(
-                        new[] {typeof (Settings)}).Invoke(new object[] {settings});
+                    instance = (T)loadedType.GetConstructor(
+                        new[] { typeof(Settings) }).Invoke(new object[] { settings });
 
                     if (!section[key].Settings.IsNullOrEmpty())
                     {

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

@@ -0,0 +1,35 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+
+namespace AutoRest.Core.Logging
+{
+    public static class LogEntrySeverityConsoleColor
+    {
+        private static IDictionary<LogEntrySeverity, ConsoleColor> _dict = new Dictionary<LogEntrySeverity, ConsoleColor>
+        {
+            { LogEntrySeverity.Fatal, ConsoleColor.Red },
+            { LogEntrySeverity.Error, ConsoleColor.Red },
+            { LogEntrySeverity.Warning, ConsoleColor.Yellow },
+            { LogEntrySeverity.Info, ConsoleColor.White },
+        };
+
+        /// <summary>
+        /// Get the console color associated with the severity of the message
+        /// </summary>
+        /// <param name="severity">Severity of the log message.</param>
+        /// <returns>The color to set the console for messages of this severity</returns>
+        public static ConsoleColor GetColorForSeverity(this LogEntrySeverity severity)
+        {
+            ConsoleColor color;
+            if (!_dict.TryGetValue(severity, out color))
+            {
+                throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, "No color defined for severity {0}", severity));
+            }
+            return color;
+        }
+    }
+}

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

@@ -90,68 +90,28 @@ public static void LogError(string message, params object[] args)
             LogError(null, message, args);
         }
 
-        /// <summary>
-        /// Writes the LogEntry collection to the provided TextWriter.
-        /// </summary>
-        /// <param name="writer">TextWriter for output.</param>
-        /// <param name="verbose">If set to true, output includes full exception stack.</param>
-        public static void WriteErrors(TextWriter writer, bool verbose)
+        public static void WriteMessages(TextWriter writer, LogEntrySeverity severity)
         {
-            if (writer == null)
-            {
-                throw new ArgumentNullException("writer");
-            }
-            foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Error ||
-                                                        e.Severity == LogEntrySeverity.Fatal)
-                .OrderByDescending(e => e.Severity))
-            {
-                string prefix = "";
-                if (logEntry.Severity == LogEntrySeverity.Fatal)
-                {
-                    prefix = "[FATAL] ";
-                }
-                writer.WriteLine("error: {0}{1}", prefix, logEntry.Message);
-                if (logEntry.Exception != null && verbose)
-                {
-                    writer.WriteLine("{0}", logEntry.Exception);
-                }
-            }
+            WriteMessages(writer, severity, false);
         }
 
-        /// <summary>
-        /// Writes the LogEntrySeverity.Warning messages to the provided TextWriter. 
-        /// </summary>
-        /// <param name="writer">TextWriter for output.</param>
-        public static void WriteWarnings(TextWriter writer)
+        public static void WriteMessages(TextWriter writer, LogEntrySeverity severity, bool verbose)
         {
             if (writer == null)
             {
                 throw new ArgumentNullException("writer");
             }
-            foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Warning))
+            foreach (var logEntry in Entries.Where(e => e.Severity == severity))
             {
-                writer.WriteLine("{0}: {1}", 
-                    logEntry.Severity.ToString().ToUpperInvariant(), 
-                    logEntry.Message);
-            }
-        }
-
-        /// <summary>
-        /// Writes the LogEntrySeverity.Info messages to the provdied TextWriter.
-        /// </summary>
-        /// <param name="writer">TextWriter for output.</param>
-        public static void WriteInfos(TextWriter writer)
-        {
-            if (writer == null)
-            {
-                throw new ArgumentNullException("writer");
-            }
-
-            foreach (var logEntry in Entries.Where(e => e.Severity == LogEntrySeverity.Info))
-            {
-                writer.WriteLine("{0}: {1}", 
-                    logEntry.Severity.ToString().ToUpperInvariant(), 
+                // Write the severity and message to console
+                writer.WriteLine("{0}: {1}",
+                    logEntry.Severity.ToString().ToUpperInvariant(),
                     logEntry.Message);
+                // If verbose is on and the entry has an exception, show it
+                if (logEntry.Exception != null && verbose)
+                {
+                    writer.WriteLine("{0}", logEntry.Exception);
+                }
             }
         }
     }

src/core/AutoRest.Core/Modeler.cs

@@ -2,6 +2,8 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using AutoRest.Core.ClientModel;
+using AutoRest.Core.Validation;
+using System.Collections.Generic;
 
 namespace AutoRest.Core
 {
@@ -17,5 +19,7 @@ protected Modeler(Settings settings)
         }
 
         public abstract ServiceClient Build();
+
+        public abstract ServiceClient Build(out IEnumerable<ValidationMessage> messages);
     }
 }

src/core/AutoRest.Core/NoOpCodeGenerator.cs

@@ -0,0 +1,56 @@
+// 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.ClientModel;
+using System.Threading.Tasks;
+
+namespace AutoRest.Core
+{
+    public class NoOpCodeGenerator: CodeGenerator
+    {
+        public NoOpCodeGenerator(Settings settings) : base(settings)
+        {
+        }
+
+        public override string Description
+        {
+            get
+            {
+                return "No op code generator";
+            }
+        }
+
+        public override string ImplementationFileExtension
+        {
+            get
+            {
+                return string.Empty;
+            }
+        }
+
+        public override string Name
+        {
+            get
+            {
+                return "No op code generator";
+            }
+        }
+
+        public override string UsageInstructions
+        {
+            get
+            {
+                return string.Empty;
+            }
+        }
+
+        public override Task Generate(ServiceClient serviceClient)
+        {
+            return Task.FromResult(0);
+        }
+
+        public override void NormalizeClientModel(ServiceClient serviceClient)
+        {
+        }
+    }
+}