Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow user to provide TS program instance in parser options #3481

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/parser/README.md
Expand Up @@ -64,6 +64,8 @@ interface ParserOptions {
tsconfigRootDir?: string;
extraFileExtensions?: string[];
warnOnUnsupportedTypeScriptVersion?: boolean;

program?: import('typescript').Program;
}
```

Expand Down Expand Up @@ -211,6 +213,14 @@ Default `false`.

This option allows you to request that when the `project` setting is specified, files will be allowed when not included in the projects defined by the provided `tsconfig.json` files. **Using this option will incur significant performance costs. This option is primarily included for backwards-compatibility.** See the **`project`** section above for more information.

### `parserOptions.program`

Default `undefined`.

This option allows you to programmatically provide an instance of a TypeScript Program object that will provide type information to rules.
This will override any program that would have been computed from `parserOptions.project` or `parserOptions.createDefaultProgram`.
All linted files must be part of the provided program.

## Supported TypeScript Version

Please see [`typescript-eslint`](https://github.com/typescript-eslint/typescript-eslint) for the supported TypeScript version.
Expand Down
13 changes: 8 additions & 5 deletions packages/parser/tests/lib/services.ts
Expand Up @@ -4,6 +4,7 @@ import glob from 'glob';
import { ParserOptions } from '../../src/parser';
import {
createSnapshotTestBlock,
createTSProgram,
formatSnapshotName,
testServices,
} from '../tools/test-utils';
Expand All @@ -30,15 +31,17 @@ function createConfig(filename: string): ParserOptions {
//------------------------------------------------------------------------------

describe('services', () => {
const program = createTSProgram(path.resolve(FIXTURES_DIR, 'tsconfig.json'));
testFiles.forEach(filename => {
const code = fs.readFileSync(path.join(FIXTURES_DIR, filename), 'utf8');
const config = createConfig(filename);
it(
formatSnapshotName(filename, FIXTURES_DIR, '.ts'),
createSnapshotTestBlock(code, config),
);
it(`${formatSnapshotName(filename, FIXTURES_DIR, '.ts')} services`, () => {
const snapshotName = formatSnapshotName(filename, FIXTURES_DIR, '.ts');
it(snapshotName, createSnapshotTestBlock(code, config));
it(`${snapshotName} services`, () => {
testServices(code, config);
});
it(`${snapshotName} services with provided program`, () => {
testServices(code, { ...config, program });
});
});
});
26 changes: 26 additions & 0 deletions packages/parser/tests/tools/test-utils.ts
@@ -1,4 +1,7 @@
import { TSESTree } from '@typescript-eslint/typescript-estree';
import * as fs from 'fs';
import * as path from 'path';
import * as ts from 'typescript';
import * as parser from '../../src/parser';
import { ParserOptions } from '../../src/parser';

Expand Down Expand Up @@ -89,3 +92,26 @@ export function formatSnapshotName(
.replace(fixturesDir + '/', '')
.replace(fileExtension, '')}`;
}

export function createTSProgram(configFile: string): ts.Program {
const projectDirectory = path.dirname(configFile);
const config = ts.readConfigFile(configFile, ts.sys.readFile);
expect(config.error).toBeUndefined();
const parseConfigHost: ts.ParseConfigHost = {
fileExists: fs.existsSync,
readDirectory: ts.sys.readDirectory,
readFile: file => fs.readFileSync(file, 'utf8'),
useCaseSensitiveFileNames: true,
};
const parsed = ts.parseJsonConfigFileContent(
config.config,
parseConfigHost,
path.resolve(projectDirectory),
{ noEmit: true },
);
expect(parsed.errors).toHaveLength(0);
const host = ts.createCompilerHost(parsed.options, true);
const program = ts.createProgram(parsed.fileNames, parsed.options, host);

return program;
}
3 changes: 3 additions & 0 deletions packages/types/package.json
Expand Up @@ -48,5 +48,8 @@
"_ts3.4/*"
]
}
},
"devDependencies": {
"typescript": ">=2.6.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use the root package.json to control the TS version installed in the repo.

You can use the peerDependenciesMeta to add an optional peer dep on typescript, which we'll need to do to ensure package managers can understand the new requirement

Suggested change
"typescript": ">=2.6.0"
"typescript": "*"

}
}
2 changes: 2 additions & 0 deletions packages/types/src/parser-options.ts
@@ -1,4 +1,5 @@
import { Lib } from './lib';
import { Program } from 'typescript';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets ensure that this isn't ever pulled in as a runtime dep on this package.

Suggested change
import { Program } from 'typescript';
import type { Program } from 'typescript';


type DebugLevel = boolean | ('typescript-eslint' | 'eslint' | 'typescript')[];

Expand Down Expand Up @@ -41,6 +42,7 @@ interface ParserOptions {
extraFileExtensions?: string[];
filePath?: string;
loc?: boolean;
program?: Program;
project?: string | string[];
projectFolderIgnoreList?: (string | RegExp)[];
range?: boolean;
Expand Down
7 changes: 7 additions & 0 deletions packages/typescript-estree/README.md
Expand Up @@ -208,6 +208,13 @@ interface ParseAndGenerateServicesOptions extends ParseOptions {
*/
tsconfigRootDir?: string;

/**
* Instance of a TypeScript Program object to be used for type information.
* This overrides any program or programs that would have been computed from the `project` option.
* All linted files must be part of the provided program.
*/
program?: import('typescript').Program;

/**
***************************************************************************************
* IT IS RECOMMENDED THAT YOU DO NOT USE THIS OPTION, AS IT CAUSES PERFORMANCE ISSUES. *
Expand Down
3 changes: 3 additions & 0 deletions packages/typescript-estree/package.json
Expand Up @@ -64,6 +64,9 @@
"jest-specific-snapshot": "*",
"make-dir": "*",
"tmp": "^0.2.1",
"typescript": "^4.3.2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep this as *
We use the root package.json to control the TS version installed in the repo.

Suggested change
"typescript": "^4.3.2"
"typescript": "*"

},
"peerDependencies": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an implicit peer dependency via the peerDependenciesMeta property.

The reason we don't explicitly declare the peer dependency is because on older versions of yarn and npm, they do not understand peerDependenciesMeta.
So if we add an explicit peer dep - then they will log warnings at install time.

In the past this annoyed many users 😅 that had our packages installed (but not used) transitively.

TL;DR - remove the explicit peer dep

"typescript": "*"
},
"peerDependenciesMeta": {
Expand Down
Expand Up @@ -3,19 +3,12 @@ import path from 'path';
import { getProgramsForProjects } from './createWatchProgram';
import { firstDefined } from '../node-utils';
import { Extra } from '../parser-options';
import { ASTAndProgram } from './shared';
import { ASTAndProgram, getAstFromProgram } from './shared';

const log = debug('typescript-eslint:typescript-estree:createProjectProgram');

const DEFAULT_EXTRA_FILE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'];

function getExtension(fileName: string | undefined): string | null {
if (!fileName) {
return null;
}
return fileName.endsWith('.d.ts') ? '.d.ts' : path.extname(fileName);
}

/**
* @param code The code of the file being linted
* @param createDefaultProgram True if the default program should be created
Expand All @@ -31,18 +24,7 @@ function createProjectProgram(

const astAndProgram = firstDefined(
getProgramsForProjects(code, extra.filePath, extra),
currentProgram => {
const ast = currentProgram.getSourceFile(extra.filePath);

// working around https://github.com/typescript-eslint/typescript-eslint/issues/1573
const expectedExt = getExtension(extra.filePath);
const returnedExt = getExtension(ast?.fileName);
if (expectedExt !== returnedExt) {
return;
}

return ast && { ast, program: currentProgram };
},
currentProgram => getAstFromProgram(currentProgram, extra),
);

if (!astAndProgram && !createDefaultProgram) {
Expand Down
25 changes: 25 additions & 0 deletions packages/typescript-estree/src/create-program/shared.ts
@@ -1,5 +1,6 @@
import path from 'path';
import * as ts from 'typescript';
import { Program } from 'typescript';
import { Extra } from '../parser-options';

interface ASTAndProgram {
Expand Down Expand Up @@ -93,6 +94,29 @@ function getScriptKind(
}
}

function getExtension(fileName: string | undefined): string | null {
if (!fileName) {
return null;
}
return fileName.endsWith('.d.ts') ? '.d.ts' : path.extname(fileName);
}

function getAstFromProgram(
currentProgram: Program,
extra: Extra,
): ASTAndProgram | undefined {
const ast = currentProgram.getSourceFile(extra.filePath);

// working around https://github.com/typescript-eslint/typescript-eslint/issues/1573
const expectedExt = getExtension(extra.filePath);
const returnedExt = getExtension(ast?.fileName);
if (expectedExt !== returnedExt) {
return undefined;
}

return ast && { ast, program: currentProgram };
}

export {
ASTAndProgram,
canonicalDirname,
Expand All @@ -101,4 +125,5 @@ export {
ensureAbsolutePath,
getCanonicalFileName,
getScriptKind,
getAstFromProgram,
};
@@ -0,0 +1,28 @@
import debug from 'debug';
import { Program } from 'typescript';
import { Extra } from '../parser-options';
import { ASTAndProgram, getAstFromProgram } from './shared';

const log = debug('typescript-eslint:typescript-estree:useProvidedProgram');

function useProvidedProgram(
programInstance: Program,
extra: Extra,
): ASTAndProgram | undefined {
log('Retrieving ast for %s from provided program instance', extra.filePath);

const astAndProgram = getAstFromProgram(programInstance, extra);

if (!astAndProgram) {
const errorLines = [
'"parserOptions.program" has been provided for @typescript-eslint/parser.',
`The file was not found in the provided program instance: ${extra.filePath}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filepath should be made relative.
This ensures that error messages are kept is stable and portable.

`The file does not match your project config: ${path.relative(
extra.tsconfigRootDir || process.cwd(),
extra.filePath,
)}.`,

];

throw new Error(errorLines.join('\n'));
}

return astAndProgram;
}

export { useProvidedProgram };
8 changes: 8 additions & 0 deletions packages/typescript-estree/src/parser-options.ts
Expand Up @@ -20,6 +20,7 @@ export interface Extra {
loc: boolean;
log: (message: string) => void;
preserveNodeMaps?: boolean;
program: null | Program;
projects: CanonicalPath[];
range: boolean;
strict: boolean;
Expand Down Expand Up @@ -169,6 +170,13 @@ interface ParseAndGenerateServicesOptions extends ParseOptions {
*/
tsconfigRootDir?: string;

/**
* Instance of a TypeScript Program object to be used for type information.
* This overrides any program or programs that would have been computed from the `project` option.
* All linted files must be part of the provided program.
*/
program?: Program;

/**
***************************************************************************************
* IT IS RECOMMENDED THAT YOU DO NOT USE THIS OPTION, AS IT CAUSES PERFORMANCE ISSUES. *
Expand Down
48 changes: 32 additions & 16 deletions packages/typescript-estree/src/parser.ts
Expand Up @@ -18,6 +18,8 @@ import {
ensureAbsolutePath,
getCanonicalFileName,
} from './create-program/shared';
import { Program } from 'typescript';
import { useProvidedProgram } from './create-program/useProvidedProgram';

const log = debug('typescript-eslint:typescript-estree:parser');

Expand Down Expand Up @@ -61,10 +63,12 @@ function enforceString(code: unknown): string {
*/
function getProgramAndAST(
code: string,
programInstance: Program | null,
shouldProvideParserServices: boolean,
shouldCreateDefaultProgram: boolean,
): ASTAndProgram {
return (
(programInstance && useProvidedProgram(programInstance, extra)) ||
(shouldProvideParserServices &&
createProjectProgram(code, shouldCreateDefaultProgram, extra)) ||
(shouldProvideParserServices &&
Expand Down Expand Up @@ -105,6 +109,7 @@ function resetExtra(): void {
loc: false,
log: console.log, // eslint-disable-line no-console
preserveNodeMaps: true,
program: null,
projects: [],
range: false,
strict: false,
Expand Down Expand Up @@ -264,22 +269,32 @@ function applyParserOptionsToExtra(options: TSESTreeOptions): void {
// NOTE - ensureAbsolutePath relies upon having the correct tsconfigRootDir in extra
extra.filePath = ensureAbsolutePath(extra.filePath, extra);

const projectFolderIgnoreList = (
options.projectFolderIgnoreList ?? ['**/node_modules/**']
)
.reduce<string[]>((acc, folder) => {
if (typeof folder === 'string') {
acc.push(folder);
}
return acc;
}, [])
// prefix with a ! for not match glob
.map(folder => (folder.startsWith('!') ? folder : `!${folder}`));
// NOTE - prepareAndTransformProjects relies upon having the correct tsconfigRootDir in extra
extra.projects = prepareAndTransformProjects(
options.project,
projectFolderIgnoreList,
);
if (typeof options.program === 'object') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - we don't want to log if the user explicitly passes null here.

Suggested change
if (typeof options.program === 'object') {
if (typeof options.program === 'object' && options.program != null) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this if (options.program && typeof options.program === 'object')

extra.program = options.program;
log(
'parserOptions.program was provided, so parserOptions.project will be ignored.',
);
}

if (extra.program === null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - just for safety so we can handle the undefined case as well, in case something somewhere is wacky.

Suggested change
if (extra.program === null) {
if (extra.program == null) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this if (!extra.program)

// providing a program overrides project resolution
const projectFolderIgnoreList = (
options.projectFolderIgnoreList ?? ['**/node_modules/**']
)
.reduce<string[]>((acc, folder) => {
if (typeof folder === 'string') {
acc.push(folder);
}
return acc;
}, [])
// prefix with a ! for not match glob
.map(folder => (folder.startsWith('!') ? folder : `!${folder}`));
// NOTE - prepareAndTransformProjects relies upon having the correct tsconfigRootDir in extra
extra.projects = prepareAndTransformProjects(
options.project,
projectFolderIgnoreList,
);
}

if (
Array.isArray(options.extraFileExtensions) &&
Expand Down Expand Up @@ -453,6 +468,7 @@ function parseAndGenerateServices<T extends TSESTreeOptions = TSESTreeOptions>(
extra.projects && extra.projects.length > 0;
const { ast, program } = getProgramAndAST(
code,
extra.program,
shouldProvideParserServices,
extra.createDefaultProgram,
)!;
Expand Down