Flag redundant allowed classes within required in the linter

.github/workflows/lint-scripts/password-rules-parse.js

@@ -1,10 +1,18 @@
 // Copyright (c) 2026 Apple Inc. Licensed under MIT License.
 //
-// Validates every "password-rules" string in quirks/password-rules.json by
-// loading tools/PasswordRulesParser.js and calling parsePasswordRules() on it.
-// Any parse error (e.g. ",required" instead of "; required" as in #907, or
-// '-'/']' in the wrong position inside a character class) causes CI to fail
-// with a clear message identifying the offending domain.
+// Lints every "password-rules" string in quirks/password-rules.json:
+//
+//   1. Validates it parses cleanly by loading tools/PasswordRulesParser.js and
+//      calling parsePasswordRules() on it. Any parse error (e.g. ",required"
+//      instead of "; required" as in #907, or '-'/']' in the wrong position
+//      inside a character class) causes CI to fail with a clear message
+//      identifying the offending domain.
+//
+//   2. Flags entries whose "allowed" list redundantly names a class that is
+//      already covered by a "required" rule. Required rules are implicitly
+//      allowed, so writing e.g. `required: upper, lower; allowed: upper,
+//      lower, [!@#];` is redundant and should be shortened to
+//      `required: upper, lower; allowed: [!@#];`.
 
 "use strict";
 
@@ -30,28 +38,81 @@ const sandbox = {
 vm.createContext(sandbox);
 vm.runInContext(parserSource, sandbox, { filename: parserPath });
 
+// These are semantic supersets used to say "allow everything"; they subsume required
+// classes by design and are not redundant.
+const ALWAYS_ALLOWED_SUPERSETS = new Set(["ascii-printable", "unicode"]);
+
+function namedClassNames(value) {
+    if (!Array.isArray(value)) {
+        return [];
+    }
+    const names = [];
+    for (const entry of value) {
+        // NamedCharacterClass instances have a `name` getter and no `characters`.
+        if (entry && typeof entry.name === "string" && !("characters" in entry)) {
+            names.push(entry.name);
+        }
+    }
+    return names;
+}
+
+function findRedundantAllowedClasses(parsedRules) {
+    const requiredNames = new Set();
+    let allowedRule = null;
+    for (const rule of parsedRules) {
+        if (rule.name === "required") {
+            for (const name of namedClassNames(rule.value)) {
+                requiredNames.add(name);
+            }
+        } else if (rule.name === "allowed") {
+            allowedRule = rule;
+        }
+    }
+    if (!allowedRule || requiredNames.size === 0) {
+        return [];
+    }
+    return namedClassNames(allowedRule.value)
+        .filter((name) => !ALWAYS_ALLOWED_SUPERSETS.has(name))
+        .filter((name) => requiredNames.has(name));
+}
+
 let failedDomains = 0;
 for (const domain of Object.keys(rules)) {
     const ruleString = rules[domain]["password-rules"];
     sandbox.capturedErrors.length = 0;
     sandbox.__ruleString = ruleString;
+    let parsed = null;
     try {
-        vm.runInContext("parsePasswordRules(__ruleString)", sandbox);
+        // Pass formatRulesForMinifiedVersion=true so required classes are not
+        // silently copied into the allowed rule.
+        parsed = vm.runInContext("parsePasswordRules(__ruleString, true)", sandbox);
     } catch (e) {
         sandbox.capturedErrors.push(`threw ${e.message}`);
     }
-    if (sandbox.capturedErrors.length > 0) {
+
+    // Log parse errors and warnings before required/allowed redundancy issues.
+    const messages = sandbox.capturedErrors.slice();
+    if (parsed) {
+        const redundant = findRedundantAllowedClasses(parsed);
+        if (redundant.length > 0) {
+            const list = redundant.map((n) => `\`${n}\``).join(", ");
+            messages.push(`Redundancy in \`allowed:\` (already covered by \`required:\`): ${redundant.join(", ")}`);
+            messages.push(`Remove ${list} from the \`allowed:\` list — required classes are implicitly allowed.`);
+        }
+    }
+
+    if (messages.length > 0) {
         failedDomains++;
         console.error(`${domain}: ${ruleString}`);
-        for (const message of sandbox.capturedErrors) {
+        for (const message of messages) {
             console.error(`  ${message}`);
         }
     }
 }
 
 if (failedDomains > 0) {
-    console.error(`\n${failedDomains} domain(s) in password-rules.json failed to parse.`);
+    console.error(`\n${failedDomains} domain(s) in password-rules.json failed linting.`);
     process.exit(1);
 }
 
-console.log(`OK: ${Object.keys(rules).length} password-rules entries parsed cleanly.`);
+console.log(`OK: ${Object.keys(rules).length} password-rules entries parsed cleanly with no redundant required/allowed classes.`);

.github/workflows/lint.yml

@@ -43,7 +43,7 @@ jobs:
     - name: Lint Sort Order
       run: cat quirks/password-rules.json | grep "^    \"" | sort -c
     - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
-    - name: Lint Rule Syntax
+    - name: Lint Rules
       run: node .github/workflows/lint-scripts/password-rules-parse.js
 
   websites-shared-credentials: