-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Serialize inferred type alias when type alias symbol is not accessible #10953
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
Changes from all commits
f26921b
dc501f4
a4864d0
e881081
e60e72e
f137c92
58976ff
94cd9ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1758,15 +1758,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, | ||
|
@@ -1830,7 +1838,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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make :) |
||
let aliasesToMakeVisible: AnyImportSyntax[]; | ||
if (forEach(symbol.declarations, declaration => !getIsDeclarationVisible(declaration))) { | ||
return undefined; | ||
|
@@ -1846,14 +1854,19 @@ namespace ts { | |
if (anyImportSyntax && | ||
!(getModifierFlags(anyImportSyntax) & ModifierFlags.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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems not completely correct. Because You wouldn't be making the alias visible yet returning that the declaration is visible. At the consumer side say result of all this is that the symbol is visible. (not the false case that you have marked) then you definitely going to get error in the .d.ts file because you should have made the alias visible for the correct emit of the declaration file but it isn't going to be emitted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I get it why it works because here you just check if this is visible but the tracking of the symbol does happen again when actually writing the symbol. Probably we need a comment here explaining that. |
||
// 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; | ||
} | ||
|
@@ -1888,7 +1901,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 | ||
|
@@ -2163,7 +2176,9 @@ 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) { | ||
else if (!(flags & TypeFormatFlags.InTypeAlias) && type.flags & (TypeFlags.Anonymous | TypeFlags.UnionOrIntersection) && type.aliasSymbol && | ||
isSymbolAccessible(type.aliasSymbol, enclosingDeclaration, SymbolFlags.Type, /*shouldComputeAliasesToMakeVisible*/ false).accessibility === SymbolAccessibility.Accessible) { | ||
// Only write out inferred type with its corresponding type-alias if type-alias is visible | ||
const typeArguments = type.aliasTypeArguments; | ||
writeSymbolTypeReference(type.aliasSymbol, typeArguments, 0, typeArguments ? typeArguments.length : 0, nextFlags); | ||
} | ||
|
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 |
---|---|---|
@@ -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 |
---|---|---|
@@ -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 |
---|---|---|
@@ -0,0 +1,37 @@ | ||
=== tests/cases/compiler/declarationEmitClassMemberNameConflict2.ts === | ||
|
||
const Bar = 'bar'; | ||
>Bar : Symbol(Bar, Decl(declarationEmitClassMemberNameConflict2.ts, 1, 5)) | ||
|
||
enum Hello { | ||
>Hello : Symbol(Hello, Decl(declarationEmitClassMemberNameConflict2.ts, 1, 18)) | ||
|
||
World | ||
>World : Symbol(Hello.World, Decl(declarationEmitClassMemberNameConflict2.ts, 3, 12)) | ||
} | ||
|
||
enum Hello1 { | ||
>Hello1 : Symbol(Hello1, Decl(declarationEmitClassMemberNameConflict2.ts, 5, 1)) | ||
|
||
World1 | ||
>World1 : Symbol(Hello1.World1, Decl(declarationEmitClassMemberNameConflict2.ts, 7, 13)) | ||
} | ||
|
||
class Foo { | ||
>Foo : Symbol(Foo, Decl(declarationEmitClassMemberNameConflict2.ts, 9, 1)) | ||
|
||
// Same names + string => OK | ||
Bar = Bar; | ||
>Bar : Symbol(Foo.Bar, Decl(declarationEmitClassMemberNameConflict2.ts, 11, 11)) | ||
>Bar : Symbol(Bar, Decl(declarationEmitClassMemberNameConflict2.ts, 1, 5)) | ||
|
||
// Same names + enum => OK | ||
Hello = Hello; | ||
>Hello : Symbol(Foo.Hello, Decl(declarationEmitClassMemberNameConflict2.ts, 13, 14)) | ||
>Hello : Symbol(Hello, Decl(declarationEmitClassMemberNameConflict2.ts, 1, 18)) | ||
|
||
// Different names + enum => OK | ||
Hello2 = Hello1; | ||
>Hello2 : Symbol(Foo.Hello2, Decl(declarationEmitClassMemberNameConflict2.ts, 16, 18)) | ||
>Hello1 : Symbol(Hello1, Decl(declarationEmitClassMemberNameConflict2.ts, 5, 1)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a Node containing reference to the symbol?