Skip to content

Add a more strict type checking mode to identify core description opportunities #457

@samreid

Description

@samreid

Core description requires certain options for certain types of objects, details are in https://github.com/phetsims/phet-info/blob/main/doc/core-description-quickstart-guide.md. We may be able to automatically detect cases where description is lacking using the type system. One way to do so would be to use more complex types and an overridden config during type checking. But a simpler way may be to switch the source code into different "modes" like so:

  // The context response is spoken when this radio button is pressed. Only spoken if the value changes.
  // The response is spoken after the Property has been set.
  accessibleContextResponse?: TAlertable; // WHEN STRICT: accessibleContextResponse: TAlertable;

and vice-versa:

  accessibleContextResponse: TAlertable; // WHEN NONSTRICT: accessibleContextResponse?: TAlertable;

Then with a script like so (GPT-5-thinking, untested), we can flip back and forth between the modes:

Details
#!/usr/bin/env node
// Usage: node scripts/flip-strict.js STRICT
//     or: node scripts/flip-strict.js NONSTRICT
//
// Mutates .ts/.tsx files in-place by swapping lines that end with:
//   // WHEN STRICT: <...>
// or
//   // WHEN NONSTRICT: <...>
//
// STRICT mode:  "<lhs> // WHEN STRICT: <rhs>"     ->  "<rhs> // WHEN NONSTRICT: <lhs>"
// NONSTRICT:    "<lhs> // WHEN NONSTRICT: <rhs>"  ->  "<rhs> // WHEN STRICT: <lhs>"

const fs = require('node:fs');
const path = require('node:path');

const MODE = (process.argv[2] || '').toUpperCase();
if (MODE !== 'STRICT' && MODE !== 'NONSTRICT') {
  console.error('flip-strict: pass mode STRICT or NONSTRICT');
  process.exit(2);
}

// configure roots and filters here if you need
const ROOT = process.cwd();
const INCLUDE_EXT = new Set(['.ts', '.tsx']);
const EXCLUDE_DIR = new Set(['node_modules', '.git', 'dist', 'build', 'out']);

const LINE_RE = /^(?<indent>\s*)(?<before>.*?)(?:\s*)\/\/\s*WHEN\s+(?<tag>STRICT|NONSTRICT)\s*:\s*(?<after>.*)\s*$/;

let filesChanged = 0;
let linesChanged = 0;

function shouldSkipDir(name) {
  return EXCLUDE_DIR.has(name) || name.startsWith('.');
}

function walk(dir) {
  for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
    const abs = path.join(dir, entry.name);
    if (entry.isDirectory()) {
      if (!shouldSkipDir(entry.name)) walk(abs);
      continue;
    }
    const ext = path.extname(entry.name);
    if (!INCLUDE_EXT.has(ext)) continue;
    transformFile(abs);
  }
}

function transformFile(file) {
  const original = fs.readFileSync(file, 'utf8');
  const lines = original.split(/\r?\n/);
  let changed = false;

  for (let i = 0; i < lines.length; i++) {
    const m = lines[i].match(LINE_RE);
    if (!m) continue;

    const { indent, before, tag, after } = m.groups;
    // Only flip lines that match the requested MODE
    if (tag !== MODE) continue;

    // Build swapped line:
    //   STRICT  -> emit "<after> // WHEN NONSTRICT: <before>"
    //   NONSTRICT -> emit "<after> // WHEN STRICT: <before>"
    const opposite = MODE === 'STRICT' ? 'NONSTRICT' : 'STRICT';
    const newLine =
      `${indent}${after.trim()} ` +
      `// WHEN ${opposite}: ${before.trim()}`;

    if (newLine !== lines[i]) {
      lines[i] = newLine;
      changed = true;
      linesChanged++;
    }
  }

  if (changed) {
    fs.writeFileSync(file, lines.join('\n'));
    filesChanged++;
  }
}

walk(ROOT);
console.log(`flip-strict(${MODE}): changed ${linesChanged} line(s) across ${filesChanged} file(s).`);

Some caveats here:

  • This would trigger type errors in "demo" and "example" code that would need patches
  • We would not want to accidentally commit the strict mode
  • It's a new "system" for people to manage
  • We could just explicitly list the search/replace patterns in a central place
  • This can only catch things enforceable by the type system, and many parts of the core description requirements are outside this system.
Details
Subject: [PATCH] hello
---
Index: sun/js/demo/buttons/demoRadioButtons.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/demo/buttons/demoRadioButtons.ts b/sun/js/demo/buttons/demoRadioButtons.ts
--- a/sun/js/demo/buttons/demoRadioButtons.ts	(revision 96764296efd3d877d2d28b273c60acec0ed5c164)
+++ b/sun/js/demo/buttons/demoRadioButtons.ts	(date 1755913518675)
@@ -71,6 +71,7 @@
       }
     };
   } );
+  // @ts-expect-error // WHEN NONSTRICT: // hello world
   const aquaRadioButtonGroup = new VerticalAquaRadioButtonGroup( aquaRadioButtonProperty, aquaRadioButtonGroupContent, {
     spacing: 8,
     enabledProperty: buttonsEnabledProperty
Index: sun/js/AquaRadioButton.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/AquaRadioButton.ts b/sun/js/AquaRadioButton.ts
--- a/sun/js/AquaRadioButton.ts	(revision 96764296efd3d877d2d28b273c60acec0ed5c164)
+++ b/sun/js/AquaRadioButton.ts	(date 1755913593549)
@@ -33,6 +33,10 @@
 import { TAlertable } from '../../utterance-queue/js/Utterance.js';
 import sun from './sun.js';
 
+// type A11yAudit = 'on';
+//
+// type MaybeRequire<T, K extends keyof T> = A11yAudit extends 'on' ? T & Required<Pick<T, K>> : T;
+
 type SelfOptions = {
 
   // color used to fill the center of the button when it's selected
@@ -58,7 +62,7 @@
 
   // The context response is spoken when this radio button is pressed. Only spoken if the value changes.
   // The response is spoken after the Property has been set.
-  accessibleContextResponse?: TAlertable;
+  accessibleContextResponse: TAlertable; // WHEN NONSTRICT: accessibleContextResponse?: TAlertable;
 
   // pointer areas
   touchAreaXDilation?: number;
@@ -139,7 +143,7 @@
       appendLabel: true,
       appendDescription: true,
       accessibleNameBehavior: Voicing.BASIC_ACCESSIBLE_NAME_BEHAVIOR,
-      accessibleContextResponse: null,
+      // accessibleContextResponse: null,
 
       // The group of radio buttons is responsible for implementing the Voicing output on focus.
       voicingFocusListener: null
Index: scenery-phet/js/TimeSpeedRadioButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/scenery-phet/js/TimeSpeedRadioButtonGroup.ts b/scenery-phet/js/TimeSpeedRadioButtonGroup.ts
--- a/scenery-phet/js/TimeSpeedRadioButtonGroup.ts	(revision bd810d7b3088cca7bd56abf54599ebfd081bcdeb)
+++ b/scenery-phet/js/TimeSpeedRadioButtonGroup.ts	(date 1755913705743)
@@ -73,6 +73,8 @@
     // by default, radio buttons match height of label before maxWidth scaling adjustments
     if ( !options.radioButtonOptions || options.radioButtonOptions.radius === undefined ) {
       if ( !options.radioButtonOptions ) {
+
+        // @ts-expect-error // WHEN NONSTRICT: // hello world
         options.radioButtonOptions = {};
       }
       options.radioButtonOptions.radius = new Text( 'test', options.labelOptions ).height / 2;
@@ -86,6 +88,8 @@
         value: speed,
         createNode: () => new Text( stringProperty, options.labelOptions ),
         tandemName: SPEED_LABEL_MAP.get( speed ).tandemName,
+
+        // @ts-expect-error // WHEN NONSTRICT: // hello world
         options: {
           accessibleName: stringProperty
         }
Index: joist/js/Helper.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/Helper.ts b/joist/js/Helper.ts
--- a/joist/js/Helper.ts	(revision 0943309f6d38f96ec118b72a58a8915a20fd009d)
+++ b/joist/js/Helper.ts	(date 1755913678443)
@@ -485,6 +485,8 @@
     ], {
       orientation: 'horizontal',
       enabledProperty: this.inputBasedPickingProperty,
+
+      // @ts-expect-error // WHEN NONSTRICT: // hello world
       radioButtonOptions: {
         xSpacing: 3
       },
Index: forces-and-motion-basics/js/netforce/view/PullerColorControl.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/forces-and-motion-basics/js/netforce/view/PullerColorControl.ts b/forces-and-motion-basics/js/netforce/view/PullerColorControl.ts
--- a/forces-and-motion-basics/js/netforce/view/PullerColorControl.ts	(revision 4d20548e6abbef0fc8a066161e54317ef26c4f4f)
+++ b/forces-and-motion-basics/js/netforce/view/PullerColorControl.ts	(date 1755898369014)
@@ -8,6 +8,7 @@
  * @author Luisa Vargas (PhET Interactive Simulations)
  */
 
+import StringProperty from '../../../../axon/js/StringProperty.js';
 import StringUnionProperty from '../../../../axon/js/StringUnionProperty.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
 import PreferencesDialogConstants from '../../../../joist/js/preferences/PreferencesDialogConstants.js';
@@ -31,6 +32,7 @@
      * Create an item for the radio button group.
      * @param value - value associated with the radio button
      * @param labelStringProperty - label that appears on the radio button
+     * @param tandemName
      */
     const createItem = ( value: 'blueRed' | 'purpleOrange', labelStringProperty: TReadOnlyProperty<string>, tandemName: string ) => {
       return {
@@ -41,7 +43,8 @@
           maxWidth: 500
         } ),
         options: {
-          accessibleName: labelStringProperty
+          accessibleName: labelStringProperty,
+          accessibleContextResponse: new StringProperty( 'hello world' )
         }
       };
     };
Index: sun/js/demo/buttons/demoAquaRadioButtonGroup.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/demo/buttons/demoAquaRadioButtonGroup.ts b/sun/js/demo/buttons/demoAquaRadioButtonGroup.ts
--- a/sun/js/demo/buttons/demoAquaRadioButtonGroup.ts	(revision 96764296efd3d877d2d28b273c60acec0ed5c164)
+++ b/sun/js/demo/buttons/demoAquaRadioButtonGroup.ts	(date 1755913637751)
@@ -44,6 +44,8 @@
         }
       };
     } );
+
+  // @ts-expect-error // WHEN NONSTRICT: // hello world
   const verticalGroup = new AquaRadioButtonGroup( verticalProperty, verticalItems, {
     orientation: 'vertical',
 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions