Skip to content

Remove unneeded ExportType and ExportNamespace flags #16766

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.

Sign up for GitHub

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
1 commit merged into from
Jul 13, 2017
Merged

Remove unneeded ExportType and ExportNamespace flags #16766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 5 additions & 9 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,8 @@ namespace ts {
}
}
else {
// Exported module members are given 2 symbols: A local symbol that is classified with an ExportValue,
// ExportType, or ExportContainer flag, and an associated export symbol with all the correct flags set
// on it. There are 2 main reasons:
// Exported module members are given 2 symbols: A local symbol that is classified with an ExportValue flag,
// and an associated export symbol with all the correct flags set on it. There are 2 main reasons:
//
// 1. We treat locals and exports of the same name as mutually exclusive within a container.
// That means the binder will issue a Duplicate Identifier error if you mix locals and exports
Expand All @@ -438,10 +437,7 @@ namespace ts {
(node as JSDocTypedefTag).name.kind === SyntaxKind.Identifier &&
((node as JSDocTypedefTag).name as Identifier).isInJSDocNamespace;
if ((!isAmbientModule(node) && (hasExportModifier || container.flags & NodeFlags.ExportContext)) || isJSDocTypedefInJSDocNamespace) {
const exportKind =
(symbolFlags & SymbolFlags.Value ? SymbolFlags.ExportValue : 0) |
(symbolFlags & SymbolFlags.Type ? SymbolFlags.ExportType : 0) |
(symbolFlags & SymbolFlags.Namespace ? SymbolFlags.ExportNamespace : 0);
const exportKind = symbolFlags & SymbolFlags.Value ? SymbolFlags.ExportValue : 0;
const local = declareSymbol(container.locals, /*parent*/ undefined, node, exportKind, symbolExcludes);
local.exportSymbol = declareSymbol(container.symbol.exports, container.symbol, node, symbolFlags, symbolExcludes);
node.localSymbol = local;
Expand Down Expand Up @@ -2288,7 +2284,7 @@ namespace ts {
// When we create a property via 'exports.foo = bar', the 'exports.foo' property access
// expression is the declaration
setCommonJsModuleIndicator(node);
declareSymbol(file.symbol.exports, file.symbol, <PropertyAccessExpression>node.left, SymbolFlags.Property | SymbolFlags.Export, SymbolFlags.None);
declareSymbol(file.symbol.exports, file.symbol, <PropertyAccessExpression>node.left, SymbolFlags.Property | SymbolFlags.ExportValue, SymbolFlags.None);
}

function isExportsOrModuleExportsOrAlias(node: Node): boolean {
Expand Down Expand Up @@ -2329,7 +2325,7 @@ namespace ts {

// 'module.exports = expr' assignment
setCommonJsModuleIndicator(node);
declareSymbol(file.symbol.exports, file.symbol, node, SymbolFlags.Property | SymbolFlags.Export | SymbolFlags.ValueModule, SymbolFlags.None);
declareSymbol(file.symbol.exports, file.symbol, node, SymbolFlags.Property | SymbolFlags.ExportValue | SymbolFlags.ValueModule, SymbolFlags.None);
}

function bindThisPropertyAssignment(node: BinaryExpression) {
Expand Down
30 changes: 17 additions & 13 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18809,7 +18809,7 @@ namespace ts {
// local symbol is undefined => this declaration is non-exported.
// however symbol might contain other declarations that are exported
symbol = getSymbolOfNode(node);
if (!(symbol.flags & SymbolFlags.Export)) {
if (!symbol.exportSymbol) {
// this is a pure local symbol (all declarations are non-exported) - no need to check anything
return;
}
Expand All @@ -18820,11 +18820,9 @@ namespace ts {
return;
}

// we use SymbolFlags.ExportValue, SymbolFlags.ExportType and SymbolFlags.ExportNamespace
// to denote disjoint declarationSpaces (without making new enum type).
let exportedDeclarationSpaces = SymbolFlags.None;
let nonExportedDeclarationSpaces = SymbolFlags.None;
let defaultExportedDeclarationSpaces = SymbolFlags.None;
let exportedDeclarationSpaces = DeclarationSpaces.None;
let nonExportedDeclarationSpaces = DeclarationSpaces.None;
let defaultExportedDeclarationSpaces = DeclarationSpaces.None;
for (const d of symbol.declarations) {
const declarationSpaces = getDeclarationSpaces(d);
const effectiveDeclarationFlags = getEffectiveDeclarationFlags(d, ModifierFlags.Export | ModifierFlags.Default);
Expand Down Expand Up @@ -18864,28 +18862,34 @@ namespace ts {
}
}

function getDeclarationSpaces(d: Declaration): SymbolFlags {
const enum DeclarationSpaces {
None = 0,
ExportValue = 1 << 0,
ExportType = 1 << 1,
ExportNamespace = 1 << 2,
}
function getDeclarationSpaces(d: Declaration): DeclarationSpaces {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this called? Why doesn't it have to change to accommodate the new enum?

Copy link
Author

@ghost ghost Jul 13, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

This is internal to checkExportsOnMergedDeclarations. It was just re-using the SymbolFlags enum, but these weren't really symbol flags. So I just gave it its own enum.

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, I missed the call on line 18827 (search and context weren't easy to get on the bus). Now it makes sense.

switch (d.kind) {
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
return SymbolFlags.ExportType;
return DeclarationSpaces.ExportType;
case SyntaxKind.ModuleDeclaration:
return isAmbientModule(d) || getModuleInstanceState(d) !== ModuleInstanceState.NonInstantiated
? SymbolFlags.ExportNamespace | SymbolFlags.ExportValue
: SymbolFlags.ExportNamespace;
? DeclarationSpaces.ExportNamespace | DeclarationSpaces.ExportValue
: DeclarationSpaces.ExportNamespace;
case SyntaxKind.ClassDeclaration:
case SyntaxKind.EnumDeclaration:
return SymbolFlags.ExportType | SymbolFlags.ExportValue;
return DeclarationSpaces.ExportType | DeclarationSpaces.ExportValue;
case SyntaxKind.ImportEqualsDeclaration:
let result = SymbolFlags.None;
let result = DeclarationSpaces.None;
const target = resolveAlias(getSymbolOfNode(d));
forEach(target.declarations, d => { result |= getDeclarationSpaces(d); });
return result;
case SyntaxKind.VariableDeclaration:
case SyntaxKind.BindingElement:
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.ImportSpecifier: // https://github.com/Microsoft/TypeScript/pull/7591
return SymbolFlags.ExportValue;
return DeclarationSpaces.ExportValue;
default:
Debug.fail((ts as any).SyntaxKind[d.kind]);
}
Expand Down
13 changes: 5 additions & 8 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2890,13 +2890,11 @@ namespace ts {
TypeParameter = 1 << 18, // Type parameter
TypeAlias = 1 << 19, // Type alias
ExportValue = 1 << 20, // Exported value marker (see comment in declareModuleMember in binder)
ExportType = 1 << 21, // Exported type marker (see comment in declareModuleMember in binder)
ExportNamespace = 1 << 22, // Exported namespace marker (see comment in declareModuleMember in binder)
Alias = 1 << 23, // An alias for another symbol (see comment in isAliasSymbolDeclaration in checker)
Prototype = 1 << 24, // Prototype property (no source representation)
ExportStar = 1 << 25, // Export * declaration
Optional = 1 << 26, // Optional property
Transient = 1 << 27, // Transient symbol (created during type check)
Alias = 1 << 21, // An alias for another symbol (see comment in isAliasSymbolDeclaration in checker)
Prototype = 1 << 22, // Prototype property (no source representation)
ExportStar = 1 << 23, // Export * declaration
Optional = 1 << 24, // Optional property
Transient = 1 << 25, // Transient symbol (created during type check)

Enum = RegularEnum | ConstEnum,
Variable = FunctionScopedVariable | BlockScopedVariable,
Expand Down Expand Up @@ -2941,7 +2939,6 @@ namespace ts {
BlockScoped = BlockScopedVariable | Class | Enum,

PropertyOrAccessor = Property | Accessor,
Export = ExportNamespace | ExportType | ExportValue,

ClassMember = Method | Accessor | Property,

Expand Down
6 changes: 2 additions & 4 deletions src/services/importTracker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ namespace ts.FindAllReferences {

function getExport(): ExportedSymbol | ImportedSymbol | undefined {
const parent = node.parent!;
if (symbol.flags & SymbolFlags.Export) {
if (symbol.exportSymbol) {
if (parent.kind === SyntaxKind.PropertyAccessExpression) {
// When accessing an export of a JS module, there's no alias. The symbol will still be flagged as an export even though we're at the use.
// So check that we are at the declaration.
Expand All @@ -449,9 +449,7 @@ namespace ts.FindAllReferences {
: undefined;
}
else {
const { exportSymbol } = symbol;
Debug.assert(!!exportSymbol);
return exportInfo(exportSymbol, getExportKindForDeclaration(parent));
return exportInfo(symbol.exportSymbol, getExportKindForDeclaration(parent));
}
}
else {
Expand Down