Skip to content

Fix @implements emit for namespaced base types #38688

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

Merged
merged 8 commits into from
Jun 12, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"node": ">=4.2.0"
},
"devDependencies": {
"@octokit/rest": "latest",
"@octokit/rest": "17.11.2",
"@types/browserify": "latest",
"@types/chai": "latest",
"@types/convert-source-map": "latest",
Expand Down
48 changes: 34 additions & 14 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3856,16 +3856,21 @@ namespace ts {
}

function isTypeSymbolAccessible(typeSymbol: Symbol, enclosingDeclaration: Node | undefined): boolean {
const access = isSymbolAccessible(typeSymbol, enclosingDeclaration, SymbolFlags.Type, /*shouldComputeAliasesToMakeVisible*/ false);
const access = isSymbolAccessibleWorker(typeSymbol, enclosingDeclaration, SymbolFlags.Type, /*shouldComputeAliasesToMakeVisible*/ false, /*allowModules*/ true);
return access.accessibility === SymbolAccessibility.Accessible;
}

function isValueSymbolAccessible(typeSymbol: Symbol, enclosingDeclaration: Node | undefined): boolean {
const access = isSymbolAccessible(typeSymbol, enclosingDeclaration, SymbolFlags.Value, /*shouldComputeAliasesToMakeVisible*/ false);
const access = isSymbolAccessibleWorker(typeSymbol, enclosingDeclaration, SymbolFlags.Value, /*shouldComputeAliasesToMakeVisible*/ false, /*allowModules*/ true);
return access.accessibility === SymbolAccessibility.Accessible;
}

function isAnySymbolAccessible(symbols: Symbol[] | undefined, enclosingDeclaration: Node | undefined, initialSymbol: Symbol, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean): SymbolAccessibilityResult | undefined {
function isSymbolAccessibleByFlags(typeSymbol: Symbol, enclosingDeclaration: Node | undefined, flags: SymbolFlags): boolean {
const access = isSymbolAccessibleWorker(typeSymbol, enclosingDeclaration, flags, /*shouldComputeAliasesToMakeVisible*/ false, /*allowModules*/ false);
return access.accessibility === SymbolAccessibility.Accessible;
}

function isAnySymbolAccessible(symbols: Symbol[] | undefined, enclosingDeclaration: Node | undefined, initialSymbol: Symbol, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean, allowModules: boolean): SymbolAccessibilityResult | undefined {
if (!length(symbols)) return;

let hadAccessibleChain: Symbol | undefined;
Expand All @@ -3880,7 +3885,7 @@ namespace ts {
return hasAccessibleDeclarations;
}
}
else {
else if (allowModules) {
if (some(symbol.declarations, hasNonGlobalAugmentationExternalModuleSymbol)) {
if (shouldComputeAliasesToMakeVisible) {
earlyModuleBail = true;
Expand Down Expand Up @@ -3920,7 +3925,7 @@ namespace ts {
containers = [getSymbolOfNode(firstDecl.parent)];
}
}
const parentResult = isAnySymbolAccessible(containers, enclosingDeclaration, initialSymbol, initialSymbol === symbol ? getQualifiedLeftMeaning(meaning) : meaning, shouldComputeAliasesToMakeVisible);
const parentResult = isAnySymbolAccessible(containers, enclosingDeclaration, initialSymbol, initialSymbol === symbol ? getQualifiedLeftMeaning(meaning) : meaning, shouldComputeAliasesToMakeVisible, allowModules);
if (parentResult) {
return parentResult;
}
Expand Down Expand Up @@ -3950,8 +3955,12 @@ namespace ts {
* @param shouldComputeAliasToMakeVisible a boolean value to indicate whether to return aliases to be mark visible in case the symbol is accessible
*/
function isSymbolAccessible(symbol: Symbol | undefined, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean): SymbolAccessibilityResult {
return isSymbolAccessibleWorker(symbol, enclosingDeclaration, meaning, shouldComputeAliasesToMakeVisible, /*allowModules*/ true);
}

function isSymbolAccessibleWorker(symbol: Symbol | undefined, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean, allowModules: boolean): SymbolAccessibilityResult {
if (symbol && enclosingDeclaration) {
const result = isAnySymbolAccessible([symbol], enclosingDeclaration, symbol, meaning, shouldComputeAliasesToMakeVisible);
const result = isAnySymbolAccessible([symbol], enclosingDeclaration, symbol, meaning, shouldComputeAliasesToMakeVisible, allowModules);
if (result) {
return result;
}
Expand Down Expand Up @@ -6211,7 +6220,7 @@ namespace ts {
const constructSignatures = serializeSignatures(SignatureKind.Construct, interfaceType, baseType, SyntaxKind.ConstructSignature) as ConstructSignatureDeclaration[];
const indexSignatures = serializeIndexSignatures(interfaceType, baseType);

const heritageClauses = !length(baseTypes) ? undefined : [createHeritageClause(SyntaxKind.ExtendsKeyword, mapDefined(baseTypes, b => trySerializeAsTypeReference(b)))];
const heritageClauses = !length(baseTypes) ? undefined : [createHeritageClause(SyntaxKind.ExtendsKeyword, mapDefined(baseTypes, b => trySerializeAsTypeReference(b, SymbolFlags.Value)))];
addResult(createInterfaceDeclaration(
/*decorators*/ undefined,
/*modifiers*/ undefined,
Expand Down Expand Up @@ -6371,15 +6380,15 @@ namespace ts {
const typeParamDecls = map(localParams, p => typeParameterToDeclaration(p, context));
const classType = getDeclaredTypeOfClassOrInterface(symbol);
const baseTypes = getBaseTypes(classType);
const implementsTypes = getImplementsTypes(classType);
const implementsExpressions = mapDefined(getImplementsTypes(classType), serializeImplementedType);
const staticType = getTypeOfSymbol(symbol);
const isClass = !!staticType.symbol?.valueDeclaration && isClassLike(staticType.symbol.valueDeclaration);
const staticBaseType = isClass
? getBaseConstructorTypeOfClass(staticType as InterfaceType)
: anyType;
const heritageClauses = [
...!length(baseTypes) ? [] : [createHeritageClause(SyntaxKind.ExtendsKeyword, map(baseTypes, b => serializeBaseType(b, staticBaseType, localName)))],
...!length(implementsTypes) ? [] : [createHeritageClause(SyntaxKind.ImplementsKeyword, map(implementsTypes, b => serializeBaseType(b, staticBaseType, localName)))]
...!length(implementsExpressions) ? [] : [createHeritageClause(SyntaxKind.ImplementsKeyword, implementsExpressions)]
];
const symbolProps = getNonInterhitedProperties(classType, baseTypes, getPropertiesOfType(classType));
const publicSymbolProps = filter(symbolProps, s => {
Expand Down Expand Up @@ -6870,7 +6879,7 @@ namespace ts {
}

function serializeBaseType(t: Type, staticType: Type, rootName: string) {
const ref = trySerializeAsTypeReference(t);
const ref = trySerializeAsTypeReference(t, SymbolFlags.Value);
if (ref) {
return ref;
}
Expand All @@ -6882,23 +6891,34 @@ namespace ts {
return createExpressionWithTypeArguments(/*typeArgs*/ undefined, createIdentifier(tempName));
}

function trySerializeAsTypeReference(t: Type) {
function trySerializeAsTypeReference(t: Type, flags: SymbolFlags) {
let typeArgs: TypeNode[] | undefined;
let reference: Expression | undefined;

// We don't use `isValueSymbolAccessible` below. since that considers alternative containers (like modules)
// which we can't write out in a syntactically valid way as an expression
if ((t as TypeReference).target && getAccessibleSymbolChain((t as TypeReference).target.symbol, enclosingDeclaration, SymbolFlags.Value, /*useOnlyExternalAliasing*/ false)) {
if ((t as TypeReference).target && isSymbolAccessibleByFlags((t as TypeReference).target.symbol, enclosingDeclaration, flags)) {
typeArgs = map(getTypeArguments(t as TypeReference), t => typeToTypeNodeHelper(t, context));
reference = symbolToExpression((t as TypeReference).target.symbol, context, SymbolFlags.Type);
}
else if (t.symbol && getAccessibleSymbolChain(t.symbol, enclosingDeclaration, SymbolFlags.Value, /*useOnlyExternalAliasing*/ false)) {
else if (t.symbol && isSymbolAccessibleByFlags(t.symbol, enclosingDeclaration, flags)) {
reference = symbolToExpression(t.symbol, context, SymbolFlags.Type);
}
if (reference) {
return createExpressionWithTypeArguments(typeArgs, reference);
}
}

function serializeImplementedType(t: Type) {
const ref = trySerializeAsTypeReference(t, SymbolFlags.Type);
if (ref) {
return ref;
}
if (t.symbol) {
return createExpressionWithTypeArguments(/*typeArgs*/ undefined, symbolToExpression(t.symbol, context, SymbolFlags.Type));
}
}

function getUnusedName(input: string, symbol?: Symbol): string {
if (symbol) {
if (context.remappedSymbolNames!.has("" + getSymbolId(symbol))) {
Expand Down Expand Up @@ -24063,7 +24083,7 @@ namespace ts {
// if jsx emit was not react as there wont be error being emitted
reactSym.isReferenced = SymbolFlags.All;

// If react symbol is alias, mark it as refereced
// If react symbol is alias, mark it as referenced
if (reactSym.flags & SymbolFlags.Alias && !getTypeOnlyAliasDeclaration(reactSym)) {
markAliasSymbolAsReferenced(reactSym);
}
Expand Down
37 changes: 37 additions & 0 deletions tests/baselines/reference/jsdocImplements_namespacedInterface.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//// [tests/cases/conformance/jsdoc/jsdocImplements_namespacedInterface.ts] ////

//// [defs.d.ts]
declare namespace N {
interface A {
mNumber(): number;
}
interface AT<T> {
gen(): T;
}
}
//// [a.js]
/** @implements N.A */
class B {
mNumber() {
return 0;
}
}
/** @implements {N.AT<string>} */
class BAT {
gen() {
return "";
}
}




//// [a.d.ts]
/** @implements N.A */
declare class B implements N.A {
mNumber(): number;
}
/** @implements {N.AT<string>} */
declare class BAT implements N.AT<string> {
gen(): string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
=== /defs.d.ts ===
declare namespace N {
>N : Symbol(N, Decl(defs.d.ts, 0, 0))

interface A {
>A : Symbol(A, Decl(defs.d.ts, 0, 21))

mNumber(): number;
>mNumber : Symbol(A.mNumber, Decl(defs.d.ts, 1, 17))
}
interface AT<T> {
>AT : Symbol(AT, Decl(defs.d.ts, 3, 5))
>T : Symbol(T, Decl(defs.d.ts, 4, 17))

gen(): T;
>gen : Symbol(AT.gen, Decl(defs.d.ts, 4, 21))
>T : Symbol(T, Decl(defs.d.ts, 4, 17))
}
}
=== /a.js ===
/** @implements N.A */
class B {
>B : Symbol(B, Decl(a.js, 0, 0))

mNumber() {
>mNumber : Symbol(B.mNumber, Decl(a.js, 1, 9))

return 0;
}
}
/** @implements {N.AT<string>} */
class BAT {
>BAT : Symbol(BAT, Decl(a.js, 5, 1))

gen() {
>gen : Symbol(BAT.gen, Decl(a.js, 7, 11))

return "";
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
=== /defs.d.ts ===
declare namespace N {
interface A {
mNumber(): number;
>mNumber : () => number
}
interface AT<T> {
gen(): T;
>gen : () => T
}
}
=== /a.js ===
/** @implements N.A */
class B {
>B : B

mNumber() {
>mNumber : () => number

return 0;
>0 : 0
}
}
/** @implements {N.AT<string>} */
class BAT {
>BAT : BAT

gen() {
>gen : () => string

return "";
>"" : ""
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// @allowJs: true
// @checkJs: true
// @declaration: true
// @emitDeclarationOnly: true
// @outDir: ./out

// @Filename: /defs.d.ts
declare namespace N {
interface A {
mNumber(): number;
}
interface AT<T> {
gen(): T;
}
}
// @Filename: /a.js
/** @implements N.A */
class B {
mNumber() {
return 0;
}
}
/** @implements {N.AT<string>} */
class BAT {
gen() {
return "";
}
}