chore(null): fix yarn null:find on windows (#3340)

#### Description of changes

The primary impact/motivation here was to fix an issue where `yarn null:find` wasn't outputting all the files it was supposed to (as it turns out, this was windows-specific). The fix for this is on line 39 of `import-finder.js` (different parts of the tool weren't in agreement about normalizing slashes, which was breaking some of the filtering logic).

As part of fixing/debugging this, I did a fair amount of refactoring/cleanup to null:find. I didn't separate out the cleanup into a separate PR like I normally would because this is a short-lived dev-only tool that doesn't require as thorough review as most of the repo's code.

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: related to #2869
- [x] Ran `yarn fastpass`
- [n/a] Added/updated relevant unit test(s) (and ran `yarn test`)
- [n/a] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS

tools/strict-null-checks/auto-add.js

@@ -6,7 +6,7 @@ const path = require('path');
 const fs = require('fs');
 const child_process = require('child_process');
 const config = require('./config');
-const { forStrictNullCheckEligibleFiles } = require('./eligible-file-finder');
+const { getUncheckedLeafFiles } = require('./eligible-file-finder');
 const { collapseCompletedDirectories } = require('./collapse-completed-directories');
 const { writeTsconfigSync } = require('./write-tsconfig');
 
@@ -16,7 +16,7 @@ const tsconfigPath = path.join(repoRoot, config.targetTsconfig);
 
 const buildCompletePattern = /Found (\d+) errors?\. Watching for file changes\./gi;
 
-forStrictNullCheckEligibleFiles(repoRoot, () => {}).then(async files => {
+getUncheckedLeafFiles(repoRoot).then(async files => {
     const child = child_process.spawn('node', [tscPath, '-p', tsconfigPath, '--watch']);
     for (const file of files) {
         await tryAutoAddStrictNulls(child, tsconfigPath, file);

tools/strict-null-checks/eligible-file-finder.js

@@ -11,7 +11,7 @@ const config = require('./config');
  * @param {string} srcRoot
  * @param {{ includeTests: boolean }} [options]
  */
-const forEachFileInSrc = (srcRoot, options) => {
+const getAllTsFiles = (srcRoot, options) => {
     return new Promise((resolve, reject) => {
         glob(`${srcRoot}/**/*.@(ts|tsx)`, (err, files) => {
             if (err) {
@@ -31,14 +31,9 @@ const forEachFileInSrc = (srcRoot, options) => {
 
 /**
  * @param {string} repoRoot
- * @param {(file: string) => void} forEach
  * @param {{ includeTests: boolean }} [options]
  */
-async function forStrictNullCheckEligibleFiles(repoRoot, forEach, options) {
-    const srcRoot = path.join(repoRoot, 'src');
-
-    const checkedFiles = await getCheckedFiles(repoRoot);
-
+async function getUncheckedLeafFiles(repoRoot, options) {
     const imports = new Map();
     const getMemoizedImportsForFile = (file, srcRoot) => {
         if (imports.has(file)) {
@@ -49,35 +44,22 @@ async function forStrictNullCheckEligibleFiles(repoRoot, forEach, options) {
         return importList;
     };
 
-    const files = await forEachFileInSrc(srcRoot, options);
+    const srcRoot = path.join(repoRoot, 'src');
 
-    return files
-        .filter(file => !checkedFiles.has(file))
-        .filter(file => !config.skippedFiles.has(path.relative(srcRoot, file)))
-        .filter(file => {
-            const allProjImports = getMemoizedImportsForFile(file, srcRoot);
+    const checkedFiles = await getCheckedFiles(repoRoot);
 
-            const nonCheckedImports = allProjImports
-                .filter(x => x !== file)
-                .filter(imp => {
-                    if (checkedFiles.has(imp)) {
-                        return false;
-                    }
-                    // Don't treat cycles as blocking
-                    const impImports = getMemoizedImportsForFile(imp, srcRoot);
-                    return (
-                        impImports.filter(x => x !== file).filter(x => !checkedFiles.has(x))
-                            .length !== 0
-                    );
-                });
+    const allFiles = await getAllTsFiles(srcRoot, options);
 
-            const isEdge = nonCheckedImports.length === 0;
+    const allUncheckedFiles = allFiles
+        .filter(file => !checkedFiles.has(file))
+        .filter(file => !config.skippedFiles.has(path.relative(srcRoot, file)));
 
-            if (isEdge) {
-                forEach(file);
-            }
-            return isEdge;
-        });
+    const areAllImportsChecked = file => {
+        const allImports = getMemoizedImportsForFile(file, srcRoot);
+        return allImports.every(imp => checkedFiles.has(imp));
+    };
+
+    return allUncheckedFiles.filter(areAllImportsChecked);
 }
 
 async function getCheckedFiles(tsconfigDir) {
@@ -105,7 +87,7 @@ async function getCheckedFiles(tsconfigDir) {
 }
 
 module.exports = {
-    forEachFileInSrc,
-    forStrictNullCheckEligibleFiles,
+    getAllTsFiles,
+    getUncheckedLeafFiles,
     getCheckedFiles,
 };

tools/strict-null-checks/find.js

@@ -3,31 +3,35 @@
 
 // @ts-check
 const path = require('path');
+const process = require('process');
 const { repoRoot } = require('./config');
-const { forStrictNullCheckEligibleFiles, forEachFileInSrc } = require('./eligible-file-finder');
+const { getUncheckedLeafFiles, getAllTsFiles } = require('./eligible-file-finder');
 const { getImportsForFile } = require('./import-finder');
 
 const srcRoot = path.join(repoRoot, 'src');
 
-let sort = true;
-let filter;
-let printDependedOnCount = true;
-let includeTests = false;
-
-if (false) {
-    // Generate test files listing
-    sort = false;
-    filter = x => x.endsWith('.test.ts');
-    printDependedOnCount = false;
-    includeTests = true;
+if (process.argv.includes('--help')) {
+    console.log(
+        'yarn null:find [--sort=name|count] [--show-count] [--include-tests] [--filter file_path_substring]',
+    );
+    process.exit(0);
 }
+const sortBy = process.argv.includes('--sort=name') ? 'name' : 'count';
+const printDependedOnCount = process.argv.includes('--show-count');
+const includeTests = process.argv.includes('--include-tests');
+const filterArgIndex = process.argv.indexOf('--filter') + 1;
+const filterArg = filterArgIndex === 0 ? null : process.argv[filterArgIndex];
+
+const filter = filterArg && (file => file.includes(filterArg));
+
+async function main() {
+    const eligibleFiles = await getUncheckedLeafFiles(repoRoot, { includeTests });
 
-forStrictNullCheckEligibleFiles(repoRoot, () => {}, { includeTests }).then(async eligibleFiles => {
     const eligibleSet = new Set(eligibleFiles);
 
     const dependedOnCount = new Map(eligibleFiles.map(file => [file, 0]));
 
-    for (const file of await forEachFileInSrc(srcRoot)) {
+    for (const file of await getAllTsFiles(srcRoot)) {
         if (eligibleSet.has(file)) {
             // Already added
             continue;
@@ -44,19 +48,25 @@ forStrictNullCheckEligibleFiles(repoRoot, () => {}, { includeTests }).then(async
     if (filter) {
         out = out.filter(x => filter(x[0]));
     }
-    if (sort) {
+    if (sortBy === 'count') {
         out = out.sort((a, b) => b[1] - a[1]);
+    } else if (sortBy === 'name') {
+        out = out.sort((a, b) => a[0].localeCompare(b[0]));
     }
     for (const pair of out) {
         console.log(
-            toFormattedFilePath(pair[0]),
-            // + (printDependedOnCount ? ` — Depended on by **${pair[1]}** files` : ''),
+            toFormattedFilePath(pair[0]) +
+                (printDependedOnCount ? ` — Depended on by **${pair[1]}** files` : ''),
         );
     }
-});
+}
 
 function toFormattedFilePath(file) {
-    // return `"./${path.relative(srcRoot, file)}",`;
     const relativePath = path.relative(srcRoot, file).replace(/\\/g, '/');
     return `"./src/${relativePath}",`;
 }
+
+main().catch(error => {
+    console.error(error.stack);
+    process.exit(1);
+});

tools/strict-null-checks/import-finder.js

@@ -6,34 +6,41 @@ const path = require('path');
 const ts = require('typescript');
 const fs = require('fs');
 
-module.exports.getImportsForFile = function getImportsForFile(file, srcRoot) {
-    const fileInfo = ts.preProcessFile(fs.readFileSync(file).toString());
+module.exports.getImportsForFile = function getImportsForFile(parentFilePath, srcRoot) {
+    const fileInfo = ts.preProcessFile(fs.readFileSync(parentFilePath).toString());
     return fileInfo.importedFiles
         .map(importedFile => importedFile.fileName)
         .filter(fileName => !/\.scss$/.test(fileName)) // remove css imports
-        .filter(x => /\//.test(x)) // remove node modules (the import must contain '/')
-        .filter(x => !/^(@uifabric|lodash|react-dom)\//.test(x)) // remove node module usages with slashes in name
+        .filter(fileName => /\//.test(fileName)) // remove node modules (the import must contain '/')
+        .filter(fileName => !/^(@uifabric|lodash|react-dom|axe-core)\//.test(fileName)) // remove node module usages with slashes in name
         .map(fileName => {
             if (/(^\.\/)|(^\.\.\/)/.test(fileName)) {
-                return path.join(path.dirname(file), fileName);
+                return path.join(path.dirname(parentFilePath), fileName);
             }
             return path.join(srcRoot, fileName);
         })
-        .map(fileName => {
-            for (const ext of ['ts', 'tsx', 'js', 'jsx', 'd.ts']) {
-                const candidate = `${fileName}.${ext}`;
+        .map(filePathWithoutExtension => {
+            for (const ext of ['ts', 'tsx', 'd.ts', 'js', 'jsx']) {
+                const candidate = `${filePathWithoutExtension}.${ext}`;
                 if (fs.existsSync(candidate)) {
                     return candidate;
                 }
             }
 
             for (const indexFile of ['index.ts', 'index.js']) {
-                const candidate = path.join(fileName, indexFile);
+                const candidate = path.join(filePathWithoutExtension, indexFile);
                 if (fs.existsSync(candidate)) {
                     return candidate;
                 }
             }
 
-            throw new Error(`Unresolved import ${fileName} in ${file}`);
+            throw new Error(`Unresolved import ${filePathWithoutExtension} in ${parentFilePath}`);
+        })
+        .map(unnormalizedPath => unnormalizedPath.replace(/\\/g, '/'))
+        .map(filePath => {
+            if (filePath === parentFilePath) {
+                throw new Error(`self-import in ${parentFilePath}`);
+            }
+            return filePath;
         });
 };

tools/strict-null-checks/progress.js

@@ -3,12 +3,12 @@
 
 // @ts-check
 const { repoRoot } = require('./config');
-const { getCheckedFiles, forEachFileInSrc } = require('./eligible-file-finder');
+const { getCheckedFiles, getAllTsFiles } = require('./eligible-file-finder');
 
 async function main() {
     const datestamp = new Date().toDateString();
     const doneCount = (await getCheckedFiles(repoRoot)).size;
-    const totalCount = (await forEachFileInSrc(`${repoRoot}/src`)).length;
+    const totalCount = (await getAllTsFiles(`${repoRoot}/src`)).length;
     const percentage = 100 * (doneCount / totalCount);
     const formattedPercentage = percentage.toFixed(0) + '%';