Respond to minor PR comments

This commit is contained in:
Andy Hanson 2017-03-27 11:46:38 -07:00
parent aa1c860f08
commit cf6c24cd02
4 changed files with 48 additions and 36 deletions

View file

@ -520,17 +520,18 @@ namespace ts.FindAllReferences.Core {
}
if (indirectUsers.length) {
const indirectSearch = (() => {
switch (exportInfo.exportKind) {
case ExportKind.Named:
return state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
case ExportKind.Default:
// Search for a property access to '.default'. This can't be renamed.
return state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
case ExportKind.ExportEquals:
return undefined;
}
})();
let indirectSearch: Search | undefined;
switch (exportInfo.exportKind) {
case ExportKind.Named:
indirectSearch = state.createSearch(exportLocation, exportSymbol, ImportExport.Export);
break;
case ExportKind.Default:
// Search for a property access to '.default'. This can't be renamed.
indirectSearch = state.isForRename ? undefined : state.createSearch(exportLocation, exportSymbol, ImportExport.Export, { text: "default" });
break;
case ExportKind.ExportEquals:
break;
}
if (indirectSearch) {
for (const indirectUser of indirectUsers) {
state.cancellationToken.throwIfCancellationRequested();
@ -549,15 +550,11 @@ namespace ts.FindAllReferences.Core {
/** Search for all occurences of an identifier in a source file (and filter out the ones that match). */
function searchForName(sourceFile: SourceFile, search: Search, state: State): void {
if (sourceFileHasName(sourceFile, search.escapedText)) {
if (getNameTable(sourceFile).get(search.escapedText) !== undefined) {
getReferencesInSourceFile(sourceFile, search, state);
}
}
function sourceFileHasName(sourceFile: SourceFile, escapedName: string): boolean {
return getNameTable(sourceFile).get(escapedName) !== undefined;
}
function getPropertySymbolOfDestructuringAssignment(location: Node, checker: TypeChecker): Symbol | undefined {
return isArrayLiteralOrObjectLiteralDestructuringPattern(location.parent.parent) &&
checker.getPropertySymbolOfDestructuringAssignment(<Identifier>location);
@ -613,7 +610,9 @@ namespace ts.FindAllReferences.Core {
return undefined;
}
// The only symbols with parents *not* globally acessible are exports of external modules, and only then if they do not have `export as namespace`.
// If the symbol has a parent, it's globally visible.
// Unless that parent is an external module, then we should only search in the module (and recurse on the export later).
// But if the parent is a module that has `export as namespace`, then the symbol *is* globally visible.
if (parent && !((parent.flags & SymbolFlags.Module) && isExternalModuleSymbol(parent) && !parent.globalExports)) {
return undefined;
}

View file

@ -36,7 +36,7 @@ namespace ts.FindAllReferences {
type ImporterOrCallExpression = Importer | CallExpression;
/** Returns import statements that directly reference the exporting module, and a list of files that may access the module through a namespace. */
function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: ImporterOrCallExpression[][], { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
function getImportersForExport(sourceFiles: SourceFile[], allDirectImports: Map<ImporterOrCallExpression[]>, { exportingModuleSymbol, exportKind }: ExportInfo, checker: TypeChecker): { directImports: Importer[], indirectUsers: SourceFile[] } {
const markSeenDirectImport = nodeSeenTracker<ImporterOrCallExpression>();
const markSeenIndirectUser = nodeSeenTracker<SourceFileLike>();
const directImports: Importer[] = [];
@ -148,7 +148,7 @@ namespace ts.FindAllReferences {
}
function getDirectImports(moduleSymbol: Symbol): ImporterOrCallExpression[] | undefined {
return allDirectImports[getSymbolId(moduleSymbol)];
return allDirectImports.get(getSymbolId(moduleSymbol).toString());
}
}
@ -173,7 +173,7 @@ namespace ts.FindAllReferences {
function handleImport(decl: Importer): void {
if (decl.kind === SyntaxKind.ImportEqualsDeclaration) {
if (isProperImportEquals(decl)) {
if (isExternalModuleImportEquals(decl)) {
handleNamespaceImportLike(decl.name);
}
return;
@ -274,17 +274,17 @@ namespace ts.FindAllReferences {
}
/** Returns a map from a module symbol Id to all import statements that directly reference the module. */
function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): ImporterOrCallExpression[][] {
const map: ImporterOrCallExpression[][] = [];
function getDirectImportsMap(sourceFiles: SourceFile[], checker: TypeChecker): Map<ImporterOrCallExpression[]> {
const map = createMap<ImporterOrCallExpression[]>();
for (const sourceFile of sourceFiles) {
forEachImport(sourceFile, (importDecl, moduleSpecifier) => {
const moduleSymbol = checker.getSymbolAtLocation(moduleSpecifier);
if (moduleSymbol) {
const id = getSymbolId(moduleSymbol);
let imports = map[id];
const id = getSymbolId(moduleSymbol).toString();
let imports = map.get(id);
if (!imports) {
imports = map[id] = [];
map.set(id, imports = []);
}
imports.push(importDecl);
}
@ -371,8 +371,7 @@ namespace ts.FindAllReferences {
* @param comingFromExport If we are doing a search for all exports, don't bother looking backwards for the imported symbol, since that's the reason we're here.
*/
export function getImportOrExportSymbol(node: Node, symbol: Symbol, checker: TypeChecker, comingFromExport: boolean): ImportedSymbol | ExportedSymbol | undefined {
const ex = getExport();
return ex || comingFromExport ? ex : getImport();
return comingFromExport ? getExport() : getExport() || getImport();
function getExport(): ExportedSymbol | ImportedSymbol | undefined {
const { parent } = node;
@ -459,7 +458,9 @@ namespace ts.FindAllReferences {
const { parent } = node;
switch (parent.kind) {
case SyntaxKind.ImportEqualsDeclaration:
return (parent as ImportEqualsDeclaration).name === node ? { isNamedImport: false } : undefined;
return (parent as ImportEqualsDeclaration).name === node && isExternalModuleImportEquals(parent as ImportEqualsDeclaration)
? { isNamedImport: false }
: undefined;
case SyntaxKind.ImportSpecifier:
// For a rename import `{ foo as bar }`, don't search for the imported symbol. Just find local uses of `bar`.
return (parent as ImportSpecifier).propertyName ? undefined : { isNamedImport: true };
@ -522,7 +523,7 @@ namespace ts.FindAllReferences {
return node.kind === SyntaxKind.ModuleDeclaration && (node as ModuleDeclaration).name.kind === SyntaxKind.StringLiteral;
}
function isProperImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
function isExternalModuleImportEquals({ moduleReference }: ImportEqualsDeclaration): boolean {
return moduleReference.kind === SyntaxKind.ExternalModuleReference && moduleReference.expression.kind === SyntaxKind.StringLiteral
}
}

View file

@ -31,6 +31,13 @@ namespace ts.Rename {
return getRenameInfoError(Diagnostics.You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library);
}
// Cannot rename `default` as in `import { default as foo } from "./someModule";
if (node.kind === SyntaxKind.Identifier &&
(node as Identifier).originalKeywordKind === SyntaxKind.DefaultKeyword &&
symbol.parent.flags & ts.SymbolFlags.Module) {
return undefined;
}
const displayName = stripQuotes(getDeclaredName(typeChecker, symbol, node));
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
return kind ? getRenameInfoSuccess(displayName, typeChecker.getFullyQualifiedName(symbol), kind, SymbolDisplay.getSymbolModifiers(symbol), node, sourceFile) : undefined;
@ -82,11 +89,8 @@ namespace ts.Rename {
}
function nodeIsEligibleForRename(node: Node): boolean {
if (node.kind === SyntaxKind.Identifier) {
// Cannot rename `default` as in `import { default as foo } from "./someModule";
return (node as Identifier).originalKeywordKind !== SyntaxKind.DefaultKeyword;
}
return node.kind === SyntaxKind.StringLiteral ||
return node.kind === ts.SyntaxKind.Identifier ||
node.kind === SyntaxKind.StringLiteral ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
isThis(node);
}

View file

@ -9,12 +9,20 @@
// @Filename: /c.ts
////import { a } from "./b";
////a.[|default|]();
////
////declare const x: { [|{| "isWriteAccess": true, "isDefinition": true |}default|]: number };
////x.[|default|];
verify.singleReferenceGroup("function f(): void");
const [r0, r1, r2, r3] = test.ranges();
const [r0, r1] = test.ranges();
verify.singleReferenceGroup("function f(): void", [r0, r1]);
verify.singleReferenceGroup("(property) default: number", [r2, r3]);
verify.rangesAreRenameLocations([r0]);
// Can't rename a default import.
goTo.rangeStart(r1);
verify.renameInfoFailed();
// Can rename a default property.
verify.rangesAreRenameLocations([r2, r3]);