diff --git a/README.md b/README.md index 5fc8b37..ac64542 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ npx --no -- commitlint --edit "$1" - **type** — one of the allowed types listed below. - **scope** — a ClickUp-style task key (e.g. `PROJ-123`), `NO-TASK`, a GitHub issue number (e.g. `#42`), or a [dependency scope](#dependency-scopes). -- **subject** — short description in sentence case. +- **subject** — short description in sentence case, optionally preceded by a [tag](#subject-tags). ### Examples @@ -81,38 +81,54 @@ Dependency updates have no task behind them, so the scope names the kind of depe ``` update(wp-plugin): Update translatepress-multilingual to v3.2.4 update(wp-theme): Update ollie-pro to v2.6.1 +remove(wp-plugin): Remove akismet build(deps): Update npm-run-all2 to v9.0.3 build(deps-dev): Update svgo to v3.3.4 build(composer): Update humbug/php-scoper to v0.18.19 chore(actions): Update actions/checkout to v7 ``` -Allowed: `deps`, `deps-dev`, `wp-plugin`, `wp-theme`, `npm`, `composer`, `actions`, `wporg`, `linchpin`. These are emitted by [linchpin/renovatebot-config](https://github.com/linchpin/renovatebot-config); anything else must still be a task key, `NO-TASK`, or an issue number. +`wp-plugin` and `wp-theme` are **scopes**, never types. The type says what happened — `update` for a bump, `remove` for a package that is gone — and the scope says what it happened to. + +Allowed: `deps`, `deps-dev`, `wp-plugin`, `wp-theme`, `npm`, `composer`, `actions`, `wporg`, `linchpin`. Anything else must still be a task key, `NO-TASK`, or an issue number. + +[linchpin/renovatebot-config](https://github.com/linchpin/renovatebot-config) emits `wp-plugin`, `wp-theme`, `deps`, `npm` and `composer`. `wporg` and `linchpin` are kept for hand-written commits — a package's registry now travels in a [tag](#subject-tags) instead, which leaves the scope free to say what kind of thing was updated. `build` is the [Angular convention's type for external dependencies](https://www.conventionalcommits.org/en/v1.0.0-beta.4/), and `update` reads better for a WordPress plugin bump — both are valid here. -### `wp-plugin` / `wp-theme` as a type instead of a scope +### Subject tags -`wp-plugin` and `wp-theme` are also valid **types**, not just scopes: +A subject may open with an optional bracketed tag, carrying context the scope slot has no room for. The convention is a package's registry: ``` -wp-plugin(wporg): Update akismet to v5.3 -wp-plugin(linchpin): Update some-plugin to v3.0 - Major -wp-theme(deps): Update twentytwentyfour to v2.0 +update(wp-plugin): [.org] Update akismet to v5.3 +update(wp-plugin): [packagist] Update gravityforms to v3 +update(wp-theme): [.org] Update ollie-pro to v2.6.1 ``` -This exists for one reason: [release-please](https://github.com/googleapis/release-please)'s `changelog-sections` groups strictly by commit **type** — `changelog-sections[].type` is the only key that schema offers, there is no scope key — so a repo that wants WordPress plugin and theme updates in their own changelog section (rather than folded into whatever section `update` or `build` maps to) has no way to get one except by making `wp-plugin`/`wp-theme` the type. `wporg` and `linchpin` are then available as scopes to say which registry the package came from. +`[.org]` is anything public — [wp-packages.org](https://wp-packages.org) and [wpackagist.org](https://wpackagist.org) are two routes to the same wordpress.org packages, so they share a tag. `[packagist]` is our own [packagist.linchpin.com](https://packagist.linchpin.com). Both are emitted by [linchpin/renovatebot-config](https://github.com/linchpin/renovatebot-config). + +The tag is not an enumerated list — anything matching `[\w.\-]+` is accepted, so `[hotfix]` or `[wp-packages]` work equally well. It is a label for whoever reads the log, not a routing key. What the charset buys you is that a tag stays a label: `[see PROJ-1 for why]` is rejected, and so is an unclosed `[`, which would otherwise swallow the rest of the header. + +Tags sit outside the captured subject, so `subject-case` and friends judge the sentence rather than the label. [release-please](https://github.com/googleapis/release-please) parses with its own pattern and keeps the tag visible in the changelog bullet: + +```markdown +### Changes to Existing Features 💅 + +* **wp-plugin:** [.org] Update akismet to v5.3 +* **wp-plugin:** [packagist] Update gravityforms to v3 +``` -Both forms lint cleanly. Use the scope form (`update(wp-plugin):`) unless a repo's `release-please-config.json` specifically defines a dedicated section for the `wp-plugin`/`wp-theme` type — [linchpin/renovatebot-config](https://github.com/linchpin/renovatebot-config) is the current example that does. +Because release-please groups strictly by commit **type** — `changelog-sections[].type` is the only key its schema offers, there is no scope key — WordPress updates share whatever section `update` maps to rather than getting one of their own. The scope renders as the bold prefix on each bullet, which is what keeps plugin and theme lines apart inside it. ## Rules | Rule | Level | Description | | --- | --- | --- | -| `type-enum` | error | Type must be one of: `add`, `improve`, `build`, `chore`, `ci`, `docs`, `feat`, `feature`, `fix`, `perf`, `refactor`, `remove`, `revert`, `style`, `test`, `update`, `wp-plugin`, `wp-theme` | +| `type-enum` | error | Type must be one of: `add`, `improve`, `build`, `chore`, `ci`, `docs`, `feat`, `feature`, `fix`, `perf`, `refactor`, `remove`, `revert`, `style`, `test`, `update` | | `subject-case` | warning | Subject must be in sentence-case | -The config also sets a custom `parserPreset.parserOpts.headerPattern` that enforces the scope format. +The config also sets a custom `parserPreset.parserOpts.headerPattern` that enforces the scope format and strips an optional subject tag. ### Failure messages diff --git a/index.js b/index.js index 19a37b2..3cf05cb 100644 --- a/index.js +++ b/index.js @@ -1,21 +1,42 @@ 'use strict'; -// wp-plugin and wp-theme are deliberately in both TYPES and DEP_SCOPES. As a scope -// (`update(wp-plugin): ...`) they keep a WordPress update legible in a mostly automated -// log. As a type (`wp-plugin(wporg): ...`) they let a repo route WordPress plugin and -// theme updates to their own release-please changelog section - `changelog-sections[].type` -// is the only key that schema offers, so a dedicated section is only reachable through -// type. Neither usage is deprecated; pick whichever a repo's release-please-config needs. -const TYPES = ['add', 'improve', 'build', 'chore', 'ci', 'docs', 'feat', 'feature', 'fix', 'perf', 'refactor', 'remove', 'revert', 'style', 'test', 'update', 'wp-plugin', 'wp-theme']; +// wp-plugin and wp-theme are scopes, not types. The type says what happened - `update` for a +// bump, `remove` for a package that is gone - and the scope says what it happened to. +// +// They were briefly types as well, in 1.3.0, so that release-please could give WordPress +// updates their own changelog section: `changelog-sections[].type` is the only key that schema +// offers, so a dedicated section is reachable through type and nothing else. The team chose the +// scope form regardless, which means WordPress updates share whatever section `update` maps to. +// That is the accepted trade - the scope still renders as the bold prefix on each changelog +// bullet, so plugin and theme lines stay apart inside the shared section. +const TYPES = ['add', 'improve', 'build', 'chore', 'ci', 'docs', 'feat', 'feature', 'fix', 'perf', 'refactor', 'remove', 'revert', 'style', 'test', 'update']; // Dependency updates have no task behind them, so the scope slot carries the kind of // dependency instead - which is what the wider ecosystem does too (`build(deps)`, // `chore(deps-dev)`). wp-plugin and wp-theme keep WordPress updates obvious at a glance -// in a log that is mostly automated. wporg and linchpin name a WordPress package's source -// for repos that promote wp-plugin/wp-theme to a type instead, and so need a scope other -// than the type itself. deps-dev precedes deps so the longer one wins. +// in a log that is mostly automated. wporg and linchpin are kept for hand-written commits; +// nothing emits them now that a package's source travels in a TAG instead, which leaves the +// scope free to say what kind of thing was updated. deps-dev precedes deps so the longer +// one wins. const DEP_SCOPES = ['deps-dev', 'deps', 'wp-plugin', 'wp-theme', 'npm', 'composer', 'actions', 'wporg', 'linchpin']; +// An optional bracketed tag between the colon and the subject, carrying context the scope has +// no room for - `[.org]` for a wordpress.org package, `[packagist]` for packagist.linchpin.com. +// +// Deliberately not an enumerated list: this is a label for a reader, not a routing key. The +// charset is what keeps it a label - it stops `[see PROJ-1 for why]` from turning the tag into +// a sentence, and stops an unclosed bracket from swallowing the rest of the header. +// +// Non-capturing in headerPattern on purpose. The subject group has to keep capturing the +// sentence, so subject-case reads `Update akismet to v5`, not `[.org] Update akismet to v5`. +const TAG_CHARS = '[\\w.\\-]+'; +const TAG = `(?:\\[${TAG_CHARS}\\]\\s*)?`; + +// The same shape, for explain(). `[^\]]*` rather than TAG_CHARS so that a malformed tag is +// caught and quoted back instead of silently failing to match; group 1 is the tag body. +const TAG_AT_START = /^\[([^\]]*)\](\s*)/; +const VALID_TAG = new RegExp(`^${TAG_CHARS}$`); + // A ClickUp-style task key, NO-TASK, a GitHub issue number, or a dependency scope. const SCOPE = new RegExp(`^(?:[A-Z]+-\\d+|NO-TASK|#\\d+|${DEP_SCOPES.join('|')})$`); @@ -27,6 +48,7 @@ const FORMAT = '(): '; const SCOPE_HELP = 'a task key such as PROJ-123, NO-TASK, a GitHub issue number such as #42, ' + `or a dependency scope (${DEP_SCOPES.join(', ')})`; const EXAMPLE = 'Example: feat(PROJ-123): Add new feature'; +const TAG_EXAMPLE = '[.org] or [packagist]'; /** * Explain what is wrong with a header, in the terms the author needs to fix it. @@ -75,17 +97,39 @@ function explain(header) { problems.push(`"${scope}" is not a valid scope.${hint}\n Use ${SCOPE_HELP}.`); } - if (!subject.trim()) { + // The tag is optional structure, not part of the subject, so strip it before judging what is + // left. Without this a leading `[` reads as a subject that does not start with a letter - + // true to the letter, and useless for fixing an otherwise fine header. + let body = subject; + const tag = subject.match(TAG_AT_START); + + if (tag) { + if (!VALID_TAG.test(tag[1])) { + problems.push( + `"[${tag[1]}]" is not a valid tag. A tag holds letters, digits, dots, hyphens or ` + + `underscores - for example ${TAG_EXAMPLE}.` + ); + } + body = subject.slice(tag[0].length); + } else if (subject.startsWith('[')) { + // Left as the whole subject deliberately: the closing bracket is the one problem worth + // reporting, and treating the rest as a subject would add a spurious second complaint. + problems.push( + `The tag is missing its closing bracket. Write it as ${TAG_EXAMPLE}, then the subject.` + ); + } + + if (!body.trim()) { problems.push('The subject is missing. Describe the change after the colon.'); } // The strict pattern stops at the first character it cannot take, so a header can look // fine yet parse to a truncated subject. Worth saying out loud rather than silently // shipping a half-sentence into the changelog. - if (problems.length === 0 && subject.trim()) { - const usable = subject.match(/^[\w\d\s,-]*/)[0].trim(); + if (problems.length === 0 && body.trim()) { + const usable = body.match(/^[\w\d\s,-]*/)[0].trim(); if (!usable) { - problems.push(`The subject must start with a letter or number. "${subject}" does not.`); + problems.push(`The subject must start with a letter or number. "${body}" does not.`); } } @@ -122,7 +166,7 @@ module.exports = { headerPattern: new RegExp( `^(${TYPES.join('|')})` + `\\(((?:[A-Z]+-\\d+)|(?:NO-TASK)|(?:#\\d+)|(?:${DEP_SCOPES.join('|')}))\\)` - + ':\\s?([\\w\\d\\s,\\-]*)' + + `:\\s?${TAG}([\\w\\d\\s,\\-]*)` ), headerCorrespondence: ['type', 'scope', 'subject'], }, diff --git a/index.test.js b/index.test.js index 0cdef73..0723f47 100644 --- a/index.test.js +++ b/index.test.js @@ -139,6 +139,10 @@ describe('@linchpinagency/commitlint-config', () => { test.each([ 'update(wp-plugin): Update translatepress-multilingual to v3.2.4', 'update(wp-theme): Update ollie-pro to v2.6.1', + 'remove(wp-plugin): Remove akismet', + 'remove(wp-theme): Remove twentytwentyfour', + 'update(wporg): Update a public package', + 'update(linchpin): Update a packagist.linchpin.com package', 'build(deps): Update npm-run-all2 to v9.0.3', 'build(deps-dev): Update svgo to v3.3.4', 'build(composer): Update humbug/php-scoper to v0.18.19', @@ -164,10 +168,11 @@ describe('@linchpinagency/commitlint-config', () => { }); }); - // wp-plugin and wp-theme double as types, not just scopes, so a repo whose - // release-please-config gives WordPress plugins/themes their own changelog section can - // emit one. release-please groups strictly by type, so this is the only way in. - describe('wp-plugin and wp-theme as types', () => { + // 1.3.0 allowed wp-plugin and wp-theme as types so release-please could give WordPress + // updates a dedicated changelog section. The team settled on the scope form instead, so the + // type form is gone. Asserted rather than assumed: it is a one-word edit to put it back, and + // the two forms are indistinguishable at a glance in a config file. + describe('wp-plugin and wp-theme are scopes, not types', () => { const { explain } = config; const pattern = config.parserPreset.parserOpts.headerPattern; @@ -175,19 +180,75 @@ describe('@linchpinagency/commitlint-config', () => { 'wp-plugin(wporg): Update akismet to v5.3', 'wp-plugin(linchpin): Update some-plugin to v3.0', 'wp-theme(deps): Update twentytwentyfour to v2.0', + ])('rejects the type form %s', (header) => { + expect(explain(header)).toContain('is not a valid type'); + expect(header).not.toMatch(pattern); + }); + + test.each(['wp-plugin', 'wp-theme'])('%s is absent from type-enum', (type) => { + const [, , types] = config.rules['type-enum']; + expect(types).not.toContain(type); + }); + + test('the scope form is what replaces it', () => { + expect(explain('update(wp-plugin): Update translatepress-multilingual to v3.2.4')).toBeNull(); + expect(explain('update(wp-theme): Update ollie-pro to v2.6.1')).toBeNull(); + }); + }); + + // An optional bracketed tag carries what the scope slot no longer can - which registry a + // WordPress package came from. linchpin/renovatebot-config emits `[.org]` and `[packagist]`. + describe('subject tags', () => { + const { explain } = config; + const pattern = config.parserPreset.parserOpts.headerPattern; + + test.each([ + 'update(wp-plugin): [.org] Update akismet to v5.3', + 'update(wp-plugin): [packagist] Update gravityforms to v3', + 'update(wp-theme): [.org] Update Themes wordpress.org', + 'update(wp-theme): [packagist] Update Themes packagist.linchpin.com - Major', + 'fix(PROJ-123): [hotfix] Correct the redirect', + 'update(wp-plugin): [wp-packages] Update akismet to v5', ])('accepts %s', (header) => { expect(explain(header)).toBeNull(); expect(header).toMatch(pattern); }); - test('wporg and linchpin are valid scopes', () => { - expect(explain('wp-plugin(wporg): Update something')).toBeNull(); - expect(explain('wp-plugin(linchpin): Update something')).toBeNull(); + // The whole reason the tag is a non-capturing group. If it leaked into the subject then + // subject-case would be judging the label rather than the sentence. + test('a tag is not part of the captured subject', () => { + const [, type, scope, subject] = 'update(wp-plugin): [.org] Update akismet to v5'.match(pattern); + expect(type).toBe('update'); + expect(scope).toBe('wp-plugin'); + expect(subject).toBe('Update akismet to v5'); }); - test('the scope form documented above still works alongside the type form', () => { - expect(explain('update(wp-plugin): Update translatepress-multilingual to v3.2.4')).toBeNull(); - expect(explain('update(wp-theme): Update ollie-pro to v2.6.1')).toBeNull(); + test('an untagged subject is unaffected', () => { + const [, , , subject] = 'update(wp-plugin): Update akismet to v5'.match(pattern); + expect(subject).toBe('Update akismet to v5'); + }); + + test('a malformed tag is reported as a tag problem', () => { + expect(explain('update(wp-plugin): [] Update akismet')).toContain('is not a valid tag'); + expect(explain('update(wp-plugin): [a b] Update akismet')).toContain('is not a valid tag'); + }); + + test('an unclosed tag names the missing bracket', () => { + expect(explain('update(wp-plugin): [unclosed Update akismet')).toContain('closing bracket'); + }); + + // Each of these used to collapse into "The subject must start with a letter or number", + // which is true of a leading `[` and no help at all in fixing it. + test.each([ + 'update(wp-plugin): [] Update akismet', + 'update(wp-plugin): [a b] Update akismet', + 'update(wp-plugin): [unclosed Update akismet', + ])('does not misreport %s as a subject problem', (header) => { + expect(explain(header)).not.toContain('must start with a letter'); + }); + + test('a tag with no subject after it still reports a missing subject', () => { + expect(explain('update(wp-plugin): [.org]')).toContain('subject is missing'); }); }); });