Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ruleset-migrator): implement ruleset migrator #1698

Merged
merged 17 commits into from
Jul 1, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ commands:

test-node:
steps:
- run: npx lerna run pretest
- run:
name: Run node tests
command: yarn test.jest --coverage --maxWorkers=2
Expand Down Expand Up @@ -119,6 +120,7 @@ jobs:
steps:
- checkout
- install-and-build
- run: npx lerna run pretest
- run:
name: Run browser tests
command: yarn test.karma
Expand Down
8 changes: 8 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ module.exports = {
},
testMatch: ['<rootDir>/packages/functions/src/**/__tests__/**/*.{test,spec}.ts'],
},
{
...projectDefault,
displayName: {
name: '@stoplight/spectral-ruleset-migrator',
color: 'blueBright',
},
testMatch: ['<rootDir>/packages/ruleset-migrator/src/**/__tests__/**/*.{test,spec}.ts'],
},
{
...projectDefault,
displayName: '@stoplight/spectral-parsers',
Expand Down
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
},
"scripts": {
"clean": "rimraf packages/*/{dist,.cache}",
"prebuild": "lerna run prebuild",
"build": "tsc --build ./tsconfig.build.json",
"postbuild": "lerna run postbuild",
"lint": "yarn lint.prettier && yarn lint.eslint && yarn lint.changelog",
"lint.changelog": "kacl lint",
"lint.fix": "yarn lint.prettier --write && yarn lint.eslint --fix",
"lint.eslint": "eslint --cache --cache-location .cache/ --ext=.js,.mjs,.ts packages test-harness",
"lint.prettier": "prettier --ignore-path .eslintignore --ignore-unknown --check packages/core/src/meta/*.json docs/**/*.md README.md",
"pretest": "lerna run pretest",
"test": "yarn test.karma && yarn test.jest --coverage --maxWorkers=2",
"test.harness": "jest -c ./test-harness/jest.config.js",
"test.jest": "jest --silent",
Expand Down
1 change: 1 addition & 0 deletions packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"@stoplight/json": "3.15.0",
"@stoplight/path": "1.3.2",
"@stoplight/spectral-core": "^0.0.0",
"@stoplight/spectral-ruleset-migrator": "^0.0.0",
"@stoplight/spectral-parsers": "^0.0.0",
"@stoplight/spectral-ref-resolver": "^0.0.0",
"@stoplight/spectral-runtime": "^0.0.0",
Expand Down
17 changes: 17 additions & 0 deletions packages/cli/src/services/__tests__/__fixtures__/ruleset.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"rules": {
"info-matches-stoplight": {
"message": "Info must contain Stoplight",
"given": "$.info",
"recommended": true,
"type": "style",
"then": {
"field": "title",
"function": "pattern",
"functionOptions": {
"match": "Stoplight"
}
}
}
}
}
14 changes: 14 additions & 0 deletions packages/cli/src/services/__tests__/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,20 @@ describe('Linter service', () => {
);
});
});

describe('given legacy ruleset', () => {
it('outputs warnings', async () => {
const output = await run(`lint ${validOas3SpecPath} -r ${join(__dirname, '__fixtures__/ruleset.json')}`);
expect(output).toEqual(expect.arrayContaining([expect.objectContaining({ code: 'info-matches-stoplight' })]));
expect(output).toEqual(
expect.not.arrayContaining([
expect.objectContaining({
message: 'Info object should contain `contact` object',
}),
]),
);
});
});
});

describe('when loading specification files from web', () => {
Expand Down
35 changes: 31 additions & 4 deletions packages/cli/src/services/linter/utils/getRuleset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ import { Optional } from '@stoplight/types';
import { Ruleset, RulesetDefinition } from '@stoplight/spectral-core';
import * as fs from 'fs';
import * as path from '@stoplight/path';
import { isAbsolute } from '@stoplight/path';
import * as process from 'process';
import { extname } from '@stoplight/path';
import { migrateRuleset } from '@stoplight/spectral-ruleset-migrator';

// eslint-disable-next-line @typescript-eslint/require-await
const AsyncFunction = (async (): Promise<void> => void 0).constructor as FunctionConstructor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I even want to know?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, long story short - in rare circumstances a "top-level" await might be inserted.
This won't be a problem soon-ish once we hop on ESM, but for the time being it's a plain CommonJS.
The above is needed for us to be able to do

AsyncFunction('module.exports = await /* the rest of the code goes here */)'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Narrator: it turns out he really did not want to know :trollface:


async function getDefaultRulesetFile(): Promise<Optional<string>> {
const cwd = process.cwd();
Expand All @@ -19,17 +23,40 @@ async function getDefaultRulesetFile(): Promise<Optional<string>> {
export async function getRuleset(rulesetFile: Optional<string>): Promise<Ruleset> {
if (rulesetFile === void 0) {
rulesetFile = await getDefaultRulesetFile();
} else if (!isAbsolute(rulesetFile)) {
} else if (!path.isAbsolute(rulesetFile)) {
rulesetFile = path.join(process.cwd(), rulesetFile);
}

if (rulesetFile === void 0) {
return new Ruleset({ rules: {} });
}

const ruleset = (await import(rulesetFile)) as { default: RulesetDefinition } | RulesetDefinition;
let ruleset;

if (/(json|ya?ml)$/.test(extname(rulesetFile))) {
const m: { exports?: RulesetDefinition } = {};
const paths = [path.dirname(rulesetFile)];

await AsyncFunction(
'module, require',
await migrateRuleset(rulesetFile, {
cwd: process.cwd(),
format: 'commonjs',
fs,
}),
// eslint-disable-next-line @typescript-eslint/no-var-requires
)(m, (id: string) => require(require.resolve(id, { paths })) as unknown);

ruleset = m.exports;
} else {
const imported = (await import(rulesetFile)) as { default: unknown } | unknown;
ruleset =
typeof imported === 'object' && imported !== null && 'default' in imported
? (imported as Record<'default', unknown>).default
: imported;
}

return new Ruleset('default' in ruleset ? ruleset.default : ruleset, {
return new Ruleset(ruleset, {
severity: 'recommended',
source: rulesetFile,
});
Expand Down
1 change: 1 addition & 0 deletions packages/cli/tsconfig.build.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
{ "path": "../core/tsconfig.build.json" },
{ "path": "../parsers/tsconfig.build.json" },
{ "path": "../ref-resolver/tsconfig.build.json" },
{ "path": "../ruleset-migrator/tsconfig.build.json" },
{ "path": "../runtime/tsconfig.build.json" },
]
}
10 changes: 3 additions & 7 deletions packages/core/src/ruleset/__tests__/ruleset.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,9 @@ describe('Ruleset', () => {

describe('error handling', () => {
it('given empty ruleset, should throw a user friendly error', () => {
expect(
() =>
new Ruleset(
// @ts-expect-error: invalid ruleset
{},
),
).toThrowError(new RulesetValidationError('Ruleset must have rules or extends or overrides defined'));
expect(() => new Ruleset({})).toThrowError(
new RulesetValidationError('Ruleset must have rules or extends or overrides defined'),
);
});
});

Expand Down
25 changes: 16 additions & 9 deletions packages/core/src/ruleset/ruleset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Format } from './format';
import { mergeRule } from './mergers/rules';
import { DEFAULT_PARSER_OPTIONS } from '..';
import { mergeRulesets } from './mergers/rulesets';
import { isPlainObject } from '@stoplight/json';

const STACK_SYMBOL = Symbol('@stoplight/spectral/ruleset/#stack');
const DEFAULT_RULESET_FILE = /^\.?spectral\.(ya?ml|json|m?js)$/;
Expand All @@ -28,10 +29,24 @@ export class Ruleset {
public readonly formats = new Set<Format>();
public readonly overrides: RulesetOverridesDefinition | null;
public readonly aliases: RulesetAliasesDefinition | null;
public readonly definition: RulesetDefinition;

readonly #context: RulesetContext & { severity: FileRulesetSeverityDefinition };

constructor(public readonly definition: RulesetDefinition, context?: RulesetContext) {
constructor(readonly maybeDefinition: unknown, context?: RulesetContext) {
let definition: RulesetDefinition;
if (isPlainObject(maybeDefinition) && 'extends' in maybeDefinition) {
const { extends: _, ...def } = maybeDefinition;
// we don't want to validate extends - this is going to happen later on (line 29)
assertValidRuleset({ extends: [], ...def });
definition = maybeDefinition as RulesetDefinition;
} else {
assertValidRuleset(maybeDefinition);
definition = maybeDefinition;
}

this.definition = definition;

this.#context = {
severity: 'recommended',
...context,
Expand All @@ -43,14 +58,6 @@ export class Ruleset {

stack.set(this.definition, this);

if ('extends' in definition) {
const { extends: _, ...def } = definition;
// we don't want to validate extends - this is going to happen later on (line 29)
assertValidRuleset({ extends: [], ...def });
} else {
assertValidRuleset(definition);
}

this.extends =
'extends' in definition
? (Array.isArray(definition.extends) ? definition.extends : [definition.extends]).reduce<Ruleset[]>(
Expand Down
93 changes: 93 additions & 0 deletions packages/ruleset-migrator/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# @stoplight/spectral-ruleset-migrator

This project serves as a converter between the legacy ruleset format and a new one.
It's used internally, albeit it can be used externally too, also in browsers.

For the time being there are two output formats supported: commonjs & esm.
The migrator loads the ruleset, apply a number of conversions and return a valid JS code that can be executed later on.

## Examples

```yaml
# .spectral.yaml
extends: spectral:oas
formats: [oas2, json-schema-loose]
rules:
oas3-schema: warning
valid-type:
message: Type must be valid
given: $..type
then:
function: pattern
functionOptions:
mustMatch: ^(string|number)$
```

```js
// .spectral.js (CommonJS)
const { oas: oas } = require("@stoplight/spectral-rulesets");
const { oas2: oas2, jsonSchemaLoose: jsonSchemaLoose } = require("@stoplight/spectral-formats");
const { pattern: pattern } = require("@stoplight/spectral-functions");
module.exports = {
extends: oas,
formats: [oas2, jsonSchemaLoose],
rules: {
"oas3-schema": "warning",
"valid-type": {
message: "Type must be valid",
given: "$..type",
then: {
function: pattern,
functionOptions: {
mustMatch: "^(string|number)$",
},
},
},
},
};
```

```js
// .spectral.js (ES Module)
import { oas } from "@stoplight/spectral-rulesets";
import { oas2, jsonSchemaLoose } from "@stoplight/spectral-formats";
import { pattern } from "@stoplight/spectral-functions";
export default {
extends: oas,
formats: [oas2, jsonSchemaLoose],
rules: {
"oas3-schema": "warning",
"valid-type": {
message: "Type must be valid",
given: "$..type",
then: {
function: pattern,
functionOptions: {
mustMatch: "^(string|number)$",
},
},
},
},
};
```

## Usage

### Via @stoplight/spectral-cli

```ts
npx @stoplight/spectral-cli ruleset migrate
```

### Programmatically

```ts
const { migrateRuleset } = require("@stoplight/spectral-ruleset-migrator");
const fs = require("fs");
const path = require("path");

migrateRuleset(path.join(__dirname, "spectral.json"), {
fs,
format: "commonjs", // esm available too, but not recommended for now
}).then(fs.promises.writeFile.bind(fs.promises, path.join(__dirname, ".spectral.js")));
```
43 changes: 43 additions & 0 deletions packages/ruleset-migrator/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"name": "@stoplight/spectral-ruleset-migrator",
"version": "0.0.0",
"homepage": "https://github.com/stoplightio/spectral",
"bugs": "https://github.com/stoplightio/spectral/issues",
"author": "Stoplight <support@stoplight.io>",
"engines": {
"node": ">=12"
},
"license": "Apache-2.0",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"files": [
"/dist"
],
"browser": {
"./dist/require-resolve.js": false
},
"repository": {
"type": "git",
"url": "https://github.com/stoplightio/spectral.git"
},
"dependencies": {
"@stoplight/json": "3.15.0",
"@stoplight/path": "1.3.2",
"@stoplight/spectral-functions": "^0.0.0",
"@stoplight/spectral-runtime": "^0.0.0",
"@stoplight/types": "12.3.0",
"@stoplight/yaml": "4.2.2",
"ajv": "^8.6.0",
"ast-types": "0.14.2",
"astring": "^1.7.5"
},
"devDependencies": {
"json-schema-to-typescript": "^10.1.4",
"memfs": "^3.2.2",
"prettier": "^2.3.2"
},
"scripts": {
"pretest": "ts-node ./scripts/generate-test-fixtures.ts && yarn prebuild",
"prebuild": "ts-node ./scripts/compile-schemas.ts"
}
}
14 changes: 14 additions & 0 deletions packages/ruleset-migrator/scripts/compile-schemas.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import * as fs from 'fs';
import * as path from '@stoplight/path';
import { compile } from 'json-schema-to-typescript';

import schema from '../src/validation/schema';

compile(schema, 'Ruleset', {
bannerComment: '/*eslint-disable*/',
style: {
singleQuote: true,
},
}).then(ts => {
return fs.promises.writeFile(path.join(__dirname, '../src/validation/types.ts'), ts);
});
Loading