Skip to content

Commit

Permalink
fix: External symbols should not be referenced
Browse files Browse the repository at this point in the history
As discussed in #1166.
  • Loading branch information
Gerrit0 committed Jan 14, 2020
1 parent ef0c17e commit 8005b21
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 12 deletions.
34 changes: 28 additions & 6 deletions src/lib/converter/context.ts
Expand Up @@ -205,12 +205,8 @@ export class Context {
* @param callback The callback that should be executed.
*/
withSourceFile(node: ts.SourceFile, callback: Function) {
let isExternal = !this.fileNames.includes(node.fileName);
if (!isExternal && this.externalPattern) {
isExternal = this.externalPattern.some(mm => mm.match(node.fileName));
}

if (isExternal && this.converter.excludeExternals) {
const isExternal = this.isExternalFile(node.fileName);
if (this.isOutsideDocumentation(node.fileName, isExternal)) {
return;
}

Expand Down Expand Up @@ -343,6 +339,32 @@ export class Context {
return target;
}

/**
* Determines if the given file is outside of the project's generated documentation.
* This is not, and is not intended to be, foolproof. Plugins may remove reflections
* in the file that this method returns true for, See GH#1166 for discussion on tradeoffs.
*
* @param fileName
* @internal
*/
isOutsideDocumentation(fileName: string, isExternal = this.isExternalFile(fileName)) {
return isExternal && this.converter.excludeExternals;
}

/**
* Checks if the given file is external.
*
* @param fileName
* @internal
*/
isExternalFile(fileName: string) {
let isExternal = !this.fileNames.includes(fileName);
if (!isExternal && this.externalPattern) {
isExternal = this.externalPattern.some(mm => mm.match(fileName));
}
return isExternal;
}

/**
* Convert the given list of type parameter declarations into a type mapping.
*
Expand Down
8 changes: 7 additions & 1 deletion src/lib/converter/factories/reference.ts
Expand Up @@ -28,11 +28,17 @@ export function createReferenceType(context: Context, symbol: ts.Symbol | undefi
return new ReferenceType(name, context.checker.getFullyQualifiedName(symbol));
}

export function createReferenceReflection(context: Context, source: ts.Symbol, target: ts.Symbol): ReferenceReflection {
export function createReferenceReflection(context: Context, source: ts.Symbol, target: ts.Symbol): ReferenceReflection | undefined {
if (!(context.scope instanceof ContainerReflection)) {
throw new Error('Cannot add reference to a non-container');
}

// If any declaration is outside, the symbol should be considered outside. Some declarations may
// be inside due to declaration merging.
if (target.declarations.some(d => context.isOutsideDocumentation(d.getSourceFile().fileName))) {
return;
}

const reflection = new ReferenceReflection(source.name, [ReferenceState.Unresolved, context.checker.getFullyQualifiedName(target)], context.scope);
reflection.flags.setFlag(ReflectionFlag.Exported, true); // References are exported by necessity
if (!context.scope.children) {
Expand Down
3 changes: 2 additions & 1 deletion src/lib/converter/nodes/export.ts
Expand Up @@ -93,7 +93,8 @@ export class ExportDeclarationConverter extends ConverterNodeComponent<ts.Export
}
const source = context.checker.tryGetMemberInModuleExports(key.toString().replace(/^__/, '_'), thisModule);
if (source) {
createReferenceReflection(context, source, symbol);
const target = context.resolveAliasedSymbol(symbol);
createReferenceReflection(context, source, target);
}
});
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/converter/exports/mod.ts
Expand Up @@ -27,3 +27,9 @@ export default function() {
* @hidden
*/
export const hidden = true;

/**
* This export is from a file external to the documentation that will not be included in the resulting docs.
* No reference should be created.
*/
export { Node } from 'typescript';
4 changes: 2 additions & 2 deletions src/test/converter/exports/specs-without-exported.json
Expand Up @@ -32,7 +32,7 @@
"flags": {
"isExported": true
},
"target": 11
"target": 10
},
{
"id": 17,
Expand All @@ -42,7 +42,7 @@
"flags": {
"isExported": true
},
"target": 12
"target": 10
},
{
"id": 19,
Expand Down
4 changes: 2 additions & 2 deletions src/test/converter/exports/specs.json
Expand Up @@ -32,7 +32,7 @@
"flags": {
"isExported": true
},
"target": 21
"target": 20
},
{
"id": 29,
Expand All @@ -42,7 +42,7 @@
"flags": {
"isExported": true
},
"target": 22
"target": 20
},
{
"id": 31,
Expand Down

0 comments on commit 8005b21

Please sign in to comment.