Improve some rules

changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## What's New (07//2021)
+
+### Changed validation rules
+- DescriptionAndTitleMissing  - description & title are not required for reference properties. 
+- PathResourceProviderMatchNamespace  - fix the false alarm when the provider namespace in swagger file path is null.
+- TopLevelResourcesListBySubscription - fix false positive caused by unused model.
 ## What's New (06/22/2021)
 
 ### Changed validation rules

src/dotnet/AutoRest/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@microsoft.azure/classic-openapi-validator",
-  "version": "1.1.9",
+  "version": "1.1.10",
   "description": "The classic OpenAPI validator plugin for AutoRest",
   "scripts": {
     "start": "dotnet ./bin/netcoreapp2.0/AutoRest.dll --server"

src/dotnet/OpenAPI.Validator.Tests/OpenAPIModelerValidationTests.cs

@@ -85,11 +85,11 @@ public void EmptyClientNameValidation()
         public void PathResourceProviderMatchNamespaceValidation()
         {
             var messages = GetValidationMessagesForRule<PathResourceProviderMatchNamespace>("network-interfaces-api.json");
-            Assert.Equal(messages.Count(), 1);
+            Assert.Equal(messages.Count(), 0);
 
             // resource provider in path doesn't match with namespace.
             messages = GetValidationMessagesForRule<PathResourceProviderMatchNamespace>("resource-manager/Microsoft.Network/network-interface-invalid.json");
-            Assert.Equal(messages.Count(), 0);
+            Assert.Equal(messages.Count(), 1);
         }
 
         [Fact]

src/dotnet/OpenAPI.Validator/Model/Utilities/ValidationUtilities.cs

@@ -20,7 +20,7 @@ public static class ValidationUtilities
         private static readonly Regex ResourceProviderPathPattern = new Regex(@"/providers/(?<resPath>[^{/]+)/", RegexOptions.IgnoreCase);
         private static readonly Regex PropNameRegEx = new Regex(@"^[a-z0-9\$-]+([A-Z]{1,3}[a-z0-9\$-]+)+$|^[a-z0-9\$-]+$|^[a-z0-9\$-]+([A-Z]{1,3}[a-z0-9\$-]+)*[A-Z]{1,3}$");
 
-        private static readonly Regex ResourceProviderNamespaceRegex = new Regex(@"(resource-manager|data-plane)/(?<namespace>[\w\.]+)/");
+        private static readonly Regex ResourceProviderNamespaceRegex = new Regex(@"(resource-manager|data-plane)/(?<namespace>[\w\.]+)/(?<potentialNamespace>[\w\.]+)");
 
         public static readonly Regex ListBySidRegEx = new Regex(@".+_(List|ListBySubscriptionId|ListBySubscription|ListBySubscriptions)$", RegexOptions.IgnoreCase);
         public static readonly Regex ListByRgRegEx = new Regex(@".+_ListByResourceGroup$", RegexOptions.IgnoreCase);
@@ -475,6 +475,23 @@ public static IEnumerable<string> GetResourceProviders(Dictionary<string, Dictio
             return resourceProviders;
         }
 
+        /// <summary>
+        /// Returns array of resource providers
+        /// </summary>
+        /// <param name="path"> path to look for</param>
+        /// <returns>Array of resource providers</returns>
+        public static IEnumerable<string> GetResourceProvidersByPath(string path)
+        {
+            IEnumerable<string> resourceProviders =  ResourceProviderPathPattern.Matches(path)
+                                                    .OfType<Match>()
+                                                    .Select(match => match.Groups["resPath"].Value.ToString())
+                                                    .Where(res => res != null)
+                                                    .Distinct()
+                                                    .ToList();
+
+            return resourceProviders;
+        }
+
         /// <summary>
         /// Return namespace from swagger file path.
         /// </summary>
@@ -487,7 +504,17 @@ public static string GetRPNamespaceFromFilePath(string path)
             if (match.Success)
             {
                 string key = match.Groups[2].Value;
-                return key;
+                // match , if path like resource-manager/Microsoft.Fabric/preview/2016-05-01/
+                if (key.StartsWith("Microsoft.",StringComparison.OrdinalIgnoreCase))
+                {
+                    return key;
+                }
+                // match , if path like resource-manager/fabric/Microsoft.Fabric.Admin/preview/2016-05-01/
+                key = match.Groups[3].Value;
+                if (key.StartsWith("Microsoft.", StringComparison.OrdinalIgnoreCase))
+                {
+                    return key;
+                }
             }
             return "";
         }

src/dotnet/OpenAPI.Validator/Validation/DescriptionAndTitleMissing.cs

@@ -43,7 +43,7 @@ public override IEnumerable<ValidationMessage> GetValidationMessages(Dictionary<
         {
             foreach (KeyValuePair<string, Schema> definition in definitions)
             {
-                if (string.IsNullOrWhiteSpace(definition.Value.Description) && string.IsNullOrWhiteSpace(definition.Value.Title))
+                if (string.IsNullOrWhiteSpace(definition.Value.Reference) && string.IsNullOrWhiteSpace(definition.Value.Description) && string.IsNullOrWhiteSpace(definition.Value.Title))
                 {
                     yield return new ValidationMessage(new FileObjectPath(context.File, context.Path.AppendProperty(definition.Key)), this, string.Format(ModelTypeFormatter, definition.Key));
                 }

src/dotnet/OpenAPI.Validator/Validation/PathResourceProvderMatchNamespace.cs

@@ -51,13 +51,19 @@ public class PathResourceProviderMatchNamespace : TypedRule<Dictionary<string, D
         /// </summary>
         /// <param name="paths"></param>
         /// <returns></returns>
-        public override bool IsValid(Dictionary<string, Dictionary<string, Operation>> paths, RuleContext context, out object[] formatParameters)
+        public override IEnumerable<ValidationMessage> GetValidationMessages(Dictionary<string, Dictionary<string, Operation>> paths, RuleContext context)
         {
-            IEnumerable<string> resourceProviders = ValidationUtilities.GetResourceProviders(paths);
             string resourceProviderNamespace = ValidationUtilities.GetRPNamespaceFromFilePath(context.File.ToString());
-            formatParameters = new[] { string.Join(", ", resourceProviders) };
-            string lastResourceProvider = resourceProviders?.ToList().Count() > 0 ? resourceProviders.Last() : null;
-            return resourceProviders.ToList().Count <=1 || lastResourceProvider == resourceProviderNamespace;
+            foreach (var pair in paths) {
+                IEnumerable<string> resourceProviders = ValidationUtilities.GetResourceProvidersByPath(pair.Key);
+                var formatParameters = new[] { string.Join(", ", resourceProviders) };
+                string lastResourceProvider = resourceProviders?.ToList().Count() > 0 ? resourceProviders.Last() : null;
+                if (resourceProviderNamespace != "" && resourceProviders.ToList().Count > 0 && lastResourceProvider != resourceProviderNamespace)
+                {
+                    yield return new ValidationMessage(new FileObjectPath(context.File, context.Path.AppendProperty(pair.Key)), this, formatParameters);
+                }
+            }
+
         }
     }
 }

src/typescript/azure-openapi-validator/rules/utilities/resourceUtils.ts

@@ -132,7 +132,6 @@ export class ResourceUtils {
     }
   }
 
-
   /**
    *  Get all resources which allOf a x-ms-resource
    */
@@ -158,7 +157,11 @@ export class ResourceUtils {
   }
 
   public getAllResourceNames() {
-    const modelNames = [...this.getSpecificOperationModels("get", "200").keys(),...this.getSpecificOperationModels("put", "200").keys(), ...this.getSpecificOperationModels("put", "201").keys()]
+    const modelNames = [
+      ...this.getSpecificOperationModels("get", "200").keys(),
+      ...this.getSpecificOperationModels("put", "200").keys(),
+      ...this.getSpecificOperationModels("put", "201").keys()
+    ]
     return modelNames.filter(m => this.checkResource(m))
   }
 
@@ -228,14 +231,13 @@ export class ResourceUtils {
     const resources = new Set<string>()
     for (const entry of fullModels.entries()) {
       const paths = entry[1]
-      paths
-        .some(p => {
-          const hierarchy = this.getResourcesTypeHierarchy(p)
-          if (hierarchy.length > 0) {
-            resources.add(entry[0])
-            return true
-          }
-        })
+      paths.some(p => {
+        const hierarchy = this.getResourcesTypeHierarchy(p)
+        if (hierarchy.length > 0) {
+          resources.add(entry[0])
+          return true
+        }
+      })
     }
     return resources.values()
   }
@@ -294,21 +296,20 @@ export class ResourceUtils {
     return hierarchy
   }
 
-  private dereference(ref: string, visited:Set<string>) {
+  private dereference(ref: string, visited: Set<string>) {
     if (visited.has(ref)) {
       return undefined
     }
     visited.add(ref)
     const model = this.getResourceByName(this.stripDefinitionPath(ref))
     if (model && model.$ref) {
       return this.dereference(model.$ref, visited)
-    }
-    else {
+    } else {
       return model
     }
   }
 
-  private getUnwrappedModel(property:any) {
+  private getUnwrappedModel(property: any) {
     if (property) {
       const ref = property.$ref
       return ref ? this.dereference(ref, new Set<string>()) : property
@@ -359,7 +360,9 @@ export class ResourceUtils {
       }
       modelEntry[1].forEach(path => {
         if (path.match(this.SpecificResourcePathRegEx)) {
-          const possibleCollectionApiPath = path.substr(0, path.lastIndexOf("/{"))
+          const firstProviderIndex = path.lastIndexOf("/providers")
+          const lastIndex = path.lastIndexOf("/{")
+          const possibleCollectionApiPath = path.substr(firstProviderIndex, lastIndex - firstProviderIndex)
           /*
           * case 1:"providers/Microsoft.Compute/virtualMachineScaleSets/{virtualMachineScaleSetName}/virtualMachines"
             case 2: "providers/Microsoft.Compute/virtualMachineScaleSets/{vmScaleSetName}/virtualMachines":
@@ -369,19 +372,19 @@ export class ResourceUtils {
             /**
              * path may end with "/", so here we remove it
              */
-            p => p.replace(/{[^\/]+}/gi, "{}").replace(/\/$/gi, "") === possibleCollectionApiPath.replace(/{[^\/]+}/gi, "{}")
+            p =>
+              p
+                .substr(p.lastIndexOf("/providers"))
+                .replace(/{[^\/]+}/gi, "{}")
+                .replace(/\/$/gi, "") === possibleCollectionApiPath.replace(/{[^\/]+}/gi, "{}")
           )
           if (matchedPaths && matchedPaths.length >= 1) {
-            matchedPaths.forEach(m =>
-              this.getModelFromPath(m)
-                ? collectionApis.push({
-                    specificGetPath: [path],
-                    collectionGetPath: [possibleCollectionApiPath],
-                    modelName: this.getModelFromPath(m),
-                    childModelName: modelEntry[0]
-                  })
-                : false
-            )
+            collectionApis.push({
+              specificGetPath: [path],
+              collectionGetPath: matchedPaths,
+              modelName: this.getModelFromPath(matchedPaths[0]),
+              childModelName: modelEntry[0]
+            })
           }
         }
       })
@@ -402,8 +405,6 @@ export class ResourceUtils {
         }
         if (index === -1) {
           collectionApis.push(collectionInfo)
-        } else {
-          collectionApis[index] = collectionInfo
         }
       }
     }
@@ -494,7 +495,7 @@ export class ResourceUtils {
     if (!model) {
       return undefined
     }
-    return this.getPropertyOfModel(model,propertyName)
+    return this.getPropertyOfModel(model, propertyName)
   }
 
   public getPropertyOfModel(sourceModel, propertyName: string) {
@@ -518,9 +519,8 @@ export class ResourceUtils {
           if (property) {
             return this.getUnwrappedModel(property)
           }
-        }
-        else {
-          const property = this.getPropertyOfModel(element,propertyName)
+        } else {
+          const property = this.getPropertyOfModel(element, propertyName)
           if (property) {
             return property
           }

src/typescript/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@microsoft.azure/openapi-validator",
-  "version": "1.10.0",
+  "version": "1.10.1",
   "description": "Azure OpenAPI Validator",
   "main": "src/index.js",
   "scripts": {