Compute selector specificity at parse time. (#1514)

* Add Specificity class * Add Specificity to selectors * Improve possibility of random tests (property testing) * Add some specifity tests * Avoid overflow in tests * Add some more tests * Use selector specificity directly * Clean up * Typos * Move property testing instructions to README * Improve RNG wrapping * Improve doc * Clean up * Clean up * Extract API --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Siteimprove · Nov 29, 2023 · dfa47dc · dfa47dc
1 parent f390f50
commit dfa47dc
Show file tree

Hide file tree

Showing 37 changed files with 904 additions and 283 deletions.
diff --git a/.changeset/eleven-shrimps-tap.md b/.changeset/eleven-shrimps-tap.md
@@ -0,0 +1,5 @@
+---
+"@siteimprove/alfa-selector": minor
+---
+
+**Added:** `Selector` now contain their own `Specificity`.
diff --git a/.changeset/rude-rivers-check.md b/.changeset/rude-rivers-check.md
@@ -0,0 +1,8 @@
+---
+"@siteimprove/alfa-test": minor
+---
+
+**Added:** Test can now accept a `Controller` to generate random tests.
+
+See the [README](./README.md) for more information.
+
diff --git a/docs/review/api/alfa-selector.api.md b/docs/review/api/alfa-selector.api.md
@@ -7,6 +7,8 @@
 import { Array as Array_2 } from '@siteimprove/alfa-array';
 import { Element } from '@siteimprove/alfa-dom';
 import { Equatable } from '@siteimprove/alfa-equatable';
+import { Hash } from '@siteimprove/alfa-hash';
+import { Hashable } from '@siteimprove/alfa-hash';
 import { Iterable as Iterable_2 } from '@siteimprove/alfa-iterable';
 import * as json from '@siteimprove/alfa-json';
 import { Nth } from '@siteimprove/alfa-css';

diff --git a/docs/review/api/alfa-test.api.md b/docs/review/api/alfa-test.api.md
@@ -24,6 +24,21 @@ export interface Assertions {
     throws(block: Function, error?: RegExp | Function | Object | Error, message?: string): void;
 }
 
+// @public (undocumented)
+export interface Controller<T = number> {
+    // (undocumented)
+    iterations: number;
+    // (undocumented)
+    seed?: number;
+    // (undocumented)
+    wrapper: (rng: RNG<number>, iteration: number) => RNG<T>;
+}
+
+// Warning: (ae-internal-missing-underscore) The name "defaultController" should be prefixed with an underscore because the declaration is marked as @internal
+//
+// @internal (undocumented)
+export const defaultController: Controller<number>;
+
 // Warning: (ae-internal-missing-underscore) The name "format" should be prefixed with an underscore because the declaration is marked as @internal
 //
 // @internal (undocumented)
@@ -37,6 +52,14 @@ export interface Notifier {
     error(message: string): void;
 }
 
+// @public (undocumented)
+export type RNG<T = number> = () => T;
+
+// Warning: (ae-internal-missing-underscore) The name "seedableRNG" should be prefixed with an underscore because the declaration is marked as @internal
+//
+// @internal
+export function seedableRNG(seed: number): RNG<number>;
+
 // Warning: (ae-forgotten-export) The symbol "Frame" needs to be exported by the entry point index.d.ts
 // Warning: (ae-internal-missing-underscore) The name "stack" should be prefixed with an underscore because the declaration is marked as @internal
 //
@@ -46,10 +69,10 @@ export function stack(error: Error): Iterable<Frame>;
 // Warning: (ae-internal-mixed-release-tag) Mixed release tags are not allowed for "test" because one of its declarations is marked as @internal
 //
 // @public (undocumented)
-export function test(name: string, assertion: (assert: Assertions) => void | Promise<void>): Promise<void>;
+export function test<T = number>(name: string, assertion: (assert: Assertions, rng: RNG<T>, seed: number) => void | Promise<void>, controller?: Partial<Controller<T>>): Promise<void>;
 
 // @internal (undocumented)
-export function test(name: string, assertion: (assert: Assertions) => void | Promise<void>, notifier: Notifier): Promise<void>;
+export function test<T = number>(name: string, assertion: (assert: Assertions, rng: RNG<T>, seed: number) => void | Promise<void>, notifier: Notifier, controller?: Partial<Controller<T>>): Promise<void>;
 
 // (No @packageDocumentation comment for this package)
 

diff --git a/packages/alfa-cascade/src/selector-map.ts b/packages/alfa-cascade/src/selector-map.ts
@@ -329,14 +329,11 @@ export namespace SelectorMap {
 
       // For style rules that are presentational hints, the specificity will
       // always be 0 regardless of the selector.
-      if (StyleRule.isStyleRule(rule) && rule.hint) {
-        this._specificity = 0;
-      }
-
-      // Otherwise, determine the specificity of the selector.
-      else {
-        this._specificity = getSpecificity(selector);
-      }
+      // Otherwise, use the specificity of the selector.
+      this._specificity =
+        StyleRule.isStyleRule(rule) && rule.hint
+          ? 0
+          : selector.specificity.value;
     }
 
     public get rule(): Rule {
@@ -457,58 +454,6 @@ function getKeySelector(selector: Selector): Id | Class | Type | null {
   return null;
 }
 
-type Specificity = number;
-
-// The number of bits to use for every component of the specificity computation.
-// As bitwise operations in JavaScript are limited to 32 bits, we can only use
-// at most 10 bits per component as 3 components are used.
-const componentBits = 10;
-
-// The maximum value that any given component can have. Since we can only use 10
-// bits for every component, this in effect means that any given component count
-// must be strictly less than 1024.
-const componentMax = (1 << componentBits) - 1;
-
-/**
- * {@link https://www.w3.org/TR/selectors/#specificity}
- */
-function getSpecificity(selector: Selector): Specificity {
-  let a = 0;
-  let b = 0;
-  let c = 0;
-
-  const queue: Array<Selector> = [selector];
-
-  while (queue.length > 0) {
-    const selector = queue.pop()!;
-
-    if (isId(selector)) {
-      a++;
-    } else if (
-      isClass(selector) ||
-      isAttribute(selector) ||
-      isPseudoClass(selector)
-    ) {
-      b++;
-    } else if (isType(selector) || isPseudoElement(selector)) {
-      c++;
-    } else if (isComplex(selector)) {
-      queue.push(selector.left, selector.right);
-    } else if (isCompound(selector)) {
-      queue.push(...selector.selectors);
-    }
-  }
-
-  // Concatenate the components to a single number indicating the specificity of
-  // the selector. This allows us to treat specificities as simple numbers and
-  // hence use normal comparison operators when comparing specificities.
-  return (
-    (Math.min(a, componentMax) << (componentBits * 2)) |
-    (Math.min(b, componentMax) << (componentBits * 1)) |
-    Math.min(c, componentMax)
-  );
-}
-
 /**
  * Check if a selector can be rejected based on an ancestor filter.
  */

diff --git a/packages/alfa-map/test/property.spec.ts b/packages/alfa-map/test/property.spec.ts
@@ -1,55 +1,14 @@
-import { test } from "@siteimprove/alfa-test";
+import { RNG, test } from "@siteimprove/alfa-test";
 
 import { Hash } from "@siteimprove/alfa-hash";
 import { Iterable } from "@siteimprove/alfa-iterable";
 
 import { Map } from "../src/map";
 
-/**
- * In order to do correct property based testing on Map (and hash tables), we
- * need:
- * * a seedable Pseudo-Random Numbers Generator, so we can re-use the seed to
- *   investigate problems;
- * * full control over the hashes of the keys, notably not relying on
- *   Hash.writeObject since the objects list might be "polluted" by previous
- *   actions
- *
- * The PRNG doesn't need to be very good (i.e. not cryptographic-grade). We also
- * wants the hashes to be both among a relatively small set (so that collisions
- * are likely and can be tested without huge number of items); and relatively
- * spread over the space so that Sparses are frequent and can be tested.
- *
- * 16 bits entropy seems a nice number since it leaves room for ~65,000 keys. By
- * making runs of 1000 keys, collisions are very likely to happen and there
- * is a relatively small number of iterations so tests don't run forever…
- *
- * We also need to output the failing seed systematically to allow investigation
- * in case of problems…
- *
- * Since #has looks for keys using hashes while Iterable.includes(map.keys())
- * is iterating through the keys without using hashes, both are systematically
- * used in tests as a way to ensure that no hash somehow becomes unreachable.
- */
-
-/**
- * PRNG taken from
- * {@link https://stackoverflow.com/questions/521295/seeding-the-random-number-generator-in-javascript}
- */
-
-function mulberry32(seed: number) {
-  return function () {
-    let t = (seed += 0x6d2b79f5);
-    t = Math.imul(t ^ (t >>> 15), t | 1);
-    t ^= t + Math.imul(t ^ (t >>> 7), t | 61);
-    return ((t ^ (t >>> 14)) >>> 0) / 4294967296;
-  };
-}
-
 /**
  * Turning a float between 0 and 1 into a 32 bits int, and keeping only
  * 16 bits to reduce entropy while keeping good coverage of the space…
  */
-
 function makeInt(x: number): number {
   return Math.floor(x * 2 ** 32) & 0b01010101010101010101010101010101;
 }
@@ -76,85 +35,91 @@ function key(id: number): Key {
   return self;
 }
 
-function randKey(rng: () => number): () => Key {
+function wrapper(rng: RNG<number>): RNG<Key> {
   return () => key(makeInt(rng()));
 }
 
-test(`#add and #delete behave when used in bulk`, (t) => {
-  const iterations = 1000;
-  const seed = Math.random();
-  const rng = mulberry32(seed);
-  const keyGen = randKey(rng);
-
-  let map: Map<Key, boolean> = Map.empty();
-  const keys: Array<Key> = [];
-
-  // Adding elements
-  for (let i = 0; i < iterations; i++) {
-    const key = keyGen();
-
-    t.deepEqual(map.size, i, `Pre-add map.size() fails with seed ${seed}`);
-    t.deepEqual(
-      map.has(key),
-      false,
-      `Pre-add map.has() fails with seed ${seed}`,
-    );
-    t.deepEqual(
-      Iterable.includes(map.keys(), key),
-      false,
-      `Pre-add includes fails with seed ${seed}`,
-    );
-
-    map = map.set(key, true);
-    keys.push(key);
-
-    t.deepEqual(map.size, i + 1, `Post-add map.size() fails with seed ${seed}`);
-    t.deepEqual(
-      map.has(key),
-      true,
-      `Post-add map.has() fails with seed ${seed}`,
-    );
-    t.deepEqual(
-      Iterable.includes(map.keys(), key),
-      true,
-      `Post-add includes fails with seed ${seed}`,
-    );
-  }
-
-  // Removing those elements. Note that the keys array keep them in an order
-  // fairly different from the #keys() enumeration (which is essentially
-  // depth-first order in the Map, hence depends on hashes).
-  // Hopefully, this creates enough entropy to test various scenarios.
-  for (let i = iterations; i > 0; i--) {
-    const key = keys[iterations - i];
-    t.deepEqual(map.size, i, `Pre-delete map.size() fails with seed ${seed}`);
-    t.deepEqual(
-      map.has(key),
-      true,
-      `Pre-delete map.has() fails with seed ${seed}`,
-    );
-    t.deepEqual(
-      Iterable.includes(map.keys(), key),
-      true,
-      `Pre-delete includes fails with seed ${seed}`,
-    );
-
-    map = map.delete(key);
-
-    t.deepEqual(
-      map.size,
-      i - 1,
-      `Post-delete map.size() fails with seed ${seed}`,
-    );
-    t.deepEqual(
-      map.has(key),
-      false,
-      `Post-delete map.has() fails with seed ${seed}`,
-    );
-    t.deepEqual(
-      Iterable.includes(map.keys(), key),
-      false,
-      `Post-delete includes fails with seed ${seed}`,
-    );
-  }
-});
+test<Key>(
+  `#add and #delete behave when used in bulk`,
+  (t, rng, seed) => {
+    // How many elements are we adding/removing in each iteration of the test?
+    const size = 1000;
+
+    let map: Map<Key, boolean> = Map.empty();
+    const keys: Array<Key> = [];
+
+    // Adding elements
+    for (let i = 0; i < size; i++) {
+      const key = rng();
+
+      t.deepEqual(map.size, i, `Pre-add map.size() fails with seed ${seed}`);
+      t.deepEqual(
+        map.has(key),
+        false,
+        `Pre-add map.has() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        Iterable.includes(map.keys(), key),
+        false,
+        `Pre-add includes fails with seed ${seed}`,
+      );
+
+      map = map.set(key, true);
+      keys.push(key);
+
+      t.deepEqual(
+        map.size,
+        i + 1,
+        `Post-add map.size() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        map.has(key),
+        true,
+        `Post-add map.has() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        Iterable.includes(map.keys(), key),
+        true,
+        `Post-add includes fails with seed ${seed}`,
+      );
+    }
+
+    // Removing those elements. Note that the keys array keep them in an order
+    // fairly different from the #keys() enumeration (which is essentially
+    // depth-first order in the Map, hence depends on hashes).
+    // Hopefully, this creates enough entropy to test various scenarios.
+    for (let i = size; i > 0; i--) {
+      const key = keys[size - i];
+      t.deepEqual(map.size, i, `Pre-delete map.size() fails with seed ${seed}`);
+      t.deepEqual(
+        map.has(key),
+        true,
+        `Pre-delete map.has() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        Iterable.includes(map.keys(), key),
+        true,
+        `Pre-delete includes fails with seed ${seed}`,
+      );
+
+      map = map.delete(key);
+
+      t.deepEqual(
+        map.size,
+        i - 1,
+        `Post-delete map.size() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        map.has(key),
+        false,
+        `Post-delete map.has() fails with seed ${seed}`,
+      );
+      t.deepEqual(
+        Iterable.includes(map.keys(), key),
+        false,
+        `Post-delete includes fails with seed ${seed}`,
+      );
+    }
+  },
+  { wrapper },
+);
diff --git a/packages/alfa-selector/package.json b/packages/alfa-selector/package.json
@@ -23,6 +23,7 @@
     "@siteimprove/alfa-css": "workspace:^0.69.0",
     "@siteimprove/alfa-dom": "workspace:^0.69.0",
     "@siteimprove/alfa-equatable": "workspace:^0.69.0",
+    "@siteimprove/alfa-hash": "workspace:^0.69.0",
     "@siteimprove/alfa-iterable": "workspace:^0.69.0",
     "@siteimprove/alfa-json": "workspace:^0.69.0",
     "@siteimprove/alfa-map": "workspace:^0.69.0",