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(ts-estree): add preserveNodeMaps option #494

Merged
merged 2 commits into from May 5, 2019
Merged
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
14 changes: 13 additions & 1 deletion packages/typescript-estree/README.md
Expand Up @@ -66,7 +66,19 @@ Parses the given string of code with the options provided and returns an ESTree-
* When value is `false`, no logging will occur.
* When value is not provided, `console.log()` will be used.
*/
loggerFn: undefined
loggerFn: undefined,

/**
* Allows the user to control whether or not two-way AST node maps are preserved
* during the AST conversion process.
*
* By default: the AST node maps are NOT preserved, unless `project` has been specified,
* in which case the maps are made available on the returned `parserServices`.
*
* NOTE: If `preserveNodeMaps` is explicitly set by the user, it will be respected,
* regardless of whether or not `project` is in use.
*/
preserveNodeMaps: undefined
}
```

Expand Down
8 changes: 3 additions & 5 deletions packages/typescript-estree/src/ast-converter.ts
Expand Up @@ -7,7 +7,7 @@ import { Extra } from './parser-options';
export default function astConverter(
ast: ts.SourceFile,
extra: Extra,
shouldProvideParserServices: boolean,
shouldPreserveNodeMaps: boolean,
) {
/**
* The TypeScript compiler produced fundamental parse errors when parsing the
Expand All @@ -23,7 +23,7 @@ export default function astConverter(
const instance = new Converter(ast, {
errorOnUnknownASTType: extra.errorOnUnknownASTType || false,
useJSXTextNode: extra.useJSXTextNode || false,
shouldProvideParserServices,
shouldPreserveNodeMaps,
});

const estree = instance.convertProgram();
Expand All @@ -42,9 +42,7 @@ export default function astConverter(
estree.comments = convertComments(ast, extra.code);
}

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

return { estree, astMaps };
}
6 changes: 3 additions & 3 deletions packages/typescript-estree/src/convert.ts
Expand Up @@ -26,7 +26,7 @@ const SyntaxKind = ts.SyntaxKind;
interface ConverterOptions {
errorOnUnknownASTType: boolean;
useJSXTextNode: boolean;
shouldProvideParserServices: boolean;
shouldPreserveNodeMaps: boolean;
}

/**
Expand Down Expand Up @@ -168,7 +168,7 @@ export class Converter {
node: ts.Node,
result: TSESTree.BaseNode | null,
) {
if (result && this.options.shouldProvideParserServices) {
if (result && this.options.shouldPreserveNodeMaps) {
if (!this.tsNodeToESTreeNodeMap.has(node)) {
this.tsNodeToESTreeNodeMap.set(node, result);
}
Expand Down Expand Up @@ -217,7 +217,7 @@ export class Converter {
result.loc = getLocFor(result.range[0], result.range[1], this.ast);
}

if (result && this.options.shouldProvideParserServices) {
if (result && this.options.shouldPreserveNodeMaps) {
this.esTreeNodeToTSNodeMap.set(result, node);
}
return result as T;
Expand Down
2 changes: 2 additions & 0 deletions packages/typescript-estree/src/parser-options.ts
Expand Up @@ -18,6 +18,7 @@ export interface Extra {
projects: string[];
tsconfigRootDir: string;
extraFileExtensions: string[];
preserveNodeMaps?: boolean;
}

export interface ParserOptions {
Expand All @@ -34,6 +35,7 @@ export interface ParserOptions {
filePath?: string;
tsconfigRootDir?: string;
extraFileExtensions?: string[];
preserveNodeMaps?: boolean;
}

export interface ParserWeakMap<TKey, TValueBase> {
Expand Down
27 changes: 24 additions & 3 deletions packages/typescript-estree/src/parser.ts
Expand Up @@ -57,6 +57,7 @@ function resetExtra(): void {
code: '',
tsconfigRootDir: process.cwd(),
extraFileExtensions: [],
preserveNodeMaps: undefined,
};
}

Expand Down Expand Up @@ -241,6 +242,18 @@ function applyParserOptionsToExtra(options: ParserOptions): void {
) {
extra.extraFileExtensions = options.extraFileExtensions;
}
/**
* 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) {
extra.preserveNodeMaps = true;
}
}

function warnAboutTSVersion(): void {
Expand Down Expand Up @@ -372,11 +385,19 @@ 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, shouldProvideParserServices);
const { estree, astMaps } = convert(ast, extra, shouldPreserveNodeMaps);
/**
* 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 @@ -395,11 +416,11 @@ export function parseAndGenerateServices<
services: {
program: shouldProvideParserServices ? program : undefined,
esTreeNodeToTSNodeMap:
shouldProvideParserServices && astMaps
shouldPreserveNodeMaps && astMaps
? astMaps.esTreeNodeToTSNodeMap
: undefined,
tsNodeToESTreeNodeMap:
shouldProvideParserServices && astMaps
shouldPreserveNodeMaps && astMaps
? astMaps.tsNodeToESTreeNodeMap
: undefined,
},
Expand Down
18 changes: 9 additions & 9 deletions packages/typescript-estree/tests/lib/convert.ts
Expand Up @@ -18,7 +18,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect(instance.convertProgram()).toMatchSnapshot();
});
Expand All @@ -29,7 +29,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect((instance as any).deeplyCopy(ast.statements[0])).toMatchSnapshot();
});
Expand All @@ -40,7 +40,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect((instance as any).deeplyCopy(ast.statements[0])).toMatchSnapshot();
});
Expand All @@ -51,7 +51,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect(
(instance as any).deeplyCopy((ast.statements[0] as any).expression),
Expand All @@ -64,7 +64,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect((instance as any).deeplyCopy(ast)).toMatchSnapshot();
});
Expand All @@ -75,7 +75,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: true,
useJSXTextNode: false,
shouldProvideParserServices: false,
shouldPreserveNodeMaps: false,
});
expect(() => instance.convertProgram()).toThrow(
'Unknown AST_NODE_TYPE: "TSJSDocNullableType"',
Expand All @@ -93,7 +93,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: true,
shouldPreserveNodeMaps: true,
});
instance.convertProgram();
const maps = instance.getASTMaps();
Expand Down Expand Up @@ -127,7 +127,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: true,
shouldPreserveNodeMaps: true,
});
instance.convertProgram();
const maps = instance.getASTMaps();
Expand Down Expand Up @@ -160,7 +160,7 @@ describe('convert', () => {
const instance = new Converter(ast, {
errorOnUnknownASTType: false,
useJSXTextNode: false,
shouldProvideParserServices: true,
shouldPreserveNodeMaps: true,
});
const program = instance.convertProgram();
const maps = instance.getASTMaps();
Expand Down
101 changes: 101 additions & 0 deletions packages/typescript-estree/tests/lib/parse.ts
Expand Up @@ -88,6 +88,7 @@ describe('parse()', () => {
tokens: expect.any(Array),
tsconfigRootDir: expect.any(String),
useJSXTextNode: false,
preserveNodeMaps: false,
},
false,
);
Expand Down Expand Up @@ -126,4 +127,104 @@ describe('parse()', () => {
}).toThrow('Decorators are not valid here.');
});
});

describe('preserveNodeMaps', () => {
const code = 'var a = true';
const baseConfig: ParserOptions = {
comment: true,
tokens: true,
range: true,
loc: true,
};

it('should not impact the use of parse()', () => {
const resultWithNoOptionSet = parser.parse(code, baseConfig);
const resultWithOptionSetToTrue = parser.parse(code, {
...baseConfig,
preserveNodeMaps: true,
});
const resultWithOptionSetToFalse = parser.parse(code, {
...baseConfig,
preserveNodeMaps: false,
});
const resultWithOptionSetExplicitlyToUndefined = parser.parse(code, {
...baseConfig,
preserveNodeMaps: undefined,
});

expect(resultWithNoOptionSet).toMatchObject(resultWithOptionSetToTrue);
expect(resultWithNoOptionSet).toMatchObject(resultWithOptionSetToFalse);
expect(resultWithNoOptionSet).toMatchObject(
resultWithOptionSetExplicitlyToUndefined,
);
});

it('should not preserve node maps by default for parseAndGenerateServices(), unless `project` is set', () => {
const noOptionSet = parser.parseAndGenerateServices(code, baseConfig);

expect(noOptionSet.services.esTreeNodeToTSNodeMap).toBeUndefined();
expect(noOptionSet.services.tsNodeToESTreeNodeMap).toBeUndefined();

const withProjectNoOptionSet = parser.parseAndGenerateServices(code, {
...baseConfig,
project: './tsconfig.json',
});

expect(withProjectNoOptionSet.services.esTreeNodeToTSNodeMap).toEqual(
expect.any(WeakMap),
);
expect(withProjectNoOptionSet.services.tsNodeToESTreeNodeMap).toEqual(
expect.any(WeakMap),
);
});

it('should preserve node maps for parseAndGenerateServices() when option is `true`, regardless of `project` config', () => {
const optionSetToTrue = parser.parseAndGenerateServices(code, {
...baseConfig,
preserveNodeMaps: true,
});

expect(optionSetToTrue.services.esTreeNodeToTSNodeMap).toEqual(
expect.any(WeakMap),
);
expect(optionSetToTrue.services.tsNodeToESTreeNodeMap).toEqual(
expect.any(WeakMap),
);

const withProjectOptionSetToTrue = parser.parseAndGenerateServices(code, {
...baseConfig,
preserveNodeMaps: true,
project: './tsconfig.json',
});

expect(withProjectOptionSetToTrue.services.esTreeNodeToTSNodeMap).toEqual(
expect.any(WeakMap),
);
expect(withProjectOptionSetToTrue.services.tsNodeToESTreeNodeMap).toEqual(
expect.any(WeakMap),
);
});

it('should not preserve node maps for parseAndGenerateServices() when option is `false`, regardless of `project` config', () => {
const optionSetToFalse = parser.parseAndGenerateServices(code, {
...baseConfig,
preserveNodeMaps: false,
});

expect(optionSetToFalse.services.esTreeNodeToTSNodeMap).toBeUndefined();
expect(optionSetToFalse.services.tsNodeToESTreeNodeMap).toBeUndefined();

const withProjectOptionSetToFalse = parser.parseAndGenerateServices(
code,
{ ...baseConfig, preserveNodeMaps: false, project: './tsconfig.json' },
);

expect(
withProjectOptionSetToFalse.services.esTreeNodeToTSNodeMap,
).toBeUndefined();
expect(
withProjectOptionSetToFalse.services.tsNodeToESTreeNodeMap,
).toBeUndefined();
});
});
});