Skip to content

Feat/639 #1196

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

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
02aad30
Merge pull request #2 from TypeStrong/feat/639
paztis Jan 30, 2020
b8a6d42
correct good export naming in library mode
Jan 30, 2020
44b98fc
correct good export naming in library mode: unit test update
Jan 30, 2020
754ec6e
[module naming] fix remaining index.d after module naming. Remove use…
Feb 4, 2020
fd02c12
update declaration ".d" removal unit test
Feb 4, 2020
4b48ba9
update double quote removal in unit tests
Feb 4, 2020
b2874ce
correct unit tests for double quote useless escaping
Feb 4, 2020
8ba5964
in case of mono-repo (node_modules outside the project), trunk the mo…
Feb 5, 2020
55f62ad
library export declaration refactoring
Feb 5, 2020
9e90746
only consider the program files paths for basePath construction
Feb 5, 2020
ad615b3
wording fix
Feb 5, 2020
da313ad
update export specs
Feb 5, 2020
42ebbee
typescript getFullyQualifiedName wrapping to prefix by sourceFile nam…
Feb 6, 2020
db1f74f
typescript getFullyQualifiedName wrapping to prefix by sourceFile nam…
Feb 6, 2020
cf8ed0c
don't forgot to flag as exported the default case
Feb 6, 2020
33b12ae
always create ReferenceReflection in export {A} declaration: event if…
Feb 6, 2020
a7b2b36
referenceReflexion in namespaces fix: copy the sourcefile export logic
Feb 6, 2020
1d3556d
tslint fix
Feb 6, 2020
5eb1a7b
correct isExported support
May 4, 2020
c6765e7
update type reference management with real FQN
May 4, 2020
47fb938
correct export symbol comparison (do not base it only on name)
May 6, 2020
ebfd032
context remaining symbol reflections support added
May 6, 2020
f85c6b8
shortcut sub-modules namings by removing parent name from it
May 6, 2020
d24a418
add all referenceType symbol support
May 6, 2020
f61becc
upgrade typescript version
May 6, 2020
fcfc68b
remaining symbols declaration refactoring
May 11, 2020
cbb97ee
correct file extension problem in FQN generation
May 11, 2020
3ceae58
test specs alignment on updates
May 11, 2020
ccfcf4a
tslint
May 11, 2020
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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"mockery": "^2.1.0",
"nyc": "15.0.0",
"tslint": "^5.20.1",
"typescript": "3.7.x"
"typescript": "3.8.x"
},
"files": [
"bin",
Expand All @@ -67,7 +67,7 @@
],
"scripts": {
"pretest": "node scripts/copy_test_files.js",
"test": "nyc --reporter=html --reporter=text-summary mocha --timeout=10000 'dist/test/**/*.test.js'",
"test": "nyc --reporter=html --reporter=text-summary mocha --timeout=20000 'dist/test/**/*.test.js'",
"prerebuild_specs": "npm run pretest",
"rebuild_specs": "node scripts/rebuild_specs.js",
"build": "tsc --project .",
Expand Down
109 changes: 106 additions & 3 deletions src/lib/converter/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import { IMinimatch } from 'minimatch';

import { Logger } from '../utils/loggers';
import { createMinimatch } from '../utils/paths';
import { Reflection, ProjectReflection, ContainerReflection, Type } from '../models/index';
import { Reflection, ProjectReflection, ContainerReflection, Type, ReflectionFlag, ReflectionKind } from '../models/index';

import { createTypeParameter } from './factories/type-parameter';
import { Converter } from './converter';
import { createDeclaration } from './factories';

/**
* The context describes the current state the converter is in.
Expand Down Expand Up @@ -87,6 +88,11 @@ export class Context {
*/
visitStack: ts.Node[];

/**
* A map of remaining symbols to process.
*/
remainingSymbolReflections = new Map<string, ts.Symbol>();

/**
* The pattern that should be used to flag external source files.
*/
Expand Down Expand Up @@ -179,7 +185,10 @@ export class Context {
*/
registerReflection(reflection: Reflection, symbol?: ts.Symbol) {
if (symbol) {
this.project.registerReflection(reflection, this.checker.getFullyQualifiedName(symbol));
const fqn = this.getFullyQualifiedName(symbol);
this.project.registerReflection(reflection, fqn);

this.removeRemainingSymbolReflection(fqn);
} else {
this.project.registerReflection(reflection);
}
Expand Down Expand Up @@ -304,7 +313,7 @@ export class Context {
}

if (baseNode.symbol) {
const id = this.checker.getFullyQualifiedName(baseNode.symbol);
const id = this.getFullyQualifiedName(baseNode.symbol);
if (this.inheritedChildren && this.inheritedChildren.includes(id)) {
return target;
} else {
Expand Down Expand Up @@ -398,6 +407,100 @@ export class Context {

return typeParameters;
}

/**
* Typscript getFullyQualifiedName method don't always return unique string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that's correct. If a declaration is not exported, it won't contain the source file path... It seems wrong to me to add it since it might conflict with a FQN created by the TS version. That said, I haven't read through everything yet so there might be a good reason for it that I'm not seeing...

Copy link
Author

Choose a reason for hiding this comment

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

In 2 different files I was having the same named export.
And typescript wasn't providing me a unique identifier (no file path in front)
The result was bad reference link in the documentation.

Here in case the fqn doesn't contain a doc, I prefix it with the file path and it has resolved my problem

* in this case prefix it with sourcefile name
*
* @param symbol
*/
getFullyQualifiedName(symbol: ts.Symbol): string {
let fullyQualifiedName = this.checker.getFullyQualifiedName(symbol);
if (!fullyQualifiedName.startsWith('"')) {
if (symbol.declarations) {
const node = symbol.declarations[0];
if (node) {
const sourceFile = node.getSourceFile();
let filePath = sourceFile.fileName;

const hiddenExtensions = ['.d.ts', '.ts', '.js'];
hiddenExtensions.forEach((hiddenExtension) => {
if (filePath.endsWith(hiddenExtension)) {
filePath = filePath.substr(0, filePath.length - hiddenExtension.length);
}
});

fullyQualifiedName = `"${filePath}".${fullyQualifiedName}`;
}
}
}

return fullyQualifiedName;
}

saveRemainingSymbolReflection(fqn: string, symbol: ts.Symbol) {
if (!this.remainingSymbolReflections.has(fqn) && !this.project.getReflectionFromFQN(fqn)) {
this.remainingSymbolReflections.set(fqn, symbol);
}
}

removeRemainingSymbolReflection(fqn: string) {
this.remainingSymbolReflections.delete(fqn);
}

hasRemainingSymbolReflections() {
return (this.remainingSymbolReflections.size > 0);
}

convertRemainingSymbolReflections() {
if (this.hasRemainingSymbolReflections()) {
this.remainingSymbolReflections.forEach((symbol, fqn) => {
const declaration = symbol.declarations?.[0];

if (declaration) {
const declareRecursively = (node: ts.Node): Reflection | undefined => {
let scope: Reflection | undefined = undefined;
const parentNode = node.parent;
if (parentNode) {
if (parentNode.symbol) {
const parentFQN = this.getFullyQualifiedName(parentNode.symbol);
const parentReflection = this.project.getReflectionFromFQN(parentFQN);

if (parentReflection) {
scope = parentReflection;
}
}

if (!scope) {
scope = declareRecursively(parentNode);
}
}

let reflection: Reflection | undefined = undefined;
if (scope) {
this.withScope(scope, () => {
reflection = this.converter.convertNode(this, node);
});
} else if (node.kind === ts.SyntaxKind.SourceFile) {
const sourceFile = node as ts.SourceFile;
this.withSourceFile(sourceFile, () => {
reflection = createDeclaration(this, sourceFile, ReflectionKind.Module, sourceFile.fileName);
reflection?.setFlag(ReflectionFlag.Exported, false);
});
}

return reflection;
};

const reflection = declareRecursively(declaration);
if (reflection) {
reflection.setFlag(ReflectionFlag.Exported, true);
}
}
});
this.remainingSymbolReflections.clear();
}
}
}

function isNamedNode(node: ts.Node): node is ts.Node & { name: ts.Identifier | ts.ComputedPropertyName } {
Expand Down
4 changes: 3 additions & 1 deletion src/lib/converter/converter.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 * as _ from 'lodash';

import { Application } from '../application';
Expand Down Expand Up @@ -383,6 +382,9 @@ export class Converter extends ChildableComponent<Application, ConverterComponen
});
}

// convert all missing reflections
context.convertRemainingSymbolReflections();

return [];
}

Expand Down
20 changes: 13 additions & 7 deletions src/lib/converter/factories/declaration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ export function createDeclaration(context: Context, node: ts.Declaration, kind:

// Test whether the node is exported
let isExported: boolean;
if (kind === ReflectionKind.Module || kind === ReflectionKind.Global) {
if (kind === ReflectionKind.Global) {
isExported = true;
} else if (container.kind === ReflectionKind.Global) {
} else if (!container || container.kind === ReflectionKind.Global) {
// In file mode, everything is exported.
isExported = true;
} else if (container.kindOf([ReflectionKind.Namespace, ReflectionKind.Module])) {
const symbol = context.getSymbolAtLocation(node);
if (!symbol) {
if (!symbol || !node.parent) {
isExported = false;
} else {
let parentNode = node.parent;
Expand All @@ -87,11 +87,17 @@ export function createDeclaration(context: Context, node: ts.Declaration, kind:
}
const parentSymbol = context.getSymbolAtLocation(parentNode);
if (!parentSymbol) {
// This is a file with no imports/exports, so everything is
// global and therefore exported.
isExported = true;
isExported = false;
} else if (parentSymbol.exports) {
isExported = false;
parentSymbol.exports.forEach((exportSymbol) => {
if (!isExported) {
const realSymbol = context.resolveAliasedSymbol(exportSymbol);
isExported = (realSymbol === symbol);
}
});
} else {
isExported = !!parentSymbol.exports?.get(symbol.escapedName);
isExported = false;
}
}
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/converter/factories/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export { createComment } from './comment';
export { createDeclaration } from './declaration';
export { createParameter } from './parameter';
export { createReferenceType } from './reference';
export { createReferenceType, createReferenceReflection } from './reference';
export { createSignature } from './signature';
export { createTypeParameter } from './type-parameter';
7 changes: 5 additions & 2 deletions src/lib/converter/factories/reference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ export function createReferenceType(context: Context, symbol: ts.Symbol | undefi
name = checker.symbolToString(symbol.parent) + '.' + name;
}

return new ReferenceType(name, context.checker.getFullyQualifiedName(symbol));
const FQN = context.getFullyQualifiedName(symbol);
context.saveRemainingSymbolReflection(FQN, symbol);

return new ReferenceType(name, FQN);
}

export function createReferenceReflection(context: Context, source: ts.Symbol, target: ts.Symbol): ReferenceReflection | undefined {
Expand All @@ -39,7 +42,7 @@ export function createReferenceReflection(context: Context, source: ts.Symbol, t
return;
}

const reflection = new ReferenceReflection(source.name, [ReferenceState.Unresolved, context.checker.getFullyQualifiedName(target)], context.scope);
const reflection = new ReferenceReflection(source.name, [ReferenceState.Unresolved, context.getFullyQualifiedName(target)], context.scope);
reflection.flags.setFlag(ReflectionFlag.Exported, true); // References are exported by necessity
if (!context.scope.children) {
context.scope.children = [];
Expand Down
28 changes: 24 additions & 4 deletions src/lib/converter/nodes/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as ts from 'typescript';
import { resolve } from 'path';

import { Reflection, ReflectionKind, ReflectionFlag } from '../../models/index';
import { createDeclaration } from '../factories/index';
import { createDeclaration, createReferenceReflection } from '../factories/index';
import { Context } from '../context';
import { Component, ConverterNodeComponent } from '../components';
import { BindOption, SourceFileMode } from '../../utils';
Expand Down Expand Up @@ -72,7 +72,11 @@ export class BlockConverter extends ConverterNodeComponent<ts.SourceFile|ts.Bloc
result = createDeclaration(context, node, ReflectionKind.Module, node.fileName);
context.withScope(result, () => {
this.convertVisibleDeclarations(context, node);
result!.setFlag(ReflectionFlag.Exported);

// only mark as exported the input files
if (isInputFile(node)) {
result!.setFlag(ReflectionFlag.Exported);
}
});
} else {
this.convertVisibleDeclarations(context, node);
Expand Down Expand Up @@ -112,8 +116,24 @@ export class BlockConverter extends ConverterNodeComponent<ts.SourceFile|ts.Bloc

for (const symbol of context.checker.getExportsOfModule(moduleSymbol)) {
const resolved = context.resolveAliasedSymbol(symbol);
for (const declaration of resolved.declarations) {
this.owner.convertNode(context, declaration);

// export declaration is always unique: no need of loop
const declaration = resolved.declarations?.[0];
if (declaration) {
const declarationReflection = this.owner.convertNode(context, declaration);
if (declarationReflection) {
if (!declarationReflection.kindOf([ReflectionKind.ClassOrInterface, ReflectionKind.SomeModule])) {
// rename the declaration to the exported one
declarationReflection.name = symbol.name;
declarationReflection.flags.setFlag(ReflectionFlag.Exported, true);
} else if (declarationReflection.name !== symbol.name) {
// create a extra reference to the declaration
declarationReflection.flags.setFlag(ReflectionFlag.Exported, false);
createReferenceReflection(context, symbol, resolved);
} else {
declarationReflection.flags.setFlag(ReflectionFlag.Exported, true);
}
}
}
}
}
Expand Down
53 changes: 42 additions & 11 deletions src/lib/converter/nodes/export.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import * as ts from 'typescript';

import { Reflection, ReflectionFlag, DeclarationReflection, ContainerReflection } from '../../models/index';
import {
Reflection,
ReflectionFlag,
DeclarationReflection,
ContainerReflection,
ReflectionKind
} from '../../models/index';
import { Context } from '../context';
import { Component, ConverterNodeComponent } from '../components';
import { createReferenceReflection } from '../factories/reference';
import { SourceFileMode } from '../../utils';
import { BindOption, SourceFileMode } from '../../utils';

@Component({name: 'node:export'})
export class ExportConverter extends ConverterNodeComponent<ts.ExportAssignment> {
Expand Down Expand Up @@ -32,7 +38,7 @@ export class ExportConverter extends ConverterNodeComponent<ts.ExportAssignment>
return;
}

const reflection = project.getReflectionFromFQN(context.checker.getFullyQualifiedName(declaration.symbol));
const reflection = project.getReflectionFromFQN(context.getFullyQualifiedName(declaration.symbol));
if (node.isExportEquals && reflection instanceof DeclarationReflection) {
reflection.setFlag(ReflectionFlag.ExportAssignment, true);
}
Expand All @@ -58,9 +64,12 @@ export class ExportConverter extends ConverterNodeComponent<ts.ExportAssignment>
export class ExportDeclarationConverter extends ConverterNodeComponent<ts.ExportDeclaration> {
supports = [ts.SyntaxKind.ExportDeclaration];

@BindOption('mode')
mode!: SourceFileMode;

convert(context: Context, node: ts.ExportDeclaration): Reflection | undefined {
// It doesn't make sense to convert export declarations if we are pretending everything is global.
if (this.application.options.getValue('mode') === SourceFileMode.File) {
if (this.mode === SourceFileMode.File) {
return;
}

Expand All @@ -70,17 +79,39 @@ export class ExportDeclarationConverter extends ConverterNodeComponent<ts.Export
}

if (node.exportClause) { // export { a, a as b }
node.exportClause.elements.forEach(specifier => {
const elements = (node.exportClause as ts.NamedExports).elements || [];
elements.forEach(specifier => {
const source = context.getSymbolAtLocation(specifier.name);
const target = context.resolveAliasedSymbol(context.getSymbolAtLocation(specifier.propertyName ?? specifier.name));
if (source && target) {
// If the original declaration is in this file, export {} was used with something
// defined in this file and we don't need to create a reference unless the name is different.
if (!node.moduleSpecifier && !specifier.propertyName) {
return;
}
if (this.mode === SourceFileMode.Library) {
// export declaration is always unique: no need of loop
const declaration = target.declarations?.[0];
if (declaration) {
const declarationReflection = this.owner.convertNode(context, declaration);
if (declarationReflection) {
if (!declarationReflection.kindOf([ReflectionKind.ClassOrInterface, ReflectionKind.SomeModule])) {
// rename the declaration to the exported one
declarationReflection.name = source.name;
declarationReflection.flags.setFlag(ReflectionFlag.Exported, true);
} else if (declarationReflection.name !== source.name) {
// create a extra reference to the declaration
declarationReflection.flags.setFlag(ReflectionFlag.Exported, false);
createReferenceReflection(context, source, target);
} else {
declarationReflection.flags.setFlag(ReflectionFlag.Exported, true);
}
}
}
} else {
// If the original declaration is in this file, export {} was used with something
// defined in this file and we don't need to create a reference unless the name is different.
if (!node.moduleSpecifier && !specifier.propertyName) {
return;
}

createReferenceReflection(context, source, target);
createReferenceReflection(context, source, target);
}
}
});
} else if (node.moduleSpecifier) { // export * from ...
Expand Down
2 changes: 1 addition & 1 deletion src/lib/converter/plugins/DecoratorPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class DecoratorPlugin extends ConverterComponent {

const type = context.checker.getTypeAtLocation(identifier);
if (type && type.symbol) {
const fqn = context.checker.getFullyQualifiedName(type.symbol);
const fqn = context.getFullyQualifiedName(type.symbol);
info.type = new ReferenceType(info.name, fqn);

if (callExpression && callExpression.arguments) {
Expand Down
Loading