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

[Release-2.0.5] Port fix from master to release-2.0.5: Serialize type alias when type-alias is not accessible and emit generic #11392

Merged
merged 9 commits into from
Oct 6, 2016
Merged
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
53 changes: 34 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
@@ -1744,15 +1744,23 @@ namespace ts {
return false;
}

function isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessibilityResult {
/**
* Check if the given symbol in given enclosing declaration is accessible and mark all associated alias to be visible if requested
*
* @param symbol a Symbol to check if accessible
* @param enclosingDeclaration a Node containing reference to the symbol
* @param meaning a SymbolFlags to check if such meaning of the symbol is accessible
* @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, enclosingDeclaration: Node, meaning: SymbolFlags, shouldComputeAliasesToMakeVisible: boolean): SymbolAccessibilityResult {
if (symbol && enclosingDeclaration && !(symbol.flags & SymbolFlags.TypeParameter)) {
const initialSymbol = symbol;
let meaningToLook = meaning;
while (symbol) {
// Symbol is accessible if it by itself is accessible
const accessibleSymbolChain = getAccessibleSymbolChain(symbol, enclosingDeclaration, meaningToLook, /*useOnlyExternalAliasing*/ false);
if (accessibleSymbolChain) {
const hasAccessibleDeclarations = hasVisibleDeclarations(accessibleSymbolChain[0]);
const hasAccessibleDeclarations = hasVisibleDeclarations(accessibleSymbolChain[0], shouldComputeAliasesToMakeVisible);
if (!hasAccessibleDeclarations) {
return <SymbolAccessibilityResult>{
accessibility: SymbolAccessibility.NotAccessible,
@@ -1816,7 +1824,7 @@ namespace ts {
return isAmbientModule(declaration) || (declaration.kind === SyntaxKind.SourceFile && isExternalOrCommonJsModule(<SourceFile>declaration));
}

function hasVisibleDeclarations(symbol: Symbol): SymbolVisibilityResult {
function hasVisibleDeclarations(symbol: Symbol, shouldComputeAliasToMakeVisible: boolean): SymbolVisibilityResult {
let aliasesToMakeVisible: AnyImportSyntax[];
if (forEach(symbol.declarations, declaration => !getIsDeclarationVisible(declaration))) {
return undefined;
@@ -1832,14 +1840,19 @@ namespace ts {
if (anyImportSyntax &&
!(anyImportSyntax.flags & NodeFlags.Export) && // import clause without export
isDeclarationVisible(<Declaration>anyImportSyntax.parent)) {
getNodeLinks(declaration).isVisible = true;
if (aliasesToMakeVisible) {
if (!contains(aliasesToMakeVisible, anyImportSyntax)) {
aliasesToMakeVisible.push(anyImportSyntax);
// In function "buildTypeDisplay" where we decide whether to write type-alias or serialize types,
// we want to just check if type- alias is accessible or not but we don't care about emitting those alias at that time
// since we will do the emitting later in trackSymbol.
if (shouldComputeAliasToMakeVisible) {
getNodeLinks(declaration).isVisible = true;
if (aliasesToMakeVisible) {
if (!contains(aliasesToMakeVisible, anyImportSyntax)) {
aliasesToMakeVisible.push(anyImportSyntax);
}
}
else {
aliasesToMakeVisible = [anyImportSyntax];
}
}
else {
aliasesToMakeVisible = [anyImportSyntax];
}
return true;
}
@@ -1874,7 +1887,7 @@ namespace ts {
const symbol = resolveName(enclosingDeclaration, (<Identifier>firstIdentifier).text, meaning, /*nodeNotFoundErrorMessage*/ undefined, /*nameArg*/ undefined);

// Verify if the symbol is accessible
return (symbol && hasVisibleDeclarations(symbol)) || <SymbolVisibilityResult>{
return (symbol && hasVisibleDeclarations(symbol, /*shouldComputeAliasToMakeVisible*/ true)) || <SymbolVisibilityResult>{
accessibility: SymbolAccessibility.NotAccessible,
errorSymbolName: getTextOfNode(firstIdentifier),
errorNode: firstIdentifier
@@ -2152,14 +2165,16 @@ namespace ts {
// The specified symbol flags need to be reinterpreted as type flags
buildSymbolDisplay(type.symbol, writer, enclosingDeclaration, SymbolFlags.Type, SymbolFormatFlags.None, nextFlags);
}
else if (!(flags & TypeFormatFlags.InTypeAlias) && type.flags & (TypeFlags.Anonymous | TypeFlags.UnionOrIntersection) && type.aliasSymbol) {
if (type.flags & TypeFlags.Anonymous || !(flags & TypeFormatFlags.UseTypeAliasValue)) {
const typeArguments = type.aliasTypeArguments;
writeSymbolTypeReference(type.aliasSymbol, typeArguments, 0, typeArguments ? typeArguments.length : 0, nextFlags);
}
else {
writeUnionOrIntersectionType(<UnionOrIntersectionType>type, nextFlags);
}
else if (!(flags & TypeFormatFlags.InTypeAlias) && ((type.flags & TypeFlags.Anonymous && !(<AnonymousType>type).target) || type.flags & TypeFlags.UnionOrIntersection) && type.aliasSymbol &&
Copy link
Member

Choose a reason for hiding this comment

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

This condition looks too complicated. Can you make it readable by adding comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a comment already here

isSymbolAccessible(type.aliasSymbol, enclosingDeclaration, SymbolFlags.Type, /*shouldComputeAliasesToMakeVisible*/ false).accessibility === SymbolAccessibility.Accessible) {
// We emit inferred type as type-alias at the current location if all the following is true
// the input type is has alias symbol that is accessible
// the input type is a union, intersection or anonymous type that is fully instantiated (if not we want to keep dive into)
// e.g.: export type Bar<X, Y> = () => [X, Y];
// export type Foo<Y> = Bar<any, Y>;
// export const y = (x: Foo<string>) => 1 // we want to emit as ...x: () => [any, string])
const typeArguments = type.aliasTypeArguments;
writeSymbolTypeReference(type.aliasSymbol, typeArguments, 0, typeArguments ? typeArguments.length : 0, nextFlags);
}
else if (type.flags & TypeFlags.UnionOrIntersection) {
writeUnionOrIntersectionType(<UnionOrIntersectionType>type, nextFlags);
10 changes: 5 additions & 5 deletions src/compiler/declarationEmitter.ts
Original file line number Diff line number Diff line change
@@ -306,7 +306,7 @@ namespace ts {
}

function trackSymbol(symbol: Symbol, enclosingDeclaration?: Node, meaning?: SymbolFlags) {
handleSymbolAccessibilityError(resolver.isSymbolAccessible(symbol, enclosingDeclaration, meaning));
handleSymbolAccessibilityError(resolver.isSymbolAccessible(symbol, enclosingDeclaration, meaning, /*shouldComputeAliasesToMakeVisible*/ true));
recordTypeReferenceDirectivesIfNecessary(resolver.getTypeReferenceDirectivesForSymbol(symbol, meaning));
}

@@ -327,7 +327,7 @@ namespace ts {
}
else {
errorNameNode = declaration.name;
resolver.writeTypeOfDeclaration(declaration, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction | TypeFormatFlags.UseTypeAliasValue, writer);
resolver.writeTypeOfDeclaration(declaration, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction, writer);
errorNameNode = undefined;
}
}
@@ -341,7 +341,7 @@ namespace ts {
}
else {
errorNameNode = signature.name;
resolver.writeReturnTypeOfSignatureDeclaration(signature, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction | TypeFormatFlags.UseTypeAliasValue, writer);
resolver.writeReturnTypeOfSignatureDeclaration(signature, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction, writer);
errorNameNode = undefined;
}
}
@@ -563,7 +563,7 @@ namespace ts {
write(tempVarName);
write(": ");
writer.getSymbolAccessibilityDiagnostic = getDefaultExportAccessibilityDiagnostic;
resolver.writeTypeOfExpression(node.expression, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction | TypeFormatFlags.UseTypeAliasValue, writer);
resolver.writeTypeOfExpression(node.expression, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction, writer);
write(";");
writeLine();
write(node.isExportEquals ? "export = " : "export default ");
@@ -1025,7 +1025,7 @@ namespace ts {
}
else {
writer.getSymbolAccessibilityDiagnostic = getHeritageClauseVisibilityError;
resolver.writeBaseConstructorTypeOfClass(<ClassLikeDeclaration>enclosingDeclaration, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction | TypeFormatFlags.UseTypeAliasValue, writer);
resolver.writeBaseConstructorTypeOfClass(<ClassLikeDeclaration>enclosingDeclaration, enclosingDeclaration, TypeFormatFlags.UseTypeOfFunction, writer);
}

function getHeritageClauseVisibilityError(symbolAccessibilityResult: SymbolAccessibilityResult): SymbolAccessibilityDiagnostic {
3 changes: 1 addition & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
@@ -1949,7 +1949,6 @@ namespace ts {
UseFullyQualifiedType = 0x00000080, // Write out the fully qualified type name (eg. Module.Type, instead of Type)
InFirstTypeArgument = 0x00000100, // Writing first type argument of the instantiated type
InTypeAlias = 0x00000200, // Writing type in type alias declaration
UseTypeAliasValue = 0x00000400, // Serialize the type instead of using type-alias. This is needed when we emit declaration file.
}

export const enum SymbolFormatFlags {
@@ -2052,7 +2051,7 @@ namespace ts {
writeReturnTypeOfSignatureDeclaration(signatureDeclaration: SignatureDeclaration, enclosingDeclaration: Node, flags: TypeFormatFlags, writer: SymbolWriter): void;
writeTypeOfExpression(expr: Expression, enclosingDeclaration: Node, flags: TypeFormatFlags, writer: SymbolWriter): void;
writeBaseConstructorTypeOfClass(node: ClassLikeDeclaration, enclosingDeclaration: Node, flags: TypeFormatFlags, writer: SymbolWriter): void;
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags): SymbolAccessibilityResult;
isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags, shouldComputeAliasToMarkVisible: boolean): SymbolAccessibilityResult;
isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult;
// Returns the constant value this property access resolves to, or 'undefined' for a non-constant
getConstantValue(node: EnumMember | PropertyAccessExpression | ElementAccessExpression): number;
12 changes: 6 additions & 6 deletions tests/baselines/reference/controlFlowBinaryOrExpression.types
Original file line number Diff line number Diff line change
@@ -62,21 +62,21 @@ declare function isHTMLCollection(sourceObj: any): sourceObj is HTMLCollection;
>HTMLCollection : HTMLCollection

type EventTargetLike = {a: string} | HTMLCollection | NodeList;
>EventTargetLike : EventTargetLike
>EventTargetLike : NodeList | HTMLCollection | { a: string; }
>a : string
>HTMLCollection : HTMLCollection
>NodeList : NodeList

var sourceObj: EventTargetLike = <any>undefined;
>sourceObj : EventTargetLike
>EventTargetLike : EventTargetLike
>sourceObj : NodeList | HTMLCollection | { a: string; }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be sourceObj: EventTargetLike?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like it should. However what happen is that the file is an external module. So when we are in BuildTypeDisplay, checking whether EventTargetLike is visible, we decide it is not accessible as the type is not exported

>EventTargetLike : NodeList | HTMLCollection | { a: string; }
><any>undefined : any
>undefined : undefined

if (isNodeList(sourceObj)) {
>isNodeList(sourceObj) : boolean
>isNodeList : (sourceObj: any) => sourceObj is NodeList
>sourceObj : EventTargetLike
>sourceObj : NodeList | HTMLCollection | { a: string; }

sourceObj.length;
>sourceObj.length : number
@@ -87,7 +87,7 @@ if (isNodeList(sourceObj)) {
if (isHTMLCollection(sourceObj)) {
>isHTMLCollection(sourceObj) : boolean
>isHTMLCollection : (sourceObj: any) => sourceObj is HTMLCollection
>sourceObj : EventTargetLike
>sourceObj : NodeList | HTMLCollection | { a: string; }

sourceObj.length;
>sourceObj.length : number
@@ -99,7 +99,7 @@ if (isNodeList(sourceObj) || isHTMLCollection(sourceObj)) {
>isNodeList(sourceObj) || isHTMLCollection(sourceObj) : boolean
>isNodeList(sourceObj) : boolean
>isNodeList : (sourceObj: any) => sourceObj is NodeList
>sourceObj : EventTargetLike
>sourceObj : NodeList | HTMLCollection | { a: string; }
>isHTMLCollection(sourceObj) : boolean
>isHTMLCollection : (sourceObj: any) => sourceObj is HTMLCollection
>sourceObj : HTMLCollection | { a: string; }
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
//// [declarationEmitArrayTypesFromGenericArrayUsage.ts]
interface A extends Array<string> { }


//// [declarationEmitArrayTypesFromGenericArrayUsage.js]


//// [declarationEmitArrayTypesFromGenericArrayUsage.d.ts]
interface A extends Array<string> {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== tests/cases/compiler/declarationEmitArrayTypesFromGenericArrayUsage.ts ===
interface A extends Array<string> { }
>A : Symbol(A, Decl(declarationEmitArrayTypesFromGenericArrayUsage.ts, 0, 0))
>Array : Symbol(Array, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== tests/cases/compiler/declarationEmitArrayTypesFromGenericArrayUsage.ts ===
interface A extends Array<string> { }
>A : A
>Array : T[]

Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//// [declarationEmit_bindingPatterns.ts]
//// [declarationEmitBindingPatterns.ts]

const k = ({x: z = 'y'}) => { }

var a;
function f({} = a, [] = a, { p: {} = a} = a) {
}

//// [declarationEmit_bindingPatterns.js]
//// [declarationEmitBindingPatterns.js]
var k = function (_a) {
var _b = _a.x, z = _b === void 0 ? 'y' : _b;
};
@@ -18,7 +18,7 @@ function f(_a, _b, _c) {
}


//// [declarationEmit_bindingPatterns.d.ts]
//// [declarationEmitBindingPatterns.d.ts]
declare const k: ({x: z}: {
x?: string;
}) => void;
17 changes: 17 additions & 0 deletions tests/baselines/reference/declarationEmitBindingPatterns.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
=== tests/cases/compiler/declarationEmitBindingPatterns.ts ===

const k = ({x: z = 'y'}) => { }
>k : Symbol(k, Decl(declarationEmitBindingPatterns.ts, 1, 5))
>x : Symbol(x)
>z : Symbol(z, Decl(declarationEmitBindingPatterns.ts, 1, 12))

var a;
>a : Symbol(a, Decl(declarationEmitBindingPatterns.ts, 3, 3))

function f({} = a, [] = a, { p: {} = a} = a) {
>f : Symbol(f, Decl(declarationEmitBindingPatterns.ts, 3, 6))
>a : Symbol(a, Decl(declarationEmitBindingPatterns.ts, 3, 3))
>a : Symbol(a, Decl(declarationEmitBindingPatterns.ts, 3, 3))
>a : Symbol(a, Decl(declarationEmitBindingPatterns.ts, 3, 3))
>a : Symbol(a, Decl(declarationEmitBindingPatterns.ts, 3, 3))
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
=== tests/cases/compiler/declarationEmit_bindingPatterns.ts ===
=== tests/cases/compiler/declarationEmitBindingPatterns.ts ===

const k = ({x: z = 'y'}) => { }
>k : ({x: z}: { x?: string; }) => void
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//// [declarationEmit_classMemberNameConflict.ts]
//// [declarationEmitClassMemberNameConflict.ts]

export class C1 {
C1() { } // has to be the same as the class name
@@ -36,7 +36,7 @@ export class C4 {
}
}

//// [declarationEmit_classMemberNameConflict.js]
//// [declarationEmitClassMemberNameConflict.js]
"use strict";
var C1 = (function () {
function C1() {
@@ -93,7 +93,7 @@ var C4 = (function () {
exports.C4 = C4;


//// [declarationEmit_classMemberNameConflict.d.ts]
//// [declarationEmitClassMemberNameConflict.d.ts]
export declare class C1 {
C1(): void;
bar(): (t: typeof C1) => void;
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
=== tests/cases/compiler/declarationEmitClassMemberNameConflict.ts ===

export class C1 {
>C1 : Symbol(C1, Decl(declarationEmitClassMemberNameConflict.ts, 0, 0))

C1() { } // has to be the same as the class name
>C1 : Symbol(C1.C1, Decl(declarationEmitClassMemberNameConflict.ts, 1, 17))

bar() {
>bar : Symbol(C1.bar, Decl(declarationEmitClassMemberNameConflict.ts, 2, 12))

return function (t: typeof C1) {
>t : Symbol(t, Decl(declarationEmitClassMemberNameConflict.ts, 5, 25))
>C1 : Symbol(C1, Decl(declarationEmitClassMemberNameConflict.ts, 0, 0))

};
}
}

export class C2 {
>C2 : Symbol(C2, Decl(declarationEmitClassMemberNameConflict.ts, 8, 1))

C2: any // has to be the same as the class name
>C2 : Symbol(C2.C2, Decl(declarationEmitClassMemberNameConflict.ts, 10, 17))

bar() {
>bar : Symbol(C2.bar, Decl(declarationEmitClassMemberNameConflict.ts, 11, 11))

return function (t: typeof C2) {
>t : Symbol(t, Decl(declarationEmitClassMemberNameConflict.ts, 14, 25))
>C2 : Symbol(C2, Decl(declarationEmitClassMemberNameConflict.ts, 8, 1))

};
}
}

export class C3 {
>C3 : Symbol(C3, Decl(declarationEmitClassMemberNameConflict.ts, 17, 1))

get C3() { return 0; } // has to be the same as the class name
>C3 : Symbol(C3.C3, Decl(declarationEmitClassMemberNameConflict.ts, 19, 17))

bar() {
>bar : Symbol(C3.bar, Decl(declarationEmitClassMemberNameConflict.ts, 20, 26))

return function (t: typeof C3) {
>t : Symbol(t, Decl(declarationEmitClassMemberNameConflict.ts, 23, 25))
>C3 : Symbol(C3, Decl(declarationEmitClassMemberNameConflict.ts, 17, 1))

};
}
}

export class C4 {
>C4 : Symbol(C4, Decl(declarationEmitClassMemberNameConflict.ts, 26, 1))

set C4(v) { } // has to be the same as the class name
>C4 : Symbol(C4.C4, Decl(declarationEmitClassMemberNameConflict.ts, 28, 17))
>v : Symbol(v, Decl(declarationEmitClassMemberNameConflict.ts, 29, 11))

bar() {
>bar : Symbol(C4.bar, Decl(declarationEmitClassMemberNameConflict.ts, 29, 17))

return function (t: typeof C4) {
>t : Symbol(t, Decl(declarationEmitClassMemberNameConflict.ts, 32, 25))
>C4 : Symbol(C4, Decl(declarationEmitClassMemberNameConflict.ts, 26, 1))

};
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
=== tests/cases/compiler/declarationEmit_classMemberNameConflict.ts ===
=== tests/cases/compiler/declarationEmitClassMemberNameConflict.ts ===

export class C1 {
>C1 : C1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//// [declarationEmit_classMemberNameConflict2.ts]
//// [declarationEmitClassMemberNameConflict2.ts]

const Bar = 'bar';

@@ -21,7 +21,7 @@ class Foo {
Hello2 = Hello1;
}

//// [declarationEmit_classMemberNameConflict2.js]
//// [declarationEmitClassMemberNameConflict2.js]
var Bar = 'bar';
var Hello;
(function (Hello) {
@@ -44,7 +44,7 @@ var Foo = (function () {
}());


//// [declarationEmit_classMemberNameConflict2.d.ts]
//// [declarationEmitClassMemberNameConflict2.d.ts]
declare const Bar: string;
declare enum Hello {
World = 0,
Loading