fix(rulesets): tweak server variables function

packages/rulesets/src/oas/__tests__/oas3-server-variables.test.ts

@@ -70,7 +70,6 @@ testRule('oas3-server-variables', [
       servers: [
         {
           url: '{protocol}://stoplight.io:{port}',
-          variables: {},
         },
       ],
       paths: {
@@ -85,7 +84,6 @@ testRule('oas3-server-variables', [
             servers: [
               {
                 url: 'https://{env}.stoplight.io',
-                variables: {},
               },
             ],
           },
@@ -95,7 +93,7 @@ testRule('oas3-server-variables', [
     errors: [
       {
         message: 'Not all server\'s variables are described with "variables" object. Missed: protocol, port.',
-        path: ['servers', '0', 'variables'],
+        path: ['servers', '0'],
         severity: DiagnosticSeverity.Error,
       },
       {
@@ -105,7 +103,7 @@ testRule('oas3-server-variables', [
       },
       {
         message: 'Not all server\'s variables are described with "variables" object. Missed: env.',
-        path: ['paths', '/', 'get', 'servers', '0', 'variables'],
+        path: ['paths', '/', 'get', 'servers', '0'],
         severity: DiagnosticSeverity.Error,
       },
     ],
@@ -124,6 +122,7 @@ testRule('oas3-server-variables', [
             },
             env: {
               enum: ['staging', 'integration'],
+              default: 'staging',
             },
           },
         },
@@ -139,6 +138,7 @@ testRule('oas3-server-variables', [
                 },
                 env: {
                   enum: ['staging', 'integration'],
+                  default: 'staging',
                 },
               },
             },
@@ -182,7 +182,48 @@ testRule('oas3-server-variables', [
   },
 
   {
-    name: 'server has an unlisted default',
+    name: 'server variable has a missing default',
+    document: {
+      openapi: '3.1.0',
+      servers: [
+        {
+          url: 'https://{env}.stoplight.io',
+          variables: {
+            env: {
+              enum: ['staging', 'integration'],
+            },
+          },
+        },
+      ],
+      paths: {
+        '/': {
+          servers: [
+            {
+              url: 'https://stoplight.io:{port}',
+              variables: {
+                port: {},
+              },
+            },
+          ],
+        },
+      },
+    },
+    errors: [
+      {
+        code: 'oas3-server-variables',
+        message: 'Server Variable "env" has a missing default.',
+        path: ['servers', '0', 'variables', 'env'],
+      },
+      {
+        code: 'oas3-server-variables',
+        message: 'Server Variable "port" has a missing default.',
+        path: ['paths', '/', 'servers', '0', 'variables', 'port'],
+      },
+    ],
+  },
+
+  {
+    name: 'server variable has an unlisted default',
     document: {
       openapi: '3.1.0',
       servers: [
@@ -231,17 +272,17 @@ testRule('oas3-server-variables', [
     },
     errors: [
       {
-        message: 'Server Variable "port" has a default not listed in the enum',
+        message: 'Server Variable "port" has a default not listed in the enum.',
         path: ['servers', '0', 'variables', 'port', 'default'],
         severity: DiagnosticSeverity.Error,
       },
       {
-        message: 'Server Variable "env" has a default not listed in the enum',
+        message: 'Server Variable "env" has a default not listed in the enum.',
         path: ['paths', '/', 'servers', '0', 'variables', 'env', 'default'],
         severity: DiagnosticSeverity.Error,
       },
       {
-        message: 'Server Variable "env" has a default not listed in the enum',
+        message: 'Server Variable "env" has a default not listed in the enum.',
         path: ['components', 'links', 'Address', 'server', 'variables', 'env', 'default'],
         severity: DiagnosticSeverity.Error,
       },
@@ -258,6 +299,7 @@ testRule('oas3-server-variables', [
           variables: {
             port: {
               enum: ['invalid port', 'another-one', '443'],
+              default: '443',
             },
           },
         },
@@ -266,6 +308,7 @@ testRule('oas3-server-variables', [
           variables: {
             username: {
               enum: ['stoplight', 'io'],
+              default: 'stoplight',
             },
           },
         },
@@ -278,6 +321,7 @@ testRule('oas3-server-variables', [
               variables: {
                 base: {
                   enum: ['http', 'https', 'ftp', 'ftps', 'ssh', 'smtp'],
+                  default: 'https',
                 },
               },
             },

packages/rulesets/src/oas/index.ts

@@ -702,6 +702,7 @@ const ruleset = {
         function: serverVariables,
         functionOptions: {
           checkSubstitutions: true,
+          requireDefault: true,
         },
       },
     },

packages/rulesets/src/shared/functions/serverVariables/index.ts

@@ -23,6 +23,7 @@ type Input = {
 
 type Options = {
   checkSubstitutions?: boolean;
+  requireDefault?: boolean;
 } | null;
 
 export default createRulesetFunction<Input, Options>(
@@ -72,73 +73,120 @@ export default createRulesetFunction<Input, Options>(
           type: 'boolean',
           default: 'false',
         },
+        requireDefault: {
+          type: 'boolean',
+          default: 'false',
+        },
       },
       additionalProperties: false,
     },
   },
   function serverVariables({ url, variables }, opts, ctx) {
-    if (variables === void 0) return;
-
     const results: IFunctionResult[] = [];
 
     const foundVariables = parseUrlVariables(url);
-    const definedVariablesKeys = Object.keys(variables);
-
-    const redundantVariables = getRedundantProps(foundVariables, definedVariablesKeys);
-    for (const variable of redundantVariables) {
-      results.push({
-        message: `Server's "variables" object has unused defined "${variable}" url variable.`,
-        path: [...ctx.path, 'variables', variable],
-      });
-    }
+    const definedVariablesKeys = variables === void 0 ? [] : Object.keys(variables);
+
+    accumulateRedundantVariables(results, ctx.path, foundVariables, definedVariablesKeys);
 
     if (foundVariables.length === 0) return results;
 
-    const missingVariables = getMissingProps(foundVariables, definedVariablesKeys);
-    if (missingVariables.length > 0) {
-      results.push({
-        message: `Not all server's variables are described with "variables" object. Missed: ${missingVariables.join(
-          ', ',
-        )}.`,
-        path: [...ctx.path, 'variables'],
-      });
-    }
+    accumulateMissingVariables(results, ctx.path, foundVariables, definedVariablesKeys);
+
+    if (variables === void 0) return results;
 
     const variablePairs: [key: string, values: string[]][] = [];
 
     for (const key of definedVariablesKeys) {
-      if (redundantVariables.includes(key)) continue;
+      if (!foundVariables.includes(key)) continue;
 
-      const values = variables[key];
+      const variable = variables[key];
 
-      if ('enum' in values) {
-        variablePairs.push([key, values.enum]);
+      if ('enum' in variable) {
+        variablePairs.push([key, variable.enum]);
 
-        if ('default' in values && !values.enum.includes(values.default)) {
-          results.push({
-            message: `Server Variable "${key}" has a default not listed in the enum`,
-            path: [...ctx.path, 'variables', key, 'default'],
-          });
-        }
+        checkVariableEnumValues(results, ctx.path, key, variable.enum, variable.default);
+      } else if ('default' in variable) {
+        variablePairs.push([key, [variable.default]]);
       } else {
-        variablePairs.push([key, [values.default ?? '']]);
+        variablePairs.push([key, []]);
+      }
+
+      if (!('default' in variable) && opts?.requireDefault === true) {
+        results.push({
+          message: `Server Variable "${key}" has a missing default.`,
+          path: [...ctx.path, 'variables', key],
+        });
       }
     }
 
-    if (opts?.checkSubstitutions === true && variablePairs.length > 0) {
+    if (opts?.checkSubstitutions === true) {
       checkSubstitutions(results, ctx.path, url, variablePairs);
     }
 
     return results;
   },
 );
 
+function accumulateRedundantVariables(
+  results: IFunctionResult[],
+  path: JsonPath,
+  foundVariables: string[],
+  definedVariablesKeys: string[],
+): void {
+  if (definedVariablesKeys.length === 0) return;
+
+  const redundantVariables = getRedundantProps(foundVariables, definedVariablesKeys);
+  for (const variable of redundantVariables) {
+    results.push({
+      message: `Server's "variables" object has unused defined "${variable}" url variable.`,
+      path: [...path, 'variables', variable],
+    });
+  }
+}
+
+function accumulateMissingVariables(
+  results: IFunctionResult[],
+  path: JsonPath,
+  foundVariables: string[],
+  definedVariablesKeys: string[],
+): void {
+  const missingVariables =
+    definedVariablesKeys.length === 0 ? foundVariables : getMissingProps(foundVariables, definedVariablesKeys);
+
+  if (missingVariables.length > 0) {
+    results.push({
+      message: `Not all server's variables are described with "variables" object. Missed: ${missingVariables.join(
+        ', ',
+      )}.`,
+      path: [...path, 'variables'],
+    });
+  }
+}
+
+function checkVariableEnumValues(
+  results: IFunctionResult[],
+  path: JsonPath,
+  name: string,
+  enumValues: string[],
+  defaultValue: string | undefined,
+): void {
+  if (defaultValue !== void 0 && !enumValues.includes(defaultValue)) {
+    results.push({
+      message: `Server Variable "${name}" has a default not listed in the enum.`,
+      path: [...path, 'variables', name, 'default'],
+    });
+  }
+}
+
 function checkSubstitutions(
   results: IFunctionResult[],
   path: JsonPath,
   url: string,
   variables: [key: string, values: string[]][],
 ): void {
+  if (variables.length === 0) return;
+
   const invalidUrls: string[] = [];
 
   for (const substitutedUrl of applyUrlVariables(url, variables)) {