From d5f11ba084084896c6c341765e38962b89f54cb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Sun, 15 Mar 2020 19:26:48 +0800 Subject: [PATCH] feat(ruleset): allow require calls in Node.JS (#1011) * feat(ruleset): allow require calls in Node.JS * test: cover require * docs: add a note on require * docs: remove comment Co-Authored-By: Phil Sturgeon * doc: note on require Co-Authored-By: Phil Sturgeon * test: minor tweak Co-authored-by: Phil Sturgeon --- docs/guides/custom-functions.md | 4 +- .../custom-functions-oas-ruleset.json | 12 +++ .../__fixtures__/functions/deepHasIn.js | 11 +++ src/__tests__/linter.jest.test.ts | 16 ++++ src/rulesets/__tests__/__fixtures__/foo.js | 1 + .../__tests__/evaluators.jest.test.ts | 41 +++++++++ .../__tests__/evaluators.karma.test.ts | 13 +++ src/rulesets/__tests__/evaluators.test.ts | 20 ++--- src/rulesets/__tests__/reader.jest.test.ts | 6 ++ src/rulesets/evaluators.ts | 86 +++++++++++++++++-- .../mergers/__tests__/functions.jest.test.ts | 11 +++ src/rulesets/mergers/functions.ts | 1 + src/rulesets/reader.ts | 4 +- src/spectral.ts | 4 +- src/types/ruleset.ts | 1 + 15 files changed, 210 insertions(+), 21 deletions(-) create mode 100644 src/__tests__/__fixtures__/functions/deepHasIn.js create mode 100644 src/rulesets/__tests__/__fixtures__/foo.js create mode 100644 src/rulesets/__tests__/evaluators.jest.test.ts create mode 100644 src/rulesets/__tests__/evaluators.karma.test.ts diff --git a/docs/guides/custom-functions.md b/docs/guides/custom-functions.md index a687ab66e..e20eb1e3e 100644 --- a/docs/guides/custom-functions.md +++ b/docs/guides/custom-functions.md @@ -183,10 +183,10 @@ export default (obj) => { }; ``` -The following code won't work properly either: +Require calls will work only in Node.js, and will cause errors for anyone trying to use the ruleset in the browser. If your ruleset is definitely going to only be used in the context of NodeJS then using them is ok, but if you are distributing your rulesets to the public we recommend avoiding the use of `require()` to increase portability. ```js -const foo = require('./foo'); // require is not available (see note below) +const foo = require('./foo'); module.exports = (obj) => { for (const [key, value] of Object.entries(obj)) { diff --git a/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json b/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json index 8c514da87..6aa600903 100644 --- a/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json +++ b/src/__tests__/__fixtures__/custom-functions-oas-ruleset.json @@ -2,6 +2,7 @@ "extends": ["spectral:oas"], "functions": [ "truthy", + "deepHasIn", [ "hasIn", { @@ -48,6 +49,17 @@ "foo": true } } + }, + "has-bar-get-operation": { + "message": "{{error}}", + "given": "$.paths", + "type": "style", + "then": { + "function": "deepHasIn", + "functionOptions": { + "path": "/bar.get" + } + } } } } diff --git a/src/__tests__/__fixtures__/functions/deepHasIn.js b/src/__tests__/__fixtures__/functions/deepHasIn.js new file mode 100644 index 000000000..85dfc2889 --- /dev/null +++ b/src/__tests__/__fixtures__/functions/deepHasIn.js @@ -0,0 +1,11 @@ +const { hasIn } = require('lodash'); + +module.exports = (targetVal, opts) => { + if (!(hasIn(targetVal, opts.path))) { + return [ + { + message: `Object does not have ${opts.prop} property`, + }, + ]; + } +}; diff --git a/src/__tests__/linter.jest.test.ts b/src/__tests__/linter.jest.test.ts index 770c13a1d..8712462b2 100644 --- a/src/__tests__/linter.jest.test.ts +++ b/src/__tests__/linter.jest.test.ts @@ -66,6 +66,22 @@ describe('Linter', () => { ); }); + it('should support require calls', async () => { + await spectral.loadRuleset(customFunctionOASRuleset); + expect( + await spectral.run({ + info: {}, + paths: {}, + }), + ).toEqual([ + expect.objectContaining({ + code: 'has-bar-get-operation', + message: 'Object does not have undefined property', + path: ['paths'], + }), + ]); + }); + it('should respect the scope of defined functions (ruleset-based)', async () => { await spectral.loadRuleset(customDirectoryFunctionsRuleset); expect(await spectral.run({})).toEqual([ diff --git a/src/rulesets/__tests__/__fixtures__/foo.js b/src/rulesets/__tests__/__fixtures__/foo.js new file mode 100644 index 000000000..0568f2abe --- /dev/null +++ b/src/rulesets/__tests__/__fixtures__/foo.js @@ -0,0 +1 @@ +module.exports = () => 'hello!'; diff --git a/src/rulesets/__tests__/evaluators.jest.test.ts b/src/rulesets/__tests__/evaluators.jest.test.ts new file mode 100644 index 000000000..1bac44841 --- /dev/null +++ b/src/rulesets/__tests__/evaluators.jest.test.ts @@ -0,0 +1,41 @@ +import * as path from '@stoplight/path'; +import { evaluateExport } from '../evaluators'; + +describe('Code evaluators', () => { + describe('Export evaluator', () => { + it('supports require', () => { + expect(evaluateExport(`module.exports = require('./foo')`, path.join(__dirname, '__fixtures__/a.js'))()).toEqual( + 'hello!', + ); + + expect( + evaluateExport( + `module.exports = () => require('path').join('/', 'hello!')`, + path.join(__dirname, '__fixtures__/a.js'), + )(), + ).toEqual(require('path').join('/', 'hello!')); + + expect( + evaluateExport( + `module.exports = () => require('@stoplight/path').join('/', 'hello!')`, + path.join(__dirname, '__fixtures__/a.js'), + )(), + ).toEqual(path.join('/', 'hello!')); + }); + + it('supports require.resolve', () => { + expect( + path.normalize( + evaluateExport( + `module.exports = () => require.resolve('./foo', { paths: ['${path.join(__dirname, '__fixtures__')}'] } )`, + null, + )(), + ), + ).toEqual(path.join(__dirname, '__fixtures__/foo.js')); + }); + + it.each(['cache', 'extensions'])('exposes %s', member => { + expect(evaluateExport(`module.exports = () => require['${member}']`, null)()).toStrictEqual(require[member]); + }); + }); +}); diff --git a/src/rulesets/__tests__/evaluators.karma.test.ts b/src/rulesets/__tests__/evaluators.karma.test.ts new file mode 100644 index 000000000..50ef9f695 --- /dev/null +++ b/src/rulesets/__tests__/evaluators.karma.test.ts @@ -0,0 +1,13 @@ +import { evaluateExport } from '../evaluators'; + +describe('Code evaluators', () => { + describe('Export evaluator', () => { + it('does not support require', () => { + expect(evaluateExport.bind(null, `require('./d')`, null)).toThrowError(ReferenceError); + expect(evaluateExport.bind(null, `require.resolve('./d')`, null)).toThrowError(ReferenceError); + expect(evaluateExport.bind(null, `require.main`, null)).toThrowError(ReferenceError); + expect(evaluateExport.bind(null, `require.cache`, null)).toThrowError(ReferenceError); + expect(evaluateExport.bind(null, `require.extensions`, null)).toThrowError(ReferenceError); + }); + }); +}); diff --git a/src/rulesets/__tests__/evaluators.test.ts b/src/rulesets/__tests__/evaluators.test.ts index 31d558d67..92551967e 100644 --- a/src/rulesets/__tests__/evaluators.test.ts +++ b/src/rulesets/__tests__/evaluators.test.ts @@ -3,61 +3,61 @@ import { evaluateExport } from '../evaluators'; describe('Code evaluators', () => { describe('Export evaluator', () => { it('detects CJS default export', () => { - const exported = evaluateExport(`module.exports = function a(x, y) {}`); + const exported = evaluateExport(`module.exports = function a(x, y) {}`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'a'); expect(exported).toHaveProperty('length', 2); }); it('detects CJS-ES compatible default export', () => { - const exported = evaluateExport(`exports.default = function b(x, y) {}`); + const exported = evaluateExport(`exports.default = function b(x, y) {}`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'b'); expect(exported).toHaveProperty('length', 2); }); it('detects CJS-ES compatible default export variant #2', () => { - const exported = evaluateExport(`module.exports.default = function c(x, y, z) {}`); + const exported = evaluateExport(`module.exports.default = function c(x, y, z) {}`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'c'); expect(exported).toHaveProperty('length', 3); }); it('detects AMD export', () => { - const exported = evaluateExport(`define(['exports'], () => function d(x){} )`); + const exported = evaluateExport(`define(['exports'], () => function d(x){} )`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'd'); expect(exported).toHaveProperty('length', 1); }); it('detects anonymous AMD export', () => { - const exported = evaluateExport(`define(() => function d(x){} )`); + const exported = evaluateExport(`define(() => function d(x){} )`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'd'); expect(exported).toHaveProperty('length', 1); }); it('detects context-based export', () => { - const exported = evaluateExport(`this.returnExports = function e() {}`); + const exported = evaluateExport(`this.returnExports = function e() {}`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'e'); expect(exported).toHaveProperty('length', 0); }); it('detects context-based export', () => { - const exported = evaluateExport(`this.returnExports = function e() {}`); + const exported = evaluateExport(`this.returnExports = function e() {}`, null); expect(exported).toBeInstanceOf(Function); expect(exported).toHaveProperty('name', 'e'); expect(exported).toHaveProperty('length', 0); }); it('throws error if no default export can be found', () => { - expect(() => evaluateExport(`exports.a = function b(x, y) {}`)).toThrow(); + expect(() => evaluateExport(`exports.a = function b(x, y) {}`, null)).toThrow(); }); it('throws error default export is not a function', () => { - expect(() => evaluateExport(`module.exports = 2`)).toThrow(); - expect(() => evaluateExport(`this.returnExports = {}`)).toThrow(); + expect(() => evaluateExport(`module.exports = 2`, null)).toThrow(); + expect(() => evaluateExport(`this.returnExports = {}`, null)).toThrow(); }); }); }); diff --git a/src/rulesets/__tests__/reader.jest.test.ts b/src/rulesets/__tests__/reader.jest.test.ts index 953be8ed1..a62737434 100644 --- a/src/rulesets/__tests__/reader.jest.test.ts +++ b/src/rulesets/__tests__/reader.jest.test.ts @@ -563,11 +563,13 @@ describe('Rulesets reader', () => { name: 'foo.cjs', ref: 'random-id-0', schema: null, + source: path.join(fooRuleset, '../functions/foo.cjs.js'), }, 'random-id-0': { name: 'foo.cjs', code: fooCJSFunction, schema: null, + source: path.join(fooRuleset, '../functions/foo.cjs.js'), }, }); @@ -592,11 +594,13 @@ describe('Rulesets reader', () => { name: 'bar', ref: expect.stringMatching(/^random-id-[01]$/), schema: null, + source: path.join(customFunctionsDirectoryRuleset, '../customFunctions/bar.js'), }, truthy: { name: 'truthy', ref: expect.stringMatching(/^random-id-[01]$/), schema: null, + source: path.join(customFunctionsDirectoryRuleset, '../customFunctions/truthy.js'), }, }), ); @@ -613,12 +617,14 @@ describe('Rulesets reader', () => { name: 'bar', code: barFunction, schema: null, + source: path.join(customFunctionsDirectoryRuleset, '../customFunctions/bar.js'), }); expect(truthyFunctionDef).toEqual({ name: 'truthy', code: truthyFunction, schema: null, + source: path.join(customFunctionsDirectoryRuleset, '../customFunctions/truthy.js'), }); expect(ruleset.functions.bar).toHaveProperty('name', 'bar'); diff --git a/src/rulesets/evaluators.ts b/src/rulesets/evaluators.ts index 57e5a8f57..a67c5c23a 100644 --- a/src/rulesets/evaluators.ts +++ b/src/rulesets/evaluators.ts @@ -1,12 +1,79 @@ -import { Optional } from '@stoplight/types'; +import { join, stripRoot } from '@stoplight/path'; +import { Dictionary, Optional } from '@stoplight/types'; import { isObject } from 'lodash'; import { IFunction, JSONSchema } from '../types'; import { decorateIFunctionWithSchemaValidation } from './validation'; -export type CJSExport = Partial<{ exports: object | ESCJSCompatibleExport }>; +export type CJSExport = Partial<{ exports: object | ESCJSCompatibleExport; require: NodeJS.Require }>; export type ESCJSCompatibleExport = Partial<{ default: unknown }>; export type ContextExport = Partial<{ returnExports: unknown }>; +function requireUnavailable() { + throw new ReferenceError('require() is supported only in the Node.JS environment'); +} + +function stubRequire(): NodeJS.Require { + function req() { + requireUnavailable(); + } + + const descriptors: Dictionary = { + resolve: { + enumerable: true, + get: requireUnavailable, + }, + + main: { + enumerable: true, + get: requireUnavailable, + }, + + cache: { + enumerable: true, + get: requireUnavailable, + }, + + extensions: { + enumerable: true, + get: requireUnavailable, + }, + }; + + return Object.defineProperties(req, descriptors); +} + +function proxyRequire(source: string): NodeJS.Require { + const actualRequire = require; + function req(p: string) { + if (p.startsWith('.')) { + p = join(source, '..', stripRoot(p)); + } + + return actualRequire.call(null, p); + } + + return Object.defineProperties(req, Object.getOwnPropertyDescriptors(actualRequire)); +} + +const isRequiredSupported = + typeof require === 'function' && + typeof require.main === 'object' && + require.main !== null && + 'paths' in require.main && + 'cache' in require; + +const createRequire = (source: string | null): NodeJS.Require => { + if (!isRequiredSupported) { + return stubRequire(); + } + + if (source === null) { + return require; + } + + return proxyRequire(source); +}; + const createDefine = (exports: CJSExport) => { const define = (nameOrFactory: string | string[] | Function, factory: Function): Optional => { if (typeof nameOrFactory === 'function') { @@ -32,15 +99,17 @@ const isESCJSCompatibleExport = (obj: unknown): obj is ESCJSCompatibleExport => // note: this code is hand-crafted and cover cases we want to support // be aware of using it in your own project if you need to support a variety of module systems -export const evaluateExport = (body: string): Function => { +export const evaluateExport = (body: string, source: string | null): Function => { + const req = createRequire(source); const mod: CJSExport = { exports: {}, + require: req, }; const exports: ESCJSCompatibleExport | unknown = {}; const root: ContextExport = {}; const define = createDefine(mod); - Function('module, exports, define', String(body)).call(root, mod, exports, define); + Function('module, exports, define, require', String(body)).call(root, mod, exports, define, req); let maybeFn: unknown; @@ -61,8 +130,13 @@ export const evaluateExport = (body: string): Function => { return maybeFn; }; -export const compileExportedFunction = (code: string, name: string, schema: JSONSchema | null) => { - const exportedFn = evaluateExport(code) as IFunction; +export const compileExportedFunction = ( + code: string, + name: string, + source: string | null, + schema: JSONSchema | null, +) => { + const exportedFn = evaluateExport(code, source) as IFunction; const fn = schema !== null ? decorateIFunctionWithSchemaValidation(exportedFn, schema) : exportedFn; diff --git a/src/rulesets/mergers/__tests__/functions.jest.test.ts b/src/rulesets/mergers/__tests__/functions.jest.test.ts index 8c73da171..93a92bce0 100644 --- a/src/rulesets/mergers/__tests__/functions.jest.test.ts +++ b/src/rulesets/mergers/__tests__/functions.jest.test.ts @@ -18,6 +18,7 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'foo()', schema: null, + source: null, }, }; @@ -37,6 +38,7 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'foo()', schema: null, + source: null, }); }); @@ -46,6 +48,7 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'foo()', schema: null, + source: null, }, }; const sources: RulesetFunctionCollection = { @@ -53,6 +56,7 @@ describe('Ruleset functions merging', () => { name: 'foo.c', code: 'foo.a()', schema: null, + source: 'foo', }, }; @@ -62,11 +66,13 @@ describe('Ruleset functions merging', () => { name: 'foo.c', code: 'foo.a()', schema: null, + source: 'foo', }); expect(target).toHaveProperty('foo', { name: 'foo.c', ref: 'random-id-0', schema: null, + source: 'foo', }); }); @@ -76,6 +82,7 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'foo()', schema: null, + source: null, }, }; @@ -84,11 +91,13 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'a.foo.c();', schema: null, + source: null, }, bar: { name: 'bar', code: 'bar()', schema: null, + source: null, }, }; @@ -112,11 +121,13 @@ describe('Ruleset functions merging', () => { name: 'foo', code: 'a.foo.c();', schema: null, + source: null, }); expect(target).toHaveProperty('random-id-1', { name: 'bar', code: 'bar()', schema: null, + source: null, }); }); diff --git a/src/rulesets/mergers/functions.ts b/src/rulesets/mergers/functions.ts index 020a1a64f..be7b52dbf 100644 --- a/src/rulesets/mergers/functions.ts +++ b/src/rulesets/mergers/functions.ts @@ -18,6 +18,7 @@ export function mergeFunctions( name: def.name, schema: def.schema, ref: newName, + source: def.source, }; } diff --git a/src/rulesets/reader.ts b/src/rulesets/reader.ts index 166a58526..86eef2b1f 100644 --- a/src/rulesets/reader.ts +++ b/src/rulesets/reader.ts @@ -130,15 +130,17 @@ const createRulesetProcessor = ( rulesetFunctions.map(async fn => { const fnName = Array.isArray(fn) ? fn[0] : fn; const fnSchema = Array.isArray(fn) ? fn[1] : null; + const source = await findFile(rulesetFunctionsBaseDir, `./${fnName}.js`); try { resolvedFunctions[fnName] = { name: fnName, - code: await readFile(await findFile(rulesetFunctionsBaseDir, `./${fnName}.js`), { + code: await readFile(source, { timeout: readOpts && readOpts.timeout, encoding: 'utf8', }), schema: fnSchema, + source, }; } catch (ex) { console.warn(`Function '${fnName}' could not be loaded: ${ex.message}`); diff --git a/src/spectral.ts b/src/spectral.ts index 120bc7a2d..2bd8b7e92 100644 --- a/src/spectral.ts +++ b/src/spectral.ts @@ -156,7 +156,7 @@ export class Spectral { this.setFunctions( Object.entries(ruleset.functions).reduce( - (fns, [key, { code, ref, name, schema }]) => { + (fns, [key, { code, ref, name, source, schema }]) => { if (code === void 0) { if (ref !== void 0) { ({ code } = ruleset.functions[ref]); @@ -168,7 +168,7 @@ export class Spectral { return fns; } - fns[key] = compileExportedFunction(code, name, schema); + fns[key] = compileExportedFunction(code, name, source, schema); return fns; }, { diff --git a/src/types/ruleset.ts b/src/types/ruleset.ts index 1651a1298..19fcadeeb 100644 --- a/src/types/ruleset.ts +++ b/src/types/ruleset.ts @@ -15,6 +15,7 @@ export interface IRulesetFunctionDefinition { ref?: string; schema: JSONSchema | null; name: string; + source: string | null; } export type RulesetFunctionCollection = Dictionary;