Skip to content

fix(estree): correct location and range for heritage clause #8

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 16, 2019
Merged

fix(estree): correct location and range for heritage clause #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
324f3eb
fix(estree): correct location and range for heritage clause
armano2 Jan 15, 2019
82b08fc
fix(estree): make sure that heritage is in treeNodeMaps
armano2 Jan 15, 2019
d400729
chore: add note to fixtures-to-test
armano2 Jan 15, 2019
6c2558a
chore: update babel PR status
armano2 Jan 15, 2019
ea4c119
chore: update nots
armano2 Jan 15, 2019
e0ae9c0
Merge branch 'master' into heritage-location
armano2 Jan 16, 2019
35b93a9
Merge branch 'master' into heritage-location
JamesHenry Jan 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 20 additions & 34 deletions packages/typescript-estree/src/convert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,31 +272,6 @@ export default function convert(config: ConvertConfig): ESTreeNode | null {
};
}

/**
* Converts a child into a specified heritage node.
* @param {AST_NODE_TYPES} nodeType Type of node to be used
* @param {ts.ExpressionWithTypeArguments} child The TypeScript AST node to convert.
* @returns {ESTreeNode} The heritage node.
*/
function convertHeritageClause(
nodeType: AST_NODE_TYPES,
child: ts.ExpressionWithTypeArguments
): ESTreeNode {
const expression = convertChild(child.expression)!;
const classImplementsNode: ESTreeNode = {
type: nodeType,
loc: expression.loc,
range: expression.range,
expression
};
if (child.typeArguments && child.typeArguments.length) {
classImplementsNode.typeParameters = convertTypeArgumentsToTypeParameters(
child.typeArguments
);
}
return classImplementsNode;
}

/**
* Converts an array of ts.Node parameters into an array of ESTreeNode params
* @param {ts.Node[]} parameters An array of ts.Node params to be converted
Expand Down Expand Up @@ -1465,9 +1440,7 @@ export default function convert(config: ConvertConfig): ESTreeNode | null {
});

if (implementsClause) {
result.implements = implementsClause.types.map(el =>
convertHeritageClause(AST_NODE_TYPES.TSClassImplements, el)
);
result.implements = implementsClause.types.map(convertChild);
}

if (hasModifier(SyntaxKind.DeclareKeyword, node)) {
Expand Down Expand Up @@ -2334,6 +2307,23 @@ export default function convert(config: ConvertConfig): ESTreeNode | null {
break;
}

case SyntaxKind.ExpressionWithTypeArguments: {
Object.assign(result, {
type:
parent && parent.kind === SyntaxKind.InterfaceDeclaration
? AST_NODE_TYPES.TSInterfaceHeritage
: AST_NODE_TYPES.TSClassImplements,
expression: convertChild(node.expression)
});

if (node.typeArguments && node.typeArguments.length) {
result.typeParameters = convertTypeArgumentsToTypeParameters(
node.typeArguments
);
}
break;
}

case SyntaxKind.InterfaceDeclaration: {
const interfaceHeritageClauses = node.heritageClauses || [];

Expand Down Expand Up @@ -2386,15 +2376,11 @@ export default function convert(config: ConvertConfig): ESTreeNode | null {
for (const heritageClause of interfaceHeritageClauses) {
if (heritageClause.token === SyntaxKind.ExtendsKeyword) {
for (const n of heritageClause.types) {
interfaceExtends.push(
convertHeritageClause(AST_NODE_TYPES.TSInterfaceHeritage, n)
);
interfaceExtends.push(convertChild(n));
}
} else if (heritageClause.token === SyntaxKind.ImplementsKeyword) {
for (const n of heritageClause.types) {
interfaceImplements.push(
convertHeritageClause(AST_NODE_TYPES.TSInterfaceHeritage, n)
);
interfaceImplements.push(convertChild(n));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/typescript-estree/src/temp-types-based-on-js-source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export interface ESTreeNode {
value?: string;
expression?: ESTreeNode | null;
decorators?: (ESTreeNode | null)[];
implements?: ESTreeNode[];
extends?: ESTreeNode[];
implements?: (ESTreeNode | null)[];
extends?: (ESTreeNode | null)[];
const?: boolean;
declare?: boolean;
global?: boolean;
Expand Down
16 changes: 7 additions & 9 deletions packages/typescript-estree/tests/ast-alignment/fixtures-to-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,17 +329,14 @@ tester.addFixturePatternConfig('typescript/basics', {
*/
'arrow-function-with-type-parameters',
/**
* Babel error: parameterName is not included into range of TSTypeAnnotation
* TODO: report it to babel
* PR for this type predicate ranges has been merged into Babel: https://github.com/babel/babel/pull/9339
* TODO: remove me in next babel > 7.2.3
*/
'type-guard-in-method',
/**
* there is difference in range between babel and ts-estree
*/
'class-with-implements-generic-multiple',
'class-with-implements-generic',
'export-declare-const-named-enum',
'interface-with-extends-type-parameters',
'interface-with-optional-properties',
/**
* Babel parses it as TSQualifiedName
Expand All @@ -350,6 +347,7 @@ tester.addFixturePatternConfig('typescript/basics', {
/**
* Babel bug for parsing exported abstract interface
* https://github.com/babel/babel/issues/9304
* PR: https://github.com/babel/babel/pull/9336
*/
'abstract-interface',
/**
Expand Down Expand Up @@ -395,8 +393,8 @@ tester.addFixturePatternConfig('typescript/basics', {
*/
'type-assertion-arrow-function',
/**
* Babel parses this incorrectly
* https://github.com/babel/babel/issues/9325
* PR for type parsing declare and abstract classes has been merged into Babel: https://github.com/babel/babel/pull/9328
* TODO: remove me in next babel > 7.2.3
*/
'class-multi-line-keyword-declare',
'class-multi-line-keyword-abstract'
Expand Down Expand Up @@ -454,8 +452,8 @@ tester.addFixturePatternConfig('typescript/types', {
fileType: 'ts',
ignore: [
/**
* Babel bug for range of Identifier in TSIndexSignature
* https://github.com/babel/babel/issues/9319
* PR for type Identifier in TSIndexSignature ranges has been merged into Babel: https://github.com/babel/babel/pull/9335
* TODO: remove me in next babel > 7.2.3
*/
'index-signature',
'index-signature-readonly',
Expand Down
12 changes: 6 additions & 6 deletions packages/typescript-estree/tests/lib/__snapshots__/typescript.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14823,7 +14823,7 @@ Object {
},
"loc": Object {
"end": Object {
"column": 24,
"column": 27,
"line": 1,
},
"start": Object {
Expand All @@ -14833,7 +14833,7 @@ Object {
},
"range": Array [
21,
24,
27,
],
"type": "TSClassImplements",
"typeParameters": Object {
Expand Down Expand Up @@ -15155,7 +15155,7 @@ Object {
},
"loc": Object {
"end": Object {
"column": 24,
"column": 30,
"line": 1,
},
"start": Object {
Expand All @@ -15165,7 +15165,7 @@ Object {
},
"range": Array [
21,
24,
30,
],
"type": "TSClassImplements",
"typeParameters": Object {
Expand Down Expand Up @@ -51372,7 +51372,7 @@ Object {
},
"loc": Object {
"end": Object {
"column": 28,
"column": 31,
"line": 1,
},
"start": Object {
Expand All @@ -51382,7 +51382,7 @@ Object {
},
"range": Array [
25,
28,
31,
],
"type": "TSInterfaceHeritage",
"typeParameters": Object {
Expand Down