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

Reduce reliance on internal TS apis #949

Merged
merged 4 commits into from
Mar 22, 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
2 changes: 1 addition & 1 deletion src/lib/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export class Application extends ChildableComponent<Application, AbstractCompone
* @param out The path and file name of the target file.
* @returns TRUE if the json file could be written successfully, otherwise FALSE.
*/
public generateJson(input: any, out: string): boolean {
public generateJson(input: ProjectReflection | string[], out: string): boolean {
const project = input instanceof ProjectReflection ? input : this.convert(input);
if (!project) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/converter/convert-expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export function convertExpression(expression: ts.Expression): string {
case ts.SyntaxKind.FalseKeyword:
return 'false';
default:
const source = _ts.getSourceFileOfNode(<ts.Node> expression);
const source = expression.getSourceFile();
return source.text.substring(expression.pos, expression.end);
}
}
15 changes: 3 additions & 12 deletions src/lib/converter/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { ParameterType } from '../utils/options/declaration';
import { Reflection, Type, ProjectReflection } from '../models/index';
import { Context } from './context';
import { ConverterComponent, ConverterNodeComponent, ConverterTypeComponent, TypeTypeConverter, TypeNodeConverter } from './components';
import { CompilerHost } from './utils/compiler-host';
import { Component, Option, ChildableComponent, ComponentClass } from '../utils/component';
import { normalizePath } from '../utils/fs';
import { createMinimatch } from '../utils/paths';
Expand Down Expand Up @@ -83,11 +82,6 @@ export class Converter extends ChildableComponent<Application, ConverterComponen
})
excludeProtected!: boolean;

/**
* Defined in the initialize method
*/
private compilerHost!: CompilerHost;

/**
* Defined in the initialize method
*/
Expand Down Expand Up @@ -199,7 +193,6 @@ export class Converter extends ChildableComponent<Application, ConverterComponen
* must expose the settings that should be used and serves as a global logging endpoint.
*/
initialize() {
this.compilerHost = new CompilerHost(this);
this.nodeConverters = {};
this.typeTypeConverters = [];
this.typeNodeConverters = [];
Expand Down Expand Up @@ -281,13 +274,11 @@ export class Converter extends ChildableComponent<Application, ConverterComponen
* @param fileNames Array of the file names that should be compiled.
*/
convert(fileNames: string[]): ConverterResult {
for (let i = 0, c = fileNames.length; i < c; i++) {
fileNames[i] = normalizePath(_ts.normalizeSlashes(fileNames[i]));
}
const normalizedFiles = fileNames.map(normalizePath);

const program = ts.createProgram(fileNames, this.application.options.getCompilerOptions(), this.compilerHost);
const program = ts.createProgram(normalizedFiles, this.application.options.getCompilerOptions());
const checker = program.getTypeChecker();
const context = new Context(this, fileNames, checker, program);
const context = new Context(this, normalizedFiles, checker, program);

this.trigger(Converter.EVENT_BEGIN, context);

Expand Down
40 changes: 28 additions & 12 deletions src/lib/converter/factories/comment.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as ts from 'typescript';
import * as _ts from '../../ts-internal';
import { toArray } from 'lodash';

import { Comment, CommentTag } from '../../models/comments/index';

Expand Down Expand Up @@ -32,14 +32,7 @@ export function createComment(node: ts.Node): Comment | undefined {
* @return TRUE if the given node is the topmost module declaration, FALSE otherwise.
*/
function isTopmostModuleDeclaration(node: ts.ModuleDeclaration): boolean {
if (node.nextContainer && node.nextContainer.kind === ts.SyntaxKind.ModuleDeclaration) {
let next = <ts.ModuleDeclaration> node.nextContainer;
if (node.name.end + 1 === next.name.pos) {
return false;
}
}

return true;
return node.getChildren().some(ts.isModuleBlock);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Toss namespace A.B.C {} into https://ts-ast-viewer.com/ and it's immediately obvious that a module declaration is "topmost" if it has a module block as an immediate child.

}

/**
Expand All @@ -65,6 +58,29 @@ function getRootModuleDeclaration(node: ts.ModuleDeclaration): ts.Node {
return node;
}

/**
* Derived from the internal ts utility
* https://github.com/Microsoft/TypeScript/blob/v3.2.2/src/compiler/utilities.ts#L954
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for including the source link

* @param node
* @param text
*/
function getJSDocCommentRanges(node: ts.Node, text: string): ts.CommentRange[] {
const hasTrailingCommentRanges = [
ts.SyntaxKind.Parameter,
ts.SyntaxKind.FunctionExpression,
ts.SyntaxKind.ArrowFunction,
ts.SyntaxKind.ParenthesizedExpression
].includes(node.kind);

let commentRanges = toArray(ts.getLeadingCommentRanges(text, node.pos));
if (hasTrailingCommentRanges) {
commentRanges = toArray(ts.getTrailingCommentRanges(text, node.pos)).concat(commentRanges);
}

// True if the comment starts with '/**' but not if it is '/**/'
return commentRanges.filter(({ pos }) => text.substr(pos, 3) === '/**' && text[pos + 4] !== '/');
}

/**
* Return the raw comment string for the given node.
*
Expand All @@ -82,9 +98,9 @@ export function getRawComment(node: ts.Node): string | undefined {
}
}

const sourceFile = _ts.getSourceFileOfNode(node);
const comments = _ts.getJSDocCommentRanges(node, sourceFile.text);
if (comments && comments.length) {
const sourceFile = node.getSourceFile();
const comments = getJSDocCommentRanges(node, sourceFile.text);
if (comments.length) {
let comment: ts.CommentRange;
if (node.kind === ts.SyntaxKind.SourceFile) {
if (comments.length === 1) {
Expand Down
3 changes: 1 addition & 2 deletions src/lib/converter/factories/parameter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as ts from 'typescript';
import * as _ts from '../../ts-internal';

import { ReflectionFlag, ReflectionKind, ParameterReflection, SignatureReflection } from '../../models/reflections/index';
import { Context } from '../context';
Expand All @@ -26,7 +25,7 @@ export function createParameter(context: Context, node: ts.ParameterDeclaration)
const parameter = new ParameterReflection(node.symbol.name, ReflectionKind.Parameter, signature);
context.registerReflection(parameter, node);
context.withScope(parameter, () => {
if (_ts.isBindingPattern(node.name)) {
if (ts.isArrayBindingPattern(node.name) || ts.isObjectBindingPattern(node.name)) {
parameter.type = context.converter.convertType(context, node.name);
parameter.name = '__namedParameters';
} else {
Expand Down
12 changes: 7 additions & 5 deletions src/lib/converter/nodes/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Reflection, ReflectionFlag, ReflectionKind, DeclarationReflection } fro
import { createDeclaration } from '../factories/index';
import { Context } from '../context';
import { Component, ConverterNodeComponent } from '../components';
import { toArray } from 'lodash';

@Component({name: 'node:class'})
export class ClassConverter extends ConverterNodeComponent<ts.ClassDeclaration> {
Expand Down Expand Up @@ -50,8 +51,9 @@ export class ClassConverter extends ConverterNodeComponent<ts.ClassDeclaration>
});
}

const baseType = _ts.getEffectiveBaseTypeNode(node);
if (baseType) {
const extendsClause = toArray(node.heritageClauses).find(h => h.token === ts.SyntaxKind.ExtendsKeyword);
if (extendsClause) {
const baseType = extendsClause.types[0];
const type = context.getTypeAtLocation(baseType);
if (!context.isInherit) {
if (!reflection!.extendedTypes) {
Expand All @@ -74,9 +76,9 @@ export class ClassConverter extends ConverterNodeComponent<ts.ClassDeclaration>
}
}

const implementedTypes = _ts.getClassImplementsHeritageClauseElements(node);
if (implementedTypes && implementedTypes.length) {
const implemented = this.owner.convertTypes(context, implementedTypes);
const implementsClause = toArray(node.heritageClauses).find(h => h.token === ts.SyntaxKind.ImplementsKeyword);
if (implementsClause) {
const implemented = this.owner.convertTypes(context, implementsClause.types);
reflection!.implementedTypes = (reflection!.implementedTypes || []).concat(implemented);
}
});
Expand Down
9 changes: 5 additions & 4 deletions src/lib/converter/nodes/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Reflection, ReflectionKind, DeclarationReflection } from '../../models/
import { createDeclaration } from '../factories/index';
import { Context } from '../context';
import { Component, ConverterNodeComponent } from '../components';
import { toArray } from 'lodash';

@Component({name: 'node:interface'})
export class InterfaceConverter extends ConverterNodeComponent<ts.InterfaceDeclaration> {
Expand Down Expand Up @@ -32,14 +33,14 @@ export class InterfaceConverter extends ConverterNodeComponent<ts.InterfaceDecla

context.withScope(reflection, node.typeParameters, () => {
if (node.members) {
node.members.forEach((member, isInherit) => {
node.members.forEach((member) => {
this.owner.convertNode(context, member);
});
}

const baseTypes = _ts.getInterfaceBaseTypeNodes(node);
if (baseTypes) {
baseTypes.forEach((baseType) => {
const extendsClause = toArray(node.heritageClauses).find(h => h.token === ts.SyntaxKind.ExtendsKeyword);
if (extendsClause) {
extendsClause.types.forEach((baseType) => {
const type = context.getTypeAtLocation(baseType);
if (!context.isInherit) {
if (!reflection!.extendedTypes) {
Expand Down
11 changes: 7 additions & 4 deletions src/lib/converter/nodes/variable-statement.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as ts from 'typescript';
import * as _ts from '../../ts-internal';

import { Reflection } from '../../models/index';
import { Context } from '../context';
Expand All @@ -24,7 +23,7 @@ export class VariableStatementConverter extends ConverterNodeComponent<ts.Variab
convert(context: Context, node: ts.VariableStatement): Reflection {
if (node.declarationList && node.declarationList.declarations) {
node.declarationList.declarations.forEach((variableDeclaration) => {
if (_ts.isBindingPattern(variableDeclaration.name)) {
if (ts.isArrayBindingPattern(variableDeclaration.name) || ts.isObjectBindingPattern(variableDeclaration.name)) {
this.convertBindingPattern(context, variableDeclaration.name);
} else {
this.owner.convertNode(context, variableDeclaration);
Expand All @@ -42,10 +41,14 @@ export class VariableStatementConverter extends ConverterNodeComponent<ts.Variab
* @param node The binding pattern node that should be analyzed.
*/
convertBindingPattern(context: Context, node: ts.BindingPattern) {
(node.elements as ts.NodeArray<ts.BindingElement>).forEach((element: ts.BindingElement) => {
node.elements.forEach((element) => {
this.owner.convertNode(context, element);

if (_ts.isBindingPattern(element.name)) {
if (!ts.isBindingElement(element)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This additional check is required since the internal isBindingPattern check also ensures that node exists, while the is*BindingPattern do not

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be better to inline this with the if block below?

return;
}

if (ts.isArrayBindingPattern(element.name) || ts.isObjectBindingPattern(element.name)) {
this.convertBindingPattern(context, element.name);
}
});
Expand Down
7 changes: 4 additions & 3 deletions src/lib/converter/nodes/variable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export class VariableConverter extends ConverterNodeComponent<ts.VariableDeclara

/**
* Analyze the given variable declaration node and create a suitable reflection.
* TODO: the type of `node` is incorrect, it should be a union of ts.PropertySignature | ts.PropertyDeclaration | ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leaving resolving this to a later PR since changing to the actual type caused quite a few type errors.

*
* @param context The context object describing the current state the converter is in.
* @param node The variable declaration node that should be analyzed.
Expand All @@ -50,9 +51,9 @@ export class VariableConverter extends ConverterNodeComponent<ts.VariableDeclara

let name: string | undefined;
let isBindingPattern: boolean;
if (_ts.isBindingPattern(node.name)) {
if (node['propertyName']) {
name = _ts.declarationNameToString(node['propertyName']);
if (ts.isArrayBindingPattern(node.name) || ts.isObjectBindingPattern(node.name)) {
if (ts.isBindingElement(node) && node.propertyName) {
name = node.propertyName.getText();
isBindingPattern = true;
} else {
return;
Expand Down
6 changes: 3 additions & 3 deletions src/lib/converter/plugins/DecoratorPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ export class DecoratorPlugin extends ConverterComponent {
args.forEach((arg: ts.Expression, index: number) => {
if (index < signature.parameters.length) {
const parameter = signature.parameters[index];
result[parameter.name] = _ts.getTextOfNode(arg);
result[parameter.name] = arg.getText();
} else {
if (!result['...']) {
result['...'] = [];
}
result['...'].push(_ts.getTextOfNode(arg));
result['...'].push(arg.getText());
}
});

Expand Down Expand Up @@ -90,7 +90,7 @@ export class DecoratorPlugin extends ConverterComponent {
}

const info: Decorator = {
name: _ts.getTextOfNode(identifier)
name: identifier.getText()
};

const type = context.checker.getTypeAtLocation(identifier);
Expand Down
4 changes: 2 additions & 2 deletions src/lib/converter/plugins/SourcePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ export class SourcePlugin extends ConverterComponent {
if (!node) {
return;
}
const sourceFile = _ts.getSourceFileOfNode(node);
const fileName = sourceFile.fileName;
const sourceFile = node.getSourceFile();
const fileName = sourceFile.fileName;
const file: SourceFile = this.getSourceFile(fileName, context.project);

let position: ts.LineAndCharacter;
Expand Down
5 changes: 2 additions & 3 deletions src/lib/converter/types/alias.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as ts from 'typescript';
import * as _ts from '../../ts-internal';

import { ReferenceType } from '../../models/index';
import { Component, ConverterTypeComponent, TypeNodeConverter } from '../components';
Expand Down Expand Up @@ -42,7 +41,7 @@ export class AliasConverter extends ConverterTypeComponent implements TypeNodeCo
symbolName.shift();
}

let nodeName = _ts.getTextOfNode(node.typeName).split('.');
let nodeName = node.typeName.getText().split('.');
if (!nodeName.length) {
return false;
}
Expand Down Expand Up @@ -71,7 +70,7 @@ export class AliasConverter extends ConverterTypeComponent implements TypeNodeCo
* @returns A type reference pointing to the type alias definition.
*/
convertNode(context: Context, node: ts.TypeReferenceNode): ReferenceType {
const name = _ts.getTextOfNode(node.typeName);
const name = node.typeName.getText();
const result = new ReferenceType(name, ReferenceType.SYMBOL_ID_RESOLVE_BY_NAME);

if (node.typeArguments) {
Expand Down
3 changes: 1 addition & 2 deletions src/lib/converter/types/type-parameter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as ts from 'typescript';
import * as _ts from '../../ts-internal';

import { Type, TypeParameterType } from '../../models/types/index';
import { Component, ConverterTypeComponent, TypeNodeConverter } from '../components';
Expand Down Expand Up @@ -37,7 +36,7 @@ export class TypeParameterConverter extends ConverterTypeComponent implements Ty
*/
convertNode(context: Context, node: ts.TypeReferenceNode): Type | undefined {
if (node.typeName) {
const name = _ts.getTextOfNode(node.typeName);
const name = node.getText();
if (context.typeParameters && context.typeParameters[name]) {
return context.typeParameters[name].clone();
}
Expand Down