Skip to content

Commit

Permalink
feat: always return parserServices, add getParserServices from utils
Browse files Browse the repository at this point in the history
BREAKING: previously plugins expected that `parserServices` would be `undefined` if full type information wasn't available.
  • Loading branch information
bradzacher committed Jul 18, 2019
1 parent 66f9741 commit 01de581
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 138 deletions.
17 changes: 2 additions & 15 deletions packages/eslint-plugin-tslint/src/rules/config.ts
@@ -1,7 +1,4 @@
import {
ESLintUtils,
ParserServices,
} from '@typescript-eslint/experimental-utils';
import { ESLintUtils } from '@typescript-eslint/experimental-utils';
import memoize from 'lodash.memoize';
import { Configuration, RuleSeverity } from 'tslint';
import { CustomLinter } from '../custom-linter';
Expand Down Expand Up @@ -99,17 +96,7 @@ export default createRule<Options, MessageIds>({
create(context) {
const fileName = context.getFilename();
const sourceCode = context.getSourceCode().text;
const parserServices: ParserServices | undefined = context.parserServices;

/**
* The user needs to have configured "project" in their parserOptions
* for @typescript-eslint/parser
*/
if (!parserServices || !parserServices.program) {
throw new Error(
`You must provide a value for the "parserOptions.project" property for @typescript-eslint/parser`,
);
}
const parserServices = ESLintUtils.getParserServices(context);

/**
* The TSLint rules configuration passed in by the user
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-tslint/tests/index.spec.ts
Expand Up @@ -148,7 +148,7 @@ describe('tslint/error', () => {
linter.defineRule('tslint/config', rule);

expect(() => linter.verify(code, config)).toThrow(
`You must provide a value for the "parserOptions.project" property for @typescript-eslint/parser`,
'You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.',
);
}

Expand Down
33 changes: 0 additions & 33 deletions packages/eslint-plugin/src/util/getParserServices.ts

This file was deleted.

10 changes: 7 additions & 3 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -2,10 +2,14 @@ import { ESLintUtils } from '@typescript-eslint/experimental-utils';

export * from './astUtils';
export * from './createRule';
export * from './getParserServices';
export * from './misc';
export * from './types';

// this is done for convenience - saves migrating all of the old rules
const { applyDefault, deepMerge, isObjectNotArray } = ESLintUtils;
export { applyDefault, deepMerge, isObjectNotArray };
const {
applyDefault,
deepMerge,
isObjectNotArray,
getParserServices,
} = ESLintUtils;
export { applyDefault, deepMerge, isObjectNotArray, getParserServices };
Expand Up @@ -104,8 +104,11 @@ function validateTableRules(
// not perfect but should be good enough
const ruleFileContents = fs.readFileSync(
path.resolve(__dirname, `../../src/rules/${ruleName}.ts`),
'utf-8',
);
const usesTypeInformation = /getParserServices\(([^,]+\)|.+?, false\))/.test(
ruleFileContents,
);
const usesTypeInformation = ruleFileContents.includes('getParserServices');
validateTableBoolean(
usesTypeInformation,
rowNeedsTypeInfo,
Expand Down
40 changes: 40 additions & 0 deletions packages/experimental-utils/src/eslint-utils/getParserServices.ts
@@ -0,0 +1,40 @@
import { ParserServices, TSESLint } from '../';

const ERROR_MESSAGE =
'You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.';

/**
* Try to retrieve typescript parser service from context
*/
export function getParserServices<
TMessageIds extends string,
TOptions extends readonly any[]
>(
context: TSESLint.RuleContext<TMessageIds, TOptions>,
allowWithoutFullTypeInformation: boolean = false,
): ParserServices {
// backwards compatability check
// old versions of the parser would not return any parserServices unless parserOptions.project was set
if (
!context.parserServices ||
!context.parserServices.program ||
!context.parserServices.esTreeNodeToTSNodeMap ||
!context.parserServices.tsNodeToESTreeNodeMap
) {
throw new Error(ERROR_MESSAGE);
}

const hasFullTypeInformation =
typeof context.parserServices.hasFullTypeInformation === 'boolean'
? context.parserServices.hasFullTypeInformation
: // backwards compatible
true;

// if a rule requries full type information, then hard fail if it doesn't exist
// this forces the user to supply parserOptions.project
if (!hasFullTypeInformation && !allowWithoutFullTypeInformation) {
throw new Error(ERROR_MESSAGE);
}

return context.parserServices;
}
1 change: 1 addition & 0 deletions packages/experimental-utils/src/eslint-utils/index.ts
@@ -1,4 +1,5 @@
export * from './applyDefault';
export * from './batchedSingleLineTests';
export * from './getParserServices';
export * from './RuleCreator';
export * from './deepMerge';
2 changes: 1 addition & 1 deletion packages/experimental-utils/src/ts-eslint/Rule.ts
Expand Up @@ -169,7 +169,7 @@ interface RuleContext<
/**
* An object containing parser-provided services for rules
*/
parserServices?: ParserServices;
parserServices: ParserServices;

/**
* Returns an array of the ancestors of the currently-traversed node, starting at
Expand Down
2 changes: 1 addition & 1 deletion packages/typescript-estree/src/ast-converter.ts
Expand Up @@ -42,7 +42,7 @@ export default function astConverter(
estree.comments = convertComments(ast, extra.code);
}

const astMaps = shouldPreserveNodeMaps ? instance.getASTMaps() : undefined;
const astMaps = instance.getASTMaps();

return { estree, astMaps };
}
2 changes: 1 addition & 1 deletion packages/typescript-estree/src/convert.ts
Expand Up @@ -58,7 +58,7 @@ export class Converter {
*/
constructor(ast: ts.SourceFile, options: ConverterOptions) {
this.ast = ast;
this.options = options;
this.options = { ...options };
}

getASTMaps() {
Expand Down
7 changes: 4 additions & 3 deletions packages/typescript-estree/src/parser-options.ts
Expand Up @@ -45,7 +45,8 @@ export interface ParserWeakMap<TKey, TValueBase> {
}

export interface ParserServices {
program: Program | undefined;
esTreeNodeToTSNodeMap: ParserWeakMap<TSESTree.Node, TSNode> | undefined;
tsNodeToESTreeNodeMap: ParserWeakMap<TSNode, TSESTree.Node> | undefined;
hasFullTypeInformation: boolean;
program: Program;
esTreeNodeToTSNodeMap: ParserWeakMap<TSESTree.Node, TSNode>;
tsNodeToESTreeNodeMap: ParserWeakMap<TSNode, TSESTree.Node>;
}
42 changes: 17 additions & 25 deletions packages/typescript-estree/src/parser.ts
Expand Up @@ -9,6 +9,7 @@ import { TSESTree } from './ts-estree';
import {
calculateProjectParserOptions,
createProgram,
unusedVarsOptions,
} from './tsconfig-parser';

/**
Expand Down Expand Up @@ -57,7 +58,7 @@ function resetExtra(): void {
code: '',
tsconfigRootDir: process.cwd(),
extraFileExtensions: [],
preserveNodeMaps: undefined,
preserveNodeMaps: true,
};
}

Expand Down Expand Up @@ -90,8 +91,11 @@ function getASTFromProject(code: string, options: TSESTreeOptions) {
function getASTAndDefaultProject(code: string, options: TSESTreeOptions) {
const fileName = options.filePath || getFileName(options);
const program = createProgram(code, fileName, extra);
const ast = program && program.getSourceFile(fileName);
return ast && { ast, program };
if (program) {
const ast = program.getSourceFile(fileName);
return ast && { ast, program };
}
return null;
}

/**
Expand Down Expand Up @@ -142,6 +146,7 @@ function createNewProgram(code: string) {
noResolve: true,
target: ts.ScriptTarget.Latest,
jsx: extra.jsx ? ts.JsxEmit.Preserve : undefined,
...unusedVarsOptions,
},
compilerHost,
);
Expand Down Expand Up @@ -245,13 +250,10 @@ function applyParserOptionsToExtra(options: TSESTreeOptions): void {
/**
* Allow the user to enable or disable the preservation of the AST node maps
* during the conversion process.
*
* NOTE: For backwards compatibility we also preserve node maps in the case where `project` is set,
* and `preserveNodeMaps` is not explicitly set to anything.
*/
extra.preserveNodeMaps =
typeof options.preserveNodeMaps === 'boolean' && options.preserveNodeMaps;
if (options.preserveNodeMaps === undefined && extra.projects.length > 0) {
if (options.preserveNodeMaps === undefined) {
extra.preserveNodeMaps = true;
}
}
Expand Down Expand Up @@ -291,6 +293,7 @@ export interface ParseAndGenerateServicesResult<T extends TSESTreeOptions> {
// Public
//------------------------------------------------------------------------------

// note - cannot migrate this to an import statement because it will make TSC copy the package.json to the dist folder
export const version: string = require('../package.json').version;

export function parse<T extends TSESTreeOptions = TSESTreeOptions>(
Expand Down Expand Up @@ -385,19 +388,13 @@ export function parseAndGenerateServices<
options,
shouldProvideParserServices,
);
/**
* Determine whether or not two-way maps of converted AST nodes should be preserved
* during the conversion process
*/
const shouldPreserveNodeMaps =
extra.preserveNodeMaps !== undefined
? extra.preserveNodeMaps
: shouldProvideParserServices;
/**
* Convert the TypeScript AST to an ESTree-compatible one, and optionally preserve
* mappings between converted and original AST nodes
*/
const { estree, astMaps } = convert(ast, extra, shouldPreserveNodeMaps);
const preserveNodeMaps =
typeof extra.preserveNodeMaps === 'boolean' ? extra.preserveNodeMaps : true;
const { estree, astMaps } = convert(ast, extra, preserveNodeMaps);
/**
* Even if TypeScript parsed the source code ok, and we had no problems converting the AST,
* there may be other syntactic or semantic issues in the code that we can optionally report on.
Expand All @@ -414,15 +411,10 @@ export function parseAndGenerateServices<
return {
ast: estree as AST<T>,
services: {
program: shouldProvideParserServices ? program : undefined,
esTreeNodeToTSNodeMap:
shouldPreserveNodeMaps && astMaps
? astMaps.esTreeNodeToTSNodeMap
: undefined,
tsNodeToESTreeNodeMap:
shouldPreserveNodeMaps && astMaps
? astMaps.tsNodeToESTreeNodeMap
: undefined,
hasFullTypeInformation: shouldProvideParserServices,
program,
esTreeNodeToTSNodeMap: astMaps.esTreeNodeToTSNodeMap,
tsNodeToESTreeNodeMap: astMaps.tsNodeToESTreeNodeMap,
},
};
}
Expand Down
21 changes: 13 additions & 8 deletions packages/typescript-estree/src/tsconfig-parser.ts
Expand Up @@ -6,12 +6,10 @@ import { Extra } from './parser-options';
// Environment calculation
//------------------------------------------------------------------------------

/**
* Default compiler options for program generation from single root file
*/
const defaultCompilerOptions: ts.CompilerOptions = {
allowNonTsExtensions: true,
allowJs: true,
// these flags are required to make no-unused-vars work
export const unusedVarsOptions = {
noUnusedLocals: true,
noUnusedParameters: true,
};

/**
Expand Down Expand Up @@ -99,7 +97,10 @@ export function calculateProjectParserOptions(
// create compiler host
const watchCompilerHost = ts.createWatchCompilerHost(
tsconfigPath,
/*optionsToExtend*/ { allowNonTsExtensions: true } as ts.CompilerOptions,
/*optionsToExtend*/ {
allowNonTsExtensions: true,
...unusedVarsOptions,
} as ts.CompilerOptions,
ts.sys,
ts.createSemanticDiagnosticsBuilderProgram,
diagnosticReporter,
Expand Down Expand Up @@ -202,7 +203,11 @@ export function createProgram(code: string, filePath: string, extra: Extra) {

const commandLine = ts.getParsedCommandLineOfConfigFile(
tsconfigPath,
defaultCompilerOptions,
{
allowNonTsExtensions: true,
allowJs: true,
...unusedVarsOptions,
},
{ ...ts.sys, onUnRecoverableConfigFileDiagnostic: () => {} },
);

Expand Down
12 changes: 6 additions & 6 deletions packages/typescript-estree/tests/lib/convert.ts
Expand Up @@ -98,8 +98,8 @@ describe('convert', () => {
instance.convertProgram();
const maps = instance.getASTMaps();

function checkMaps(child: any) {
child.forEachChild((node: any) => {
function checkMaps(child: ts.SourceFile | ts.Node) {
child.forEachChild(node => {
if (
node.kind !== ts.SyntaxKind.EndOfFileToken &&
node.kind !== ts.SyntaxKind.JsxAttributes &&
Expand Down Expand Up @@ -132,8 +132,8 @@ describe('convert', () => {
instance.convertProgram();
const maps = instance.getASTMaps();

function checkMaps(child: any) {
child.forEachChild((node: any) => {
function checkMaps(child: ts.SourceFile | ts.Node) {
child.forEachChild(node => {
if (
node.kind !== ts.SyntaxKind.EndOfFileToken &&
node.kind !== ts.SyntaxKind.JsxAttributes
Expand Down Expand Up @@ -165,8 +165,8 @@ describe('convert', () => {
const program = instance.convertProgram();
const maps = instance.getASTMaps();

function checkMaps(child: any) {
child.forEachChild((node: any) => {
function checkMaps(child: ts.SourceFile | ts.Node) {
child.forEachChild(node => {
if (node.kind !== ts.SyntaxKind.EndOfFileToken) {
expect(ast).toBe(
maps.esTreeNodeToTSNodeMap.get(maps.tsNodeToESTreeNodeMap.get(ast)),
Expand Down

0 comments on commit 01de581

Please sign in to comment.