Skip to content

Commit

Permalink
chore(strict-null): add strict null check migration tools to repo (#2957
Browse files Browse the repository at this point in the history
)

#### Description of changes

This integrates the strict null checking tools that I migrated from VS Code's as an experiment in https://github.com/dbjorge/aiweb-strict-null-check-migration-tools into the `/tools/` folder of our repo, to make it easier for others to use them.

As part of the migration, I eliminated the separate `package.json` machinery for the tools and had them just re-use the same `node_modules` as the main project (in particular, re-using the correct typescript version). I also added entry point run-scripts for the tools to our root-level package.json, so the source for the tools should be an implementation detail for most purposes of actually running null check migration work; usage is just `yarn null:find` and `yarn null:autoadd` from the root, to match the previously-added `yarn null:check`.

I've updated #2869 with instructions for how to use these scripts to contribute to the strict null check migration work.

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: tools to help address #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
  • Loading branch information
dbjorge authored Jun 29, 2020
1 parent c5a05fc commit 74792d3
Show file tree
Hide file tree
Showing 7 changed files with 298 additions and 0 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
"format:fix": "prettier --config prettier.config.js --write \"**/*\"",
"lint:check": "tslint -c ./tslint.json -p ./tsconfig.json",
"lint:fix": "tslint --fix -c ./tslint.json -p ./tsconfig.json",
"null:autoadd": "node ./tools/strict-null-checks/auto-add.js",
"null:check": "tsc -p ./tsconfig.strictNullChecks.json",
"null:find": "node ./tools/strict-null-checks/find.js",
"react-devtools": "react-devtools",
"scss:build": "tsm \"src/**/*.scss\"",
"scss:clean": "grunt clean:scss",
Expand Down
24 changes: 24 additions & 0 deletions tools/strict-null-checks/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!--
Copyright (c) Microsoft Corporation. All rights reserved.
Licensed under the MIT License.
-->

Scripts to help migrate [accessibility-insights-web](https://github.com/microsoft/accessibility-insights-web) to use strict null checks.

Based closely on the VS Code team's [vscode-strict-null-check-migration-tools](https://github.com/mjbvz/vscode-strict-null-check-migration-tools). See their excellent [migration write-up](https://code.visualstudio.com/blogs/2019/05/23/strict-null)!

## Usage

Use the `package.json` run-scripts in the root-level `package.json`:

```bash
$ # Print out a markdown-checklist of files that would be good candidates to update for null-safety
$ yarn null:find

$ # For each file in the null:find list, automatically add it to tsconfig.strictNullChecks.json
$ # if-and-only-if doing so does not introduce any new "yarn null:check" violations.
$ yarn null:autoadd

$ # Keep this command running on the side while you're fixing up null:check issues
$ yarn null:check --watch
```
60 changes: 60 additions & 0 deletions tools/strict-null-checks/auto-add.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

// @ts-check
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 repoRoot = config.repoRoot;

const buildCompletePattern = /Found (\d+) errors?\. Watching for file changes\./gi;

forStrictNullCheckEligibleFiles(repoRoot, () => {}).then(async files => {
const tscPath = path.join(repoRoot, 'node_modules', 'typescript', 'bin', 'tsc');
const tsconfigPath = path.join(repoRoot, config.targetTsconfig);

const child = child_process.spawn('node', [tscPath, '-p', tsconfigPath, '--watch']);
for (const file of files) {
await tryAutoAddStrictNulls(child, tsconfigPath, file);
}
child.kill();
});

function tryAutoAddStrictNulls(child, tsconfigPath, file) {
return new Promise(resolve => {
const relativeFilePath = path.relative(repoRoot, file).replace(/\\/g, '/');
console.log(`Trying to auto add '${relativeFilePath}'`);

const originalConfig = JSON.parse(fs.readFileSync(tsconfigPath).toString());
originalConfig.files = Array.from(new Set(originalConfig.files.sort()));

// Config on accept
const newConfig = Object.assign({}, originalConfig);
newConfig.files = Array.from(
new Set(originalConfig.files.concat('./' + relativeFilePath).sort()),
);

const listener = data => {
const textOut = data.toString();
const match = buildCompletePattern.exec(textOut);
if (match) {
const errorCount = +match[1];
if (errorCount === 0) {
console.log(`Success`);
} else {
console.log(`Errors (x${errorCount}), skipped`);
fs.writeFileSync(tsconfigPath, JSON.stringify(originalConfig, null, ' '));
}

child.stdout.removeListener('data', listener);
resolve();
}
};
child.stdout.on('data', listener);

fs.writeFileSync(tsconfigPath, JSON.stringify(newConfig, null, ' '));
});
}
8 changes: 8 additions & 0 deletions tools/strict-null-checks/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

module.exports = {
repoRoot: `${__dirname}/../..`,
targetTsconfig: 'tsconfig.strictNullChecks.json',
skippedFiles: new Set([]),
};
104 changes: 104 additions & 0 deletions tools/strict-null-checks/eligible-file-finder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

// @ts-check
const path = require('path');
const { getImportsForFile } = require('./import-finder');
const glob = require('glob');
const config = require('./config');

/**
* @param {string} srcRoot
* @param {{ includeTests: boolean }} [options]
*/
const forEachFileInSrc = (srcRoot, options) => {
return new Promise((resolve, reject) => {
glob(`${srcRoot}/**/*.@(ts|tsx)`, (err, files) => {
if (err) {
return reject(err);
}

return resolve(
files.filter(
file =>
!file.endsWith('.d.ts') &&
(options && options.includeTests ? true : !/\.test\.tsx?$/.test(file)),
),
);
});
});
};
module.exports.forEachFileInSrc = forEachFileInSrc;

/**
* @param {string} repoRoot
* @param {(file: string) => void} forEach
* @param {{ includeTests: boolean }} [options]
*/
module.exports.forStrictNullCheckEligibleFiles = async (repoRoot, forEach, options) => {
const srcRoot = path.join(repoRoot, 'src');

const tsconfig = require(path.join(repoRoot, config.targetTsconfig));
const checkedFiles = await getCheckedFiles(tsconfig, repoRoot);

const imports = new Map();
const getMemoizedImportsForFile = (file, srcRoot) => {
if (imports.has(file)) {
return imports.get(file);
}
const importList = getImportsForFile(file, srcRoot);
imports.set(file, importList);
return importList;
};

const files = await forEachFileInSrc(srcRoot, options);

return files
.filter(file => !checkedFiles.has(file))
.filter(file => !config.skippedFiles.has(path.relative(srcRoot, file)))
.filter(file => {
const allProjImports = getMemoizedImportsForFile(file, srcRoot);

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 isEdge = nonCheckedImports.length === 0;
if (isEdge) {
forEach(file);
}
return isEdge;
});
};

async function getCheckedFiles(tsconfigContent, tsconfigDir) {
const set = new Set(
tsconfigContent.files.map(f => path.join(tsconfigDir, f).replace(/\\/g, '/')),
);
const includes = tsconfigContent.include.map(include => {
return new Promise((resolve, reject) => {
glob(path.join(tsconfigDir, include), (err, files) => {
if (err) {
return reject(err);
}

for (const file of files) {
set.add(file);
}
resolve();
});
});
});
await Promise.all(includes);
return set;
}
61 changes: 61 additions & 0 deletions tools/strict-null-checks/find.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

// @ts-check
const path = require('path');
const { repoRoot } = require('./config');
const { forStrictNullCheckEligibleFiles, forEachFileInSrc } = 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;
}

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)) {
if (eligibleSet.has(file)) {
// Already added
continue;
}

for (const imp of getImportsForFile(file, srcRoot)) {
if (dependedOnCount.has(imp)) {
dependedOnCount.set(imp, dependedOnCount.get(imp) + 1);
}
}
}

let out = Array.from(dependedOnCount.entries());
if (filter) {
out = out.filter(x => filter(x[0]));
}
if (sort) {
out = out.sort((a, b) => b[1] - a[1]);
}
for (const pair of out) {
console.log(
toFormattedFilePath(pair[0]) +
(printDependedOnCount ? ` — Depended on by **${pair[1]}** files` : ''),
);
}
});

function toFormattedFilePath(file) {
// return `"./${path.relative(srcRoot, file)}",`;
return `- [ ] \`"./${path.relative(srcRoot, file)}"\``;
}
39 changes: 39 additions & 0 deletions tools/strict-null-checks/import-finder.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

// @ts-check
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());
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
.map(fileName => {
if (/(^\.\/)|(^\.\.\/)/.test(fileName)) {
return path.join(path.dirname(file), fileName);
}
return path.join(srcRoot, fileName);
})
.map(fileName => {
for (const ext of ['ts', 'tsx', 'js', 'jsx', 'd.ts']) {
const candidate = `${fileName}.${ext}`;
if (fs.existsSync(candidate)) {
return candidate;
}
}

for (const indexFile of ['index.ts', 'index.js']) {
const candidate = path.join(fileName, indexFile);
if (fs.existsSync(candidate)) {
return candidate;
}
}

throw new Error(`Unresolved import ${fileName} in ${file}`);
});
};

0 comments on commit 74792d3

Please sign in to comment.