fix(ruleset-migrator): order of functions property should not matter (#…

…2014)

packages/ruleset-migrator/package.json

@@ -21,7 +21,7 @@
     "url": "https://github.com/stoplightio/spectral.git"
   },
   "dependencies": {
-    "@stoplight/json": "3.17.0",
+    "@stoplight/json": "~3.17.0",
     "@stoplight/ordered-object-literal": "1.0.2",
     "@stoplight/path": "1.3.2",
     "@stoplight/spectral-functions": "^1.0.0",

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-4/output.cjs

@@ -1,13 +1,13 @@
-const { oas3, oas3_1 } = require('@stoplight/spectral-formats');
-const oas3$0 = _interopDefault(require('/.tmp/spectral/functions-variant-4/functions/oas3.js'));
+const { oas3: oas3$0, oas3_1 } = require('@stoplight/spectral-formats');
+const oas3 = _interopDefault(require('/.tmp/spectral/functions-variant-4/functions/oas3.js'));
 module.exports = {
-  formats: [oas3, oas3_1],
+  formats: [oas3$0, oas3_1],
   rules: {
     rule: {
       given: '$',
       then: [
         {
-          function: oas3$0,
+          function: oas3,
         },
       ],
     },

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-4/output.mjs

@@ -1,13 +1,13 @@
-import { oas3, oas3_1 } from '@stoplight/spectral-formats';
-import oas3$0 from '/.tmp/spectral/functions-variant-4/functions/oas3.js';
+import { oas3 as oas3$0, oas3_1 } from '@stoplight/spectral-formats';
+import oas3 from '/.tmp/spectral/functions-variant-4/functions/oas3.js';
 export default {
-  formats: [oas3, oas3_1],
+  formats: [oas3$0, oas3_1],
   rules: {
     rule: {
       given: '$',
       then: [
         {
-          function: oas3$0,
+          function: oas3,
         },
       ],
     },

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-5/output.cjs

@@ -0,0 +1,14 @@
+const donothing = _interopDefault(require('/.tmp/spectral/functions-variant-5/custom-functions/do-nothing.js'));
+module.exports = {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: donothing,
+      },
+    },
+  },
+};
+function _interopDefault(ex) {
+  return ex && typeof ex === 'object' && 'default' in ex ? ex['default'] : ex;
+}

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-5/output.mjs

@@ -0,0 +1,11 @@
+import donothing from '/.tmp/spectral/functions-variant-5/custom-functions/do-nothing.js';
+export default {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: donothing,
+      },
+    },
+  },
+};

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-5/ruleset.yaml

@@ -0,0 +1,8 @@
+rules:
+  rule:
+    given: $
+    then:
+      function: do-nothing
+functionsDir: custom-functions
+functions:
+  - do-nothing

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-6/output.cjs

@@ -0,0 +1,11 @@
+const { truthy } = require('@stoplight/spectral-functions');
+module.exports = {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: truthy,
+      },
+    },
+  },
+};

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-6/output.mjs

@@ -0,0 +1,11 @@
+import { truthy } from '@stoplight/spectral-functions';
+export default {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: truthy,
+      },
+    },
+  },
+};

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-6/ruleset.yaml

@@ -0,0 +1,6 @@
+rules:
+  rule:
+    given: $
+    then:
+      function: truthy
+functionsDir: custom-fns

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-7/output.cjs

@@ -0,0 +1,10 @@
+module.exports = {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: void 0,
+      },
+    },
+  },
+};

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-7/output.mjs

@@ -0,0 +1,10 @@
+export default {
+  rules: {
+    rule: {
+      given: '$',
+      then: {
+        function: void 0,
+      },
+    },
+  },
+};

packages/ruleset-migrator/src/__tests__/__fixtures__/functions-variant-7/ruleset.yaml

@@ -0,0 +1,5 @@
+rules:
+  rule:
+    given: $
+    then:
+      function: do-nothing

packages/ruleset-migrator/src/__tests__/__fixtures__/rules-variant-1/output.cjs

@@ -1,4 +1,5 @@
 const { oas2 } = require('@stoplight/spectral-formats');
+const { truthy } = require('@stoplight/spectral-functions');
 module.exports = {
   rules: {
     'oas3-schema': 'error',
@@ -11,7 +12,7 @@ module.exports = {
       given:
         "$.paths.*[?( @property === 'get' || @property === 'put' || @property === 'post' || @property === 'delete' || @property === 'options' || @property === 'head' || @property === 'patch' || @property === 'trace' )]",
       then: {
-        function: void 0,
+        function: truthy,
         functionOptions: null,
       },
     },

packages/ruleset-migrator/src/__tests__/__fixtures__/rules-variant-1/output.mjs

@@ -1,4 +1,5 @@
 import { oas2 } from '@stoplight/spectral-formats';
+import { truthy } from '@stoplight/spectral-functions';
 export default {
   rules: {
     'oas3-schema': 'error',
@@ -11,7 +12,7 @@ export default {
       given:
         "$.paths.*[?( @property === 'get' || @property === 'put' || @property === 'post' || @property === 'delete' || @property === 'options' || @property === 'head' || @property === 'patch' || @property === 'trace' )]",
       then: {
-        function: void 0,
+        function: truthy,
         functionOptions: null,
       },
     },

packages/ruleset-migrator/src/__tests__/__fixtures__/rules-variant-1/ruleset.yaml

@@ -12,5 +12,5 @@ rules:
       "$.paths.*[?( @property === 'get' || @property === 'put' || @property === 'post' || @property === 'delete' ||
       @property === 'options' || @property === 'head' || @property === 'patch' || @property === 'trace' )]"
     then:
-      function: oasOpFormDataConsumeCheck
+      function: truthy
       functionOptions: null

packages/ruleset-migrator/src/index.ts

@@ -50,18 +50,18 @@ export async function migrateRuleset(filepath: string, opts: MigrationOptions):
   };
 
   for (const transformer of transformers) {
-    transformer(ctx);
+    transformer(ctx.hooks);
   }
 
-  tree.ruleset = await process(ruleset, hooks);
+  tree.ruleset = await process(ruleset, ctx);
 
   return tree.toString();
 }
 
-async function _process(input: unknown, hooks: Set<Hook>, path: string): Promise<ExpressionKind | null> {
-  for (const [pattern, fn] of hooks) {
+async function _process(input: unknown, ctx: TransformerCtx, path: string): Promise<ExpressionKind | null> {
+  for (const [pattern, fn] of ctx.hooks) {
     if (pattern.test(path)) {
-      const output = await fn(input);
+      const output = await fn(input, ctx);
 
       if (output !== void 0) {
         return output;
@@ -71,7 +71,9 @@ async function _process(input: unknown, hooks: Set<Hook>, path: string): Promise
 
   if (Array.isArray(input)) {
     return b.arrayExpression(
-      (await Promise.all(input.map((item, i) => _process(item, hooks, `${path}/${String(i)}`)))).filter(Boolean),
+      (await Promise.all(input.map(async (item, i) => await _process(item, ctx, `${path}/${String(i)}`)))).filter(
+        Boolean,
+      ),
     );
   } else if (typeof input === 'number' || typeof input === 'boolean' || typeof input === 'string') {
     return b.literal(input);
@@ -87,7 +89,7 @@ async function _process(input: unknown, hooks: Set<Hook>, path: string): Promise
     (
       await Promise.all(
         Object.entries(input).map(async ([key, value]) => {
-          const propertyValue = await _process(value, hooks, `${path}/${key}`);
+          const propertyValue = await _process(value, ctx, `${path}/${key}`);
 
           if (propertyValue !== null) {
             return b.property('init', b.identifier(JSON.stringify(key)), propertyValue);
@@ -100,6 +102,6 @@ async function _process(input: unknown, hooks: Set<Hook>, path: string): Promise
   );
 }
 
-export async function process(input: Ruleset, hooks: Set<Hook>): Promise<namedTypes.ObjectExpression> {
-  return (await _process(input, hooks, '')) as namedTypes.ObjectExpression;
+export async function process(input: Ruleset, ctx: TransformerCtx): Promise<namedTypes.ObjectExpression> {
+  return (await _process(input, ctx, '')) as namedTypes.ObjectExpression;
 }

packages/ruleset-migrator/src/transformers/except.ts

@@ -3,8 +3,8 @@ import { Ruleset } from '../validation/types';
 
 export { transformer as default };
 
-const transformer: Transformer = function (ctx) {
-  ctx.hooks.add([
+const transformer: Transformer = function (hooks) {
+  hooks.add([
     /^$/,
     (_ruleset): void => {
       const ruleset = _ruleset as Ruleset;

packages/ruleset-migrator/src/transformers/extends.ts

@@ -22,28 +22,23 @@ async function processExtend(
     return ctx.tree.addImport(REPLACEMENTS[name], '@stoplight/spectral-rulesets');
   }
 
-  const existingCwd = ctx.cwd;
-  const filepath = ctx.tree.resolveModule(name, existingCwd);
+  const filepath = ctx.tree.resolveModule(name, ctx.cwd);
 
   if (KNOWN_JS_EXTS.test(path.extname(filepath))) {
     return ctx.tree.addImport(`${path.basename(filepath, true)}_${path.extname(filepath)}`, filepath, true);
   }
 
-  const scope = ctx.tree.scope;
-  try {
-    ctx.cwd = path.dirname(filepath);
-    ctx.tree.scope = ctx.tree.scope.fork();
-    return await process(await ctx.read(filepath, ctx.opts.fs, ctx.opts.fetch), ctx.hooks);
-  } finally {
-    ctx.tree.scope = scope;
-    ctx.cwd = existingCwd;
-  }
+  return await process(await ctx.read(filepath, ctx.opts.fs, ctx.opts.fetch), {
+    ...ctx,
+    cwd: path.dirname(filepath),
+    tree: ctx.tree.fork(),
+  });
 }
 
-const transformer: Transformer = function (ctx) {
-  ctx.hooks.add([
+const transformer: Transformer = function (hooks) {
+  hooks.add([
     /^(\/overrides\/\d+)?\/extends$/,
-    async (input): Promise<namedTypes.ArrayExpression | namedTypes.ObjectExpression | namedTypes.Identifier> => {
+    async (input, ctx): Promise<namedTypes.ArrayExpression | namedTypes.ObjectExpression | namedTypes.Identifier> => {
       const _extends = input as Ruleset['extends'];
 
       if (typeof _extends === 'string') {

packages/ruleset-migrator/src/transformers/formats.ts

@@ -18,7 +18,7 @@ const REPLACEMENTS = Object.fromEntries(
   ]),
 );
 
-function transform(ctx: TransformerCtx, input: unknown): namedTypes.ArrayExpression {
+function transform(input: unknown, ctx: TransformerCtx): namedTypes.ArrayExpression {
   assertArray(input);
 
   return b.arrayExpression(
@@ -35,10 +35,8 @@ function transform(ctx: TransformerCtx, input: unknown): namedTypes.ArrayExpress
 
 export { transformer as default };
 
-const transformer: Transformer = function (ctx) {
-  const t = transform.bind(null, ctx);
-
-  ctx.hooks.add([/^\/aliases\/[^/]+\/targets\/\d+\/formats$/, t]);
-  ctx.hooks.add([/^(\/overrides\/\d+)?\/formats$/, t]);
-  ctx.hooks.add([/^(\/overrides\/\d+)?\/rules\/[^/]+\/formats$/, t]);
+const transformer: Transformer = function (hooks) {
+  hooks.add([/^\/aliases\/[^/]+\/targets\/\d+\/formats$/, transform]);
+  hooks.add([/^(\/overrides\/\d+)?\/formats$/, transform]);
+  hooks.add([/^(\/overrides\/\d+)?\/rules\/[^/]+\/formats$/, transform]);
 };

packages/ruleset-migrator/src/transformers/functions.ts

@@ -5,34 +5,40 @@ import { Ruleset } from '../validation/types';
 
 export { transformer as default };
 
-const transformer: Transformer = function (ctx) {
-  ctx.hooks.add([
+const transformer: Transformer = function (hooks) {
+  hooks.add([
     /^$/,
-    (_ruleset): void => {
+    (_ruleset, ctx): void => {
       const ruleset = _ruleset as Ruleset;
       const { functionsDir, functions } = ruleset;
 
       if (Array.isArray(functions) && functions.length > 0) {
-        ruleset.functions = functions.map(fn =>
-          ctx.tree.resolveModule(`${fn}.js`, path.join(ctx.cwd, functionsDir ?? 'functions')),
-        );
-        delete ruleset.functionsDir;
+        for (const fn of functions) {
+          assertString(fn);
+          const resolved = ctx.tree.resolveModule(
+            `${fn}.js`,
+            path.join(ctx.cwd, typeof functionsDir === 'string' ? functionsDir : 'functions'),
+          );
+          const fnName = path.basename(resolved, true);
+          const identifier = ctx.tree.addImport(fnName, resolved, true);
+          ctx.tree.scope.store(`function-${fnName}`, identifier.name);
+        }
       }
     },
   ]);
 
-  ctx.hooks.add([
+  hooks.add([
     /^\/functions$/,
     (value): null => {
       assertArray(value);
+      return null;
+    },
+  ]);
 
-      for (const fn of value) {
-        assertString(fn);
-        const fnName = path.basename(fn, true);
-        const identifier = ctx.tree.addImport(fnName, fn, true);
-        ctx.tree.scope.store(`function-${fnName}`, identifier.name);
-      }
-
+  hooks.add([
+    /^\/functionsDir$/,
+    (value): null => {
+      assertString(value);
       return null;
     },
   ]);

packages/ruleset-migrator/src/transformers/rules.ts

@@ -53,8 +53,8 @@ function getDiagnosticSeverity(severity: DiagnosticSeverity | string): Diagnosti
   return Number.isNaN(Number(severity)) ? SEVERITY_MAP[severity] : Number(severity);
 }
 
-const transformer: Transformer = function (ctx) {
-  ctx.hooks.add([
+const transformer: Transformer = function (hooks) {
+  hooks.add([
     /^$/,
     (_ruleset): void => {
       const ruleset = _ruleset as Ruleset;
@@ -84,9 +84,9 @@ const transformer: Transformer = function (ctx) {
     },
   ]);
 
-  ctx.hooks.add([
+  hooks.add([
     /^\/rules\/[^/]+\/then\/(?:[0-9]+\/)?function$/,
-    (value): namedTypes.Identifier | namedTypes.UnaryExpression => {
+    (value, ctx): namedTypes.Identifier | namedTypes.UnaryExpression => {
       assertString(value);
 
       if (KNOWN_FUNCTIONS.includes(value)) {