diff --git a/scripts/color-tuple-css.test.mjs b/scripts/color-tuple-css.test.mjs index a8d8b464e..ed1822cb5 100644 --- a/scripts/color-tuple-css.test.mjs +++ b/scripts/color-tuple-css.test.mjs @@ -98,61 +98,770 @@ function stylesheet() { } /** - * Does the stylesheet carry this exact class, not a longer one starting with - * it? - * - * Single classes are matched literally. A COMPOUND (`notification.is-primary`) - * is matched order-independently, because CSS does not care and Bulma does not - * either: it writes `.notification.is-primary` and `.is-primary.input`. An - * element-first-only regex called the second one absent, which is how a live - * modifier could have read as dead. - * - * Indexed by class rather than scanned per query. Walking every selector for - * every pair took the first case from a third of a second to six, because - * this file asks the question a few thousand times. + * Does the stylesheet style this class ON ITS OWN? + * + * `shipsClass` answers two questions and neither is this one. A bare class + * there means MENTIONED, which is right where a yes that should have been a + * no fails loudly — but one caller runs the other way. The assertion that + * `has-text-inherit` and its kin MUST ship passes on a yes, so a class the + * stylesheet merely mentions somewhere would satisfy it, and a helper the + * library hands the user has to be a rule of its own rather than a name in + * someone else's selector. + */ +function rendersAlone(css, cls) { + return simpleSelectors(css).sets.some( + classes => classes.size === 1 && classes.has(cls) + ); +} + +/** + * Does the stylesheet carry this class, or this exact compound? + * + * A single class is matched literally. A COMPOUND (`notification.is-primary`) + * has to appear as one SIMPLE selector carrying exactly those classes, which + * is narrower than it sounds and deliberately so. + * + * Order does not matter, because CSS does not care and Bulma does not either: + * it writes `.notification.is-primary` and `.is-primary.input`. Matching + * element-first only called the second absent, so a live modifier read as + * dead. + * + * But co-occurrence is not a match. Indexing per comma-separated part made + * `.hero.is-primary .tabs` answer the query `tabs.is-primary`, since all + * three classes sit in that one part, and that is the silent direction: a + * dead modifier reading as live, which is a missed defect rather than a false + * alarm. Parts are split into simple selectors on the combinators, and the + * class set has to match exactly, so a rule needing a third class does not + * answer for two. */ function shipsClass(css, cls) { - const parts = cls.split('.'); - const index = classIndex(css); - if (parts.length === 1) return index.has(cls); - // Start from the rarest class so the scan is over the shortest list. - const lists = parts.map(part => index.get(part) ?? new Set()); - if (lists.some(l => l.size === 0)) return false; - const smallest = lists.reduce((a, b) => (a.size <= b.size ? a : b)); - for (const id of smallest) { - if (lists.every(l => l.has(id))) return true; + // Two questions, two indexes, and they are not the same question. + // + // A COMPOUND query asks whether a modifier RENDERS. Its answer feeds a + // set compared against the colours the library warns about, so saying + // live where nothing renders drops a real defect out of that comparison + // and the test still passes. That is why it reads the subject sets and + // why the size has to match exactly: a rule needing a third class must + // not answer for two. + // + // A SINGLE-class query asks whether the stylesheet KNOWS a name. It reads + // every name mentioned anywhere, subject or not, prohibition included: + // answering it from the subject sets hid a class that only ever appears + // as an ancestor, and a colour the CSS ships and the tuple lacks is the + // first defect this file was written for. + // + // Most callers of it fail loudly on a yes they should not have been + // given and quietly on a no — the `-bis`/`-ter` sweep whose extra entry + // has nowhere to land, the `assert.ok` on a keyword that must NOT ship — + // so the wide reading is the right way round for them. The one that + // required a keyword to SHIP asks `rendersAlone` instead, because a name + // in someone else's selector is not a helper anybody can use. + // + // There used to be a caller that was quiet on a bad yes — the shade + // `deepEqual`, which compares two computed lists, so a fabricated name + // on the left could make an equality hold that should have failed. It + // asks `rendersAlone` now. That is the point worth keeping: this path is + // safe because no caller reads it in a direction where a wrong yes is + // quiet, and that is a property of the call sites rather than of the + // index. A name can still be fabricated — a comment inside a selector's + // parentheses survives the strip by design and is read — so the check + // when adding a caller is which way it fails, not whether the index can + // be wrong. + const wanted = cls.split('.'); + const { sets, names } = simpleSelectors(css); + if (wanted.length === 1) { + return names.has(cls); } - return false; + return sets.some( + classes => + classes.size === wanted.length && wanted.every(w => classes.has(w)) + ); } +/** Built once per stylesheet, keyed by its text. */ +const indexCache = new Map(); + /** - * class name → the selector parts carrying it, as a set of part ids. + * Stands in for a requirement the tokeniser does not model. * - * One pass over the stylesheet per file read. A part is one comma-separated - * selector, so a class list matching within a single part is what counts, - * rather than the whole selector list mentioning them somewhere apart. + * The exact-size rule in `shipsClass` is the whole defence against a rule + * that needs a third class answering a two-class query, so anything that + * quietly shortens a class set defeats it. Where a selector demands + * something this file will not parse, one of these goes in the set instead, + * and the query stops matching. `simpleSelectors` asserts the name is absent + * from the stylesheet before indexing, so it cannot collide with a real + * class. */ -const indexCache = new Map(); -function classIndex(css) { +const UNMODELLED = 'bestax-guard-unmodelled-requirement'; + +/** + * An at-rule whose contents apply exactly as they would at the top level. + * + * Listed the other way round on purpose. Naming the CONDITIONS instead — + * `@media`, `@supports`, `@container`, `@scope` — waves through every + * at-rule this file has not heard of, and a browser that has not heard of + * one drops it WITH its block, so nothing inside renders at all. That made + * `@media-foo`, `@page` and `@starting-style` read live, which is the + * direction this file forbids, and the commit before this one pinned it. + * + * `@layer` is the only at-rule whose contents are ordinary style rules that + * always apply. `@keyframes` and `@font-face` hold no style rules to begin + * with, so calling them conditional costs nothing. + */ +const TRANSPARENT_AT_RULE = /^@layer(?![\w-])/i; + +/** + * An at-rule whose prelude can name classes. + * + * Two of them do. `@scope (.root) to (.limit)` is a selector list outright, + * and `@supports selector(.a:has(.b))` carries one inside a condition. + * Those are classes the stylesheet knows about, and skipping every prelude + * lost them — the membership index's quiet direction. + * + * The rest stay out on their own merit rather than by this list: a + * condition carries lengths, and `IDENTIFIER_START` already refuses the + * `5rem` a decimal would offer. Keeping the list to the two that mean it + * is the narrower claim, and a name fabricated from a prelude would be + * loud rather than quiet, so the cost of being wrong here is a false + * alarm. + */ +const SELECTOR_PRELUDE = /^@(scope|supports)(?![\w-])/i; + +/** + * The half of that pair whose WHOLE prelude is selectors. + * + * No boundary of its own: nothing reaches this that `SELECTOR_PRELUDE` has + * not already matched with one, so a second copy would be unreachable + * rather than merely unexercised. + */ +const SCOPE_AT_RULE = /^@scope/i; + +/** + * A numeric escape, which names a character by code point. + * + * `.\\31 23` is the class `123`, and the tokeniser reads every escape as one + * character, so it would take `31` and lose the real name. That is the quiet + * direction, and decoding them properly means a CSS identifier reader, which + * is more than this file should carry. Every path that reads a class name + * checks for one and stops instead. + */ +const NUMERIC_ESCAPE = /\\[0-9a-f]/i; + +/** + * A character CSS ends an unterminated string at. + * + * Line feed, carriage return and form feed, all three. Bounding on the + * newline alone left the leak this closed open for a stylesheet with CR or + * FF endings, which is the same shape one character to the side. + */ +const LINE_BREAK = /[\n\r\f]/; + +/** + * A name a CSS class can actually have. + * + * An identifier may not begin with a digit, nor with a hyphen followed by + * one, so anything that does was never a class — it is a number this file + * read as one. A keyframe selector is where that happens: + * `@keyframes f{33.3%{…}}` offers `.3` to the class tokeniser and it comes + * back as the name `3`. Fabricating a name is the loud direction for the + * membership index, so this is tidiness rather than a fix, and it closes + * the shape wherever it appears rather than teaching one caller about + * keyframes. + */ +const IDENTIFIER_START = /^(?:--|-?[A-Za-z_\\])/; + +/** A class selector wherever it appears, with its name captured. */ +const CLASS_TOKEN = /\.((?:\\.|[A-Za-z0-9_-])+)/g; + +/** One class selector, escapes included. */ +const CLASS_SELECTOR = /\.(?:\\.|[A-Za-z0-9_-])+/g; + +/** + * Is this pseudo-class argument a class selector list and nothing else? + * + * Split rather than matched. Writing the list as one pattern puts a + * quantifier holding optional whitespace inside another quantifier, and the + * two `\s*` can divide a run of spaces between them in exponentially many + * ways: `.a .a .a …` against such a pattern takes seconds by a couple of + * dozen terms, and CodeQL fails the build over it. Splitting on the + * separator first leaves one unambiguous pattern per part. + */ +const onlyClasses = argument => + // No emptiness check up front: `''.split(',')` is `['']`, and the + // per-part test below rejects that already, so a clause here could never + // decide anything. + argument.split(',').every(part => { + // Whatever is left once the class selectors come out has to be nothing. + // A COMPOUND of them counts — `:not(.is-a.is-b)` prohibits carrying both + // and names no other kind of thing — but a space does not, because a + // descendant inside the argument is a shape this file does not model. + const trimmed = part.trim(); + return trimmed !== '' && trimmed.replace(CLASS_SELECTOR, '') === ''; + }); + +/** + * A selector prelude with everything that is not a class made explicit. + * + * A simple selector is only as live as its WHOLE set of requirements, and + * the exact-size rule in `shipsClass` counts classes. So anything else the + * selector demands has to end up in that count, or a rule needing more than + * the query answers it — over-reporting, and the silent direction. + * + * Three kinds of thing appear, and they are not alike: + * + * - `:not(…)` over CLASSES is a PROHIBITION. Its argument names classes the + * element must NOT carry, so reading them as present lets + * `.notification:not(.is-light)` answer `notification.is-light` — a rule + * that excludes the pair reporting it live. What remains already says + * everything the element carries, so it goes, argument and all, and + * leaves a `*` where it stood. + * + * The `*` is not decoration. A prohibition that is the WHOLE simple + * selector leaves nothing behind if it drops to empty, and the combinator + * before it then has no subject — `.hero.is-primary :not(.is-light)` + * collapses to `.hero.is-primary`, which hands the ancestor the subject + * position and says a rule renders the hero when it renders something + * inside it. The universal selector is exactly what a prohibition leaves: + * it matches everything, it holds the position, and it is already + * discarded as a qualifier. + * + * Only over classes. `:not(:last-child)` prohibits a POSITION, and + * dropping it answers live where the mirror `.box:first-child` answers + * dead, which is the same requirement read two ways. And a prohibition + * can add a requirement after all: `:not(:not(.x))` needs `.x`, and two + * drops leave nothing. So the argument has to be classes and nothing + * else, checked as written; anything else is a requirement like any + * other. A nested call counts as anything else: `:not(:is(.is-light))` + * is never matched here at all, because the pattern cannot cross the + * inner parenthesis, so the `:not` survives into the qualifier residue + * in `classesOf` and is counted there as the requirement it could not + * verify. + * - A pseudo-ELEMENT does not restrict anything. `.delete::before` styles a + * box generated for a delete, so the delete is live and calling it dead + * would be a plain misreading. A bare `::` spelling is exempted. Two + * kinds are not, and both land on the false-alarm side: the legacy + * one-colon spellings, which are indistinguishable from `:hover` without + * keeping a list, and the functional ones like `::part(x)`, which the + * argument pass takes first because it matches any name before a + * parenthesis. + * - Everything else — a pseudo-class with parentheses or without, and an + * attribute selector — narrows WHICH elements carrying those classes + * match. `.notification.is-primary:where(.is-light)` and + * `.notification.is-primary:first-child` are the same shape, and treating + * only the parenthesised one as a requirement is how this file came to + * contradict itself. Both leave an `UNMODELLED` behind, so the set grows + * rather than shrinks and the query reads dead. A false alarm is the one + * way this guard may be wrong. + * + * Arguments are handled before the caller splits on commas, because they + * contain commas: `.navbar-item:not(.is-active,.is-selected)` is ONE + * selector, and splitting first fragments it and leaves `is-active` looking + * like a class the element carries. + * + * One pass, and this is the part that took longest to get right. `[^()]*` + * cannot cross a parenthesis, so a nested call leaves its enclosing one + * unmatched, and `replace` does not re-scan what it wrote. That is the + * correct answer rather than a gap: a construct whose argument could not be + * read is a requirement, and the residue check counts it. Repeating to a + * fixed point looked like an improvement and was the opposite — it resolved + * the inner call to something CLASS-SHAPED, which `onlyClasses` then + * accepted, so `:not(:nth-last-child(1))` dropped and read live while the + * identical `:not(:last-child)` read dead. One pass keeps them the same, + * and keeps the one that is wrong on the loud side. + */ +function requirements(prelude) { + // Whatever is still inside parentheses once the passes above have run is + // the argument of a call none of them could resolve, and it has already + // been accounted for: the call's NAME survives into the qualifier + // residue in `classesOf`, which sentinels it there. Leaving the text behind is what hurts, because the caller + // splits on commas and an argument may hold them — + // `:has(:nth-child(2),.notification.is-primary,.z)` fragments into a + // clean `.notification.is-primary` with no sentinel on it, which reads + // live. Removing it is text-level and cannot turn a requirement into a + // class, which is the mistake the fixed-point loop made. Innermost first, + // and each round removes a pair, so it ends. + let text = prelude + .replace(/:([a-z-]+)\(([^()]*)\)/gi, (_, name, argument) => + name.toLowerCase() === 'not' && onlyClasses(argument) + ? '*' + : `.${UNMODELLED}` + ) + // Nothing else is substituted. A bare pseudo-class, an ID and an + // attribute selector all survive into the qualifier residue in + // `classesOf` and are counted there, so a pass apiece would be parser + // for its own sake. + // + // Pseudo-elements are the exception: they restrict nothing, so they + // must not reach the residue check and be read as a requirement. They + // leave a `*` rather than nothing, for the same reason `:not()` does — + // `::before` can BE the whole simple selector, and removing it outright + // left `.hero.is-primary ::before` as `.hero.is-primary`, which hands + // the ancestor the subject position — the same hole the prohibition + // opened, through the last SUBSTITUTION pass that was still deleting. + // The paren-residue loop below deletes too, and is allowed to: by the + // time it runs the enclosing call's name is already a sentinel outside + // the parentheses, so it can only shorten an argument. + .replace(/::[a-z-]+/gi, '*'); + while (/\([^()]*\)/.test(text)) text = text.replace(/\([^()]*\)/g, ''); + return text; +} + +/** + * The classes a simple selector requires, sentinel included. + * + * `requirements` handles everything a selector can say ABOUT an element. + * What it cannot see is the element itself: a type selector is not a token + * to substitute but the absence of one, and only the shape of a simple + * selector tells you it is there. Bulma ships the `dropdown-item` selected + * pair type-qualified and no other way, so reading `a.dropdown-item + * .is-selected` as two classes says a plain + * `