diff --git a/.changeset/lucky-badges-report.md b/.changeset/lucky-badges-report.md new file mode 100644 index 00000000..7f0f48d2 --- /dev/null +++ b/.changeset/lucky-badges-report.md @@ -0,0 +1,53 @@ +--- +'hermex': major +--- + +Packages table: a status-badge hook, and deprecation as its own rule + +Closes #86 and #107. + +**A `Flags` column on the packages table.** A badge exists if and only if a package rule produced a +violation for that row, and its icon is that violation's severity. `no-packages` and the new +`no-deprecated-packages` contribute today; a future package rule plugs into the same declaration table +(`src/utils/package-status.ts`) and gets a badge with no change to any renderer. + +**`no-deprecated-packages`, a new rule.** Deprecation used to be detected only as a by-product of +release-age enrichment, so turning release-age off silently turned deprecation detection off with it. +It is now an inventory fact recorded by the registry pass plus a rule that judges it โ€” resolved +last-match-wins against an implicit `['**']` baseline at severity `info`, the way release-age is. So a +deprecated package stays visible with no configuration, an `error` entry makes it fail `comply`, and an +`'off'` entry genuinely exempts it. Both registry-backed rules share one request per installed package, +and a repo that configures neither still makes no network calls. + +Breaking changes: + +- `--format json`: `packages[].releaseAge.deprecated` moved to `packages[].deprecated`, and `deprecated` + was removed from `release-age` entries in `ruleViolations[]`. Deprecation is now reported as its own + `ruleId: "no-deprecated-packages"` entry, which also means a repo with `release-age` configured gains + `info`-severity violations it did not have before (they do not affect `compliance.status` or the exit + code). +- The packages table's `Target` column is now `Minimum target`, in both the human table and + `--summary-file`, whose header row changed accordingly. The cell leads with the version and moves + the bump tier into the parenthetical beside the timing: `major 4.2.0 (40 days overdue)` becomes + `4.2.0 (major, 40 days overdue)`. A pending upgrade reads `3.24.0 (minor, due in 12 days)` rather + than `minor 3.24.0 (12 days remaining)`, and a row with nothing to check at all renders `โ€”` + rather than blank, so it can be told apart from ๐ŸŸข ("checked, nothing to do"). +- The `[BANNED]` / `[RESTRICTED]` / `[DEPRECATED]` prefixes on package names are gone, replaced by + lowercase `forbidden` / `deprecated` badges in a new Flags column. `[RESTRICTED]` + named a config concept that never existed โ€” both badges came from one `no-packages` rule โ€” and it + collapsed `warn` and `info` into a single word. +- The gray `[not enforced]` suffix is gone; the severity icon carries that on its own. + +Fixes, visible in the same output: + +- A release-age breach governed by an `info`-severity entry rendered red, and so did one governed by an + `'off'` entry that produced no violation at all. They now render blue and unadorned respectively. +- ๐Ÿ”ต meant two unrelated things at once โ€” "an info-severity violation exists" and "an upgrade is coming + due". A pending upgrade now carries the all-clear ๐ŸŸข instead, since nothing about it is a verdict. +- `release-age` named a version and then said none was available: `minor 2.30.1 (no compliant release + available)`. That branch is only reachable when a newer release exists and simply has not been taken + โ€” being on the newest release that exists breaches nothing, so no row is produced โ€” and it + contradicted `minCompliantVersion`'s own rule that being on latest counts as compliant. It now reads + `2.30.1 (minor, 455 days overdue)`. The arithmetic is unchanged; only the wording was wrong (#26). +- Chart mode measured its label padding from the bare package name while padding a badge-prefixed, + ANSI-colored string, so any flagged row pushed its bar out of column. Badges now trail the bar. diff --git a/docs/examples.md b/docs/examples.md index b077f0df..b5aab0ca 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -246,8 +246,9 @@ same set of strings), not a glob comparison โ€” write the override's `patterns` identically to the base rule you want to replace or cancel. Every rule type supports this (`no-files`, `require-files`, -`max-file-size`, `no-packages`, `require-packages`, `require-scripts`, -`require-package-fields`, `no-package-fields`, `require-engine-version`). The +`max-file-size`, `no-packages`, `no-deprecated-packages`, `require-packages`, +`require-scripts`, `require-package-fields`, `no-package-fields`, +`require-engine-version`). The exception is `require-codeowners`, which only ever holds a single rule โ€” any matching override's `require-codeowners` replaces the base one outright, and severity `'off'` clears it. @@ -381,10 +382,16 @@ no need to spell it out as `'no-package-fields': ['dependencies.x', 'devDependen Purely transitive dependencies are never flagged: they arrive through another package, so removing one isn't something your repo can do. Packages excluded by `packages.ignore` are never flagged either. -Every banned package appears in the Rules and Compliance sections, and also gets a `[BANNED]` or -`[RESTRICTED]` badge in the packages table. Since #78 that table lists every package the repo owns, so a +Every banned package appears in the Rules and Compliance sections, and also gets a `forbidden` badge in +the packages table's **Flags** column. Since #78 that table lists every package the repo owns, so a declared-but-unimported banned package has a row โ€” and a badge on it โ€” just like an imported one. +The badge used to read `[BANNED]` at `error` and `[RESTRICTED]` at anything softer, glued to the front of +the package name. That invented a distinction the config never had โ€” there is one `no-packages` rule, not +two โ€” and it collapsed `warn` and `info` into the same word. One badge names *why* the row is flagged and +the icon beside it says how hard the rule is enforced, which is what every other surface already did +([#86](https://github.com/Gallevy/hermex/issues/86)). + In the JSON output, each hit is an ordinary entry in `ruleViolations` with `ruleId: "no-packages"` โ€” `patterns` carries the rule's globs, `packageName` the package that matched, and `matchedFiles` is empty (a package isn't a file, and a declared-but-unimported one has none): @@ -402,6 +409,107 @@ In the JSON output, each hit is an ordinary entry in `ruleViolations` with `rule A glob rule that matches several packages produces one entry per package, all sharing the same `patterns`. +### Deprecated Packages + +Flags packages whose publisher has deprecated them on npm. + +```ts +export default defineConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'error', patterns: ['**'] }, + ], + }, +}); +``` + +`patterns` match package names, the same way `no-packages` does. The rules-table row carries npm's own +deprecation notice, unless you write a `message`, in which case yours is shown instead. + +**You don't have to configure it to see it.** Any package no entry matches falls back to an implicit +`['**']` entry at severity `info`: reported, counted in the tally, never part of the pass/fail verdict. +So a deprecated package shows up with no configuration at all, and you only reach for this rule to make +it enforceable โ€” or to silence it: + +```ts +rules: { + 'no-deprecated-packages': [ + { severity: 'error', patterns: ['**'] }, + // Deprecated, but we're mid-migration and don't want CI red over it. + { severity: 'off', patterns: ['@legacy/*'] }, + ], +}, +``` + +Like [release-age](#release-age-opt-in) โ€” and unlike every other rule โ€” exactly one entry governs a +package, **last match wins**. That is why `'off'` genuinely exempts a package here rather than falling +through to the baseline. + +**The baseline decides severity, not whether hermex looks.** Deprecation is a registry fact, so it costs +one request per installed dependency. That request is only made when `rules['no-deprecated-packages']` or +`rules['release-age']` is non-empty for the repo โ€” a run that configures neither makes no network calls +at all. When either is configured, both rules read the same fetched document, so enabling the second one +costs nothing extra. + +Before v3 deprecation had no rule of its own: it was detected as a by-product of release-age enrichment, +so turning release-age off silently turned deprecation detection off with it, even though the two have +nothing to do with each other ([#107](https://github.com/Gallevy/hermex/issues/107)). + +Purely transitive packages are out of scope, for the same reason they are for `no-packages`: you can't +un-deprecate someone else's dependency. + +In the JSON output the fact and the judgment are separate. The fact is `packages[].deprecated` โ€” npm's +notice for the installed version, present whenever the registry was consulted. The judgment is an +ordinary `ruleViolations` entry: + +```jsonc +{ + "ruleId": "no-deprecated-packages", + "severity": "error", + "patterns": ["**"], + "packageName": "request", + "deprecated": "request has been deprecated, see request/request#3142" +} +``` + +### Reading the packages table + +``` +โ”Œโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ฌโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ” +โ”‚ Package โ”‚ Installed โ”‚ Minimum target โ”‚ Flags โ”‚ +โ”œโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ผโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ผโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ผโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ค +โ”‚ react โ”‚ 18.3.1 โ”‚ ๐ŸŸข โ”‚ โ”‚ +โ”‚ zod โ”‚ 3.23.8 โ”‚ ๐ŸŸข 3.24.0 (minor, due in 12 days) โ”‚ โ”‚ +โ”‚ moment โ”‚ 2.29.4 โ”‚ ๐Ÿ”ด 4.2.0 (major, 40 days overdue) โ”‚ ๐Ÿ”ต deprecated โ”‚ +โ”‚ eslint โ”‚ N/A โ”‚ โ€” โ”‚ ๐Ÿ”ด forbidden โ”‚ +โ””โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ดโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ดโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”ดโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ +``` + +**Installed** and **Minimum target** appear only when release-age ran; they are that rule's display, and +its icon sits beside the target it judges. **Flags** is where every *other* package rule reports โ€” one +badge per rule that flagged the row, blank when none did. + +The target names the version to move to, then why: the bump tier, and how long the breached tier has +been out of compliance (counted from its *oldest* release, not from the recommended one โ€” see +[#24](https://github.com/Gallevy/hermex/issues/24)). `N/A` under **Installed** means the package is +declared in `package.json` but missing from the lockfile, so there is no installed version to check. + +The icons mean the same thing everywhere in hermex's output: + +| Icon | Meaning | +| --- | --- | +| ๐ŸŸข | Checked, nothing to report. | +| ๐Ÿ”ต | Reported at `info` โ€” counted in the tally, never part of the verdict. | +| ๐ŸŸก | Reported at `warn` โ€” counted, and drops `compliance.status` to `"warning"`, but `comply` still exits 0. | +| ๐Ÿ”ด | Reported at `error` โ€” `comply` exits 1. | +| *(none)* | No rule is judging this. A release-age entry at `'off'` still shows the target it would have recommended, with no icon, because nothing about it is a verdict. | +| `โ€”` | Nothing was checked: the package has no installed version, or the registry never answered for it. Distinct from ๐ŸŸข, which is the claim that hermex looked and found nothing to do. | + +Anything a badge can't hold โ€” npm's deprecation notice, a package with several resolved copies, an +overdue nested copy the current scope doesn't enforce โ€” is printed as a **Notes** line under the table. +Notes are stdout-only: `--summary-file` feeds CI checks and PR comments, where non-blocking context +rendered as a colored line reads like blame for something that isn't failing. + ### CODEOWNERS Rule Requires every scanned file to have an owner in your `CODEOWNERS` file @@ -510,7 +618,13 @@ export default defineConfig({ }); ``` -Adds an `Upgrades` column to the packages table. Deprecated packages get a `[DEPRECATED]` badge regardless of which rule entries are configured. +Splits the packages table's `Version` column into **Installed** and **Minimum target**, so the version +you have and the version that would clear the breach sit side by side. "Minimum" is the point: the target +is the most conservative release that clears the breach, not the newest one available โ€” see +[Resolution](#resolution-one-governing-entry-per-package-last-match-wins) below. + +Deprecation is a separate rule, [`no-deprecated-packages`](#deprecated-packages). It shares this rule's +registry request but does not depend on it. ### Resolution: one governing entry per package, last match wins @@ -557,7 +671,7 @@ rules: { Rule-entry patterns are checked against the lockfile directly, not just packages hermex found imported as components โ€” so a CSS-only or side-effect-only dependency (e.g. `import '@my-org/styles/button.css'`) still gets checked and can still fail `hermex comply`, even though it never shows up in component usage. -Which entry governs a package decides *severity, thresholds and scope* โ€” never *whether the package is checked*, once release-age is on for the repo at all. Every package in the packages table with an installed version gets its release age looked up, so a dependency imported purely as functions or hooks (`@my-org/toolkit`) shows a Target like any other โ€” advisory `[not enforced]` under the baseline, mandatory when a specific entry names it `error`. Before v3 that lookup was gated on JSX component usage, which had nothing to do with whether an installed version is stale and silently exempted every function-only dependency ([#171](https://github.com/Gallevy/hermex/issues/171)). The cost is one registry request per installed dependency rather than per rendered one. +Which entry governs a package decides *severity, thresholds and scope* โ€” never *whether the package is checked*, once release-age is on for the repo at all. Every package in the packages table with an installed version gets its release age looked up, so a dependency imported purely as functions or hooks (`@my-org/toolkit`) shows a Minimum target like any other โ€” advisory ๐ŸŸก under the baseline, mandatory ๐Ÿ”ด when a specific entry names it `error`. Before v3 that lookup was gated on JSX component usage, which had nothing to do with whether an installed version is stale and silently exempted every function-only dependency ([#171](https://github.com/Gallevy/hermex/issues/171)). The cost is one registry request per installed dependency rather than per rendered one. Severity only decides mandatory vs. advisory for a package that's already being enforced under its governing entry's `scope` โ€” it doesn't override `scope` itself. Under `scope: 'root'` (the default), a package matched by an `error` entry that's only ever pulled in transitively (never a direct dependency in your `package.json`) still can't fail `comply` โ€” there's no root copy to hold accountable. It still shows up as advisory context (see below), it just doesn't block the build. @@ -584,7 +698,7 @@ rules: { To give a subset of packages the opposite scope from your general policy, give them their own rule entry with `scope` set explicitly โ€” listed after the broader entry so it wins for those packages (last match wins, above). Useful when most of your tree should be checked exhaustively but a handful of packages have transitive pins you don't control down to the root, or vice versa. -**Root scope never hides nested duplicates from local output.** The human `--format human` table always shows a single **Installed** version (the exact copy the verdict was measured against โ€” the enforced baseline, which under `scope: 'tree'` may be a nested copy rather than the root version) alongside the **Target** (recommended upgrade). When a package has multiple resolved versions, or an overdue nested copy the current scope doesn't enforce, that context is printed as a Notes line beneath the table โ€” informational, not part of the pass/fail verdict. Notes are stdout-only: `--summary-file` (meant for a PR comment or CI check) only ever shows mandatory violations, so a reviewer never sees non-blocking context rendered as if it needed attention. +**Root scope never hides nested duplicates from local output.** The human `--format human` table always shows a single **Installed** version (the exact copy the verdict was measured against โ€” the enforced baseline, which under `scope: 'tree'` may be a nested copy rather than the root version) alongside the **Minimum target** (recommended upgrade). When a package has multiple resolved versions, or an overdue nested copy the current scope doesn't enforce, that context is printed as a Notes line beneath the table โ€” informational, not part of the pass/fail verdict. Notes are stdout-only: `--summary-file` (meant for a PR comment or CI check) only ever shows mandatory violations, so a reviewer never sees non-blocking context rendered as if it needed attention. For **yarn**, root-version resolution works by reading the root `package.json`'s declared dependency range and matching it exactly against the corresponding entry in the already-parsed `yarn.lock` โ€” yarn.lock itself retains no root/nested distinction, unlike npm's and pnpm's lockfile formats. If `package.json` can't be read, or a package isn't a direct dependency at all (purely transitive), hermex falls back to the highest resolved version found in the lockfile โ€” but only for *display* (the `--format human` table, and `scan`'s Version column). That fallback is never treated as an enforced root version: under `scope: 'root'`, a package with no true root resolution can never fail `comply`, regardless of which rule entry governs it. Pre-v9 pnpm lockfiles (no `importers` field) are always treated as root-only regardless of `scope` โ€” those legacy formats don't retain a root/nested distinction either. @@ -635,13 +749,13 @@ Severity is the only thing that decides which bucket a rule violation lands in; |---|---| | `version` | The hermex version that produced the report. | | `summary` | Aggregate counts: `filesAnalyzed`, `totalImports`, `totalComponents`, `totalUsagePatterns`, plus `patternCounts` โ€” per-pattern-type usage counts (`imports.named`, `usage.jsx`, โ€ฆ). | -| `packages` | Every package the repo owns โ€” see below. Carries version, `declaredIn`, usage counts, and `releaseAge` when enrichment ran. | +| `packages` | Every package the repo owns โ€” see below. Carries version, `declaredIn`, usage counts, `deprecated` (npm's notice, when the registry was consulted) and `releaseAge` when release-age ran. | | `components` | Every component found, with its source package, usage count and the files using it. The one place component names live. | | `versus` | Head-to-head comparisons configured under `versus`. | -| `ruleViolations` | **Every rule hit, in one list** โ€” `no-files`, `require-files`, `max-file-size`, `require-packages`, `no-packages`, `require-scripts`, `require-package-fields`, `no-package-fields`, `require-engine-version`, `require-codeowners`. Filter on `ruleId`. | +| `ruleViolations` | **Every rule hit, in one list** โ€” `no-files`, `require-files`, `max-file-size`, `require-packages`, `no-packages`, `no-deprecated-packages`, `require-scripts`, `require-package-fields`, `no-package-fields`, `require-engine-version`, `require-codeowners`, `release-age`. Filter on `ruleId`. | | `compliance` | The canonical verdict โ€” see above. | -`ruleViolations` is the single source of truth for rule hits. Entries share a common shape (`ruleId`, `severity`, `patterns`, `message?`, `matchedFiles`) and add per-type fields where they apply: `packageName` for `no-packages`, `fieldPath`/`actualValue` for the package-field rules, `maxSizeBytes`/`oversizeFiles` for `max-file-size`, `installedRange`/`requiredRange` for `require-engine-version`. +`ruleViolations` is the single source of truth for rule hits. Entries share a common shape (`ruleId`, `severity`, `patterns`, `message?`, `matchedFiles`) and add per-type fields where they apply: `packageName` for `no-packages`, `packageName`/`deprecated` for `no-deprecated-packages`, `fieldPath`/`actualValue` for the package-field rules, `maxSizeBytes`/`oversizeFiles` for `max-file-size`, `installedRange`/`requiredRange` for `require-engine-version`. #### Trimming the JSON with `output.*` diff --git a/fixtures/cases.ts b/fixtures/cases.ts index 3ee90b25..58025c8e 100644 --- a/fixtures/cases.ts +++ b/fixtures/cases.ts @@ -129,10 +129,19 @@ export const cases: FixtureCase[] = [ registry: true, expectExit: 1, }, + { + name: 'comply-deprecated-packages', + proves: + 'Deprecation detection with release-age switched off entirely โ€” the #107 case. Before, deprecation was a by-product of release-age enrichment, so this configuration found nothing at all and this run would have been silently compliant on that axis. Now the registry is consulted for deprecation alone: a no-deprecated-packages row appears in the Rules table carrying npm own notice, and at severity error it fails comply on its own. The only case where the registry is reached without release-age, which is exactly the path that did not exist before.', + cwd: '.', + args: ['comply', '--config', 'configs/deprecated-packages.config.ts'], + registry: true, + expectExit: 1, + }, { name: 'comply-all-rule-types', proves: - 'Every one of the eleven rule types in one run, at three severities โ€” the only case that renders max-file-size, require-engine-version, codeowners, both package-field shapes, and release-age together. release-age itself never gets a Rules-table row (its display is the Packages table) โ€” that split is what this case pins.', + 'Every one of the twelve rule types in one run, at three severities โ€” the only case that renders max-file-size, require-engine-version, codeowners, both package-field shapes, release-age and no-deprecated-packages together. release-age itself never gets a Rules-table row (its display is the Packages table) โ€” that split is what this case pins, along with the only multi-badge Status cell in the fixtures (moment is both forbidden and deprecated).', cwd: 'repos/all-rule-types', args: ['comply'], registry: true, diff --git a/fixtures/cases/comply-all-rule-types.md b/fixtures/cases/comply-all-rule-types.md index 15644c2e..62d11757 100644 --- a/fixtures/cases/comply-all-rule-types.md +++ b/fixtures/cases/comply-all-rule-types.md @@ -3,7 +3,7 @@ # `comply-all-rule-types` -Every one of the eleven rule types in one run, at three severities โ€” the only case that renders max-file-size, require-engine-version, codeowners, both package-field shapes, and release-age together. release-age itself never gets a Rules-table row (its display is the Packages table) โ€” that split is what this case pins. +Every one of the twelve rule types in one run, at three severities โ€” the only case that renders max-file-size, require-engine-version, codeowners, both package-field shapes, release-age and no-deprecated-packages together. release-age itself never gets a Rules-table row (its display is the Packages table) โ€” that split is what this case pins, along with the only multi-badge Status cell in the fixtures (moment is both forbidden and deprecated). ## What runs diff --git a/fixtures/cases/comply-deprecated-packages.md b/fixtures/cases/comply-deprecated-packages.md new file mode 100644 index 00000000..a7736c03 --- /dev/null +++ b/fixtures/cases/comply-deprecated-packages.md @@ -0,0 +1,29 @@ + + +# `comply-deprecated-packages` + +Deprecation detection with release-age switched off entirely โ€” the #107 case. Before, deprecation was a by-product of release-age enrichment, so this configuration found nothing at all and this run would have been silently compliant on that axis. Now the registry is consulted for deprecation alone: a no-deprecated-packages row appears in the Rules table carrying npm own notice, and at severity error it fails comply on its own. The only case where the registry is reached without release-age, which is exactly the path that did not exist before. + +## What runs + +| | | +| --- | --- | +| **Command** | `hermex comply --config configs/deprecated-packages.config.ts` | +| **Working directory** | `fixtures/` | +| **Config** | [`fixtures/configs/deprecated-packages.config.ts`](../configs/deprecated-packages.config.ts) | +| **Fixture** | [`fixtures`](..) โ€” [overview](../README.md) | +| **Asserted exit code** | `1` | +| **Registry** | offline, served from [`fixtures/registry/timelines.ts`](../registry/timelines.ts) โ€” never the network | + +## Recorded output + +Nothing is committed for this case's output โ€” every run compares this tree's real output against a fresh build of the target branch (`--against`, `main` by default). A change to hermex's output shows up as a diff in that run's output-review report, never as a file in this repo. + +## Run it locally + +```bash +pnpm run test:output -- --filter comply-deprecated-packages +``` + +Defined in [`fixtures/cases.ts`](../cases.ts). diff --git a/fixtures/configs/deprecated-packages.config.ts b/fixtures/configs/deprecated-packages.config.ts new file mode 100644 index 00000000..182990a0 --- /dev/null +++ b/fixtures/configs/deprecated-packages.config.ts @@ -0,0 +1,31 @@ +import type { HermexConfigInput } from '../../src/config/types.ts'; +import base from '../hermex.config.ts'; + +/** + * `no-deprecated-packages` with `release-age` left empty โ€” the #107 case. + * + * Deprecation used to be a by-product of release-age enrichment, so this + * configuration detected nothing at all: turning release-age off silently + * took deprecation with it. Here the registry is consulted for deprecation + * alone, which is what the output proves โ€” a `no-deprecated-packages` row + * in the Rules table, carrying npm's own notice, on a run where release-age + * never looked anything up. + * + * `comply` prints no Packages table without release-age data, by design: a + * badge there is a cross-reference to the Rules row above it, not a second + * finding, and `comply` is the gating view. `scan-human-default` is where + * the Status column itself is reviewed. + * + * At `error`, so it also pins that deprecation can now fail `comply` on its + * own rather than being display-only. + */ +export default { + ...base, + releaseAge: { + cacheDisabled: true, + }, + rules: { + ...base.rules, + 'no-deprecated-packages': [{ severity: 'error', patterns: ['**'] }], + }, +} satisfies HermexConfigInput; diff --git a/fixtures/repos/all-rule-types/README.md b/fixtures/repos/all-rule-types/README.md index 90415084..b1999ab6 100644 --- a/fixtures/repos/all-rule-types/README.md +++ b/fixtures/repos/all-rule-types/README.md @@ -1,11 +1,11 @@ # `repos/all-rule-types/` -A repo engineered so **every one of the eleven rule types fires at once**, at +A repo engineered so **every one of the twelve rule types fires at once**, at three different severities. ## What it proves -The primary fixture repo only ever trips three of the eleven. Without this +The primary fixture repo only ever trips three of the twelve. Without this repo the rules table has never been reviewed with a `max-file-size` row, an `require-engine-version` row, a `codeowners` row, or either of the two package-field shapes in it โ€” so nothing would catch a renderer that @@ -30,6 +30,15 @@ field reports the offending value. | `no-package-fields` | warn | `publishConfig.registry` points at an internal registry | | `require-engine-version` | error | `engines.node` is `>=16`, below the required `>=20` | | `codeowners` | info | fires **twice** โ€” see below | +| `release-age` | error | `react@18.3.1` is overdue on a major, with `19.1.0` still in-window | +| `no-deprecated-packages` | warn | the registry reports `moment@2.29.4` deprecated | + +`release-age` is the only rule that renders no row in the Rules table at +all โ€” its display is the Packages table. `no-deprecated-packages` renders in +both: a Rules row carrying npm's own notice, and a Status badge on the +Packages row. Since `moment` is also forbidden, that row is the one place in +the fixtures where two Status badges appear together at different +severities. `includes` is scoped to `src/` so `jest.config.js` is found by `no-files` without also being parsed as source โ€” the same scoping keeps @@ -62,7 +71,7 @@ reads "have the wrong owner". | Case | Command | Expects | | --- | --- | --- | -| `comply-all-rule-types` | `hermex comply` | exit 1, all eleven rows in the human table | +| `comply-all-rule-types` | `hermex comply` | exit 1, every rule type represented โ€” eleven in the Rules table, release-age in the Packages table | | `comply-all-rule-types-json` | `hermex comply --format json` | exit 1, the machine-readable shape of each rule type | ## Layout @@ -71,9 +80,9 @@ reads "have the wrong owner". .babelrc no-files hit .github/CODEOWNERS two of three src files covered assets/logo.svg 1410 B, over the 1 KB max-file-size ceiling -hermex.config.ts all eleven rules, three severities +hermex.config.ts all twelve rules, three severities jest.config.js no-files hit -package.json engines >=16, no license, publishConfig.registry, moment +package.json engines >=16, no license, publishConfig.registry, moment (forbidden + deprecated) src/legacy.tsx owned by the wrong team src/orphan.tsx owned by nobody src/owned.tsx correctly owned diff --git a/fixtures/repos/all-rule-types/hermex.config.ts b/fixtures/repos/all-rule-types/hermex.config.ts index 269fe253..8db12247 100644 --- a/fixtures/repos/all-rule-types/hermex.config.ts +++ b/fixtures/repos/all-rule-types/hermex.config.ts @@ -2,14 +2,16 @@ import type { HermexConfigInput } from '../../../src/config/types.ts'; /** * Every rule type hermex has, all firing at once, at three different - * severities. The primary fixture repo only ever trips three of the eleven โ€” + * severities. The primary fixture repo only ever trips three of the twelve โ€” * so without this repo the rules table has never been reviewed with an * `require-engine-version` row, a `require-codeowners` row, or either of the * package-field shapes in it, and nothing would catch a renderer that * mishandles `fieldPath` / `installedRange` / a long `matchedFiles` list. - * `release-age` is the eleventh and only rule that never renders a Rules-table - * row at all (its display is the Packages table) โ€” this is the only case - * that exercises it alongside the other ten in one run. + * `release-age` is the only rule that never renders a Rules-table row at + * all (its display is the Packages table) โ€” this is the only case that + * exercises it alongside the other eleven in one run. `moment` is both + * forbidden and deprecated here, which makes it the one row anywhere in the + * fixtures that carries two Status badges at different severities. * * Scoped to `src/` so `jest.config.js` is found by `no-files` without * also being parsed as source โ€” and so `assets/logo.svg`, which exists @@ -22,11 +24,15 @@ export default { }, rules: { // react@18.3.1 is overdue on a major (19.0.0, breached) with a genuine - // compliant target still in-window (19.1.0) โ€” the eleventh rule type, - // and the only one whose display lives in the Packages table rather - // than the Rules table (see src/utils/print-rules.ts's renderer - // dispatch). + // compliant target still in-window (19.1.0) โ€” the only rule whose + // display lives in the Packages table rather than the Rules table (see + // src/utils/print-rules.ts's renderer dispatch). 'release-age': [{ severity: 'error', patterns: ['react'] }], + // moment@2.29.4 carries a deprecation notice in the fixture registry, + // and is already forbidden below โ€” so its Packages row renders both + // badges at once, at two different severities, which is the only place + // the fixtures exercise a multi-badge Status cell. + 'no-deprecated-packages': [{ severity: 'warn', patterns: ['moment'] }], 'no-files': [ { severity: 'error', diff --git a/fixtures/repos/all-rule-types/package.json b/fixtures/repos/all-rule-types/package.json index e932b89a..d380e5f6 100644 --- a/fixtures/repos/all-rule-types/package.json +++ b/fixtures/repos/all-rule-types/package.json @@ -2,7 +2,7 @@ "name": "hermex-fixture-all-rule-types", "version": "1.0.0", "private": true, - "description": "A repo engineered so every one of the eleven rule types fires at once, which no other fixture does: `license` is missing (require-package-fields), `publishConfig.registry` is present (no-package-fields), assets/logo.svg is over its 1 KB ceiling (max-file-size), engines.node is below the required range (require-engine-version), `build` and `test` scripts are absent (require-scripts), moment is declared (no-packages), typescript is not (require-packages), jest.config.js and .babelrc exist (no-files), .nvmrc does not (require-files), and CODEOWNERS leaves one file unowned and another owned by the wrong team (codeowners, twice).", + "description": "A repo engineered so every one of the twelve rule types fires at once, which no other fixture does: `license` is missing (require-package-fields), `publishConfig.registry` is present (no-package-fields), assets/logo.svg is over its 1 KB ceiling (max-file-size), engines.node is below the required range (require-engine-version), `build` and `test` scripts are absent (require-scripts), moment is declared and deprecated (no-packages, no-deprecated-packages), typescript is not (require-packages), jest.config.js and .babelrc exist (no-files), .nvmrc does not (require-files), and CODEOWNERS leaves one file unowned and another owned by the wrong team (codeowners, twice).", "type": "module", "engines": { "node": ">=16" diff --git a/src/commands/pipeline.ts b/src/commands/pipeline.ts index a42d0c1e..4d440212 100644 --- a/src/commands/pipeline.ts +++ b/src/commands/pipeline.ts @@ -9,7 +9,7 @@ import { findFiles } from '../utils/file-utils'; import { findAndParseLockfile } from '../lock-parser'; import { evaluateRules } from '../rules/evaluator'; import { collectDeclaredPackages } from '../rules/shared'; -import { evaluateReleaseAge } from '../rules/release-age'; +import { evaluateRegistryRules, needsRegistry } from '../rules/registry-rules'; import { applyOverrides } from '../config/overrides'; import { runPlugins } from '../plugins'; import type { HermexConfig } from '../config/types'; @@ -122,21 +122,27 @@ export async function runPipeline( ...evaluatorViolations, ]; - // `rules['release-age']` being non-empty IS "release-age enabled" โ€” no - // separate flag (see src/config/schema.ts's releaseAge block comment). - if (resolvedConfig.rules['release-age'].length > 0) { + // A rule family being non-empty IS "that rule enabled" โ€” no separate + // flag (see src/config/schema.ts's releaseAge block comment). Both + // registry-backed families share one enrichment pass, so the gate is + // "does anything need the registry", not "is release-age on" (#107). + if (needsRegistry(resolvedConfig.rules)) { + const forReleaseAge = resolvedConfig.rules['release-age'].length > 0; if (spinner.isEnabled) - spinner.start('Fetching release age from registry...'); - const { enriched, violations, skipped } = await evaluateReleaseAge( + spinner.start( + forReleaseAge + ? 'Fetching release age from registry...' + : 'Checking the registry for deprecated packages...', + ); + const { enriched, violations, skipped } = await evaluateRegistryRules( aggregated.packageDistribution, - resolvedConfig.releaseAge, - resolvedConfig.rules['release-age'], + resolvedConfig, ); aggregated.packageDistribution = enriched; aggregated.ruleViolations = [...aggregated.ruleViolations, ...violations]; spinner.succeed( chalk.blue( - `Release age fetched${skipped > 0 ? chalk.gray(` (${skipped} packages skipped โ€” registry unreachable or not found)`) : ''}`, + `${forReleaseAge ? 'Release age fetched' : 'Registry checked'}${skipped > 0 ? chalk.gray(` (${skipped} packages skipped โ€” registry unreachable or not found)`) : ''}`, ), ); } diff --git a/src/config/overrides.ts b/src/config/overrides.ts index 96b63019..9532cdeb 100644 --- a/src/config/overrides.ts +++ b/src/config/overrides.ts @@ -37,17 +37,30 @@ export type ResolvedCodeownersRule = Resolved; * `upsertPatternRules` drops it for other rules, a package meant to be * exempted would just fall through to the next-best match (often the * baseline) instead of being exempted โ€” the opposite of what 'off' means. - * `upsertReleaseAgeRules` (below) is `upsertPatternRules` without the + * `upsertGoverningRules` (below) is `upsertPatternRules` without the * `isEnabled` filter, so 'off' entries are upserted like any other. */ export type ResolvedReleaseAgeRuleConfig = ReleaseAgeRuleConfig; +/** + * NOT `Resolved`, for exactly the reason spelled out on + * `ResolvedReleaseAgeRuleConfig` above: `no-deprecated-packages` is the + * other family resolved by last-match-wins governance against an implicit + * `['**']` baseline (`resolveDeprecatedPackagesRule` below), so an 'off' + * entry has to survive resolution in order to exempt a package. Drop it the + * way `upsertPatternRules` does for the ordinary families and the package + * falls through to the baseline instead โ€” silently turning 'off' into a + * no-op. + */ +export type ResolvedDeprecatedPackagesRuleConfig = RuleConfig; + /** The shape `RulesConfig` resolves to after `applyOverrides` โ€” see `ResolvedRuleConfig`. */ export interface ResolvedRulesConfig { 'no-files': ResolvedRuleConfig[]; 'require-files': ResolvedRuleConfig[]; 'max-file-size': ResolvedMaxFileSizeRule[]; 'no-packages': ResolvedRuleConfig[]; + 'no-deprecated-packages': ResolvedDeprecatedPackagesRuleConfig[]; 'require-packages': ResolvedRuleConfig[]; 'require-scripts': ResolvedRuleConfig[]; 'require-package-fields': ResolvedPackageFieldRule[]; @@ -93,15 +106,17 @@ function upsertPatternRules( } /** - * Same identity/replacement semantics as {@link upsertPatternRules} - * (keyed by `patterns`), but never drops an 'off' entry โ€” see - * `ResolvedReleaseAgeRuleConfig` above for why release-age needs 'off' to - * remain a real, resolvable entry instead of vanishing from the array. + * Same identity/replacement semantics as {@link upsertPatternRules} (keyed + * by `patterns`), but never drops an 'off' entry โ€” for the two families + * resolved by last-match-wins governance against an implicit baseline + * (`release-age`, `no-deprecated-packages`). See + * `ResolvedReleaseAgeRuleConfig` above for why those need 'off' to remain a + * real, resolvable entry instead of vanishing from the array. */ -function upsertReleaseAgeRules( - base: ReleaseAgeRuleConfig[], - overrides: ReleaseAgeRuleConfig[], -): ReleaseAgeRuleConfig[] { +function upsertGoverningRules( + base: T[], + overrides: T[], +): T[] { let result = base; for (const rule of overrides) { result = result.filter((r) => !patternsMatch(r.patterns, rule.patterns)); @@ -151,6 +166,10 @@ function resolveRules(rules: RulesConfig): ResolvedRulesConfig { 'require-files': upsertPatternRules([], toArray(rules['require-files'])), 'max-file-size': upsertPatternRules([], toArray(rules['max-file-size'])), 'no-packages': upsertPatternRules([], toArray(rules['no-packages'])), + 'no-deprecated-packages': upsertGoverningRules( + [], + toArray(rules['no-deprecated-packages']), + ), 'require-packages': upsertPatternRules( [], toArray(rules['require-packages']), @@ -172,7 +191,7 @@ function resolveRules(rules: RulesConfig): ResolvedRulesConfig { toArray(rules['require-engine-version']), ), 'require-codeowners': resolveCodeowners(rules['require-codeowners']), - 'release-age': upsertReleaseAgeRules([], toArray(rules['release-age'])), + 'release-age': upsertGoverningRules([], toArray(rules['release-age'])), }; } @@ -229,6 +248,12 @@ export function applyOverrides( toArray(o['no-packages']), ); } + if (o['no-deprecated-packages'] !== undefined) { + rules['no-deprecated-packages'] = upsertGoverningRules( + rules['no-deprecated-packages'], + toArray(o['no-deprecated-packages']), + ); + } if (o['require-packages'] !== undefined) { rules['require-packages'] = upsertPatternRules( rules['require-packages'], @@ -265,7 +290,7 @@ export function applyOverrides( ); } if (o['release-age'] !== undefined) { - rules['release-age'] = upsertReleaseAgeRules( + rules['release-age'] = upsertGoverningRules( rules['release-age'], toArray(o['release-age']), ); @@ -322,3 +347,39 @@ export function resolveReleaseAgeRule( } return winner; } + +/** + * The default policy for any package no authored + * `rules['no-deprecated-packages']` entry matches: reported, never + * enforced. 'info' is what preserves the long-standing behavior that a + * deprecated package is always *visible* once hermex has registry data for + * it, while keeping it out of the compliance verdict โ€” an author opts into + * enforcement with an 'error' entry, and silences it with 'off'. + * + * Like `RELEASE_AGE_BASELINE`, this sets *severity*, not *enablement*: it + * only ever applies to packages the registry was already consulted about. + * An empty `rules['no-deprecated-packages']` with release-age also off + * means no registry call is made at all, so nothing reaches this. + */ +const DEPRECATED_PACKAGES_BASELINE: ResolvedDeprecatedPackagesRuleConfig = { + severity: 'info', + patterns: ['**'], +}; + +/** + * Resolves which single `no-deprecated-packages` entry governs + * `packageName` โ€” last match wins against the implicit `['**']` baseline, + * identical in mechanic and rationale to `resolveReleaseAgeRule` above. A + * package can't be simultaneously 'error' under one entry and 'off' under + * another, so one entry has to win rather than every match firing. + */ +export function resolveDeprecatedPackagesRule( + packageName: string, + resolvedRules: ResolvedDeprecatedPackagesRuleConfig[], +): ResolvedDeprecatedPackagesRuleConfig { + let winner = DEPRECATED_PACKAGES_BASELINE; + for (const rule of [DEPRECATED_PACKAGES_BASELINE, ...resolvedRules]) { + if (micromatch.isMatch(packageName, rule.patterns)) winner = rule; + } + return winner; +} diff --git a/src/config/schema.ts b/src/config/schema.ts index 058a2291..98575c7c 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -139,6 +139,7 @@ const OverrideRulesSchema = z 'require-files': RuleConfigOrArraySchema.optional(), 'max-file-size': MaxFileSizeRuleOrArraySchema.optional(), 'no-packages': RuleConfigOrArraySchema.optional(), + 'no-deprecated-packages': RuleConfigOrArraySchema.optional(), 'require-packages': RuleConfigOrArraySchema.optional(), 'require-scripts': RuleConfigOrArraySchema.optional(), 'require-package-fields': PackageFieldRuleOrArraySchema.optional(), @@ -298,6 +299,18 @@ export const HermexConfigSchema = z 'require-files': RuleConfigOrArraySchema.default([]), 'max-file-size': MaxFileSizeRuleOrArraySchema.default([]), 'no-packages': RuleConfigOrArraySchema.default([]), + /** + * Judges the deprecation notice registry enrichment records on each + * package. `patterns` match package names, like `no-packages`. + * + * Governed the way `release-age` is โ€” one winning entry per package + * against an implicit `['**']` baseline at severity 'info' (see + * `resolveDeprecatedPackagesRule`, ./overrides) โ€” so 'off' exempts a + * package rather than being dropped. That baseline sets severity, + * not enablement: leaving this empty with release-age also empty + * means the registry is never consulted at all. + */ + 'no-deprecated-packages': RuleConfigOrArraySchema.default([]), 'require-packages': RuleConfigOrArraySchema.default([]), 'require-scripts': RuleConfigOrArraySchema.default([]), 'require-package-fields': PackageFieldRuleOrArraySchema.default([]), @@ -314,6 +327,7 @@ export const HermexConfigSchema = z 'require-files': [] as RuleConfig[], 'max-file-size': [] as MaxFileSizeRule[], 'no-packages': [] as RuleConfig[], + 'no-deprecated-packages': [] as RuleConfig[], 'require-packages': [] as RuleConfig[], 'require-scripts': [] as RuleConfig[], 'require-package-fields': [] as PackageFieldRule[], diff --git a/src/config/types.ts b/src/config/types.ts index 4a855a35..cf458f44 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -28,6 +28,7 @@ export type { ResolvedEngineVersionRule, ResolvedCodeownersRule, ResolvedReleaseAgeRuleConfig, + ResolvedDeprecatedPackagesRuleConfig, ResolvedRulesConfig, ResolvedHermexConfig, } from './overrides'; diff --git a/src/npm-registry/enricher.ts b/src/npm-registry/enricher.ts index 2df19eaf..684dbd1c 100644 --- a/src/npm-registry/enricher.ts +++ b/src/npm-registry/enricher.ts @@ -72,9 +72,11 @@ interface ReleaseAgeForVersion { /** * Computes everything version-dependent for a single installed version - * against the registry's release timeline โ€” no notion of scope, severity, - * or deprecation, which are policy/registry facts independent of which - * installed copy is being checked (#57). + * against the registry's release timeline โ€” no notion of scope or + * severity, which are policy facts independent of which installed copy is + * being checked (#57). Deprecation isn't here either, and no longer passes + * through release-age at all: it's an inventory fact recorded directly by + * `enrichFromRegistry` below (#107). */ function computeReleaseAgeForVersion( installedVersion: string, @@ -258,7 +260,6 @@ export function computeReleaseAge( installedVersion: string, allVersions: string[], timeMap: Record, - deprecated: string | undefined, thresholds: ReleaseAgeThresholds, distTags: Record | undefined, severity: 'error' | 'warn' | 'info' | 'off', @@ -324,7 +325,6 @@ export function computeReleaseAge( upgrades: baseline.upgrades, worstLevel: baseline.worstLevel, pendingUpgrade: baseline.pendingUpgrade, - deprecated, latestVersion: baseline.latestVersion, latestReleasedDaysAgo: baseline.latestReleasedDaysAgo, minCompliantVersion: baseline.minCompliantVersion, @@ -356,18 +356,33 @@ export interface ReleaseAgePolicy { scope: 'root' | 'tree'; } +/** What one package's registry lookup produced. `found` records whether the + * fetch itself succeeded, which is deliberately separate from whether an + * `entry` was computed โ€” see the merge loop below. */ +interface RegistryResult { + pkg: PackageDistribution; + found: boolean; + deprecated?: string; + entry?: ReleaseAgeEntry; +} + /** - * Fetches registry data and computes `ReleaseAgeEntry` for every package - * with an installed version โ€” unconditionally, regardless of policy (#171, - * #173): a package's `resolvePolicy` result decides only its severity, - * thresholds and scope, never whether it's looked up at all. Pure registry - * I/O plus the timeline math above; policy resolution (which rule entry - * governs a package) lives in `src/rules/release-age.ts`, one layer up. + * One registry pass, two outputs. Deprecation is an inventory fact and is + * always recorded; the `ReleaseAgeEntry` is computed only when + * `resolvePolicy` is supplied โ€” i.e. when `rules['release-age']` is + * non-empty for this repo. So both registry-backed rules cost one request + * per installed package between them, not one each (#107). + * + * Within a release-age run, enrichment stays unconditional regardless of + * policy (#171, #173): a package's `resolvePolicy` result decides only its + * severity, thresholds and scope, never whether it's looked up at all. + * Pure registry I/O plus the timeline math above; policy resolution (which + * rule entry governs a package) lives one layer up, in `src/rules/`. */ -export async function enrichWithReleaseAge( +export async function enrichFromRegistry( packages: PackageDistribution[], connection: ReleaseAgeConnection, - resolvePolicy: (packageName: string) => ReleaseAgePolicy, + resolvePolicy?: (packageName: string) => ReleaseAgePolicy, ): Promise<{ enriched: PackageDistribution[]; skipped: number }> { const authToken = connection.authToken ?? process.env['HERMEX_REGISTRY_AUTH_TOKEN']; @@ -387,8 +402,8 @@ export async function enrichWithReleaseAge( // Process in batches of CONCURRENCY for (let i = 0; i < targets.length; i += CONCURRENCY) { const batch = targets.slice(i, i + CONCURRENCY); - const results = await Promise.all( - batch.map(async (pkg) => { + const results: RegistryResult[] = await Promise.all( + batch.map(async (pkg): Promise => { const info = await getPackageInfo( pkg.packageName, NPM_REGISTRY_URL, @@ -397,13 +412,13 @@ export async function enrichWithReleaseAge( ); if (!info || !info.time) { skipped++; - return { pkg, entry: null }; + return { pkg, found: false }; } const deprecated = info.versions?.[pkg.version!]?.deprecated ?? info.deprecated; - const policy = resolvePolicy(pkg.packageName); + const policy = resolvePolicy?.(pkg.packageName); // `undefined` (never populated โ€” e.g. a hand-built PackageDistribution // in a test) is treated as "unknown, assume root" for backward @@ -412,28 +427,45 @@ export async function enrichWithReleaseAge( // means "don't enforce this under root scope" (#62). const hasRootVersion = pkg.rootVersion !== null; - const entry = computeReleaseAge( - pkg.version!, - pkg.allVersions, - info.time, - typeof deprecated === 'string' ? deprecated : undefined, - policy.thresholds, - info['dist-tags'], - policy.severity, - policy.scope, - hasRootVersion, - ); - - return { pkg, entry }; + const entry = policy + ? computeReleaseAge( + pkg.version!, + pkg.allVersions, + info.time, + policy.thresholds, + info['dist-tags'], + policy.severity, + policy.scope, + hasRootVersion, + ) + : undefined; + + return { + pkg, + found: true, + deprecated: typeof deprecated === 'string' ? deprecated : undefined, + entry, + }; }), ); - for (const { pkg, entry } of results) { - if (!entry) continue; - const idx = enriched.findIndex((p) => p.packageName === pkg.packageName); - if (idx !== -1) { - enriched[idx] = { ...enriched[idx], releaseAge: entry }; - } + // Keyed on whether the FETCH succeeded, never on whether an entry was + // computed: with release-age off there is no entry for any package, and + // `if (!entry) continue` would silently discard every deprecation fact + // this pass exists to collect (#107). + for (const result of results) { + if (!result.found) continue; + const idx = enriched.findIndex( + (p) => p.packageName === result.pkg.packageName, + ); + if (idx === -1) continue; + enriched[idx] = { + ...enriched[idx], + ...(result.deprecated !== undefined + ? { deprecated: result.deprecated } + : {}), + ...(result.entry ? { releaseAge: result.entry } : {}), + }; } } diff --git a/src/npm-registry/types.ts b/src/npm-registry/types.ts index aaf205d2..7173f2b5 100644 --- a/src/npm-registry/types.ts +++ b/src/npm-registry/types.ts @@ -38,7 +38,6 @@ export interface ReleaseAgeEntry { */ worstLevel: UpgradeLevel | null; pendingUpgrade?: PendingUpgrade; - deprecated?: string; latestVersion?: string; latestReleasedDaysAgo?: number; /** diff --git a/src/rules/__type-tests__/off-severity.type-test.ts b/src/rules/__type-tests__/off-severity.type-test.ts index 5bcc1de2..29ff175e 100644 --- a/src/rules/__type-tests__/off-severity.type-test.ts +++ b/src/rules/__type-tests__/off-severity.type-test.ts @@ -4,6 +4,15 @@ // away, and every downstream consumer trusts that via ResolvedRulesConfig / // ResolvedHermexConfig instead of re-checking at runtime. // +// Two rule families are deliberately absent below: `release-age` and +// `no-deprecated-packages` resolve by last-match-wins governance against an +// implicit `['**']` baseline, so for them 'off' must SURVIVE resolution in +// order to exempt a package โ€” dropping it would fall through to the +// baseline instead (see `ResolvedReleaseAgeRuleConfig`, +// src/config/overrides.ts). The resolved-vs-raw distinction proved here +// simply doesn't apply to those two; their equivalent guarantee is the +// explicit `severity === 'off'` check in each evaluator, covered by tests. +// // This file is never imported by src/index.ts or src/cli.ts, so it never // ships in dist/ โ€” it exists solely for `tsc --noEmit` (pnpm run // typecheck) to enforce the invariant below on every run. If any diff --git a/src/rules/registry-rules.ts b/src/rules/registry-rules.ts new file mode 100644 index 00000000..b8f3caf7 --- /dev/null +++ b/src/rules/registry-rules.ts @@ -0,0 +1,74 @@ +import type { + ResolvedHermexConfig, + ResolvedRulesConfig, +} from '../config/types'; +import type { PackageDistribution } from '../utils/aggregator'; +import { enrichFromRegistry } from '../npm-registry/enricher'; +import { detectDeprecatedPackages } from '../utils/package-rules'; +import { + evaluateReleaseAge, + releaseAgeConnection, + resolveReleaseAgeRule, +} from './release-age'; +import type { RuleViolation } from './shared'; + +/** + * Whether this repo's rules need the npm registry at all โ€” the gate that + * decides if a run makes network calls. + * + * Both registry-backed families are *explicitly* configured or off: their + * implicit `['**']` baselines (`RELEASE_AGE_BASELINE`, + * `DEPRECATED_PACKAGES_BASELINE`) set severity for packages the registry + * was already consulted about, and are deliberately not a reason to consult + * it. So an unconfigured `hermex scan` still makes zero requests. + */ +export function needsRegistry(rules: ResolvedRulesConfig): boolean { + return ( + rules['release-age'].length > 0 || + rules['no-deprecated-packages'].length > 0 + ); +} + +/** + * Runs every registry-backed rule off a single enrichment pass. + * + * `release-age` and `no-deprecated-packages` read different fields of the + * same registry document, so they share one request per installed package + * rather than paying for one each (#107). The release-age policy callback + * is supplied only when that rule is actually configured โ€” without it + * `enrichFromRegistry` records the deprecation fact and skips the timeline + * math entirely. + * + * Call only when `needsRegistry` is true; with both families empty this + * would still iterate every package to produce nothing. + */ +export async function evaluateRegistryRules( + packages: PackageDistribution[], + config: ResolvedHermexConfig, +): Promise<{ + enriched: PackageDistribution[]; + violations: RuleViolation[]; + skipped: number; +}> { + const releaseAgeRules = config.rules['release-age']; + + const { enriched, skipped } = await enrichFromRegistry( + packages, + releaseAgeConnection(config.releaseAge), + releaseAgeRules.length > 0 + ? (packageName) => resolveReleaseAgeRule(packageName, releaseAgeRules) + : undefined, + ); + + return { + enriched, + violations: [ + ...evaluateReleaseAge(enriched, releaseAgeRules), + ...detectDeprecatedPackages( + enriched, + config.rules['no-deprecated-packages'], + ), + ], + skipped, + }; +} diff --git a/src/rules/release-age.ts b/src/rules/release-age.ts index ab7d8d21..b22d9f44 100644 --- a/src/rules/release-age.ts +++ b/src/rules/release-age.ts @@ -4,56 +4,48 @@ import type { ReleaseAgeConfig, } from '../config/types'; import type { PackageDistribution } from '../utils/aggregator'; -import { - enrichWithReleaseAge, - type ReleaseAgeConnection, -} from '../npm-registry/enricher'; +import type { ReleaseAgeConnection } from '../npm-registry/enricher'; import type { RuleViolation } from './shared'; export { resolveReleaseAgeRule }; /** - * Evaluates the `release-age` rule: for every package with an installed - * version, resolves which rule entry governs it (`resolveReleaseAgeRule`, - * last-match-wins against the implicit `['**']` baseline), fetches its - * registry timeline, and โ€” when the governing entry's severity isn't - * `'off'` and the package has actually breached its threshold โ€” emits one - * atomic `ReleaseAgeViolation`. `PackageDistribution.releaseAge` is - * populated for every package regardless (the Packages table's display - * data), independent of whether a violation was also produced. + * Narrows the `releaseAge` config block down to the connection/infra fields + * the registry layer actually takes. Lives here rather than in the command + * layer so nothing above the rules has to know that block's shape. + */ +export function releaseAgeConnection( + config: ReleaseAgeConfig, +): ReleaseAgeConnection { + return { + authToken: config.authToken, + cacheTtlMs: config.cacheTtlMs, + cacheDisabled: config.cacheDisabled, + }; +} + +/** + * Evaluates the `release-age` rule over packages already enriched by + * `enrichFromRegistry`: for each one that breached its threshold, resolves + * which rule entry governs it (`resolveReleaseAgeRule`, last-match-wins + * against the implicit `['**']` baseline) and โ€” unless that entry's + * severity is `'off'` โ€” emits one atomic `ReleaseAgeViolation`. * - * Off entirely when `rules['release-age']` is empty for this repo โ€” no - * registry calls, no `PackageDistribution.releaseAge`. Call sites should - * skip calling this at all in that case rather than relying on an empty - * result, since even one call still pays for iterating `packages`. + * Pure and synchronous. The registry I/O it used to own now lives in + * `enrichFromRegistry`, shared with `no-deprecated-packages` so the two + * registry-backed rules cost one request per package between them (#107). + * `PackageDistribution.releaseAge` is populated for every package + * regardless of whether a violation was also produced โ€” it's the Packages + * table's display data, not a verdict. */ -export async function evaluateReleaseAge( +export function evaluateReleaseAge( packages: PackageDistribution[], - releaseAgeConfig: ReleaseAgeConfig, rules: ResolvedReleaseAgeRuleConfig[], -): Promise<{ - enriched: PackageDistribution[]; - violations: RuleViolation[]; - skipped: number; -}> { - if (rules.length === 0) { - return { enriched: packages, violations: [], skipped: 0 }; - } - - const connection: ReleaseAgeConnection = { - authToken: releaseAgeConfig.authToken, - cacheTtlMs: releaseAgeConfig.cacheTtlMs, - cacheDisabled: releaseAgeConfig.cacheDisabled, - }; - - const { enriched, skipped } = await enrichWithReleaseAge( - packages, - connection, - (packageName) => resolveReleaseAgeRule(packageName, rules), - ); +): RuleViolation[] { + if (rules.length === 0) return []; const violations: RuleViolation[] = []; - for (const pkg of enriched) { + for (const pkg of packages) { const entry = pkg.releaseAge; if (!entry || entry.worstLevel === null) continue; if (entry.severity === 'off') continue; @@ -68,9 +60,8 @@ export async function evaluateReleaseAge( installedVersion: entry.installedVersion, worstLevel: entry.worstLevel, scope: entry.scope, - deprecated: entry.deprecated, }); } - return { enriched, violations, skipped }; + return violations; } diff --git a/src/rules/shared.ts b/src/rules/shared.ts index 1349035d..f794ea0e 100644 --- a/src/rules/shared.ts +++ b/src/rules/shared.ts @@ -49,6 +49,20 @@ export interface NoPackagesViolation extends BaseViolation<'no-packages'> { packageName?: string; } +/** + * One violation per deprecated package โ€” atomic, like every other package + * rule. The publisher's own notice rides along so neither the rules table + * nor a JSON consumer has to go back to the registry for it. + * + * The *fact* of deprecation lives on `PackageDistribution.deprecated`, + * recorded by registry enrichment whether or not any rule judges it. This + * is the *judgment*, and the two are deliberately separate (#107). + */ +export interface NoDeprecatedPackagesViolation extends BaseViolation<'no-deprecated-packages'> { + packageName: string; + deprecated: string; +} + export interface OversizeFile { file: string; sizeBytes: number; @@ -92,7 +106,6 @@ export interface ReleaseAgeViolation extends BaseViolation<'release-age'> { installedVersion: string; worstLevel: 'minor_overdue' | 'major_overdue'; scope: 'root' | 'tree'; - deprecated?: string; } /** @@ -105,6 +118,7 @@ export type CoreRuleViolation = | MaxFileSizeViolation | RequirePackagesViolation | NoPackagesViolation + | NoDeprecatedPackagesViolation | RequireScriptsViolation | RequirePackageFieldsViolation | NoPackageFieldsViolation diff --git a/src/utils/format-utils.ts b/src/utils/format-utils.ts index 8cc8e4c7..d24fcf9a 100644 --- a/src/utils/format-utils.ts +++ b/src/utils/format-utils.ts @@ -30,10 +30,10 @@ export function formatDaysOverdue( /** * Format how long until an upgrade candidate breaches its age threshold - * @returns Formatted string (e.g., "12 days remaining", "1 day remaining") + * @returns Formatted string (e.g., "due in 12 days", "due in 1 day") */ export function formatDaysRemaining(daysRemaining: number): string { - return `${daysRemaining} day${daysRemaining === 1 ? '' : 's'} remaining`; + return `due in ${daysRemaining} day${daysRemaining === 1 ? '' : 's'}`; } /** diff --git a/src/utils/package-distribution.ts b/src/utils/package-distribution.ts index 57c130e6..d2ba1f71 100644 --- a/src/utils/package-distribution.ts +++ b/src/utils/package-distribution.ts @@ -44,6 +44,18 @@ export interface PackageDistribution { * transitive-only package as if it were a root dependency). */ rootVersion?: string | null; + /** + * npm's deprecation notice for the installed version, falling back to the + * package-level one. An inventory fact, not a policy verdict: recorded + * whenever the registry is consulted at all, independent of whether + * `release-age` is configured (#107) โ€” that coupling is what used to make + * deprecation vanish the moment release-age was turned off. Judging it is + * `rules['no-deprecated-packages']`'s job. + * + * Set by `enrichFromRegistry` (`src/npm-registry/enricher.ts`), never by + * `calculatePackageDistribution` โ€” same as `releaseAge` below. + */ + deprecated?: string; releaseAge?: ReleaseAgeEntry; } diff --git a/src/utils/package-flags.ts b/src/utils/package-flags.ts new file mode 100644 index 00000000..bb1e3c0a --- /dev/null +++ b/src/utils/package-flags.ts @@ -0,0 +1,146 @@ +import type { PackageDistribution } from './package-distribution'; +import type { + CoreRuleViolation, + NoDeprecatedPackagesViolation, + NoPackagesViolation, + RuleViolation, +} from '../rules/shared'; +import { isPluginViolation } from '../rules/shared'; +import { severityColor, severityIcon } from './severity-format'; + +/** The badge word a package rule contributes. Severity-neutral on purpose: + * the icon beside it carries how hard the rule is enforced, which is what + * every other surface already does. `[BANNED]`/`[RESTRICTED]` failed that + * test โ€” one rule, two adjectives, and no config concept behind either + * (#86). + * + * Lowercase, matching the semver words in the Minimum target column beside + * it and the rule ids these come from. The icon is already doing the + * shouting; uppercasing the word too just put two casing conventions in one + * row. */ +export type PackageFlagLabel = 'forbidden' | 'deprecated'; + +export interface PackageFlag { + ruleId: CoreRuleViolation['ruleId']; + /** A badge exists if and only if a violation does, so this is always a + * real violation's severity โ€” never a config-resolved one. That's what + * keeps a badge and its rules-table row from ever disagreeing. */ + severity: RuleViolation['severity']; + label: PackageFlagLabel; + /** Long-form context (the publisher's deprecation notice). Rendered in + * the Notes block below the table, never inside a cell โ€” a table cell is + * the wrong shape for a sentence. */ + detail?: string; +} + +/** `undefined` means this rule has nothing to say about this package. */ +type Contribute = ( + pkg: PackageDistribution, + violations: RuleViolation[], +) => Omit | undefined; + +/** + * Plugin findings are excluded before the id check, not merely to narrow + * the type: this column joins on hermex's own rules, and a plugin's id + * space is its own (#102). + */ +function findForbidViolation( + packageName: string, + violations: RuleViolation[], +): NoPackagesViolation | undefined { + return violations.find( + (v): v is NoPackagesViolation => + !isPluginViolation(v) && + v.ruleId === 'no-packages' && + v.packageName === packageName, + ); +} + +function findDeprecatedViolation( + packageName: string, + violations: RuleViolation[], +): NoDeprecatedPackagesViolation | undefined { + return violations.find( + (v): v is NoDeprecatedPackagesViolation => + !isPluginViolation(v) && + v.ruleId === 'no-deprecated-packages' && + v.packageName === packageName, + ); +} + +const forbiddenFlag: Contribute = (pkg, violations) => { + const violation = findForbidViolation(pkg.packageName, violations); + if (!violation) return undefined; + return { severity: violation.severity, label: 'forbidden' }; +}; + +const deprecatedFlag: Contribute = (pkg, violations) => { + const violation = findDeprecatedViolation(pkg.packageName, violations); + if (!violation) return undefined; + return { + severity: violation.severity, + label: 'deprecated', + detail: violation.deprecated, + }; +}; + +/** + * The single place that declares each package rule's badge โ€” the Packages + * table's counterpart to `ONE_ROW_PER_VIOLATION` in `print-rules.ts`. + * Adding a package rule to the table is one entry here and nothing else: + * no new column, no change to any renderer. + * + * An ordered array rather than a Record keyed by rule id, because array + * order IS badge order on the row โ€” a Record would make that ordering + * implicit and fragile. + * + * `release-age` deliberately has no entry. Its verdict is a version + * comparison, not a flag, so its display is the Installed/Minimum target + * columns โ€” the same kind of declared rendering choice `ReleaseAgeViolation` + * (`src/rules/shared.ts`) already makes about it rendering no rules-table + * row either. + */ +const PACKAGE_FLAG_CONTRIBUTORS: readonly { + ruleId: CoreRuleViolation['ruleId']; + contribute: Contribute; +}[] = [ + { ruleId: 'no-packages', contribute: forbiddenFlag }, + { ruleId: 'no-deprecated-packages', contribute: deprecatedFlag }, +]; + +/** + * Every package rule's badge for one row, in declaration order. `ruleId` is + * stamped on from the registry entry rather than returned by the + * contributor, so a contributor structurally cannot mislabel its own rule. + */ +export function collectPackageFlags( + pkg: PackageDistribution, + violations: RuleViolation[], +): PackageFlag[] { + const flags: PackageFlag[] = []; + for (const { ruleId, contribute } of PACKAGE_FLAG_CONTRIBUTORS) { + const flag = contribute(pkg, violations); + if (flag) flags.push({ ruleId, ...flag }); + } + return flags; +} + +/** The Flags cell: `icon label` per badge, space-joined; empty when no + * rule flagged the row. Colored โ€” cli-table3 measures with `string-width`, + * which ignores ANSI, so column math is unaffected. */ +export function formatPackageFlags(flags: PackageFlag[]): string { + return flags + .map( + (flag) => + `${severityIcon(flag.severity)} ${severityColor(flag.severity)(flag.label)}`, + ) + .join(' '); +} + +/** Notes-line facts for the badges carrying long-form context, phrased to + * read as a continuation of the badge itself ("deprecated: "). */ +export function describeFlagDetails(flags: PackageFlag[]): string[] { + return flags + .filter((flag) => flag.detail !== undefined) + .map((flag) => `${flag.label}: ${flag.detail}`); +} diff --git a/src/utils/package-rules.ts b/src/utils/package-rules.ts index 063652ad..b0a2756e 100644 --- a/src/utils/package-rules.ts +++ b/src/utils/package-rules.ts @@ -1,6 +1,11 @@ import micromatch from 'micromatch'; -import type { ResolvedHermexConfig } from '../config/types'; +import type { + ResolvedDeprecatedPackagesRuleConfig, + ResolvedHermexConfig, +} from '../config/types'; +import { resolveDeprecatedPackagesRule } from '../config/overrides'; import type { RuleViolation } from '../rules/evaluator'; +import type { PackageDistribution } from './package-distribution'; import type { PackageInventoryEntry } from './package-inventory'; import { isInstalled, isOwnedByRepo, isUsed } from './package-inventory'; @@ -87,3 +92,48 @@ export function detectRequiredPackages( } return violations; } + +/** + * Judges the deprecation notice registry enrichment already recorded on + * each package. A pure, synchronous rule over `PackageDistribution.deprecated` + * โ€” it performs no I/O of its own (#107). + * + * Unlike its siblings above it cannot run during `aggregateReports`: + * `deprecated` doesn't exist until `enrichFromRegistry` has run, so the + * pipeline calls this from the registry block instead. That's also why it + * takes `PackageDistribution[]` rather than the inventory โ€” the + * distribution is the axis enrichment writes to. + * + * Resolution is last-match-wins against an implicit `['**']` baseline + * (`resolveDeprecatedPackagesRule`), the same governance `release-age` + * uses, rather than "every matching entry fires" โ€” so exactly one entry + * governs a package and `severity: 'off'` genuinely exempts it. The + * baseline is why a deprecated package stays visible with no rule authored + * at all, at 'info', never affecting the verdict. + * + * Scope matches `detectForbiddenPackages`: only packages the repo owns + * reach here, since `calculatePackageDistribution` already selected them. A + * purely transitive deprecated package isn't the repo's to fix. + */ +export function detectDeprecatedPackages( + packages: PackageDistribution[], + rules: ResolvedDeprecatedPackagesRuleConfig[], +): RuleViolation[] { + const violations: RuleViolation[] = []; + for (const pkg of packages) { + if (!pkg.deprecated) continue; + + const rule = resolveDeprecatedPackagesRule(pkg.packageName, rules); + if (rule.severity === 'off') continue; + + violations.push({ + ruleId: 'no-deprecated-packages', + severity: rule.severity, + patterns: rule.patterns, + message: rule.message, + packageName: pkg.packageName, + deprecated: pkg.deprecated, + }); + } + return violations; +} diff --git a/src/utils/print-packages.ts b/src/utils/print-packages.ts index 8695b1e6..9c9f403a 100644 --- a/src/utils/print-packages.ts +++ b/src/utils/print-packages.ts @@ -2,40 +2,24 @@ import chalk from 'chalk'; import Table from 'cli-table3'; import type { AggregatedReport, PackageDistribution } from './aggregator'; import type { RuleViolation } from '../rules/evaluator'; -import { isPluginViolation } from '../rules/shared'; import type { AvailableUpgrade, ReleaseAgeEntry, SemverBump, } from '../npm-registry/types'; import { formatDaysOverdue, formatDaysRemaining } from './format-utils'; +import { formatSeverityTally, severityIcon } from './severity-format'; +import type { PackageFlag } from './package-flags'; import { - formatSeverityTally, - severityIcon, - severityColor, -} from './severity-format'; + collectPackageFlags, + describeFlagDetails, + formatPackageFlags, +} from './package-flags'; function printHeader() { console.log(chalk.blueBright.bold('\n๐Ÿ“ฆ Packages\n')); } -export function formatPackageName( - pkg: PackageDistribution, - banned?: RuleViolation, -): string { - let prefix = ''; - if (pkg.releaseAge?.deprecated) { - prefix += severityColor('error')('[DEPRECATED] '); - } - if (banned) { - prefix += - banned.severity === 'error' - ? severityColor('error')('[BANNED] ') - : severityColor('warn')('[RESTRICTED] '); - } - return prefix + pkg.packageName; -} - // Describe the recommended upgrade for a breached tier. // // When a genuinely in-window compliant release exists (`compliantTarget`), @@ -45,25 +29,28 @@ export function formatPackageName( // breached tier has been out of compliance, measured from its oldest breaching // release (#24). // -// Only when there's no in-window target at all does the "no compliant release -// available" wording apply โ€” every candidate is itself past its threshold, so -// a day count would imply a countdown that was never achievable (#26). +// When there is no in-window target, the breached tier's own newest release +// is still the right thing to name. Being on the newest release that exists +// never breaches at all โ€” nothing is newer than installed, so no tier is +// breached and no row is produced โ€” which means this branch is only ever +// reached when a newer release DOES exist and has simply not been taken. +// It used to read "no compliant release available", which contradicted the +// version printed beside it and contradicted `minCompliantVersion`'s own +// rule that being on latest counts as compliant (#26's wording, not its +// arithmetic โ€” the day count below is unchanged). export function describeUpgradeTarget( top: AvailableUpgrade, compliantTarget?: { version: string; bump: SemverBump }, ): string { - if (compliantTarget) { - const overdue = formatDaysOverdue( - top.breachReleasedDaysAgo, - top.thresholdDays, - ); - return `${compliantTarget.bump} ${compliantTarget.version} (${overdue})`; - } - const overdue = - top.releasedDaysAgo > top.thresholdDays - ? 'no compliant release available' - : formatDaysOverdue(top.breachReleasedDaysAgo, top.thresholdDays); - return `${top.semverBump} ${top.version} (${overdue})`; + const version = compliantTarget?.version ?? top.version; + const bump = compliantTarget?.bump ?? top.semverBump; + // Counted from the breached tier's OLDEST release: how long the tier has + // been out of compliance, not how old the recommended target is (#24). + const overdue = formatDaysOverdue( + top.breachReleasedDaysAgo, + top.thresholdDays, + ); + return `${version} (${bump}, ${overdue})`; } // Prefer a genuinely compliant, still-in-window release as the recommended @@ -148,62 +135,83 @@ export interface PackageNote { // stdout is the right place for "here's some extra context" (#59). export function describePackageNotes( pkg: PackageDistribution, + flags: PackageFlag[] = [], ): PackageNote | undefined { const facts = [ describeBundleImpact(pkg), describeAdvisoryBreaches(pkg.releaseAge), + // Appended, not prepended: a badge's long-form context (the publisher's + // deprecation notice) elaborates on the Flags column, so it reads last, + // after the facts about the package's own installed copies. + ...describeFlagDetails(flags), ].filter((fact): fact is string => Boolean(fact)); if (facts.length === 0) return undefined; return { icon: severityIcon('info'), facts }; } -export function formatUpgradeCell(releaseAge?: ReleaseAgeEntry): string { +/** + * The recommended upgrade as text, with no icon and no verdict: the most + * conservative version that clears the breach, or the countdown for one + * that is merely coming due. Empty when there is nothing to recommend. + * + * Shared verbatim with `--summary-file`, which renders its own severity in + * a leading column of its own and so wants the text alone. That sharing is + * what makes "both surfaces recommend the same version" structural rather + * than a pair of call sites kept in step by hand (#57). + */ +export function describeMinimumTarget(releaseAge?: ReleaseAgeEntry): string { if (!releaseAge) return ''; - const { worstLevel, upgrades, severity, pendingUpgrade } = releaseAge; + const { worstLevel, upgrades, pendingUpgrade } = releaseAge; if (!worstLevel) { - if (pendingUpgrade) { - return `${severityIcon('info')} ${pendingUpgrade.semverBump} ${pendingUpgrade.version} (${formatDaysRemaining(pendingUpgrade.daysRemaining)})`; - } - return severityIcon('success'); + return pendingUpgrade + ? `${pendingUpgrade.version} (${pendingUpgrade.semverBump}, ${formatDaysRemaining(pendingUpgrade.daysRemaining)})` + : ''; } const top = upgrades[0]; - if (!top) return severityIcon('success'); + if (!top) return ''; - const suffix = severity === 'warn' ? chalk.gray(' [not enforced]') : ''; - const description = describeUpgradeTarget( - top, - resolveCompliantTarget(releaseAge), - ); - - // The status reflects severity, not which tier breached: an enforced package - // fails comply whether the worst breach is minor_overdue or major_overdue - // (#28), so it renders red โ€” never a softer yellow just because the breached - // tier happens to be minor. - const icon = severityIcon(severity === 'warn' ? 'warn' : 'error'); - return `${icon} ${description}${suffix}`; + return describeUpgradeTarget(top, resolveCompliantTarget(releaseAge)); } +/** No release-age verdict for this row at all. Deliberately not blank and + * not ๐ŸŸข: an em dash says hermex has no opinion here โ€” either the package + * has no installed version to check (declared but absent from the lockfile, + * which the Installed column shows as `N/A`) or the registry never answered + * for it. ๐ŸŸข is the opposite claim: it looked, and there is nothing to do. */ +const NO_TARGET = 'โ€”'; + /** - * The `no-packages` hit for this package, if any. Filters `ruleViolations` - * by `ruleId` rather than reading a dedicated banned-packages array, which is - * what #77 removed โ€” `packageName` is what keeps the join to a table row - * exact, since a violation carries no file paths to match on. + * The human table's "Minimum target" cell: `describeMinimumTarget` prefixed + * by release-age's own verdict. + * + * An icon here means a rule is judging this package, and which icon means + * how hard. So an entry at severity 'off' renders the target text with no + * icon at all โ€” it produced no violation, and inheriting red from a + * fallback ternary (as it used to) claimed a verdict nobody made. + * + * A pending upgrade keeps the all-clear icon rather than going bare or + * borrowing a severity: nothing is overdue, so the row is genuinely fine โ€” + * it just has a date attached. Bare cells next to iconned ones read as a + * rendering failure rather than as a deliberate absence of verdict. */ -export function findForbidViolation( - pkg: PackageDistribution, - violations: RuleViolation[], -): RuleViolation | undefined { - return violations.find( - (v) => - // Plugin findings are excluded before the id check, not just to - // narrow the type: this column joins on hermex's own `no-packages` - // rule, and a plugin's id space is its own (#102). - !isPluginViolation(v) && - v.ruleId === 'no-packages' && - v.packageName === pkg.packageName, - ); +export function formatMinimumTargetCell(releaseAge?: ReleaseAgeEntry): string { + if (!releaseAge) return NO_TARGET; + const { worstLevel, upgrades, severity } = releaseAge; + const description = describeMinimumTarget(releaseAge); + + if (!worstLevel || !upgrades[0]) { + const ok = severityIcon('success'); + return description ? `${ok} ${description}` : ok; + } + + // Severity, not which tier breached: an enforced package fails comply + // whether the worst breach is minor_overdue or major_overdue (#28), so it + // renders red โ€” never a softer yellow just because the breached tier + // happens to be minor. + if (severity === 'off') return description; + return `${severityIcon(severity)} ${description}`; } export function printPackages( @@ -235,13 +243,22 @@ function printPackagesTable( printHeader(); const hasReleaseAge = packages.some((p) => p.releaseAge !== undefined); + const flags = packages.map((pkg) => collectPackageFlags(pkg, violations)); + // Only worth a column once something has actually landed in it โ€” + // otherwise every repo with no package-rule hits grows a column of blanks + // where it used to have a clean two-column table. + const hasFlags = flags.some((rowFlags) => rowFlags.length > 0); + // With release age on, "Version" splits into "Installed" (the single - // version the verdict was actually measured against) and "Target" (the - // recommended upgrade) โ€” cramming a multi-version list and an upgrade - // recommendation into one cell was exactly the ambiguity #57 reported. + // version the verdict was actually measured against) and "Minimum target" + // (the most conservative upgrade that clears the breach) โ€” cramming a + // multi-version list and an upgrade recommendation into one cell was + // exactly the ambiguity #57 reported. "Minimum" is load-bearing: the cell + // deliberately does not name the latest release (#24, #26). const head = hasReleaseAge - ? ['Package', 'Installed', 'Target'] + ? ['Package', 'Installed', 'Minimum target'] : ['Package', 'Version']; + if (hasFlags) head.push('Flags'); const table = new Table({ head, @@ -251,13 +268,19 @@ function printPackagesTable( }, }); - packages.forEach((pkg) => { - const row = [formatPackageName(pkg, findForbidViolation(pkg, violations))]; + packages.forEach((pkg, index) => { + // Just the name. The badges that used to be glued on as prefixes are a + // column of their own now (#86). + const row = [pkg.packageName]; if (hasReleaseAge) { - row.push(resolveInstalledVersion(pkg), formatUpgradeCell(pkg.releaseAge)); + row.push( + resolveInstalledVersion(pkg), + formatMinimumTargetCell(pkg.releaseAge), + ); } else { row.push(pkg.version || 'N/A'); } + if (hasFlags) row.push(formatPackageFlags(flags[index])); table.push(row); }); @@ -268,7 +291,10 @@ function printPackagesTable( // notes below the table rather than inside a cell, so the table itself // stays a clean "installed โ†’ target" comparison (#57). const notes = packages - .map((pkg) => ({ pkg, note: describePackageNotes(pkg) })) + .map((pkg, index) => ({ + pkg, + note: describePackageNotes(pkg, flags[index]), + })) .filter( (entry): entry is { pkg: PackageDistribution; note: PackageNote } => entry.note !== undefined, @@ -284,8 +310,9 @@ function printPackagesTable( // The same "N errors, M warnings" tally style as the Rules section // (`print-rules.ts`), computed from this table's own release-age // violations โ€” the only violation kind this table uniquely surfaces (a - // banned package's no-packages hit is already counted in the Rules - // tally; [BANNED] here is just a cross-reference, not a second count). + // banned or deprecated package's own hit is already counted in the Rules + // tally; the Flags badge here is just a cross-reference, not a second + // count). // A plain package count ("N packages total") said nothing about // compliance and didn't add up with anything else on screen โ€” this does: // Rules-tally + Packages-tally always equals the overall mandatory count. @@ -325,16 +352,20 @@ function printPackagesChart( (pkg.percentage / maxPercentage) * maxBarWidth, ); const emptyLength = maxBarWidth - barLength; - const label = formatPackageName( - pkg, - findForbidViolation(pkg, violations), - ).padEnd(maxLabelLength, ' '); + // Padded on the bare name โ€” which is exactly what `maxLabelLength` + // measured. Badges trail the row instead of prefixing the label: as a + // prefix they made every flagged row wider than the padding allowed + // for (and carried ANSI codes `.length` counted as visible width), + // which is the misalignment #86 reported. + const label = pkg.packageName.padEnd(maxLabelLength, ' '); const bar = chalk.green('โ–ˆ'.repeat(barLength)) + chalk.gray('โ–‘'.repeat(emptyLength)); + const badges = formatPackageFlags(collectPackageFlags(pkg, violations)); + console.log( - `${label} ${bar} ${chalk.bold(pkg.percentage.toFixed(1) + '%')} (${pkg.usageCount})`, + `${label} ${bar} ${chalk.bold(pkg.percentage.toFixed(1) + '%')} (${pkg.usageCount})${badges ? ` ${badges}` : ''}`, ); }); } diff --git a/src/utils/print-rules.ts b/src/utils/print-rules.ts index 2586496b..071e0a03 100644 --- a/src/utils/print-rules.ts +++ b/src/utils/print-rules.ts @@ -30,6 +30,8 @@ export function formatRuleType(violation: RuleViolation): string { return 'require-packages'; case 'no-packages': return 'no-packages'; + case 'no-deprecated-packages': + return 'no-deprecated-packages'; case 'require-scripts': return 'require-scripts'; case 'require-package-fields': @@ -125,9 +127,9 @@ function describeGroup(group: CoreRuleViolation[]): string { } } -/** One row per violation โ€” each subject (a forbidden package, a forbidden - * field) is individually actionable, so folding them would hide which - * specific ones to fix. Matches today's existing display for these rules, +/** One row per violation โ€” each subject (a forbidden package, a deprecated + * package, a forbidden field) is individually actionable, so folding them + * would hide which specific ones to fix. Matches today's existing display for these rules, * which already emit one violation per subject. */ function describeIndividual(v: CoreRuleViolation): string { const patterns = v.patterns.join(', '); @@ -135,6 +137,13 @@ function describeIndividual(v: CoreRuleViolation): string { if (v.ruleId === 'no-packages') return `${v.packageName ?? patterns} is forbidden${suffix}`; + // The publisher's notice stands in for a message only when the policy + // author wrote none: two gray em-dash clauses in a row read terribly, and + // between the two the author's own wording should win. Showing it at all + // matters because this row is the only place the notice reaches + // `--summary-file`, which renders Rules but never the stdout Notes block. + if (v.ruleId === 'no-deprecated-packages') + return `${v.packageName} is deprecated${suffix || chalk.gray(` โ€” ${v.deprecated}`)}`; if (v.ruleId === 'no-package-fields') return `field ${v.fieldPath ?? patterns} is forbidden in package.json${suffix}`; @@ -161,6 +170,7 @@ function describePlugin(v: PluginViolation): string { * declares each rule's rendering strategy. */ const ONE_ROW_PER_VIOLATION = new Set([ 'no-packages', + 'no-deprecated-packages', 'no-package-fields', ]); @@ -176,7 +186,8 @@ export interface Row { /** * Describes a single violation on its own โ€” for a plugin finding or a - * `no-packages`/`no-package-fields` hit this is exactly what shows up in + * `no-packages`/`no-deprecated-packages`/`no-package-fields` hit this is + * exactly what shows up in * `buildRuleRows`'s output (each already renders one row per violation); for * a fold-style rule (`no-files`, `max-file-size`, `require-codeowners`, or * any absence rule) it describes that one violation in isolation, as if it @@ -196,13 +207,14 @@ export function describeViolation(v: RuleViolation): string { * Turns a flat list of atomic violations into display rows: one row per * group for fold-style rules (`no-files`, `max-file-size`, * `require-codeowners`, and every absence rule, which are always - * single-member groups), one row per violation for `no-packages`/ - * `no-package-fields`, one row per finding for plugins, and zero rows for - * `release-age` (its display is the Packages table). Shared by the terminal - * table (`printRules`) and `--summary-file` (`write-summary-file.ts`) so - * the two surfaces can never render a different row count for the same - * violations โ€” pass violations pre-sorted (`sortViolationsBySeverity`) if - * row order matters to the caller. + * single-member groups), one row per violation for the package and field + * rules in `ONE_ROW_PER_VIOLATION`, one row per finding for plugins, and + * zero rows for `release-age` (its display is the Packages table). Shared + * by the terminal table (`printRules`) and `--summary-file` + * (`write-summary-file.ts`) so the two surfaces can never render a + * different row count for the same violations โ€” pass violations + * pre-sorted (`sortViolationsBySeverity`) if row order matters to the + * caller. */ export function buildRuleRows(violations: RuleViolation[]): Row[] { const rows: Row[] = []; diff --git a/src/utils/write-summary-file.ts b/src/utils/write-summary-file.ts index 3a0a8c4e..50a9b224 100644 --- a/src/utils/write-summary-file.ts +++ b/src/utils/write-summary-file.ts @@ -4,10 +4,10 @@ import type { ComplianceResult } from './compliance'; import { countMandatoryViolations } from './compliance'; import { buildRuleRows } from './print-rules'; import { - describeUpgradeTarget, - resolveCompliantTarget, + describeMinimumTarget, resolveInstalledVersion, } from './print-packages'; +import { collectPackageFlags, formatPackageFlags } from './package-flags'; import { formatSeverityTally, severityIcon, @@ -79,6 +79,12 @@ function buildRulesSection(aggregated: AggregatedReport): string { // summary meant for a PR comment or CI check reads any colored row/line as // something that needs attention. That context belongs in the human // `--format human` table (stdout), not in a surface used for gating (#59). +// +// A mandatory `no-deprecated-packages` hit doesn't get a row of its own +// here either, for the same reason banned packages don't: it renders an +// ordinary row in the Rules section above, which release-age never does. +// The Flags column below still carries it as a cross-reference on the +// rows that ARE listed. function buildPackagesSection(aggregated: AggregatedReport): string { const failingPackageNames = new Set( aggregated.ruleViolations @@ -97,19 +103,19 @@ function buildPackagesSection(aggregated: AggregatedReport): string { const lines: string[] = [ '### Packages', '', - '| | Package | Installed | Target |', - '|---|---|---|---|', + '| | Package | Installed | Minimum target | Flags |', + '|---|---|---|---|---|', ]; + // The target cell comes from the same function the human table renders, + // rather than re-deriving it from the same two helpers โ€” which makes + // #57's "both surfaces recommend the same version" a structural + // property instead of two call sites kept in step by hand. for (const pkg of mandatory) { - const top = pkg.releaseAge?.upgrades[0]; - const reasons: string[] = []; - if (top) - reasons.push( - describeUpgradeTarget(top, resolveCompliantTarget(pkg.releaseAge)), - ); - if (pkg.releaseAge?.deprecated) reasons.push('deprecated'); + const flags = formatPackageFlags( + collectPackageFlags(pkg, aggregated.ruleViolations), + ); lines.push( - `| ${severityIcon('error')} | \`${pkg.packageName}\` | ${resolveInstalledVersion(pkg)} | ${reasons.join(', ')} |`, + `| ${severityIcon('error')} | \`${pkg.packageName}\` | ${resolveInstalledVersion(pkg)} | ${describeMinimumTarget(pkg.releaseAge)} | ${flags} |`, ); } diff --git a/tests/config/overrides.test.ts b/tests/config/overrides.test.ts index 270ccc1b..bbc583a9 100644 --- a/tests/config/overrides.test.ts +++ b/tests/config/overrides.test.ts @@ -4,6 +4,7 @@ import { join } from 'node:path'; import { tmpdir } from 'node:os'; import { applyOverrides, + resolveDeprecatedPackagesRule, resolveReleaseAgeRule, } from '../../src/config/overrides'; import { HermexConfigSchema } from '../../src/config/schema'; @@ -1082,3 +1083,102 @@ describe('resolveReleaseAgeRule', () => { }); }); }); + +// no-deprecated-packages is the second family resolved by last-match-wins +// governance against an implicit baseline, so it inherits release-age's +// 'off' semantics rather than the ordinary "a dropped entry never fires" +// ones (#107). These tests are the executable record of that choice โ€” the +// contrast with no-packages directly above is the point. +describe('no-deprecated-packages resolution', () => { + it('keeps an "off" entry in the resolved rules array, the way release-age does', () => { + const dir = makeRepo('some-app'); + try { + const config = createConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'off', patterns: ['@internal/*'] }, + ], + }, + }); + const result = applyOverrides(config, dir); + expect(result.rules['no-deprecated-packages']).toEqual([ + { severity: 'off', patterns: ['@internal/*'] }, + ]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('drops an "off" no-packages entry, for contrast โ€” that rule fires per matching entry', () => { + const dir = makeRepo('some-app'); + try { + const config = createConfig({ + rules: { + 'no-packages': [{ severity: 'off', patterns: ['@internal/*'] }], + }, + }); + const result = applyOverrides(config, dir); + expect(result.rules['no-packages']).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('upserts an override on top of the base by patterns identity', () => { + const dir = makeRepo('@acme/internal-tools-x'); + try { + const config = createConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'error', patterns: ['@acme/*'] }, + ], + }, + overrides: [ + { + match: ['@acme/internal-tools-*'], + rules: { + 'no-deprecated-packages': [ + { severity: 'off', patterns: ['@acme/*'] }, + ], + }, + }, + ], + }); + const result = applyOverrides(config, dir); + expect(result.rules['no-deprecated-packages']).toEqual([ + { severity: 'off', patterns: ['@acme/*'] }, + ]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe('resolveDeprecatedPackagesRule', () => { + it('falls back to the info baseline for a package no entry matches', () => { + expect(resolveDeprecatedPackagesRule('request', [])).toEqual({ + severity: 'info', + patterns: ['**'], + }); + }); + + it('lets the last matching entry win', () => { + const rules = [ + { severity: 'error' as const, patterns: ['**'] }, + { severity: 'off' as const, patterns: ['@internal/*'] }, + ]; + expect(resolveDeprecatedPackagesRule('@internal/ui', rules).severity).toBe( + 'off', + ); + expect(resolveDeprecatedPackagesRule('moment', rules).severity).toBe( + 'error', + ); + }); + + it('lets an authored ["**"] entry replace the baseline outright', () => { + const rules = [{ severity: 'error' as const, patterns: ['**'] }]; + expect(resolveDeprecatedPackagesRule('anything', rules).severity).toBe( + 'error', + ); + }); +}); diff --git a/tests/helpers/mock-reports.ts b/tests/helpers/mock-reports.ts index 6163bef8..8c167a71 100644 --- a/tests/helpers/mock-reports.ts +++ b/tests/helpers/mock-reports.ts @@ -4,7 +4,12 @@ import type { PackageInventoryEntry, } from '../../src/utils/aggregator'; import type { ReleaseAgeEntry } from '../../src/npm-registry/types'; -import type { ReleaseAgeViolation } from '../../src/rules/evaluator'; +// From ./shared, not ./evaluator: evaluator re-exports only RuleViolation, so +// these two named imports never actually resolved (tests aren't typechecked). +import type { + NoDeprecatedPackagesViolation, + ReleaseAgeViolation, +} from '../../src/rules/shared'; /** * Creates a minimal UsageReport with all required fields. @@ -144,3 +149,19 @@ export function createMockReleaseAgeViolation( ...overrides, }; } + +/** A `no-deprecated-packages` hit, the shape `detectDeprecatedPackages` + * emits. Severity defaults to the rule family's own baseline (#107). */ +export function createMockDeprecatedViolation( + packageName: string, + overrides: Partial = {}, +): NoDeprecatedPackagesViolation { + return { + ruleId: 'no-deprecated-packages', + severity: 'info', + patterns: ['**'], + packageName, + deprecated: 'no longer maintained', + ...overrides, + }; +} diff --git a/tests/npm-registry/enricher.test.ts b/tests/npm-registry/enricher.test.ts index dea1124f..3c5feb7c 100644 --- a/tests/npm-registry/enricher.test.ts +++ b/tests/npm-registry/enricher.test.ts @@ -2,7 +2,7 @@ import semver from 'semver'; import micromatch from 'micromatch'; import { - enrichWithReleaseAge, + enrichFromRegistry, type ReleaseAgeConnection, type ReleaseAgePolicy, } from '../../src/npm-registry/enricher'; @@ -24,8 +24,8 @@ const DEFAULT_THRESHOLDS = { patch: 30, minor: 45, major: 60 }; /** * Adapts this file's pre-rule-ification config shape (enforceOn/scope/ * scopeExceptions as a flat global object) onto the current - * `enrichWithReleaseAge(packages, connection, resolvePolicy)` signature โ€” - * `resolvePolicy` here replicates exactly what `enrichWithReleaseAge` used + * `enrichFromRegistry(packages, connection, resolvePolicy)` signature โ€” + * `resolvePolicy` here replicates exactly what `enrichFromRegistry` used * to compute internally before policy resolution moved to * `resolveReleaseAgeRule` (`src/config/overrides.ts`): severity from a * plain `enforceOn` glob match (no special-casing empty โ€” `#173`), scope @@ -65,7 +65,7 @@ function callEnrich( : config.scope; return { severity, thresholds: config.thresholds, scope }; }; - return enrichWithReleaseAge(packages, connection, resolvePolicy); + return enrichFromRegistry(packages, connection, resolvePolicy); } function daysAgo(n: number): string { @@ -76,7 +76,7 @@ beforeEach(() => { vi.clearAllMocks(); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ skipped packages', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ skipped packages', () => { it('skips packages with no version', async () => { const pkg = createMockPackage('react', { version: null }); const { enriched } = await callEnrich([pkg], BASE_CONFIG); @@ -202,7 +202,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ skipped packages', () => { }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ upgrade detection', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ upgrade detection', () => { it('no upgrade when all newer versions are within threshold', async () => { const pkg = createMockPackage('react', { version: '18.0.0' }); mockFetch.mockResolvedValueOnce({ @@ -277,7 +277,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ upgrade detection', () => { expect(enriched[0].releaseAge?.pendingUpgrade).toBeUndefined(); }); - it('surfaces deprecation notice from versions field', async () => { + it('records a version-specific deprecation notice as an inventory fact, not on releaseAge (#107)', async () => { const pkg = createMockPackage('old-pkg', { version: '1.0.0' }); mockFetch.mockResolvedValueOnce({ name: 'old-pkg', @@ -285,11 +285,64 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ upgrade detection', () => { versions: { '1.0.0': { deprecated: 'Use new-pkg instead' } }, }); const { enriched } = await callEnrich([pkg], BASE_CONFIG); - expect(enriched[0].releaseAge?.deprecated).toBe('Use new-pkg instead'); + expect(enriched[0].deprecated).toBe('Use new-pkg instead'); + // It used to ride along on the release-age entry, which is exactly the + // coupling #107 removed. + expect('deprecated' in (enriched[0].releaseAge ?? {})).toBe(false); + }); + + it('falls back to the package-level deprecation notice when the installed version has none', async () => { + const pkg = createMockPackage('old-pkg', { version: '1.0.0' }); + mockFetch.mockResolvedValueOnce({ + name: 'old-pkg', + time: { '1.0.0': daysAgo(500) }, + deprecated: 'this package is no longer maintained', + versions: { '1.0.0': {} }, + }); + const { enriched } = await callEnrich([pkg], BASE_CONFIG); + expect(enriched[0].deprecated).toBe('this package is no longer maintained'); + }); + + it('ignores a non-string deprecated field (npm historically used booleans)', async () => { + const pkg = createMockPackage('old-pkg', { version: '1.0.0' }); + mockFetch.mockResolvedValueOnce({ + name: 'old-pkg', + time: { '1.0.0': daysAgo(500) }, + deprecated: true, + versions: { '1.0.0': {} }, + }); + const { enriched } = await callEnrich([pkg], BASE_CONFIG); + expect(enriched[0].deprecated).toBeUndefined(); + }); + + it('records deprecation with no release-age policy at all โ€” the #107 core', async () => { + const pkg = createMockPackage('old-pkg', { version: '1.0.0' }); + mockFetch.mockResolvedValueOnce({ + name: 'old-pkg', + time: { '1.0.0': daysAgo(500) }, + versions: { '1.0.0': { deprecated: 'Use new-pkg instead' } }, + }); + // No resolvePolicy: release-age is off for this repo. Deprecation must + // still be recorded โ€” turning release-age off used to silently take + // deprecation detection with it. + const { enriched } = await enrichFromRegistry([pkg], { + cacheDisabled: false, + }); + expect(enriched[0].deprecated).toBe('Use new-pkg instead'); + expect(enriched[0].releaseAge).toBeUndefined(); + }); + + it('records neither deprecation nor releaseAge when the fetch itself fails', async () => { + const pkg = createMockPackage('old-pkg', { version: '1.0.0' }); + mockFetch.mockResolvedValueOnce(null); + const { enriched, skipped } = await callEnrich([pkg], BASE_CONFIG); + expect(skipped).toBe(1); + expect(enriched[0].deprecated).toBeUndefined(); + expect(enriched[0].releaseAge).toBeUndefined(); }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ batching', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ batching', () => { it('processes multiple packages and returns all enriched', async () => { const packages = [ createMockPackage('react', { version: '18.0.0' }), @@ -307,7 +360,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ batching', () => { }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ latest version reporting (#14)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ latest version reporting (#14)', () => { it('surfaces the newest stable release per tier, not the original alpha/rc', async () => { const pkg = createMockPackage('react-router-dom', { version: '5.3.4' }); mockFetch.mockResolvedValueOnce({ @@ -367,7 +420,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ latest version reporting (#14)', () => { }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ prerelease exclusion (#20)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ prerelease exclusion (#20)', () => { it('minCompliantVersion skips a prerelease even when it is the most recent qualifying version', async () => { const pkg = createMockPackage('@acme-ui/localize', { version: '4.1.12' }); mockFetch.mockResolvedValueOnce({ @@ -418,7 +471,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ prerelease exclusion (#20)', () => { }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ minCompliantVersion for all bump tiers (#21)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ minCompliantVersion for all bump tiers (#21)', () => { it('finds a compliant release inside an intermediate major line, not just the latest', async () => { // A real-world shape: installed 10.x. 11.0.0 is old enough (400d) to // breach the 60-day major threshold and drive worstLevel to @@ -544,7 +597,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ minCompliantVersion for all bump tiers ( }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ minCompliantVersion falls back to latest (#26)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ minCompliantVersion falls back to latest (#26)', () => { it('falls back minCompliantVersion to latest for display but still reports major_overdue when a breached major tier exists (#29)', async () => { const pkg = createMockPackage('@acme-ui/stale-lib', { version: '1.0.0' }); mockFetch.mockResolvedValueOnce({ @@ -635,7 +688,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ minCompliantVersion falls back to latest }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ enforceOn severity scoping (#18)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ enforceOn severity scoping (#18)', () => { it('marks matched packages error and everything else warn when enforceOn is set', async () => { const packages = [ createMockPackage('@my-org/internal-pkg', { version: '1.0.0' }), @@ -723,7 +776,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ enforceOn severity scoping (#18)', () => }); }); -describe('enrichWithReleaseAge รขโ‚ฌโ€ overdue basis uses oldest breach, not newest target (#24)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ overdue basis uses oldest breach, not newest target (#24)', () => { it('reports breachReleasedDaysAgo from the oldest release in the tier while releasedDaysAgo stays on the newest target', async () => { // A real-world shape: the major line breached its 60-day threshold // ~1000 days ago, but the recommended upgrade target (latest) was only @@ -792,7 +845,7 @@ describe('enrichWithReleaseAge รขโ‚ฌโ€ overdue basis uses oldest breach, not ne // "a more specific rule entry sets its own scope", lives in // tests/rules/release-age.test.ts. -describe('enrichWithReleaseAge รขโ‚ฌโ€ scope (#57)', () => { +describe('enrichFromRegistry รขโ‚ฌโ€ scope (#57)', () => { // Shared fixture: relative to '1.0.0', both '2.0.0' (400d old) and '3.0.0' // (200d old) are major bumps past the 60-day threshold รขโ‚ฌโ€ so '1.0.0' alone // is major_overdue. Relative to '3.0.0', there's nothing newer at all, so diff --git a/tests/utils/format-utils.test.ts b/tests/utils/format-utils.test.ts index 9ed8a0b5..748cc761 100644 --- a/tests/utils/format-utils.test.ts +++ b/tests/utils/format-utils.test.ts @@ -31,11 +31,11 @@ describe('formatDaysOverdue', () => { describe('formatDaysRemaining', () => { it('pluralizes "days" when the remaining count is not 1', () => { - expect(formatDaysRemaining(12)).toBe('12 days remaining'); + expect(formatDaysRemaining(12)).toBe('due in 12 days'); }); it('uses the singular "day" when the remaining count is exactly 1', () => { - expect(formatDaysRemaining(1)).toBe('1 day remaining'); + expect(formatDaysRemaining(1)).toBe('due in 1 day'); }); }); diff --git a/tests/utils/package-flags.test.ts b/tests/utils/package-flags.test.ts new file mode 100644 index 00000000..1bff80e8 --- /dev/null +++ b/tests/utils/package-flags.test.ts @@ -0,0 +1,132 @@ +import { describe, expect, it } from 'vitest'; +import type { RuleViolation } from '../../src/rules/evaluator'; +import { stripAnsi } from '../../src/utils/severity-format'; +import { + collectPackageFlags, + describeFlagDetails, + formatPackageFlags, +} from '../../src/utils/package-flags'; +import { + createMockPackage, + createMockDeprecatedViolation, +} from '../helpers/mock-reports'; + +/** A no-packages hit, the shape `detectForbiddenPackages` emits (#77). */ +function forbidViolation( + packageName: string, + severity: RuleViolation['severity'] = 'error', + message?: string, +): RuleViolation { + return { + ruleId: 'no-packages', + severity, + patterns: [packageName], + message, + packageName, + }; +} + +describe('package flags', () => { + it('contributes nothing for a package no package rule flagged', () => { + const pkg = createMockPackage('react'); + expect(collectPackageFlags(pkg, [])).toEqual([]); + expect(formatPackageFlags([])).toBe(''); + }); + + it('badges a banned package as forbidden at the severity of its own violation', () => { + const pkg = createMockPackage('moment'); + const statuses = collectPackageFlags(pkg, [ + forbidViolation('moment', 'warn'), + ]); + expect(statuses).toEqual([ + { ruleId: 'no-packages', severity: 'warn', label: 'forbidden' }, + ]); + expect(stripAnsi(formatPackageFlags(statuses))).toBe('๐ŸŸก forbidden'); + }); + + // The single badge #86 asked for: one rule, one word, severity in the + // icon. [RESTRICTED] named a config concept that never existed, and + // collapsed warn and info into the same string. + it('uses the same word at every severity, varying only the icon', () => { + const pkg = createMockPackage('moment'); + const render = (severity: RuleViolation['severity']) => + stripAnsi( + formatPackageFlags( + collectPackageFlags(pkg, [forbidViolation('moment', severity)]), + ), + ); + expect(render('error')).toBe('๐Ÿ”ด forbidden'); + expect(render('warn')).toBe('๐ŸŸก forbidden'); + expect(render('info')).toBe('๐Ÿ”ต forbidden'); + }); + + it('badges a deprecated package and carries the publisher notice as detail', () => { + const pkg = createMockPackage('request', { + deprecated: 'request has been deprecated', + }); + const statuses = collectPackageFlags(pkg, [ + createMockDeprecatedViolation('request', { + deprecated: 'request has been deprecated', + }), + ]); + expect(statuses[0].label).toBe('deprecated'); + expect(statuses[0].severity).toBe('info'); + expect(statuses[0].detail).toBe('request has been deprecated'); + expect(stripAnsi(formatPackageFlags(statuses))).toBe('๐Ÿ”ต deprecated'); + }); + + it('renders both badges in contributor order when a package is banned and deprecated', () => { + const pkg = createMockPackage('moment', { deprecated: 'use dayjs' }); + const statuses = collectPackageFlags(pkg, [ + createMockDeprecatedViolation('moment', { deprecated: 'use dayjs' }), + forbidViolation('moment', 'error'), + ]); + // Declaration order in PACKAGE_FLAG_CONTRIBUTORS, not the order the + // violations happened to arrive in. + expect(stripAnsi(formatPackageFlags(statuses))).toBe( + '๐Ÿ”ด forbidden ๐Ÿ”ต deprecated', + ); + }); + + it('never joins on a plugin finding, whose rule-id space is its own (#102)', () => { + const pkg = createMockPackage('moment'); + const pluginFinding = { + ruleId: 'no-packages', + severity: 'error' as const, + patterns: ['moment'], + packageName: 'moment', + message: 'from a plugin', + plugin: 'some-plugin', + } as unknown as RuleViolation; + expect(collectPackageFlags(pkg, [pluginFinding])).toEqual([]); + }); + + it('says nothing about a package the deprecation rule exempted', () => { + // `severity: 'off'` is resolved away into "no violation", so there is + // no badge at all โ€” the fact stays on the package for JSON consumers. + const pkg = createMockPackage('moment', { deprecated: 'use dayjs' }); + expect(collectPackageFlags(pkg, [])).toEqual([]); + }); +}); + +describe('describeFlagDetails', () => { + it('returns nothing for badges that carry no long-form context', () => { + const pkg = createMockPackage('moment'); + const statuses = collectPackageFlags(pkg, [forbidViolation('moment')]); + expect(describeFlagDetails(statuses)).toEqual([]); + }); + + it('phrases a deprecation notice as a continuation of its own badge', () => { + const pkg = createMockPackage('request', { + deprecated: 'request has been deprecated', + }); + const statuses = collectPackageFlags(pkg, [ + createMockDeprecatedViolation('request', { + deprecated: 'request has been deprecated', + }), + ]); + expect(describeFlagDetails(statuses)).toEqual([ + 'deprecated: request has been deprecated', + ]); + }); +}); diff --git a/tests/utils/package-rules.test.ts b/tests/utils/package-rules.test.ts index 432917e3..eeb9d57e 100644 --- a/tests/utils/package-rules.test.ts +++ b/tests/utils/package-rules.test.ts @@ -1,12 +1,16 @@ import { describe, expect, it } from 'vitest'; import { + detectDeprecatedPackages, detectForbiddenPackages, detectRequiredPackages, } from '../../src/utils/package-rules'; import { HermexConfigSchema } from '../../src/config/schema'; import type { HermexConfigInput } from '../../src/config/schema'; import { applyOverrides } from '../../src/config/overrides'; -import { createMockInventoryEntry } from '../helpers/mock-reports'; +import { + createMockInventoryEntry, + createMockPackage, +} from '../helpers/mock-reports'; /** * Parse a partial config through the real schema, then resolve it exactly @@ -370,3 +374,129 @@ describe('detectRequiredPackages', () => { expect(detectRequiredPackages([], config)).toEqual([]); }); }); + +describe('detectDeprecatedPackages', () => { + /** A package the registry reported as deprecated. */ + const deprecated = (name: string, notice = 'no longer maintained') => + createMockPackage(name, { deprecated: notice }); + + it('reports a deprecated package at the info baseline with no rule authored', () => { + // The baseline is what preserves the long-standing behavior that a + // deprecated package is always visible once hermex has registry data for + // it, without it ever affecting the verdict (#107). + const config = createConfig({}); + + expect( + detectDeprecatedPackages( + [deprecated('request', 'request has been deprecated')], + config.rules['no-deprecated-packages'], + ), + ).toEqual([ + { + ruleId: 'no-deprecated-packages', + severity: 'info', + patterns: ['**'], + message: undefined, + packageName: 'request', + deprecated: 'request has been deprecated', + }, + ]); + }); + + it('says nothing about a package the registry did not report as deprecated', () => { + const config = createConfig({}); + + expect( + detectDeprecatedPackages( + [createMockPackage('react')], + config.rules['no-deprecated-packages'], + ), + ).toEqual([]); + }); + + it('takes severity and message from the entry that governs the package', () => { + const config = createConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'error', patterns: ['request'], message: 'use undici' }, + ], + }, + }); + + const violations = detectDeprecatedPackages( + [deprecated('request')], + config.rules['no-deprecated-packages'], + ); + + expect(violations[0].severity).toBe('error'); + expect(violations[0].message).toBe('use undici'); + expect(violations[0].patterns).toEqual(['request']); + }); + + it('lets an off entry exempt a package, falling back to the baseline for others', () => { + // The reason this rule resolves like release-age rather than like + // no-packages: an 'off' entry has to survive resolution, or the package + // falls through to the baseline and 'off' silently does nothing. + const config = createConfig({ + rules: { + 'no-deprecated-packages': [{ severity: 'off', patterns: ['request'] }], + }, + }); + + const violations = detectDeprecatedPackages( + [deprecated('request'), deprecated('left-pad')], + config.rules['no-deprecated-packages'], + ); + + expect(violations.map((v) => v.packageName)).toEqual(['left-pad']); + }); + + it('honors a glob pattern', () => { + const config = createConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'off', patterns: ['@legacy/*'] }, + ], + }, + }); + + const violations = detectDeprecatedPackages( + [deprecated('@legacy/widget'), deprecated('moment')], + config.rules['no-deprecated-packages'], + ); + + expect(violations.map((v) => v.packageName)).toEqual(['moment']); + }); + + it('emits exactly one violation per package when several entries match', () => { + // Last match wins โ€” one entry governs a package, so a package can't be + // simultaneously error under one entry and off under another. + const config = createConfig({ + rules: { + 'no-deprecated-packages': [ + { severity: 'error', patterns: ['**'] }, + { severity: 'warn', patterns: ['req*'] }, + ], + }, + }); + + const violations = detectDeprecatedPackages( + [deprecated('request')], + config.rules['no-deprecated-packages'], + ); + + expect(violations).toHaveLength(1); + expect(violations[0].severity).toBe('warn'); + }); + + it('carries the publisher notice verbatim', () => { + const config = createConfig({}); + + const violations = detectDeprecatedPackages( + [deprecated('moment', 'Moment.js is a legacy project')], + config.rules['no-deprecated-packages'], + ); + + expect(violations[0].deprecated).toBe('Moment.js is a legacy project'); + }); +}); diff --git a/tests/utils/print-utils.test.ts b/tests/utils/print-utils.test.ts index b36bc426..a847789a 100644 --- a/tests/utils/print-utils.test.ts +++ b/tests/utils/print-utils.test.ts @@ -11,12 +11,12 @@ import { describeAdvisoryBreaches, describeBundleImpact, describePackageNotes, - formatPackageName, - formatUpgradeCell, + describeMinimumTarget, + formatMinimumTargetCell, resolveCompliantTarget, resolveInstalledVersion, } from '../../src/utils/print-packages'; -import { enrichWithReleaseAge } from '../../src/npm-registry/enricher'; +import { enrichFromRegistry } from '../../src/npm-registry/enricher'; import { printComponents } from '../../src/utils/print-components'; import { printPatterns } from '../../src/utils/print-patterns'; import { printDetails } from '../../src/utils/print-details'; @@ -34,6 +34,7 @@ import { createMockPackage, createMockReleaseAge, createMockReleaseAgeViolation, + createMockDeprecatedViolation, } from '../helpers/mock-reports'; // Drive the "user report" test end-to-end through the real enricher, so it @@ -302,7 +303,12 @@ describe('printPackages', () => { expect(output).not.toContain('14 days overdue'); }); - it('omits a day count when even the recommended target is past its own threshold (#26)', () => { + // This branch is only ever reached when a newer release DOES exist and + // has not been taken โ€” being on the newest release that exists breaches + // nothing, so no row is produced at all. Naming the version and then + // denying one was available contradicted both the cell beside it and + // `minCompliantVersion`'s own rule that latest counts as compliant. + it('still names a target and counts the days when no in-window candidate exists (#26)', () => { const aggregated = makeAggregated({ packageDistribution: [ createMockPackage('react', { @@ -327,11 +333,11 @@ describe('printPackages', () => { const output = consoleSpy.mock.calls .map((call) => call.join(' ')) .join('\n'); - expect(output).toContain('no compliant release available'); - expect(output).not.toContain('days overdue'); + expect(output).toContain('18.0.0 (major, 30 days overdue)'); + expect(output).not.toContain('no compliant release available'); }); - it('renders "days remaining" for a package approaching its threshold', () => { + it('renders a due date for a package approaching its threshold', () => { const aggregated = makeAggregated({ packageDistribution: [ createMockPackage('axios', { @@ -353,7 +359,10 @@ describe('printPackages', () => { const output = consoleSpy.mock.calls .map((call) => call.join(' ')) .join('\n'); - expect(output).toContain('12 days remaining'); + expect(output).toContain('1.6.0 (minor, due in 12 days)'); + // Not a verdict โ€” nothing is overdue, so it keeps the all-clear icon + // rather than borrowing a severity or going bare (#86). + expect(output).toContain('๐ŸŸข 1.6.0'); }); it('renders chart mode with a populated package entry', () => { @@ -413,10 +422,10 @@ describe('printPackages', () => { // (#57) When release age is enabled, the table splits into an unambiguous // "Installed" column (the single version the verdict was measured - // against) and "Target" column, with the bundle-impact/advisory-breach + // against) and "Minimum target" column, with the bundle-impact/advisory-breach // context printed as a separate Notes line below the table โ€” not crammed // into a cell where it's unclear which installed copy a claim refers to. - it('shows Installed/Target columns and a Notes line for a package with an advisory breach', () => { + it('shows Installed/Minimum target columns and a Notes line for a package with an advisory breach', () => { const aggregated = makeAggregated({ packageDistribution: [ createMockPackage('multi-version-lib', { @@ -437,7 +446,7 @@ describe('printPackages', () => { .map((call) => call.join(' ')) .join('\n'); expect(output).toContain('Installed'); - expect(output).toContain('Target'); + expect(output).toContain('Minimum target'); // The table row shows only the single installed baseline, not the list. const tableLines = output.split('\n').filter((l) => l.includes('โ”‚')); expect(tableLines.some((l) => l.includes('3.0.0'))).toBe(true); @@ -451,93 +460,271 @@ describe('printPackages', () => { }); }); -describe('formatPackageName', () => { - it('prefixes a deprecated package', () => { - const pkg = createMockPackage('left-pad', { - releaseAge: createMockReleaseAge({ deprecated: 'use String.padStart' }), +describe('the Flags column (#86)', () => { + it('omits the column entirely when no package rule flagged anything', () => { + // A plain scan of a repo with no package rules keeps its clean two-column + // table rather than growing a column of blanks. + const aggregated = makeAggregated({ + packageDistribution: [createMockPackage('react', { version: '18.3.1' })], }); - expect(formatPackageName(pkg)).toContain('[DEPRECATED]'); + printPackages(aggregated, 'table'); + const output = consoleSpy.mock.calls.map((c) => c.join(' ')).join('\n'); + expect(output).not.toContain('Flags'); }); - it('prefixes an error-severity banned package as [BANNED]', () => { - const pkg = createMockPackage('moment'); - expect( - formatPackageName(pkg, forbidViolation('moment', 'error')), - ).toContain('[BANNED]'); + it('adds the column as soon as one row has a badge', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('react', { version: '18.3.1' }), + createMockPackage('moment', { version: '2.29.4' }), + ], + ruleViolations: [forbidViolation('moment', 'error')], + }); + printPackages(aggregated, 'table'); + const output = stripAnsi( + consoleSpy.mock.calls.map((c) => c.join(' ')).join('\n'), + ); + expect(output).toContain('Flags'); + expect(output).toContain('๐Ÿ”ด forbidden'); + }); + + it('keeps the package name cell bare โ€” no prefixes glued to the name', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('moment', { + version: '2.29.4', + deprecated: 'use dayjs', + }), + ], + ruleViolations: [ + forbidViolation('moment', 'error'), + createMockDeprecatedViolation('moment', { deprecated: 'use dayjs' }), + ], + }); + printPackages(aggregated, 'table'); + const output = stripAnsi( + consoleSpy.mock.calls.map((c) => c.join(' ')).join('\n'), + ); + expect(output).not.toContain('[BANNED]'); + expect(output).not.toContain('[RESTRICTED]'); + expect(output).not.toContain('[DEPRECATED]'); + expect(output).toContain('๐Ÿ”ด forbidden ๐Ÿ”ต deprecated'); }); - it('prefixes a warn-severity banned package as [RESTRICTED]', () => { - const pkg = createMockPackage('moment'); - expect(formatPackageName(pkg, forbidViolation('moment', 'warn'))).toContain( - '[RESTRICTED]', + it('surfaces the publisher notice as a Notes line rather than in a cell', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('request', { + version: '2.88.2', + deprecated: 'request has been deprecated', + }), + ], + ruleViolations: [ + createMockDeprecatedViolation('request', { + deprecated: 'request has been deprecated', + }), + ], + }); + printPackages(aggregated, 'table'); + const output = stripAnsi( + consoleSpy.mock.calls.map((c) => c.join(' ')).join('\n'), + ); + expect(output).toContain('Notes:'); + expect(output).toContain( + '๐Ÿ”ต request โ†’ deprecated: request has been deprecated', + ); + }); +}); + +describe('printPackages chart โ€” label alignment (#86)', () => { + // The bug: maxLabelLength was measured from packageName.length while the + // string actually padded was the ANSI-colored, badge-prefixed label, so any + // flagged row pushed its bar out of the column. + it('starts every bar in the same column even when a row is flagged', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('@design-system/foundation', { + usageCount: 33, + percentage: 91.7, + }), + createMockPackage('react', { usageCount: 3, percentage: 8.3 }), + ], + ruleViolations: [forbidViolation('@design-system/foundation', 'error')], + }); + printPackages(aggregated, 'chart'); + const lines = consoleSpy.mock.calls + .map((c) => stripAnsi(c.join(' '))) + .filter((l) => l.includes('โ–ˆ') || l.includes('โ–‘')); + expect(lines).toHaveLength(2); + const barStarts = lines.map((l) => + Math.min( + ...['โ–ˆ', 'โ–‘'].map((ch) => { + const i = l.indexOf(ch); + return i === -1 ? Number.POSITIVE_INFINITY : i; + }), + ), ); + expect(barStarts[0]).toBe(barStarts[1]); }); - it('combines [DEPRECATED] with [BANNED] when both apply', () => { - const pkg = createMockPackage('moment', { - releaseAge: createMockReleaseAge({ deprecated: 'use dayjs' }), + it('trails the badges after the percentage instead of prefixing the label', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('moment', { usageCount: 5, percentage: 100 }), + ], + ruleViolations: [forbidViolation('moment', 'error')], }); - const name = formatPackageName(pkg, forbidViolation('moment')); - expect(name).toContain('[DEPRECATED]'); - expect(name).toContain('[BANNED]'); + printPackages(aggregated, 'chart'); + const line = consoleSpy.mock.calls + .map((c) => stripAnsi(c.join(' '))) + .find((l) => l.includes('โ–ˆ')); + expect(line).toMatch(/^moment /); + expect(line?.trimEnd()).toMatch(/๐Ÿ”ด forbidden$/); + }); + + it('adds nothing after the percentage when no rule flagged the package', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('react', { usageCount: 5, percentage: 100 }), + ], + }); + printPackages(aggregated, 'chart'); + const line = consoleSpy.mock.calls + .map((c) => stripAnsi(c.join(' '))) + .find((l) => l.includes('โ–ˆ')); + expect(line?.trimEnd()).toMatch(/\(5\)$/); }); }); -describe('formatUpgradeCell', () => { +describe('describeMinimumTarget', () => { it('returns an empty string when there is no release-age data', () => { - expect(formatUpgradeCell(undefined)).toBe(''); + expect(describeMinimumTarget(undefined)).toBe(''); + }); + + it('returns an empty string when nothing is breached or pending', () => { + expect( + describeMinimumTarget(createMockReleaseAge({ worstLevel: null })), + ).toBe(''); + }); + + // The summary file renders its own severity in a leading column, so it + // takes the text alone โ€” sharing this function with the human table is + // what keeps the two from recommending different versions (#57). + it('never carries an icon, at any severity', () => { + const majorOverdueUpgrade = { + version: '2.0.0', + releasedDaysAgo: 10, + breachReleasedDaysAgo: 100, + semverBump: 'major' as const, + level: 'major_overdue' as const, + thresholdDays: 60, + }; + for (const severity of ['error', 'warn', 'info', 'off'] as const) { + const text = describeMinimumTarget( + createMockReleaseAge({ + worstLevel: 'major_overdue', + severity, + upgrades: [majorOverdueUpgrade], + }), + ); + expect(text).toBe('2.0.0 (major, 40 days overdue)'); + } + }); +}); + +describe('formatMinimumTargetCell', () => { + const majorOverdueUpgrade = { + version: '2.0.0', + releasedDaysAgo: 10, + breachReleasedDaysAgo: 100, + semverBump: 'major' as const, + level: 'major_overdue' as const, + thresholdDays: 60, + }; + + // The status icon reflects severity, not which tier breached: an ENFORCED + // minor_overdue fails comply just like a major would (#28), so it renders + // red. + const minorOverdueUpgrade = { + version: '1.1.0', + releasedDaysAgo: 10, + breachReleasedDaysAgo: 50, + semverBump: 'minor' as const, + level: 'minor_overdue' as const, + thresholdDays: 30, + }; + + // An em dash, not a blank and not ๐ŸŸข: hermex has no opinion here, either + // because the package has no installed version or because the registry + // never answered. ๐ŸŸข is the opposite claim โ€” it looked, nothing to do. + it('returns an em dash when there is no release-age data at all', () => { + expect(formatMinimumTargetCell(undefined)).toBe('โ€”'); }); it('returns a success icon when there is no worst level and no pending upgrade', () => { - expect(formatUpgradeCell(createMockReleaseAge({ worstLevel: null }))).toBe( - '๐ŸŸข', - ); + expect( + formatMinimumTargetCell(createMockReleaseAge({ worstLevel: null })), + ).toBe('๐ŸŸข'); }); it('returns a success icon when worstLevel is set but the upgrades list is empty', () => { expect( - formatUpgradeCell( + formatMinimumTargetCell( createMockReleaseAge({ worstLevel: 'major_overdue', upgrades: [] }), ), ).toBe('๐ŸŸข'); }); + // A pending upgrade is not a verdict โ€” nothing is overdue and no rule has + // anything to say yet. Carrying the info icon here made ๐Ÿ”ต mean two + // unrelated things in one report (#86). + it('renders a pending upgrade with the all-clear icon', () => { + const cell = formatMinimumTargetCell( + createMockReleaseAge({ + worstLevel: null, + pendingUpgrade: { + version: '2.0.0', + semverBump: 'major', + releasedDaysAgo: 48, + thresholdDays: 60, + daysRemaining: 12, + }, + }), + ); + expect(cell).toBe('๐ŸŸข 2.0.0 (major, due in 12 days)'); + // ๐Ÿ”ต used to mean both "an upgrade is coming due" and "an info-severity + // violation exists" โ€” it now means only the second (#86). + expect(cell).not.toContain('๐Ÿ”ต'); + }); + it('uses the warn icon (not error) for a major_overdue upgrade at warn severity', () => { - const cell = formatUpgradeCell( + const cell = formatMinimumTargetCell( createMockReleaseAge({ worstLevel: 'major_overdue', severity: 'warn', - upgrades: [ - { - version: '2.0.0', - releasedDaysAgo: 10, - breachReleasedDaysAgo: 100, - semverBump: 'major', - level: 'major_overdue', - thresholdDays: 60, - }, - ], + upgrades: [majorOverdueUpgrade], }), ); expect(cell).toContain('๐ŸŸก'); expect(cell).not.toContain('๐Ÿ”ด'); - expect(cell).toContain('[not enforced]'); }); - // The status icon reflects severity, not which tier breached: an ENFORCED - // minor_overdue fails comply just like a major would (#28), so it renders red - // โ€” only a non-enforced (advisory) breach is yellow. - const minorOverdueUpgrade = { - version: '1.1.0', - releasedDaysAgo: 10, - breachReleasedDaysAgo: 50, - semverBump: 'minor' as const, - level: 'minor_overdue' as const, - thresholdDays: 30, - }; + // The gray "[not enforced]" suffix is gone: ๐ŸŸก already says the package + // does not fail comply, and every other surface lets the icon carry + // severity on its own (#86). + it('no longer appends a [not enforced] suffix at warn severity', () => { + const cell = formatMinimumTargetCell( + createMockReleaseAge({ + worstLevel: 'minor_overdue', + severity: 'warn', + upgrades: [minorOverdueUpgrade], + }), + ); + expect(cell).not.toContain('not enforced'); + }); it('uses the error icon for an enforced minor_overdue worst level (#28)', () => { - const cell = formatUpgradeCell( + const cell = formatMinimumTargetCell( createMockReleaseAge({ worstLevel: 'minor_overdue', severity: 'error', @@ -548,21 +735,35 @@ describe('formatUpgradeCell', () => { expect(cell).not.toContain('๐ŸŸก'); }); - it('uses the warn icon for a non-enforced minor_overdue worst level', () => { - const cell = formatUpgradeCell( + // Both of these used to render red, because the icon came from a + // severity-is-warn-else-error ternary that swept info and off into the + // error branch (#86). + it('uses the info icon for a breach governed by an info-severity entry', () => { + const cell = formatMinimumTargetCell( createMockReleaseAge({ - worstLevel: 'minor_overdue', - severity: 'warn', - upgrades: [minorOverdueUpgrade], + worstLevel: 'major_overdue', + severity: 'info', + upgrades: [majorOverdueUpgrade], }), ); - expect(cell).toContain('๐ŸŸก'); - expect(cell).toContain('[not enforced]'); + expect(cell).toContain('๐Ÿ”ต'); + expect(cell).not.toContain('๐Ÿ”ด'); + }); + + it('renders no icon at all for a breach governed by an off entry', () => { + const cell = formatMinimumTargetCell( + createMockReleaseAge({ + worstLevel: 'major_overdue', + severity: 'off', + upgrades: [majorOverdueUpgrade], + }), + ); + expect(cell).toBe('2.0.0 (major, 40 days overdue)'); }); }); describe('describeUpgradeTarget', () => { - it('formats the semver bump, version, and days overdue', () => { + it('leads with the version, then the bump and how overdue it is', () => { expect( describeUpgradeTarget({ version: '4.2.0', @@ -572,10 +773,10 @@ describe('describeUpgradeTarget', () => { level: 'major_overdue', thresholdDays: 60, }), - ).toBe('major 4.2.0 (40 days overdue)'); + ).toBe('4.2.0 (major, 40 days overdue)'); }); - it('reports "no compliant release available" when even the recommended target is past its own threshold (#26)', () => { + it('names the newest release in the breached tier when no in-window candidate exists (#26)', () => { expect( describeUpgradeTarget({ version: '18.0.0', @@ -585,7 +786,7 @@ describe('describeUpgradeTarget', () => { level: 'major_overdue', thresholdDays: 60, }), - ).toBe('major 18.0.0 (no compliant release available)'); + ).toBe('18.0.0 (major, 30 days overdue)'); }); }); @@ -760,7 +961,7 @@ describe('describePackageNotes (#57)', () => { }); }); -describe('formatUpgradeCell โ€” stale 0.x minor line beneath a compliant newer major (user report)', () => { +describe('formatMinimumTargetCell โ€” stale 0.x minor line beneath a compliant newer major (user report)', () => { beforeEach(() => { vi.clearAllMocks(); }); @@ -783,7 +984,7 @@ describe('formatUpgradeCell โ€” stale 0.x minor line beneath a compliant newer m versions: {}, }); - const { enriched } = await enrichWithReleaseAge( + const { enriched } = await enrichFromRegistry( [pkg], { cacheDisabled: false }, // Named explicitly: severity follows the resolved policy alone, so @@ -796,7 +997,7 @@ describe('formatUpgradeCell โ€” stale 0.x minor line beneath a compliant newer m }), ); - const cell = formatUpgradeCell(enriched[0].releaseAge); + const cell = formatMinimumTargetCell(enriched[0].releaseAge); // Correct target: the compliant major, not the stale minor. expect(cell).toContain('1.0.0'); @@ -1090,6 +1291,62 @@ describe('printRules', () => { expect(output).toContain('๐ŸŸก'); }); + // Deprecation renders an ordinary rules row, which is why the summary + // file's Packages section deliberately doesn't repeat it (#107). + it('renders a no-deprecated-packages violation with the publisher notice when the rule has no message', () => { + const aggregated = makeAggregated({ + ruleViolations: [ + createMockDeprecatedViolation('request', { + severity: 'error', + deprecated: 'request has been deprecated', + }), + ], + }); + printRules(aggregated); + const output = stripAnsi( + consoleSpy.mock.calls.map((call) => call.join(' ')).join('\n'), + ); + expect(output).toContain('no-deprecated-packages'); + expect(output).toContain( + 'request is deprecated โ€” request has been deprecated', + ); + }); + + // Two gray em-dash clauses in a row read terribly, and between the policy + // author's wording and npm's the author's should win. + it("prefers the rule's own message over the publisher notice", () => { + const aggregated = makeAggregated({ + ruleViolations: [ + createMockDeprecatedViolation('request', { + severity: 'error', + message: 'use undici', + deprecated: 'request has been deprecated', + }), + ], + }); + printRules(aggregated); + const output = stripAnsi( + consoleSpy.mock.calls.map((call) => call.join(' ')).join('\n'), + ); + expect(output).toContain('request is deprecated โ€” use undici'); + expect(output).not.toContain('request has been deprecated'); + }); + + it('gives every deprecated package its own row rather than folding them', () => { + const aggregated = makeAggregated({ + ruleViolations: [ + createMockDeprecatedViolation('request'), + createMockDeprecatedViolation('left-pad'), + ], + }); + printRules(aggregated); + const output = stripAnsi( + consoleSpy.mock.calls.map((call) => call.join(' ')).join('\n'), + ); + expect(output).toContain('request is deprecated'); + expect(output).toContain('left-pad is deprecated'); + }); + // `packageName` is set by `detectForbiddenPackages`, but the rule's own // patterns are the fallback for a violation raised without one. it('falls back to the rule patterns when a no-packages violation has no packageName', () => { @@ -1998,6 +2255,22 @@ describe('printJson', () => { expect(parsed.compliance.compliant).toBe(true); }); + it('exposes deprecation as a package-level fact, not under releaseAge (#107)', () => { + const aggregated = makeAggregated({ + packageDistribution: [ + createMockPackage('request', { + deprecated: 'request has been deprecated', + releaseAge: createMockReleaseAge({ worstLevel: null }), + }), + ], + }); + printJson(aggregated, makeOutput()); + + const parsed = JSON.parse(stdoutSpy.mock.calls[0][0] as string); + expect(parsed.packages[0].deprecated).toBe('request has been deprecated'); + expect(parsed.packages[0].releaseAge.deprecated).toBeUndefined(); + }); + it('passes an explicitly provided compliance result straight through (#55)', () => { const aggregated = makeAggregated(); printJson(aggregated, makeOutput(), { diff --git a/tests/utils/write-summary-file.test.ts b/tests/utils/write-summary-file.test.ts index fd6276c2..3b64bd07 100644 --- a/tests/utils/write-summary-file.test.ts +++ b/tests/utils/write-summary-file.test.ts @@ -10,11 +10,12 @@ import { writeSummaryFile, DEFAULT_SUMMARY_TITLE, } from '../../src/utils/write-summary-file'; -import { formatUpgradeCell } from '../../src/utils/print-packages'; +import { describeMinimumTarget } from '../../src/utils/print-packages'; import { createMockPackage, createMockReleaseAge, createMockReleaseAgeViolation, + createMockDeprecatedViolation, } from '../helpers/mock-reports'; /** A no-packages hit, the shape `detectForbiddenPackages` emits (#77). */ @@ -249,7 +250,8 @@ describe('writeSummaryFile', () => { describe('Packages section', () => { it('excludes a deprecated-only package (no breached tier)', () => { const deprecated = createMockPackage('left-pad', { - releaseAge: createMockReleaseAge({ deprecated: '2020-01-01' }), + deprecated: 'no longer maintained', + releaseAge: createMockReleaseAge({}), }); const content = write( makeAggregated({ packageDistribution: [deprecated] }), @@ -311,18 +313,20 @@ describe('writeSummaryFile', () => { }), ); expect(content).toContain('### Packages'); - expect(content).toContain('| | Package | Installed | Target |'); expect(content).toContain( - '| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | major 4.2.0 (40 days overdue) |', + '| | Package | Installed | Minimum target | Flags |', + ); + expect(content).toContain( + '| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | 4.2.0 (major, 40 days overdue) | |', ); }); - it('joins both reasons on one line for a package that is enforced-overdue and deprecated', () => { + it('shows a deprecated overdue package as a deprecated badge in Flags, not crammed into the target cell', () => { const both = createMockPackage('my-internal-pkg', { + deprecated: 'no longer maintained', releaseAge: createMockReleaseAge({ worstLevel: 'major_overdue', severity: 'error', - deprecated: '2020-01-01', upgrades: [ { version: '4.2.0', @@ -343,6 +347,7 @@ describe('writeSummaryFile', () => { worstLevel: 'major_overdue', severity: 'error', }), + createMockDeprecatedViolation('my-internal-pkg'), ], }), ); @@ -350,18 +355,18 @@ describe('writeSummaryFile', () => { .split('\n') .find((l) => l.includes('my-internal-pkg')); expect(line).toBe( - '| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | major 4.2.0 (40 days overdue), deprecated |', + '| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | 4.2.0 (major, 40 days overdue) | ๐Ÿ”ต deprecated |', ); }); - it('shows only "deprecated" when a mandatory violation has no upgrade candidates', () => { + it('leaves the target cell empty when a mandatory violation has no upgrade candidates', () => { // worstLevel non-null but upgrades empty is a defensive/edge shape โ€” // exercises the `top` guard independently of the deprecated reason. const deprecatedOnlyBreach = createMockPackage('my-internal-pkg', { + deprecated: 'no longer maintained', releaseAge: createMockReleaseAge({ worstLevel: 'major_overdue', severity: 'error', - deprecated: '2020-01-01', upgrades: [], }), }); @@ -373,13 +378,16 @@ describe('writeSummaryFile', () => { worstLevel: 'major_overdue', severity: 'error', }), + createMockDeprecatedViolation('my-internal-pkg'), ], }), ); const line = content .split('\n') .find((l) => l.includes('my-internal-pkg')); - expect(line).toBe('| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | deprecated |'); + expect(line).toBe( + '| ๐Ÿ”ด | `my-internal-pkg` | 1.0.0 | | ๐Ÿ”ต deprecated |', + ); }); it('does not show a banned/restricted package (it is Rules-only, not duplicated here)', () => { @@ -470,7 +478,7 @@ describe('writeSummaryFile', () => { ], }), ); - expect(content).toContain('major 1.0.0 (155 days overdue)'); + expect(content).toContain('1.0.0 (major, 155 days overdue)'); expect(content).not.toContain('no compliant release available'); }); @@ -479,7 +487,7 @@ describe('writeSummaryFile', () => { // upgrade description from the same ReleaseAgeEntry โ€” they diverged on // this once before (#57) because only one call site passed the // compliantTarget argument. - it('agrees with formatUpgradeCell on the recommended upgrade description', () => { + it('renders the exact string describeMinimumTarget produces, not a re-derived one', () => { const releaseAge = createMockReleaseAge({ worstLevel: 'minor_overdue', severity: 'error', @@ -510,12 +518,11 @@ describe('writeSummaryFile', () => { }), ); - const tableCell = formatUpgradeCell(releaseAge); - // formatUpgradeCell is " [suffix]" โ€” extract just - // the description text and confirm the summary row contains the exact - // same substring, not a re-derived (and potentially divergent) one. - const description = tableCell.replace(/^\S+\s/, ''); - expect(content).toContain(description); + // Both surfaces call this one function now, so "they recommend the + // same version" is structural rather than two call sites kept in step + // by hand (#57) โ€” and there is no icon to strip off first, since the + // summary renders severity in its own leading column. + expect(content).toContain(describeMinimumTarget(releaseAge)); }); // Notes (#57): a package that's compliant at the enforced scope but has @@ -588,7 +595,7 @@ describe('writeSummaryFile', () => { ); const tableLine = content.split('\n').find((l) => l.startsWith('| ๐Ÿ”ด |')); expect(tableLine).toBe( - '| ๐Ÿ”ด | `multi-version-lib` | 1.0.0 | major 2.0.0 (340 days overdue) |', + '| ๐Ÿ”ด | `multi-version-lib` | 1.0.0 | 2.0.0 (major, 340 days overdue) | |', ); expect(content).not.toContain('Notes:'); expect(content).not.toContain('bundle impact');