fix: derive the remaining nine rules from the generated CSS - #95
Merged
Conversation
The rule compared the textual token, not the value it resolves to. Tailwind v4
has per-axis theme namespaces — `w-*` reads `--width-*`, `h-*` reads `--height-*`,
`size-*` reads `--size-*` — so with
@theme { --width-brand: 10rem; --height-brand: 20rem }
`w-brand h-brand` was autofixed to `size-brand`, a class that emits nothing: the
element lost its width and its height. A hardcoded `INVALID_SIZE_VALUES` blocklist
covered `screen` and, over-eagerly, every viewport unit, while the case that
actually breaks was never in it.
Now the merge is verified against the declarations Tailwind emits: the parts have
to resolve to the same value and the replacement has to reproduce it. The blocklist
is gone (`w-screen h-screen` is rejected because 100vw is not 100vh; `w-dvw h-dvw`
merges), and named theme tokens on that family are only merged when the values are
literally identical, for the same reason enforce-canonical refuses a var-backed
rewrite (#78). DS-OPTIONAL: with no entryPoint the merges that are safe whatever
the theme says still fire.
The family table is now declarative data, which made it cheap to add what was
missing: border sides/axes (widths AND colours), inset, the rounded edges and the
logical corners, gap, overflow, overscroll, border-spacing, scroll-m*/scroll-p*,
the logical inline pairs (`ms`+`me` → `mx`) and translate. `scale-x`+`scale-y` is
deliberately absent: `scale-110` also writes `--tw-scale-z`, which `scale-3d`
reads, so that merge changes the render.
Each entry is proved against the real design system in
tests/integration/shorthand-families.test.ts: the properties the replacement writes
must be exactly the union of the parts', expanded through the CSS shorthands. That
matrix caught `rounded-s` being `ss`+`es`, not the `ss`+`se` the names suggest.
Four sides now report ONE diagnostic instead of three (the merge consumes the
classes, so the axis halves are no longer reported next to it).
…porting Tailwind's own canonicalizeCandidates rewrites every v3 spelling this rule knew about (`bg-gradient-to-r` → `bg-linear-to-r`, `flex-grow` → `grow`), and the precompute already fed them into the canonical map. So each of those classes produced TWO diagnostics carrying the SAME fix: one from no-deprecated-classes' hardcoded table, one from enforce-canonical. The precompute now records the renames separately from the rest of the canonical map, derived by asking the design system, and that map decides ownership: no-deprecated-classes reports them (it says the class is deprecated, which is the more actionable message) and enforce-canonical skips them. The derived map also covers renames the hardcoded table never had — `break-words`, `order-none`, the reordered `bg-left-top`/`object-left-top` position spellings — and prunes itself when a Tailwind release stops compiling one, which is what the hardcoded table could not do. The split is not "everything the canonical map holds": `start-2` canonicalizes to `inset-s-2` and is NOT deprecated — it is the spelling Tailwind's docs use — so enforce-canonical keeps that one. Both directions are pinned down in tests/integration/deprecated-canonical-coexistence.test.ts. DS-OPTIONAL: with no entryPoint the hardcoded table stands in and the rule reports exactly what it reported before. no-unknown-classes now asks the design system which classes are deprecated instead of importing that table.
…t their shape Two holes, both of which looked like coverage. `isValid` strips the variants BEFORE validating, so the chain was never checked at all: `hoverr:flex`, `darkk:size-4`, `peer-cheked:flex` and `@mdd:flex` produce no CSS whatsoever and every one of them passed. And it accepts anything shaped like a dynamic value, because until the declaration service there was no way to ask — which is why `bg-red-5000`, `bg-red-500/foo` and `w-[]` passed for exactly the reason `w-45` legitimately does. They are the same shape; only Tailwind knows it compiles one and not the other. Both are now answered by the design system, batched per location and memoized per class, with a fall back to the old heuristic if the worker is unavailable so a broken service can never invent a diagnostic. Validating a chain by NAME was not an option: `variantOrder` holds the 71 static variants and nothing else — no `group`, no `data`, no `@md` — so a list-based check would report most real-world variants as unknown. Instead a probe utility is compiled under the chain, which costs one call per DISTINCT chain in the project. Suggestions are verified the same way before being offered, so `group-hoverr` is corrected to `group-hover` (root kept, tail fixed, confirmed to compile) and a project's own `@custom-variant thumb` spell-checks like a built-in. The declaration service now reports which classes produce no CSS and owns the interning, which collapses the two memos the rules kept into one answer per class.
…just by prefix Grouping by the leading token reported the idiomatic "light does X, dark undoes X" pairs as missing a base, because the two spellings share no prefix: `underline dark:no-underline`, `italic dark:not-italic`, `visible dark:invisible`, `uppercase dark:normal-case`, `truncate dark:text-clip`, `sr-only dark:not-sr-only`. Each pair writes the SAME CSS property, and the rule had no way to know — a hardcoded equivalence table covered exactly two groups (display, position) and would have needed one entry per pair forever. With an entry point the properties come from the design system instead, and user-written values are resolved through the declaration service so `bg-[#fff] dark:bg-[#111]` keeps grouping too. The check is ADDITIVE: a class that matched by prefix still matches, so this can only ever report less than before, never more. DS-OPTIONAL: with no entryPoint the prefix heuristic and its two static groups stand in unchanged. That limit is now asserted in the test file rather than left implicit. `isUserValued` moves to utils/class-parser.ts, shared with no-conflicting-classes.
enforce-logical / enforce-physical Three physical→logical pairs Tailwind ships were simply missing from the table: `float-left`↔`float-start` (`float: inline-start`), `clear-left`↔`clear-start` and `text-left`↔`text-start`. A codebase could be fully converted and still float things left. They are marked `exact` because the direction is the VALUE there, which also stops the prefix scan from rewriting `float-left-0` — a class that does not exist — into another one that doesn't either. Both also verify the class they propose exists, when a design system is available. A project defining `@utility ml-huge` got an autofix to `ms-huge`, which emits nothing: the fix applies and the margin disappears. enforce-physical additionally converts `inset-s-*`/`inset-e-*`. enforce-logical suggests `start-2` (the spelling Tailwind's docs use), enforce-canonical then rewrites that to `inset-s-2` (what the design system calls canonical), and this rule had no way back — its table only knew `start`. no-hardcoded-colors The value was matched from its first character only, so a colour anywhere else in it passed: `shadow-[0_1px_2px_#000]` is a hardcoded black the rule promised to catch. Anchoring also forced a companion list of "colour-bearing prefixes" which had `ring` but not `inset-ring`, `shadow` but not `inset-shadow`, and no way to spell an arbitrary property (`[color:#f00]`, `[--brand:#f00]`). Both are gone: the value carries a colour or it doesn't, whatever utility it hangs off. The scanner skips quoted strings and `url()` contents, which is what keeps `content-['#fff']` and `fill-[url(#gradient)]` out, and treats `_` as the space it stands for so a function name inside a shorthand still surfaces. no-arbitrary-value `bg-(--x)` is sugar for `bg-[var(--x)]`, and only the second was reported — so `enforce-consistent-variable-syntax` in its default `shorthand` mode rewrote the reported form into the unreported one and the violation vanished with the code unchanged in substance. Both spellings report now. The replacement guard is deliberately NOT wired into the two rules whose replacement is an arbitrary value: Tailwind takes those verbatim, so `x-[-5px]` compiles whenever `-x-[5px]` does and the guard could not answer anything. They stay purely syntactic, as their docs say.
Rule pages for the nine rules this branch touches, in both locales (prose in `_extras`, pages regenerated). Each one says what changed, what an `entryPoint` buys, and what the rule still does without one — including the limits: the pairs `no-dark-without-light` cannot group without CSS, and the sizing merges `enforce-shorthand` skips when it cannot verify them. CHANGELOG records the three groups of new diagnostics under "Breaking-ish" (they can fail a CI run that passes today), and a "Not changed, and why" section for `no-duplicate-classes` — audited, no defect — and for the two rules that deliberately did NOT get the replacement guard. Also renames tests/rules/tmp-ncv-entrypoint-option.test.ts, which landed in #94 with a draft filename.
Visitors run on every AST node, so three things from the previous commits were paying per-location or per-class costs they didn't need to: - no-unknown-classes built three intermediate arrays per location to collect the classes needing verification. Now a single loop that allocates nothing at all for the common location where every class is precomputed. - the variant-chain verdict is memoized in the rule too, keyed by entry point and chain: a file with a thousand `hover:` classes was paying a probe array and a WeakMap lookup a thousand times over. - no-dark-without-light did the same map/filter pair per location; same fix. Also fixes a duplicate the shorthand fix could introduce: `mt-2 mb-2 my-2` already carries the shorthand, so appending it produced `my-2 my-2`.
…lass strings The validation is exact now, and the way that goes wrong is by reporting something real. These are class strings copied from shadcn/ui components — compound data variants, has-[…], arbitrary selectors, in-*, supports-[…], child selectors, colour modifiers — each run against the fixture that defines what it needs. Written while checking exactly that, and it earned its place twice: the first two runs "failed" on `bg-sidebar` and `animate-in`, which turned out to be the probe's fault (a fixture that defines neither), not the rule's.
… chain from segments Three things found reviewing the diff: - the early exit for a precomputed class with no variant chain had been dropped, so every class in every file was paying a deprecation lookup and a chain check it could not need. That is the overwhelming majority of what a file contains. - the suggested chain was built with `String.replace`, which rewrites the first occurrence of the offending segment — `data-[x=md]:mdd:` could have had the wrong one replaced. It is rebuilt from the parsed segments now. - the per-location reporter closure was allocated on every visitor call. Also: the reference tables in the docs index were out of date (and were already wrong about `no-deprecated-classes` before this branch), so they now have three groups — DS-dependent, DS-optional, DS-independent — with what the design system adds for each optional rule. `settings.md` says the same in one paragraph. Test coverage: the new shorthand families were only exercised WITHOUT a design system, and the DS path compares emitted declarations, so a family could have silently stopped merging with an entryPoint configured. All 16 are now run through both paths. The CHANGELOG said "~30 families"; it is 47, of which 37 are new.
sergioazoc
added a commit
that referenced
this pull request
Jul 26, 2026
…1.5.0 review (#96) * fix: three reliability gaps left over from the 1.5.0 review Documented in the #94 review, and one of them got worse in #95. The declaration service degraded in silence. It swallowed its own failures and returned "no information", which was defensible while `no-conflicting-classes` was the only caller — no declarations, no comparison, no diagnostic. Since #95 `no-unknown-classes` asks it about VALIDITY, and there silence sends the rule back to the tolerant heuristic: a dead worker quietly reinstates the exact false negatives the service exists to remove, on a run that stays green. It throws now, like the sort and canonicalize services, and each caller takes the posture that already existed in the plugin — the two DS-dependent rules surface `designSystemUnavailable`, and `no-dark-without-light` (DS-OPTIONAL, which may never emit it) degrades to prefix-only grouping and says so via debugLog. `internDeclarations` appended lint-time values to `index.values`, which belongs to the cache artifact and is held by reference. Two caches built from one artifact would each push while holding their own text→id map, so the same value would get two ids — and `decidePair` compares ids, so two identical values would read as a conflict. Ids are now local to the cache; nothing reads them back out of the array (the decoder only resolves ids that came from `index.table`). Unreachable through the loader today, one shared object away from being a false positive nobody could explain, and the test interleaves the two caches so it fails on the old code. `definedVars` scanned only the entry CSS. Splitting the theme across files is the normal shadcn/ui layout, so `:root { --primary: … }` in an `@import` read as undefined — and that is precisely what tells `prefer-theme-tokens` a rewrite is safe, so it proposed `bg-primary` for `bg-(--primary)` and silently changed the colour. It now scans the entry and its resolved imports, reusing the collection `extractComponentClasses` already did. * feat(prefer-scale-token): report the literal that equals a token, without fixing it Closes the gap #91 reported. Since #78, `enforce-canonical` only rewrites when the emitted CSS is byte-identical — right for an autofixer, and it took the consistency signal with it for every value whose token resolves through `var()`. Nothing else covered it: the three related rules each key on something this case does not satisfy (byte-identical CSS, what canonicalizeCandidates proposes, the NAME of a variable the user wrote). Numeric equality is a fourth premise. Two families, both derived from the design system by the precompute: - the spacing scale — `p-[10px]` → `p-2.5`, `gap-[4px]` → `gap-1`, `h-[2rem]` → `h-8`, and `w-[140px]` → `w-35`, the issue's own example; - named theme tokens by value — `rounded-[0.5rem]` → `rounded-lg`, read off the emitted CSS plus the theme, so no namespace table exists to drift. Report-only, with a suggestion and NO autofix: the equivalence is numeric, so a `:root` override or a different root font size makes the two diverge. Fixing that automatically is the mistake #78 fixed, and the message says as much. The boundary is where the work was. Tailwind compiles ANY number — `w-8.425` is valid — so "has a scale equivalent" is true for practically every length, and reporting all of them would be `no-arbitrary-value` with extra steps. The cut is the granularity Tailwind's own enumerated steps use, computed by the precompute as the smallest gap between them (0.5 in the default theme) rather than chosen here. `step` only ever makes it finer. Two things the derivation gets right on its own, which is why it is a derivation: `text-[14px]` → `text-sm` is NOT reported, because `text-sm` also sets `line-height` and the single-declaration requirement drops the whole family; and the ~7 000 colour tokens are excluded because a colour can never match a literal numerically, leaving 253 comparable tokens and +8.5 KB on the cache artifact. Corrects the issue's premise, for the record: `w-[140px]` → `w-35` was never reported, before or after #78 — Tailwind does not propose it, since 35 is not one of the steps it enumerates. * docs: prefer-scale-token, and bump to 1.6.0 Rule page in both locales, with the two things worth understanding up front: why there is no autofix (the equivalence is numeric, so a `:root` override or a different root font size makes it false — autofixing that is the bug #78 fixed), and why there is a granularity at all (Tailwind compiles any number, so without one the rule fires on essentially every length). Also fixes a doc error the same misconception produced: the rule index claimed `no-unnecessary-arbitrary-value` handles `w-[200px]` → `w-50`. It does not and cannot — those emit different CSS text — and `getNamedEquivalent('w-[200px]')` returns null. That example now points at the rule that does handle it. `enforce-canonical`'s page gains the pointer to its report-only half, since that is where the signal was lost. Unrelated flake found on the way: the precompute worker test scanned the WHOLE shared cache dir for `.tmp.` stragglers, so it failed whenever a peer isolate was mid-write — which this branch made likely by changing CACHE_KEY, since every fixture then recomputes at once. Scoped to its own artifact's temp names. * fix(prefer-scale-token): a bare number is not a length Found reviewing the diff. `p-[10]` compiles to `padding: 10` — no unit, which is not a length at all — and the rule treated the bare number as px, so it suggested `p-2.5` (`padding: 10px`). That is a change of meaning dressed up as a change of spelling, which is the one thing a rule with no autofix still has to get right, because an editor applies its suggestion on a click. Values now carry whether they had a unit, and a unitless number is never the same value as a length whatever the digits say. The scale path additionally requires both sides to be the same kind of quantity, so a hypothetical unitless `--spacing` would still compare correctly. Also tightened the prefix split to require the dash that joins a utility to its value, instead of assuming the character before `[` is one. Verified the fix discriminates: without the guard, `p-[10]` and `w-[140]` are reported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Companion to #94. That PR made
no-conflicting-classesdecide from the CSS Tailwind emits; the audit behind it left nine other rules reasoning about class NAMES. This closes them: one destructive autofix, one case of double reporting, six false negatives and one family of false positives.Every finding below was reproduced against Tailwind 4.3.3 with the real design system before being fixed, and each fix is verified the same way.
The nine
enforce-shorthandw-*from--width-*,h-*from--height-*,size-*from--size-*. With@theme { --width-brand: 10rem; --height-brand: 20rem },w-brand h-brandwas rewritten tosize-brand, which does not exist — the element lost its width and its heightno-deprecated-classes+enforce-canonicalcanonicalizeCandidatesrewrites every v3 spelling, and the precompute fed them into the canonical map, sobg-gradient-to-rproduced two diagnostics carrying the same fixno-unknown-classeshoverr:flex,darkk:size-4,peer-cheked:flex,@mdd:flexpassed while producing no CSS. And it accepts anything shaped like an off-scale number, sobg-red-5000,bg-red-500/foo,w-[]passed for the same reasonw-45legitimately doesno-hardcoded-colorsshadow-[0_1px_2px_#000]passed) and the prefix list hadringbut notinset-ring,shadowbut notinset-shadow, and no way to spell[color:#f00]no-arbitrary-valuebg-(--x)is sugar forbg-[var(--x)]and only the second reported — soenforce-consistent-variable-syntax's default autofix rewrote the reported form into the unreported one and the violation vanishedenforce-logical/enforce-physicalfloat-left↔float-start,clear-left↔clear-start,text-left↔text-start. A codebase could be fully converted and still float things leftno-dark-without-lightunderline dark:no-underline,italic dark:not-italic,visible dark:invisible,uppercase dark:normal-case,truncate dark:text-clip,sr-only dark:not-sr-only— every pair writes the same CSS property, but the spellings share no prefix@utility ml-hugewas autofixed toms-huge, which emits nothingno-duplicate-classesredundantsince 1.4.0, and that rule already skips the exact-duplicate pairHow each is decided now
Eight of the nine rules are DS-OPTIONAL: with an
entryPointthey get more accurate, without one they behave exactly as before. None can emitdesignSystemUnavailable— every one of them was usable with no configuration and has to stay that way.enforce-shorthandverifies each merge against the emitted declarations: the parts must resolve to the same value and the replacement must reproduce it. TheINVALID_SIZE_VALUESblocklist is gone with it —w-screen h-screenis rejected because 100vw is not 100vh (derived, not listed) andw-dvw h-dvw, excluded by that list for no reason, merges.no-deprecated-classesderives the rename list from the design system, which also decides ownership: it reports them andenforce-canonicalskips them. The derived list covers renames the hardcoded table never had (break-words,order-none, the reordered position spellings) and prunes itself when a Tailwind release stops compiling one.start-2→inset-s-2stays withenforce-canonical: current Tailwind, just not canonical.no-unknown-classescompiles the variant chain — the only exact way, sincevariantOrderholds the 71 static variants and has nogroup, nodata, no@md, so a name list would report most real-world variants as unknown. Suggestions are confirmed against the design system before being offered, sogroup-hoverr→group-hoverkeeps the root, and a project's own@custom-variant thumbis spell-checked like a built-in.no-dark-without-lightalso matches the base by declared property. The check is additive, so configuring an entry point can only make it report less.Also in here
enforce-shorthandwent from 10 collapsible families to 47 (borders, inset, radius edges, gap, overflow, overscroll,border-spacing,scroll-m*/scroll-p*, the logical inline pairs, translate) and now reports one diagnostic per group instead of one per matching family — four sides used to produce three errors.scale-x+scale-yis deliberately NOT a family:scale-110also writes--tw-scale-z, whichscale-3dreads, so the merge would change howscale-x-110 scale-y-110 scale-3drenders. That is the kind of thing only the CSS can tell you, which is the whole point of the branch.Testing
tests/integration/shorthand-families.test.tsproves every family against the real design system: the properties the replacement writes must be exactly the union of the parts', expanded through the CSS shorthands. It caughtrounded-sbeingss+esrather than thess+sethe names suggest, and it is what keepsscaleout.no-unknown-classesgets an anti-false-positive suite of class strings copied from real shadcn/ui components — compound data variants,has-[…], arbitrary selectors,in-*,supports-[…], colour modifiers.url()contents).CACHE_KEYchanges with the precompute script). Verified end to end under the real oxlint binary with the built plugin.Breaking-ish
New diagnostics can fail a CI run that passes today — that is the point, since everything they catch produces no CSS or changes it. Details in the CHANGELOG; version bumped to 1.5.0.