Skip to content

Commit

Permalink
Introduce async SourceFileProvider interface (#97)
Browse files Browse the repository at this point in the history
* Introduce async SourceFileProvider interface

Adds a common interface that in future changes will be implemented
with a faster alternate that avoids scanning all files in the program
during initialization

As part of this work, I rework consumers of TypeScriptProgram to be
asyncronous, and add runWithConcurrentLimit to fence check in parallel.
This is needed because if we were to check all fences in parallel we
would hit the system RLIMIT by opening all fences simultaneously.

And while I'm in there, I added a progress bar to give user feedback
while performing fence checks. This does not display while performing
provider initialization, but this is less of a problem against the (yet
to come) newer implementation that doesn't perform full typescript
program initialization.

* Update runner.ts

* Update Options.ts

* Update getImportsFromFile.ts

* Prefer ?. for control flow in runWithConcurrentLimit

Co-authored-by: Maxwell Huang-Hobbs <mahuangh@microsoft.com>
Co-authored-by: Scott Mikula <mikula@gmail.com>
  • Loading branch information
3 people authored Aug 11, 2021
1 parent 8e7799d commit eb572f4
Show file tree
Hide file tree
Showing 14 changed files with 169 additions and 49 deletions.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
},
"author": "Scott Mikula <mikula@gmail.com>",
"dependencies": {
"cli-progress": "^3.9.0",
"commander": "^7.2.0",
"minimatch": "^3.0.4",
"typescript": "^4.0.3"
},
"devDependencies": {
"@types/cli-progress": "^3.9.2",
"@types/commander": "^2.12.2",
"@types/jest": "^26.0.15",
"@types/node": "^12.7.8",
Expand Down
8 changes: 1 addition & 7 deletions src/core/ImportRecord.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
import NormalizedPath from '../types/NormalizedPath';
import TypeScriptProgram from './TypeScriptProgram';
import normalizePath from '../utils/normalizePath';
import * as path from 'path';
import getOptions from '../utils/getOptions';

export default class ImportRecord {
public filePath: NormalizedPath;

constructor(
public rawImport: string,
sourceFile: NormalizedPath,
tsProgram: TypeScriptProgram
) {
const resolvedFileName = tsProgram.resolveImportFromFile(rawImport, sourceFile);
constructor(public rawImport: string, resolvedFileName: string | undefined) {
if (resolvedFileName) {
this.filePath = normalizePath(resolvedFileName);
}
Expand Down
8 changes: 8 additions & 0 deletions src/core/SourceFileProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export interface SourceFileProvider {
getSourceFiles(searchRoots?: string[]): Promise<string[]> | string[];
getImportsForFile(filePath: string): Promise<string[]> | string[];
resolveImportFromFile(
importer: string,
importSpecifier: string
): Promise<string | undefined> | string | undefined;
}
11 changes: 6 additions & 5 deletions src/core/TypeScriptProgram.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import * as path from 'path';
import * as ts from 'typescript';
import * as path from 'path';
import NormalizedPath from '../types/NormalizedPath';
import { SourceFileProvider } from './SourceFileProvider';

// Helper class for interacting with TypeScript
export default class TypeScriptProgram {
export default class TypeScriptProgram implements SourceFileProvider {
private compilerOptions: ts.CompilerOptions;
private compilerHost: ts.CompilerHost;
private program: ts.Program;

constructor(configFile: NormalizedPath) {
// Parse the config file
const projectPath = path.dirname(configFile);
const config = readConfigFile(configFile);
const projectPath = path.dirname(configFile);
const parsedConfig = ts.parseJsonConfigFileContent(config, ts.sys, projectPath);
this.compilerOptions = parsedConfig.options;

Expand All @@ -35,11 +36,11 @@ export default class TypeScriptProgram {
// Get all imports from a given file
getImportsForFile(fileName: NormalizedPath) {
let fileInfo = ts.preProcessFile(ts.sys.readFile(fileName), true, true);
return fileInfo.importedFiles;
return fileInfo.importedFiles.map(importedFile => importedFile.fileName);
}

// Resolve an imported module
resolveImportFromFile(moduleName: string, containingFile: NormalizedPath) {
resolveImportFromFile(containingFile: NormalizedPath, moduleName: string) {
const resolvedFile = ts.resolveModuleName(
moduleName,
containingFile.replace(/\\/g, '/'), // TypeScript doesn't like backslashes here
Expand Down
50 changes: 31 additions & 19 deletions src/core/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,40 @@ import * as commander from 'commander';
import RawOptions from '../types/RawOptions';
import { run } from './runner';

// Read the package version from package.json
const packageVersion = require('../../package.json').version;
async function main() {
// Read the package version from package.json
const packageVersion = require('../../package.json').version;

// Parse command line options
const program = commander
.version(packageVersion)
.option('-p, --project <string> ', 'tsconfig.json file')
.option('-r, --rootDir <string...>', 'root directories of the project');
program.parse(process.argv);
const options = program.opts() as RawOptions;
// Parse command line options
const program = commander
.version(packageVersion)
.option('-p, --project <string> ', 'tsconfig.json file')
.option('-r, --rootDir <string...>', 'root directories of the project')
.option(
'-j, --maxConcurrentFenceJobs',
'Maximum number of concurrent fence jobs to run. Default 6000'
)
.option('-b, --progressBar', 'Show a progress bar while evaluating fences');
program.parse(process.argv);
const options = program.opts() as RawOptions;

// Run good-fences
const result = run(options);
// Run good-fences
const result = await run(options);

// Write results to the console
for (const error of result.errors) {
console.error(error.detailedMessage);
}
// Write results to the console
for (const error of result.errors) {
console.error(error.detailedMessage);
}

for (const warning of result.warnings) {
console.error(warning.detailedMessage);
}

for (const warning of result.warnings) {
console.error(warning.detailedMessage);
// Indicate success or failure via the exit code
process.exitCode = result.errors.length > 0 ? 1 : 0;
}

// Indicate success or failure via the exit code
process.exitCode = result.errors.length > 0 ? 1 : 0;
main().catch(e => {
console.error('Error while running fences:', e.stack);
process.exit(1);
});
34 changes: 27 additions & 7 deletions src/core/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,40 @@ import TypeScriptProgram from './TypeScriptProgram';
import normalizePath from '../utils/normalizePath';
import { getResult } from './result';
import { validateTagsExist } from '../validation/validateTagsExist';
import { SourceFileProvider } from './SourceFileProvider';
import NormalizedPath from '../types/NormalizedPath';
import { runWithConcurrentLimit } from '../utils/runWithConcurrentLimit';

export function run(rawOptions: RawOptions) {
async function getSourceFilesNormalized(
sourceFileProvider: SourceFileProvider,
rootDirs?: string[]
): Promise<NormalizedPath[]> {
let files = await sourceFileProvider.getSourceFiles(rootDirs);
const normalizedFiles = files.map(file => normalizePath(file));
return normalizedFiles;
}

export async function run(rawOptions: RawOptions) {
// Store options so they can be globally available
setOptions(rawOptions);
let options = getOptions();

let sourceFileProvider: SourceFileProvider = new TypeScriptProgram(options.project);

// Do some sanity checks on the fences
validateTagsExist();

// Run validation
let tsProgram = new TypeScriptProgram(getOptions().project);
let files = tsProgram.getSourceFiles();
files.forEach(file => {
validateFile(normalizePath(file), tsProgram);
});
const normalizedFiles = await getSourceFilesNormalized(sourceFileProvider);

// Limit the concurrent executed promises because
// otherwise we will open all the files at the same time and
// hit the MFILE error (when we hit rlimit)
await runWithConcurrentLimit(
options.maxConcurrentFenceJobs,
normalizedFiles,
(normalizedFile: NormalizedPath) => validateFile(normalizedFile, sourceFileProvider),
options.progress
);

return getResult();
}
9 changes: 9 additions & 0 deletions src/types/Options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,13 @@ export default interface Options {
project: NormalizedPath;
rootDir: NormalizedPath[];
ignoreExternalFences: boolean;

// Maximum number of fence validation jobs that can
// be run at the same time.
//
// This should be set under the system rlimit,
// otherwise you will hit the MFILE error when
// we try to open too many files concurrently.
maxConcurrentFenceJobs: number;
progress: boolean;
}
2 changes: 2 additions & 0 deletions src/types/RawOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ export default interface RawOptions {
project?: string;
rootDir?: string | string[];
ignoreExternalFences?: boolean;
maxConcurrentJobs?: number;
progressBar?: boolean;
}
23 changes: 17 additions & 6 deletions src/utils/getImportsFromFile.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
import TypeScriptProgram from '../core/TypeScriptProgram';
import NormalizedPath from '../types/NormalizedPath';
import ImportRecord from '../core/ImportRecord';
import { SourceFileProvider } from '../core/SourceFileProvider';

export default function getImportsFromFile(filePath: NormalizedPath, tsProgram: TypeScriptProgram) {
const importedFiles = tsProgram.getImportsForFile(filePath);
return importedFiles
.map(importInfo => new ImportRecord(importInfo.fileName, filePath, tsProgram))
.filter(importRecord => importRecord.filePath);
export default async function getImportsFromFile(
sourceFilePath: NormalizedPath,
sourceFileProvider: SourceFileProvider
) {
const rawImports = await sourceFileProvider.getImportsForFile(sourceFilePath);
const resolvedImports = await Promise.all(
rawImports.map(
async rawImport =>
new ImportRecord(
rawImport,
await sourceFileProvider.resolveImportFromFile(sourceFilePath, rawImport)
)
)
);

return resolvedImports.filter(importRecord => importRecord.filePath);
}
2 changes: 2 additions & 0 deletions src/utils/getOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,7 @@ export function setOptions(rawOptions: RawOptions) {
project,
rootDir,
ignoreExternalFences: rawOptions.ignoreExternalFences,
maxConcurrentFenceJobs: rawOptions.maxConcurrentJobs || 6000,
progress: rawOptions.progressBar || false,
};
}
35 changes: 35 additions & 0 deletions src/utils/runWithConcurrentLimit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import * as CliProgress from 'cli-progress';

export async function runWithConcurrentLimit<I>(
maxBatchSize: number,
inputs: I[],
cb: (input: I) => Promise<void>,
progress: boolean
): Promise<void> {
const i = [...inputs];
let completedJobs = 0;
const initialWorkingSet = i.splice(0, maxBatchSize);

let progressBar: CliProgress.SingleBar | undefined;
if (progress) {
progressBar = new CliProgress.SingleBar(
{
etaBuffer: maxBatchSize,
},
CliProgress.Presets.shades_grey
);
progressBar.start(inputs.length, 0);
}

const queueNext = (): Promise<void> | void => {
const next = i.shift();
completedJobs += 1;
progressBar?.update(completedJobs);
if (next) {
return cb(next).then(queueNext);
}
};
await Promise.all(initialWorkingSet.map(i => cb(i).then(queueNext)));

progressBar?.stop();
}
10 changes: 7 additions & 3 deletions src/validation/validateFile.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import NormalizedPath from '../types/NormalizedPath';
import TypeScriptProgram from '../core/TypeScriptProgram';
import validateExportRules from './validateExportRules';
import getImportsFromFile from '../utils/getImportsFromFile';
import validateDependencyRules from './validateDependencyRules';
import validateImportRules from './validateImportRules';
import { SourceFileProvider } from '../core/SourceFileProvider';

export default async function validateFile(
filePath: NormalizedPath,
fileProvider: SourceFileProvider
) {
const imports = await getImportsFromFile(filePath, fileProvider);

export default function validateFile(filePath: NormalizedPath, tsProgram: TypeScriptProgram) {
const imports = getImportsFromFile(filePath, tsProgram);
for (let importRecord of imports) {
validateExportRules(filePath, importRecord);

Expand Down
4 changes: 2 additions & 2 deletions test/endToEnd/endToEndTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import GoodFencesResult from '../../src/types/GoodFencesResult';
import normalizePath from '../../src/utils/normalizePath';

describe('runner', () => {
it('returns the expected results', () => {
it('returns the expected results', async () => {
// Arrange
const expectedResults = require('./endToEndTests.expected.json');

// Act
const actualResults = run({
const actualResults = await run({
rootDir: './sample',
});

Expand Down
20 changes: 20 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,13 @@
dependencies:
"@babel/types" "^7.3.0"

"@types/cli-progress@^3.9.2":
version "3.9.2"
resolved "https://registry.yarnpkg.com/@types/cli-progress/-/cli-progress-3.9.2.tgz#6ca355f96268af39bee9f9307f0ac96145639c26"
integrity sha512-VO5/X5Ij+oVgEVjg5u0IXVe3JQSKJX+Ev8C5x+0hPy0AuWyW+bF8tbajR7cPFnDGhs7pidztcac+ccrDtk5teA==
dependencies:
"@types/node" "*"

"@types/commander@^2.12.2":
version "2.12.2"
resolved "https://registry.yarnpkg.com/@types/commander/-/commander-2.12.2.tgz#183041a23842d4281478fa5d23c5ca78e6fd08ae"
Expand Down Expand Up @@ -1100,6 +1107,14 @@ class-utils@^0.3.5:
isobject "^3.0.0"
static-extend "^0.1.1"

cli-progress@^3.9.0:
version "3.9.0"
resolved "https://registry.yarnpkg.com/cli-progress/-/cli-progress-3.9.0.tgz#25db83447deb812e62d05bac1af9aec5387ef3d4"
integrity sha512-g7rLWfhAo/7pF+a/STFH/xPyosaL1zgADhI0OM83hl3c7S43iGvJWEAV2QuDOnQ8i6EMBj/u4+NTd0d5L+4JfA==
dependencies:
colors "^1.1.2"
string-width "^4.2.0"

cliui@^6.0.0:
version "6.0.0"
resolved "https://registry.yarnpkg.com/cliui/-/cliui-6.0.0.tgz#511d702c0c4e41ca156d7d0e96021f23e13225b1"
Expand Down Expand Up @@ -1151,6 +1166,11 @@ color-name@~1.1.4:
resolved "https://registry.yarnpkg.com/color-name/-/color-name-1.1.4.tgz#c2a09a87acbde69543de6f63fa3995c826c536a2"
integrity sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==

colors@^1.1.2:
version "1.4.0"
resolved "https://registry.yarnpkg.com/colors/-/colors-1.4.0.tgz#c50491479d4c1bdaed2c9ced32cf7c7dc2360f78"
integrity sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==

combined-stream@^1.0.6, combined-stream@~1.0.6:
version "1.0.8"
resolved "https://registry.yarnpkg.com/combined-stream/-/combined-stream-1.0.8.tgz#c3d45a8b34fd730631a110a8a2520682b31d5a7f"
Expand Down

0 comments on commit eb572f4

Please sign in to comment.