Skip to content

Commit

Permalink
chore(null): update yarn null:autoadd to run multiple passes (#3346)
Browse files Browse the repository at this point in the history
#### Description of changes

Previously, if `yarn null:autoadd` added a file to the strictNullChecks config which enabled a previously-ineligible file to be considered for auto-add, you'd have to run `yarn null:autoadd` again and eat the cost of it spinning up a new tsc watch process and rescanning all the already-known-failures.

With this PR, the tool will do this for you automatically, re-using both the watch process and previously-established knowledge of known failures.

`yarn null:progress` update with this PR:

## Web strict-null progress

**42%** complete (**558**/1334 non-test files)

*Contribute at [#2869](#2869). Last update: Fri Sep 11 2020*

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: part of #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 Sep 11, 2020
1 parent 559b6a5 commit 4c36095
Show file tree
Hide file tree
Showing 4 changed files with 189 additions and 57 deletions.
103 changes: 72 additions & 31 deletions tools/strict-null-checks/auto-add.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,92 @@ const repoRoot = config.repoRoot;
const tscPath = path.join(repoRoot, 'node_modules', 'typescript', 'bin', 'tsc');
const tsconfigPath = path.join(repoRoot, config.targetTsconfig);

const buildCompletePattern = /Found (\d+) errors?\. Watching for file changes\./gi;
async function main() {
console.log('## Initializing tsc --watch process...');
const tscWatchProcess = child_process.spawn('node', [tscPath, '-p', tsconfigPath, '--watch']);
await waitForBuildComplete(tscWatchProcess);

const alreadyAttempted = new Set();

for (let pass = 1; ; pass += 1) {
let successesThisPass = 0;
const uncheckedLeafFiles = await getUncheckedLeafFiles(repoRoot);
const candidateFiles = uncheckedLeafFiles.filter(f => !alreadyAttempted.has(f));
const candidateCount = candidateFiles.length;
console.log(`## Starting pass ${pass} with ${candidateCount} candidate 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);
for (const file of candidateFiles) {
const succeeded = await tryAutoAddStrictNulls(tscWatchProcess, tsconfigPath, file);
alreadyAttempted.add(file);
if (succeeded) {
successesThisPass += 1;
}
}

console.log(`### Finished pass ${pass} (added ${successesThisPass}/${candidateCount})`);
if (successesThisPass === 0) {
break;
}
}
child.kill();

console.log('Collapsing full completed directories into "includes" patterns...');
console.log('## Stopping tsc --watch process...');
tscWatchProcess.kill();

console.log('## Collapsing fully null-checked directories into "include" patterns...');
collapseCompletedDirectories(tsconfigPath);
});
}

function tryAutoAddStrictNulls(child, tsconfigPath, file) {
return new Promise(resolve => {
const relativeFilePath = path.relative(repoRoot, file).replace(/\\/g, '/');
console.log(`Trying to auto add '${relativeFilePath}'`);
async function tryAutoAddStrictNulls(child, tsconfigPath, file) {
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()));
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()),
);
// Config on accept
const newConfig = Object.assign({}, originalConfig);
newConfig.files = Array.from(
new Set(originalConfig.files.concat('./' + relativeFilePath).sort()),
);

const buildCompetePromise = waitForBuildComplete(child);

writeTsconfigSync(tsconfigPath, newConfig);

const errorCount = await buildCompetePromise;
const success = errorCount === 0;
if (success) {
console.log(`Success`);
} else {
console.log(`Errors (x${errorCount}), skipped`);
writeTsconfigSync(tsconfigPath, originalConfig);
}

return success;
}

const buildCompletePattern = /Found (\d+) errors?\. Watching for file changes\./gi;
async function waitForBuildComplete(tscWatchProcess) {
const match = await waitForStdoutMatching(tscWatchProcess, buildCompletePattern);
const errorCount = +match[1];
return errorCount;
}

async function waitForStdoutMatching(child, pattern) {
return new Promise(resolve => {
const listener = data => {
const textOut = data.toString();
const match = buildCompletePattern.exec(textOut);
const match = pattern.exec(textOut);
if (match) {
const errorCount = +match[1];
if (errorCount === 0) {
console.log(`Success`);
} else {
console.log(`Errors (x${errorCount}), skipped`);
writeTsconfigSync(tsconfigPath, originalConfig);
}

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

writeTsconfigSync(tsconfigPath, newConfig);
});
}

main().catch(error => {
console.error(error.stack);
process.exit(1);
});
16 changes: 4 additions & 12 deletions tools/strict-null-checks/eligible-file-finder.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// Licensed under the MIT License.

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

Expand Down Expand Up @@ -34,16 +35,6 @@ const getAllTsFiles = (srcRoot, options) => {
* @param {{ includeTests: boolean }} [options]
*/
async function getUncheckedLeafFiles(repoRoot, options) {
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 srcRoot = path.join(repoRoot, 'src');

const checkedFiles = await getCheckedFiles(repoRoot);
Expand All @@ -63,7 +54,8 @@ async function getUncheckedLeafFiles(repoRoot, options) {
}

async function getCheckedFiles(tsconfigDir) {
const tsconfigContent = require(path.join(tsconfigDir, config.targetTsconfig));
const tsconfigPath = path.join(tsconfigDir, config.targetTsconfig);
const tsconfigContent = JSON.parse(fs.readFileSync(tsconfigPath).toString());

const set = new Set(
tsconfigContent.files.map(f => path.join(tsconfigDir, f).replace(/\\/g, '/')),
Expand Down
17 changes: 16 additions & 1 deletion tools/strict-null-checks/import-finder.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@ const path = require('path');
const ts = require('typescript');
const fs = require('fs');

module.exports.getImportsForFile = function getImportsForFile(parentFilePath, srcRoot) {
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;
};

function getImportsForFile(parentFilePath, srcRoot) {
const fileInfo = ts.preProcessFile(fs.readFileSync(parentFilePath).toString());
return fileInfo.importedFiles
.map(importedFile => importedFile.fileName)
Expand Down Expand Up @@ -43,4 +53,9 @@ module.exports.getImportsForFile = function getImportsForFile(parentFilePath, sr
}
return filePath;
});
}

module.exports = {
getImportsForFile,
getMemoizedImportsForFile,
};
Loading

0 comments on commit 4c36095

Please sign in to comment.