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
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
53 changes: 34 additions & 19 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions src/compiler/declarationEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand All @@ -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;
}
}
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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 ");
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions tests/baselines/reference/controlFlowBinaryOrExpression.types
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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; }
Expand Down
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;
};
Expand All @@ -18,7 +18,7 @@ function f(_a, _b, _c) {
}


//// [declarationEmit_bindingPatterns.d.ts]
//// [declarationEmitBindingPatterns.d.ts]
declare const k: ({x: z}: {
x?: string;
}) => void;
Expand Down
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
Expand Down
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
Expand Down Expand Up @@ -36,7 +36,7 @@ export class C4 {
}
}

//// [declarationEmit_classMemberNameConflict.js]
//// [declarationEmitClassMemberNameConflict.js]
"use strict";
var C1 = (function () {
function C1() {
Expand Down Expand Up @@ -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;
Expand Down
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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//// [declarationEmit_classMemberNameConflict2.ts]
//// [declarationEmitClassMemberNameConflict2.ts]

const Bar = 'bar';

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

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


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