style(spec): add out-of-line-enum rule APIC-416 (#356)

Author: millotp

.eslintrc.js

@@ -13,7 +13,16 @@ module.exports = {
       parser: 'yaml-eslint-parser',
       plugins: ["automation-custom"],
       rules: {
-        '@typescript-eslint/naming-convention': 0,
+        'yml/plain-scalar': [
+          2,
+          "always"
+          , {
+            // ignore path from ref, that must be quoted
+            ignorePatterns: [
+              '[./#a-zA-Z0-9_]+'
+            ]
+          }
+        ],
         'yml/quotes': [
           2,
           {
@@ -35,7 +44,16 @@ module.exports = {
         files: ['specs/**/*.yml'],
         rules: {
           "automation-custom/description-dot": "error",
-        }
+          "automation-custom/single-quote-ref": "error",
+        },
+        overrides: [
+          {
+            files: ['!specs/bundled/*.yml'],
+            rules: {
+              "automation-custom/out-of-line-enum": "error",
+            }
+          }
+        ]
       }
       ]
     },

.github/.cache_version

@@ -1 +1 @@
-7.1.0
+7.2.0

eslint/src/index.ts

@@ -1,7 +1,11 @@
 import { descriptionDot } from './rules/descriptionDot';
+import { outOfLineEnum } from './rules/outOfLineEnum';
+import { singleQuoteRef } from './rules/singleQuoteRef';
 
 const rules = {
   'description-dot': descriptionDot,
+  'out-of-line-enum': outOfLineEnum,
+  'single-quote-ref': singleQuoteRef,
 };
 
 export { rules };

eslint/src/rules/outOfLineEnum.ts

@@ -0,0 +1,44 @@
+import type { Rule } from 'eslint';
+
+import { isPairWithKey } from '../utils';
+
+export const outOfLineEnum: Rule.RuleModule = {
+  meta: {
+    docs: {
+      description: 'enum must be out of line',
+    },
+    messages: {
+      enumNotOutOfLine: 'enum is not out of line',
+    },
+  },
+  create(context) {
+    if (!context.parserServices.isYAML) {
+      return {};
+    }
+
+    return {
+      YAMLPair(node): void {
+        if (!isPairWithKey(node, 'enum')) {
+          return;
+        }
+        // parent is mapping, and parent is real parent that must be to the far left
+        if (node.parent.parent.loc.start.column === 0) {
+          return;
+        }
+        if (
+          isPairWithKey(
+            node.parent.parent.parent.parent?.parent?.parent?.parent ?? null,
+            'servers'
+          )
+        ) {
+          // accept out of line enum if in servers
+          return;
+        }
+        context.report({
+          node: node.parent.parent as any,
+          messageId: 'enumNotOutOfLine',
+        });
+      },
+    };
+  },
+};

eslint/src/rules/singleQuoteRef.ts

@@ -0,0 +1,55 @@
+import type { Rule } from 'eslint';
+
+import { isBLockScalar, isPairWithKey, isScalar } from '../utils';
+
+export const singleQuoteRef: Rule.RuleModule = {
+  meta: {
+    docs: {
+      description: '$ref must be wrapped in single quote',
+    },
+    messages: {
+      refNoQuote: '$ref is not wrapped in single quote',
+    },
+    fixable: 'code',
+  },
+  create(context) {
+    if (!context.parserServices.isYAML) {
+      return {};
+    }
+
+    return {
+      YAMLPair(node): void {
+        if (!isPairWithKey(node, '$ref')) {
+          return;
+        }
+        if (!isScalar(node.value)) {
+          // not our problem, something else will fail like path resolution
+          return;
+        }
+        if (node.value.style === 'single-quoted') {
+          // that's what we want
+          return;
+        }
+        if (isBLockScalar(node.value)) {
+          // another rule should take care of that case
+          return;
+        }
+        const hasDoubleQuote = node.value.style === 'double-quoted';
+        const [start, end] = node.value.range;
+        context.report({
+          node: node as any,
+          messageId: 'refNoQuote',
+          *fix(fixer) {
+            if (hasDoubleQuote) {
+              yield fixer.replaceTextRange([start, start + 1], "'");
+              yield fixer.replaceTextRange([end - 1, end], "'");
+            } else {
+              yield fixer.insertTextBeforeRange([start, start], "'");
+              yield fixer.insertTextAfterRange([end, end], "'");
+            }
+          },
+        });
+      },
+    };
+  },
+};

eslint/tests/outOfLineEnum.test.ts

@@ -0,0 +1,66 @@
+import { RuleTester } from 'eslint';
+
+import { outOfLineEnum } from '../src/rules/outOfLineEnum';
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('yaml-eslint-parser'),
+});
+
+ruleTester.run('out-of-line-enum', outOfLineEnum, {
+  valid: [
+    `
+simple:
+  type: string
+  enum: [bla, blabla]
+`,
+    `
+simple:
+  type: string
+  enum:
+    - bla
+    - blabla
+`,
+    `
+simple:
+  type: string
+  enum: [bla, blabla]
+
+useIt:
+  $ref: '#/simple'
+`,
+    `
+servers:
+  - url: http://test-server.com
+    variables:
+      region:
+        default: us
+        enum:
+          - us
+          - de
+`,
+  ],
+  invalid: [
+    {
+      code: `
+root:
+  inside:
+    type: string
+    enum: [bla, blabla]
+  `,
+      errors: [{ messageId: 'enumNotOutOfLine' }],
+    },
+    {
+      code: `
+root:
+  inside:
+    deeper:
+      type: string
+      enum: [bla, blabla]
+
+useIt:
+  $ref: '#/root/inside/deeper'
+  `,
+      errors: [{ messageId: 'enumNotOutOfLine' }],
+    },
+  ],
+});

eslint/tests/singleQuoteRef.test.ts

@@ -0,0 +1,82 @@
+import { RuleTester } from 'eslint';
+
+import { singleQuoteRef } from '../src/rules/singleQuoteRef';
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('yaml-eslint-parser'),
+});
+
+ruleTester.run('single-quote-ref', singleQuoteRef, {
+  valid: [
+    `
+simple:
+  $ref: 'random/path.yml'`,
+    `
+sameFile:
+  $ref: '#/inside'`,
+    `
+long:
+  $ref: 'some/random/file.yml#/root/object/prop'
+`,
+    `
+post:
+  description: test desc.
+  '400':
+    $ref: '../../common/responses/BadRequest.yml'
+`,
+  ],
+  invalid: [
+    {
+      code: `
+simple:
+  $ref: random/path.yml
+`,
+      errors: [{ messageId: 'refNoQuote' }],
+      output: `
+simple:
+  $ref: 'random/path.yml'
+`,
+    },
+    {
+      code: `
+long:
+  $ref: some/random/file.yml#/root/object/prop
+`,
+      errors: [{ messageId: 'refNoQuote' }],
+      output: `
+long:
+  $ref: 'some/random/file.yml#/root/object/prop'
+`,
+    },
+    {
+      code: `
+post:
+  description: test desc.
+  '400':
+    $ref: ../../common/responses/BadRequest.yml
+  '404':
+    $ref: ../../common/responses/IndexNotFound.yml
+`,
+      errors: [{ messageId: 'refNoQuote' }, { messageId: 'refNoQuote' }],
+      output: `
+post:
+  description: test desc.
+  '400':
+    $ref: '../../common/responses/BadRequest.yml'
+  '404':
+    $ref: '../../common/responses/IndexNotFound.yml'
+`,
+    },
+    {
+      code: `
+double:
+  $ref: "some/ref"
+`,
+      errors: [{ messageId: 'refNoQuote' }],
+      output: `
+double:
+  $ref: 'some/ref'
+`,
+    },
+  ],
+});

scripts/buildSpecs.ts

@@ -41,6 +41,39 @@ async function propagateTagsToOperations(
   return true;
 }
 
+async function lintCommon(verbose: boolean, useCache: boolean): Promise<void> {
+  let hash = '';
+  const cacheFile = toAbsolutePath(`specs/dist/common.cache`);
+  if (useCache) {
+    const { cacheExists, hash: newCache } = await checkForCache(
+      {
+        job: 'common specs',
+        folder: toAbsolutePath('specs/'),
+        generatedFiles: [],
+        filesToCache: ['common'],
+        cacheFile,
+      },
+      verbose
+    );
+
+    if (cacheExists) {
+      return;
+    }
+
+    hash = newCache;
+  }
+
+  const spinner = createSpinner('linting common spec', verbose).start();
+  await run(`yarn specs:lint common`, { verbose });
+
+  if (hash) {
+    spinner.text = `storing common spec cache`;
+    await fsp.writeFile(cacheFile, hash);
+  }
+
+  spinner.succeed();
+}
+
 async function buildSpec(
   client: string,
   outputFormat: string,
@@ -87,7 +120,7 @@ async function buildSpec(
   }
 
   spinner.text = `linting ${client} spec`;
-  await run(`yarn specs:lint ${client}`, { verbose });
+  await run(`yarn specs:fix ${client}`, { verbose });
 
   spinner.text = `validating ${client} spec`;
   await run(`yarn openapi lint specs/bundled/${client}.${outputFormat}`, {
@@ -113,6 +146,8 @@ export async function buildSpecs(
 ): Promise<void> {
   await fsp.mkdir(toAbsolutePath('specs/dist'), { recursive: true });
 
+  await lintCommon(verbose, useCache);
+
   await Promise.all(
     clients.map((client) => buildSpec(client, outputFormat, verbose, useCache))
   );

specs/abtesting/common/schemas/ABTest.yml

@@ -9,7 +9,7 @@ ABTest:
   additionalProperties: false
   properties:
     abTestID:
-      $ref: ../parameters.yml#/abTestID
+      $ref: '../parameters.yml#/abTestID'
     clickSignificance:
       type: number
       format: double
@@ -19,16 +19,16 @@ ABTest:
       format: double
       description: A/B test significance based on conversion data. Should be > 0.95 to be considered significant (no matter which variant is winning).
     endAt:
-      $ref: ../parameters.yml#/endAt
+      $ref: '../parameters.yml#/endAt'
     createdAt:
-      $ref: ../parameters.yml#/createdAt
+      $ref: '../parameters.yml#/createdAt'
     name:
-      $ref: ../parameters.yml#/name
+      $ref: '../parameters.yml#/name'
     status:
       type: string
       description: status of the A/B test.
     variants:
-      $ref: Variant.yml#/variants
+      $ref: 'Variant.yml#/variants'
   required:
     - status
     - name