Skip to content
Merged
Changes from 1 commit
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
Next Next commit
Clarify code checking for UMD exports and eagerly return undefined
…rather than continuing on to the for loop.
  • Loading branch information
Andy Hanson committed Jul 22, 2016
commit aded015c38dd78fa7e39a586ff98afb3505768ff
15 changes: 9 additions & 6 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17713,14 +17713,17 @@ namespace ts {
const parentSymbol = getParentOfSymbol(symbol);
if (parentSymbol) {
if (parentSymbol.flags & SymbolFlags.ValueModule && parentSymbol.valueDeclaration.kind === SyntaxKind.SourceFile) {
const symbolFile = <SourceFile> parentSymbol.valueDeclaration;
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jul 22, 2016

Choose a reason for hiding this comment

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

No space after type assertions (@weswigham can you write this rule? 😄 )

Copy link
Member

Choose a reason for hiding this comment

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

I could. ;)

const referenceFile = getSourceFileOfNode(node);
// If `node` accesses an export and that export isn't in the same file, then symbol is a namespace export, so return undefined.
if (parentSymbol.valueDeclaration === getSourceFileOfNode(node)) {
return <SourceFile>parentSymbol.valueDeclaration;
}
const symbolIsUmdExport = symbolFile !== referenceFile;
return symbolIsUmdExport ? undefined : symbolFile;
}
for (let n = node.parent; n; n = n.parent) {
if ((n.kind === SyntaxKind.ModuleDeclaration || n.kind === SyntaxKind.EnumDeclaration) && getSymbolOfNode(n) === parentSymbol) {
return <ModuleDeclaration | EnumDeclaration>n;
else {
Copy link
Member

Choose a reason for hiding this comment

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

What does this else add apart from nesting?

Copy link
Author

Choose a reason for hiding this comment

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

The old version of this code fell through when it shouldn't have, so as part of removing the fallthrough I turned it into an if-else. If you think the nesting is too much I'll change it back.

for (let n = node.parent; n; n = n.parent) {
if ((n.kind === SyntaxKind.ModuleDeclaration || n.kind === SyntaxKind.EnumDeclaration) && getSymbolOfNode(n) === parentSymbol) {
return <ModuleDeclaration | EnumDeclaration>n;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly return undefined

}
Expand Down