diff --git a/.scriptlintrc b/.scriptlintrc index b850461..6927002 100644 --- a/.scriptlintrc +++ b/.scriptlintrc @@ -1,3 +1,7 @@ { - "strict": true -} \ No newline at end of file + "strict": true, + "rules": { + "alphabetic-order": false, + "natural-order": true + } +} diff --git a/README.md b/README.md index 0f9a9ba..596d534 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Your `package.json`'s `"scripts"` section should… - _abstract script names from their implementation (`test`, not `jest`)_ - _use namespaces to categorize scripts (`"test:unit": "jest"`)_ - _use `:` as a namespace separator_ -- _have the scripts in alphabetic order_ +- _have the scripts in alphabetic or ["natural order"](https://github.com/peerigon/scriptlint/wiki/natural-order) - _have a trigger script for all hooks (ex: if you have `prefoobar`, there must be a `foobar` script)_ - _use `camelCase` for all script names_ - _not alias `devDependencies` (no `"jest": "jest"`)_ @@ -89,6 +89,9 @@ href="https://github.com/peerigon/scriptlint/wiki/uses-allowed-namespace">uses-a
  • alphabetic-order
  • +
  • natural-order +
  • correct-casing
  • no-aliases diff --git a/package.json b/package.json index 519b0be..2766dfe 100644 --- a/package.json +++ b/package.json @@ -27,8 +27,8 @@ "other:selfupdate": "updtr", "other:watch": "nodemon -e js,ts --watch src --exec 'run-p build:dev'", "prepublishOnly": "run-p build", - "pretest": "run-s build", "start": "node dist/index.js", + "pretest": "run-s build", "test": "run-s test:exports test:lint test:types test:unit:ci test:self", "test:exports": "ts-unused-exports tsconfig.json --ignoreFiles src/index.ts", "test:lint": "eslint ./src ./tests --ext js,ts,tsx", @@ -85,4 +85,4 @@ "cosmiconfig": "^7.0.0", "detect-indent": "^6.0.0" } -} +} \ No newline at end of file diff --git a/src/cliConfig.ts b/src/cliConfig.ts index fd33fa4..b2183e0 100644 --- a/src/cliConfig.ts +++ b/src/cliConfig.ts @@ -27,7 +27,8 @@ export default (argv: Array): CliConfig => { program.parse(argv); - const cliConfig: CliConfig = { packageFile }; + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const cliConfig: CliConfig = { packageFile: packageFile ?? process.cwd() }; if (program.fix !== undefined) { cliConfig.fix = program.fix; diff --git a/src/cliModule.ts b/src/cliModule.ts index 5f1dc78..2d8faab 100644 --- a/src/cliModule.ts +++ b/src/cliModule.ts @@ -28,6 +28,7 @@ export default (argv: Array) => { const userConfig = loadUserConfig(); const cliConfig = loadCliConfig(argv); + const config = { ...DEFAULT_CONFIG, ...{ json: false }, @@ -51,7 +52,7 @@ export default (argv: Array) => { writePackageScripts, readPackageScripts, } = userPackageScriptContext( - makePackageFilePath(config.packageFile ?? process.cwd()) + makePackageFilePath(config.packageFile) ); const scripts = readPackageScripts(config.ignoreScripts); diff --git a/src/constants.ts b/src/constants.ts index a19c8a3..a8e57af 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -8,7 +8,7 @@ export const DEFAULT_CONFIG: Config = { fix: false, json: false, config: false, - packageFile: undefined, + packageFile: process.cwd(), rules: {}, customRules: [], ignoreScripts: [], diff --git a/src/defaultRuleSets.ts b/src/defaultRuleSets.ts index 851a472..d9a32bb 100644 --- a/src/defaultRuleSets.ts +++ b/src/defaultRuleSets.ts @@ -1,6 +1,12 @@ import defaultRules from "./rules"; import { Rule } from "./types"; +export const optionalRules = ["natural-order"]; + +const strict = defaultRules + .map((r: Rule) => r.name) + .filter((r) => !optionalRules.includes(r)); + const ruleSets = { default: [ "mandatory-test", @@ -8,7 +14,8 @@ const ruleSets = { "mandatory-dev", "no-default-test", ], - strict: defaultRules.map((r: Rule) => r.name), + strict, + optionalRules, }; export default ruleSets; diff --git a/src/loadRules.ts b/src/loadRules.ts index 2d28126..1ebe448 100644 --- a/src/loadRules.ts +++ b/src/loadRules.ts @@ -1,4 +1,4 @@ -import defaultRuleSets from "./defaultRuleSets"; +import defaultRuleSets, {optionalRules} from "./defaultRuleSets"; import defaultRules from "./rules"; // Types import { Rule } from "./types"; @@ -32,31 +32,36 @@ const loadDefaultRulesFromSet = (strict: boolean): Array => { .filter((r): r is Rule => r !== null); }; +const loadOptionalRules = (): Array => { + return optionalRules + .map((name: string) => getRuleByName(defaultRules, name)) + .filter((r): r is Rule => r !== null); +}; + export const loadRulesFromRuleConfig = ( strict: boolean, rulesConfig?: RulesConfig, customRules?: Array ): Array => { + const optionalRules = loadOptionalRules(); const rules = loadDefaultRulesFromSet(strict); - const loadedCustomRules = - rulesConfig && customRules - ? customRules.filter((cr: Rule) => rulesConfig[cr.name]) - : []; - - const loadedRules = [...loadedCustomRules, ...rules]; - + // standard rulesets apply if (!rulesConfig) { - return loadedRules; + return rules; } - return loadedRules - .map((rule: Rule) => { - if (rule.name in rulesConfig && rulesConfig[rule.name] === false) { - return null; - } + // there's custom rules loaded + const loadedCustomRules = customRules + ? customRules.filter((cr: Rule) => rulesConfig[cr.name]) + : []; - return rule; - }) - .filter((r): r is Rule => r !== null); + // there's enabled optional rules + const enabledOptionalRules = optionalRules.filter( + ({ name }) => name in rulesConfig && Boolean(rulesConfig[name]) + ); + + return [...loadedCustomRules, ...enabledOptionalRules, ...rules].filter( + ({ name }) => !(name in rulesConfig) || rulesConfig[name] !== false + ); }; diff --git a/src/module.ts b/src/module.ts index 08135de..fc4fe7a 100644 --- a/src/module.ts +++ b/src/module.ts @@ -35,7 +35,7 @@ export default (moduleConfig: Partial) => { if (!moduleConfig.packageScripts && moduleConfig.packageFile) { const { readPackageScripts } = userPackageScriptContext( - makePackageFilePath(config.packageFile ?? "") + makePackageFilePath(config.packageFile) ); scripts = readPackageScripts(config.ignoreScripts); diff --git a/src/rules/index.ts b/src/rules/index.ts index 3fdae91..b106bf2 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -6,6 +6,7 @@ import noAliases from "./no-aliases"; import prePostTriggerDefined from "./prepost-trigger-defined"; import usesAllowedNamespace from "./uses-allowed-namespace"; import alphabeticOrder from "./alphabetic-order"; +import naturalOrder from "./natural-order"; import { Rule } from "../types"; const rules: Array = [ @@ -18,6 +19,7 @@ const rules: Array = [ usesAllowedNamespace, prePostTriggerDefined, alphabeticOrder, + naturalOrder, makeForbidUnixOperators(/rm /, "rm -rf", "rimraf"), makeForbidUnixOperators( / && /, diff --git a/src/rules/natural-order.ts b/src/rules/natural-order.ts new file mode 100644 index 0000000..2b6e90b --- /dev/null +++ b/src/rules/natural-order.ts @@ -0,0 +1,84 @@ +import { PackageScripts } from "../types"; +import { objectFromEntries } from "../utils"; + +type EntryInfo = { + order: number; + namespace: string; + entry: [string, string]; +}; + +const prepareEntryInfo = (entry: [string, string]) => { + const name = entry[0]; + const namespace = name.split(":")[0]; + + if (name.startsWith("pre")) { + return { + order: 0, + namespace: namespace.substr(3), + entry, + }; + } + if (name.startsWith("post")) { + return { + order: 2, + namespace: namespace.substr(4), + entry, + }; + } + + return { + order: 1, + namespace, + entry, + }; +}; + +export const sortScripts = (scripts: PackageScripts): PackageScripts => { + // prepare namespace and order info + const entries = Object.entries(scripts).map(prepareEntryInfo); + + return objectFromEntries( + entries + // make unique namespace groups + .map((e: EntryInfo) => e.namespace) + .filter( + (name: string, i: number, a: Array) => + a.indexOf(name) === i + ) + .sort() + .map((name: string) => + entries.filter((e: EntryInfo) => e.namespace === name) + ) + // sort inside the group + .map((group: Array) => + // sort `1-title` vs `2-title` etc. + group.sort((a: EntryInfo, b: EntryInfo) => + `${a.order}-${a.entry[0]}`.localeCompare( + `${b.order}-${b.entry[0]}` + ) + ) + ) + // flatten array + .reduce( + (flatted: Array, group: Array) => [ + ...flatted, + ...group, + ], + [] + ) + // reduce to entries again + .map((f: EntryInfo) => f.entry) + ); +}; + +export default { + name: "natural-order", + isObjectRule: true, + message: "scripts must be in 'natural' order", + validate: (scripts: PackageScripts) => { + const sorted = sortScripts(scripts); + + return Object.keys(sorted).join("|") === Object.keys(scripts).join("|"); + }, + fix: (scripts: PackageScripts) => sortScripts(scripts), +}; diff --git a/src/types.ts b/src/types.ts index 68d8296..16bca29 100644 --- a/src/types.ts +++ b/src/types.ts @@ -17,7 +17,7 @@ export type PackageScripts = { export type Config = { strict: boolean; - packageFile?: string; + packageFile: string; packageScripts?: PackageScripts; fix: boolean; json: boolean; diff --git a/src/utils.ts b/src/utils.ts index c12a3a1..551db4b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -74,3 +74,10 @@ export const patchScriptObjectEntry = ( return k === fromKey ? [toKey, value] : [k, v]; }) ); + +type ObjectFromEntries = {[key: string]: any}; +export const objectFromEntries = (iterable: Array<[string, T]>): ObjectFromEntries => iterable.reduce((obj, [key, value]) => { + obj[key] = value; + + return obj +}, {} as ObjectFromEntries) diff --git a/tests/cliConfig.test.ts b/tests/cliConfig.test.ts index e812d37..4e882cf 100644 --- a/tests/cliConfig.test.ts +++ b/tests/cliConfig.test.ts @@ -51,7 +51,7 @@ describe("cliConfig.ts", () => { "-f", ]); - expect(packageFile).not.toBeDefined(); + expect(packageFile).toEqual(process.cwd()); expect(config).toEqual({ fix: true, diff --git a/tests/loadRules.test.ts b/tests/loadRules.test.ts index 3309ac4..76de0cd 100644 --- a/tests/loadRules.test.ts +++ b/tests/loadRules.test.ts @@ -1,5 +1,6 @@ import { loadRulesFromRuleConfig, getRuleByName } from "../src/loadRules"; import defaultRules from "../src/rules"; +import {optionalRules} from "../src/defaultRuleSets"; describe("loadRules.ts", () => { const defaultRulesLoaded = loadRulesFromRuleConfig(false); @@ -28,8 +29,19 @@ describe("loadRules.ts", () => { ).toBe(customRule.name); }); + it("loads optional rules", () => { + const rulesWithCustomRule = loadRulesFromRuleConfig( + false, + { + "natural-order": true, + } + ); + + expect(rulesWithCustomRule.map(r => r.name)[0]).toBe("natural-order"); + }); + it("loads correct amount of rules", () => { - expect(strictRulesLoaded.length).toBe(defaultRules.length); + expect(strictRulesLoaded.length).toBe(defaultRules.length - optionalRules.length); }); test("getRuleByName() nulls on unknown name", () => { diff --git a/tests/rules/natural-order.test.ts b/tests/rules/natural-order.test.ts new file mode 100644 index 0000000..e81fbf6 --- /dev/null +++ b/tests/rules/natural-order.test.ts @@ -0,0 +1,76 @@ +import rule, { sortScripts } from "../../src/rules/natural-order"; + +describe("natural-order.ts", () => { + const scriptsUnsorted = { + postbuild: "echo 1", + build: "echo 1", + "test:lint": "echo 1", + "test:lint:scripts:es7": "echo 1", + dev: "echo 1", + posttest: "echo 1", + "other:update:dependencies": "echo 1", + preinstall: "echo 1", + prebuild: "echo 1", + "other:update": "echo 1", + "other:update:typings": "echo 1", + "test:lint:scripts": "echo 1", + "test:lint:scripts:babel": "echo 1", + "test:lint:styles:scss": "echo 1", + publish: "echo 1", + prepublishOnly: "echo 1", + "build:cleanup": "echo 1", + "test:lint:styles": "echo 1", + test: "echo 1", + start: "echo 1", + pretest: "echo 1", + "pretest:lint:scripts:es7": "echo 1", + "test:lint:styles:postcss": "echo 1", + }; + + const scriptsSorted = { + prebuild: "echo 1", + build: "echo 1", + "build:cleanup": "echo 1", + postbuild: "echo 1", + dev: "echo 1", + preinstall: "echo 1", + "other:update": "echo 1", + "other:update:dependencies": "echo 1", + "other:update:typings": "echo 1", + publish: "echo 1", + prepublishOnly: "echo 1", + start: "echo 1", + pretest: "echo 1", + "pretest:lint:scripts:es7": "echo 1", + test: "echo 1", + "test:lint": "echo 1", + "test:lint:scripts": "echo 1", + "test:lint:scripts:babel": "echo 1", + "test:lint:scripts:es7": "echo 1", + "test:lint:styles": "echo 1", + "test:lint:styles:postcss": "echo 1", + "test:lint:styles:scss": "echo 1", + posttest: "echo 1", + }; + + describe("validate()", () => { + it("should validate correctly", () => { + expect(rule.validate({})).toBe(true); + expect(rule.validate(scriptsUnsorted)).toBe(false); + expect(rule.validate(scriptsSorted)).toBe(true); + }); + }); + + it("should fix issues", () => { + expect(rule.validate(rule.fix(scriptsUnsorted))).toBe(true); + }); + + describe("sortScripts()", () => { + it("should sort correctly", () => { + expect(sortScripts({})).toEqual({}); + expect(Object.keys(sortScripts(scriptsUnsorted))).toEqual( + Object.keys(scriptsSorted) + ); + }); + }); +}); diff --git a/tests/userConfig.test.ts b/tests/userConfig.test.ts index 5727e95..c67cb26 100644 --- a/tests/userConfig.test.ts +++ b/tests/userConfig.test.ts @@ -5,6 +5,7 @@ const validConfig = { strict: true, fix: false, json: false, + packageFile: "/foo/bar/baz", config: false, rules: { foo: "bar" }, ignoreScripts: ["foo"], @@ -41,7 +42,7 @@ describe("userConfig.ts", () => { test("loadConfig()", () => { const loaded = loadConfig(); - expect(loaded).toMatchSnapshot(); + expect({...loaded, packageFile: undefined}).toMatchSnapshot(); }); test("loadConfig() with config missing", () => {