Skip to content

Commit

Permalink
Add exported members of all project files in the global completion li…
Browse files Browse the repository at this point in the history
…st (#19069)

* checker.ts: Remove null check on symbols

* tsserverProjectSystem.ts: add two tests

* client.ts, completions.ts, types.ts: Add codeActions member to CompletionEntryDetails

* protocol.ts, session.ts: Add codeActions member to CompletionEntryDetails protocol

* protocol.ts, session.ts, types.ts: add hasAction to CompletionEntry

* session.ts, services.ts, types.ts: Add formattingOptions parameter to getCompletionEntryDetails

* completions.ts: define SymbolOriginInfo type

* completions.ts, services.ts: Add allSourceFiles parameter to getCompletionsAtPosition

* completions.ts, services.ts: Plumb allSourceFiles into new function getSymbolsFromOtherSourceFileExports inside getCompletionData

* completions.ts: add symbolToOriginInfoMap parameter to getCompletionEntriesFromSymbols and to return value of getCompletionData

* utilities.ts: Add getOtherModuleSymbols, getUniqueSymbolIdAsString, getUniqueSymbolId

* completions.ts: Set CompletionEntry.hasAction when symbol is found in symbolToOriginInfoMap (meaning there's an import action)

* completions.ts: Populate list with possible exports (implement getSymbolsFromOtherSourceFileExports)

* completions.ts, services.ts: Plumb host and rulesProvider into getCompletionEntryDetails

* completions.ts: Add TODO comment

* importFixes.ts: Add types ImportDeclarationMap and ImportCodeFixContext

* Move getImportDeclarations into getCodeActionForImport, immediately after the implementation

* importFixes.ts: Move createChangeTracker into getCodeActionForImport, immediately after getImportDeclarations

* importFixes.ts: Add convertToImportCodeFixContext function and reference it from the getCodeActions lambda

* importFixes.ts: Add context: ImportCodeFixContext parameter to getCodeActionForImport, update call sites, destructure it, use compilerOptions in getModuleSpecifierForNewImport

* importFixes.ts: Remove moduleSymbol parameter from getImportDeclarations and use the ambient one

* importFixes.ts: Use cachedImportDeclarations from context in getCodeActionForImport

* importFixes.ts: Move createCodeAction out, immediately above convertToImportCodeFixContext

* Move the declaration for lastImportDeclaration out of the getCodeActions lambda into getCodeActionForImport

* importFixes.ts: Use symbolToken in getCodeActionForImport

* importFixes.ts: Remove useCaseSensitiveFileNames altogether from getCodeActions lambda

* importFixes.ts: Remove local getUniqueSymbolId function and add checker parameter to calls to it

* importFixes.ts: Move getCodeActionForImport out into an export, immediately below convertToImportCodeFixContext

* completions.ts: In getCompletionEntryDetails, if there's symbolOriginInfo, call getCodeActionForImport

* importFixes.ts: Create and use importFixContext within getCodeActions lambda

* importFixes.ts: Use local newLineCharacter instead of context.newLineCharacter in getCodeActionForImport

* importFixes.ts: Use local host instead of context.host in getCodeActionForImport

* importFixes.ts: Remove dummy getCanonicalFileName line

* Filter symbols after gathering exports instead of before

* Lint

* Test, fix bugs, refactor

* Suggestions from code review

* Update api baseline

* Fix bug if previousToken is not an Identifier

* Replace `startsWith` with `stringContainsCharactersInOrder`
  • Loading branch information
Andy committed Oct 17, 2017
1 parent 1ccc04f commit 4c0f3d2
Show file tree
Hide file tree
Showing 36 changed files with 1,104 additions and 582 deletions.
18 changes: 11 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ namespace ts {
const jsObjectLiteralIndexInfo = createIndexInfo(anyType, /*isReadonly*/ false);

const globals = createSymbolTable();
let ambientModulesCache: Symbol[] | undefined;
/**
* List of every ambient module with a "*" wildcard.
* Unlike other ambient modules, these can't be stored in `globals` because symbol tables only deal with exact matches.
Expand Down Expand Up @@ -25560,13 +25561,16 @@ namespace ts {
}

function getAmbientModules(): Symbol[] {
const result: Symbol[] = [];
globals.forEach((global, sym) => {
if (ambientModuleSymbolRegex.test(unescapeLeadingUnderscores(sym))) {
result.push(global);
}
});
return result;
if (!ambientModulesCache) {
ambientModulesCache = [];
globals.forEach((global, sym) => {
// No need to `unescapeLeadingUnderscores`, an escaped symbol is never an ambient module.
if (ambientModuleSymbolRegex.test(sym as string)) {
ambientModulesCache.push(global);
}
});
}
return ambientModulesCache;
}

function checkGrammarImportCallExpression(node: ImportCall): boolean {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1183,8 +1183,8 @@ namespace ts {
}
}

function isDoubleQuotedString(node: Node) {
return node.kind === SyntaxKind.StringLiteral && getSourceTextOfNodeFromSourceFile(sourceFile, node).charCodeAt(0) === CharacterCodes.doubleQuote;
function isDoubleQuotedString(node: Node): boolean {
return isStringLiteral(node) && isStringDoubleQuoted(node, sourceFile);
}
}

Expand Down
30 changes: 30 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,18 @@ namespace ts {
}
return undefined;
}

/** Like `forEach`, but suitable for use with numbers and strings (which may be falsy). */
export function firstDefined<T, U>(array: ReadonlyArray<T> | undefined, callback: (element: T, index: number) => U | undefined): U | undefined {
for (let i = 0; i < array.length; i++) {
const result = callback(array[i], i);
if (result !== undefined) {
return result;
}
}
return undefined;
}

/**
* Iterates through the parent chain of a node and performs the callback on each parent until the callback
* returns a truthy value, then returns that value.
Expand Down Expand Up @@ -261,6 +273,16 @@ namespace ts {
return undefined;
}

export function findLast<T>(array: ReadonlyArray<T>, predicate: (element: T, index: number) => boolean): T | undefined {
for (let i = array.length - 1; i >= 0; i--) {
const value = array[i];
if (predicate(value, i)) {
return value;
}
}
return undefined;
}

/** Works like Array.prototype.findIndex, returning `-1` if no element satisfying the predicate is found. */
export function findIndex<T>(array: ReadonlyArray<T>, predicate: (element: T, index: number) => boolean): number {
for (let i = 0; i < array.length; i++) {
Expand Down Expand Up @@ -1147,6 +1169,14 @@ namespace ts {
return result;
}

export function arrayToNumericMap<T>(array: ReadonlyArray<T>, makeKey: (value: T) => number): T[] {
const result: T[] = [];
for (const value of array) {
result[makeKey(value)] = value;
}
return result;
}

/**
* Creates a set from the elements of an array.
*
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3657,15 +3657,15 @@
"category": "Error",
"code": 90010
},
"Import {0} from {1}.": {
"Import '{0}' from \"{1}\".": {
"category": "Message",
"code": 90013
},
"Change '{0}' to '{1}'.": {
"category": "Message",
"code": 90014
},
"Add {0} to existing import declaration from {1}.": {
"Add '{0}' to existing import declaration from \"{1}\".": {
"category": "Message",
"code": 90015
},
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ namespace ts {
}
}

export function getEffectiveTypeRoots(options: CompilerOptions, host: { directoryExists?: (directoryName: string) => boolean, getCurrentDirectory?: () => string }): string[] | undefined {
export interface GetEffectiveTypeRootsHost {
directoryExists?(directoryName: string): boolean;
getCurrentDirectory?(): string;
}
export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined {
if (options.typeRoots) {
return options.typeRoots;
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,7 @@ namespace ts {
export interface StringLiteral extends LiteralExpression {
kind: SyntaxKind.StringLiteral;
/* @internal */ textSourceNode?: Identifier | StringLiteral | NumericLiteral; // Allows a StringLiteral to get its text from another node (used by transforms).
/** Note: this is only set when synthesizing a node, not during parsing. */
/* @internal */ singleQuote?: boolean;
}

Expand Down
15 changes: 15 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,17 @@ namespace ts {
}
}

/* @internal */
export function isAnyImportSyntax(node: Node): node is AnyImportSyntax {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
case SyntaxKind.ImportEqualsDeclaration:
return true;
default:
return false;
}
}

// Gets the nearest enclosing block scope container that has the provided node
// as a descendant, that is not the provided node.
export function getEnclosingBlockScopeContainer(node: Node): Node {
Expand Down Expand Up @@ -1375,6 +1386,10 @@ namespace ts {
return charCode === CharacterCodes.singleQuote || charCode === CharacterCodes.doubleQuote;
}

export function isStringDoubleQuoted(string: StringLiteral, sourceFile: SourceFile): boolean {
return getSourceTextOfNodeFromSourceFile(sourceFile, string).charCodeAt(0) === CharacterCodes.doubleQuote;
}

/**
* Returns true if the node is a variable declaration whose initializer is a function expression.
* This function does not test if the node is in a JavaScript file or not.
Expand Down
67 changes: 58 additions & 9 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -783,10 +783,10 @@ namespace FourSlash {
});
}

public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) {
const completions = this.getCompletionListAtCaret();
if (completions) {
this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex);
this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex, hasAction);
}
else {
this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`);
Expand Down Expand Up @@ -1127,7 +1127,7 @@ Actual: ${stringify(fullActual)}`);
}

private getCompletionEntryDetails(entryName: string) {
return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName);
return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings);
}

private getReferencesAtCaret() {
Expand Down Expand Up @@ -2289,6 +2289,29 @@ Actual: ${stringify(fullActual)}`);
this.applyCodeActions(this.getCodeFixActions(fileName, errorCode), index);
}

public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) {
this.goToMarker(markerName);

const actualCompletion = this.getCompletionListAtCaret().entries.find(e => e.name === options.name);

if (!actualCompletion.hasAction) {
this.raiseError(`Completion for ${options.name} does not have an associated action.`);
}

const details = this.getCompletionEntryDetails(options.name);
if (details.codeActions.length !== 1) {
this.raiseError(`Expected one code action, got ${details.codeActions.length}`);
}

if (details.codeActions[0].description !== options.description) {
this.raiseError(`Expected description to be:\n${options.description}\ngot:\n${details.codeActions[0].description}`);
}

this.applyCodeActions(details.codeActions);

this.verifyNewContent(options);
}

public verifyRangeIs(expectedText: string, includeWhiteSpace?: boolean) {
const ranges = this.getRanges();
if (ranges.length !== 1) {
Expand Down Expand Up @@ -2360,6 +2383,10 @@ Actual: ${stringify(fullActual)}`);
this.applyEdits(change.fileName, change.textChanges, /*isFormattingEdit*/ false);
}

this.verifyNewContent(options);
}

private verifyNewContent(options: FourSlashInterface.NewContentOptions) {
if (options.newFileContent) {
assert(!options.newRangeContent);
this.verifyCurrentFileContent(options.newFileContent);
Expand Down Expand Up @@ -2933,7 +2960,15 @@ Actual: ${stringify(fullActual)}`);
return text.substring(startPos, endPos);
}

private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
private assertItemInCompletionList(
items: ts.CompletionEntry[],
name: string,
text: string | undefined,
documentation: string | undefined,
kind: string | undefined,
spanIndex: number | undefined,
hasAction: boolean | undefined,
) {
for (const item of items) {
if (item.name === name) {
if (documentation !== undefined || text !== undefined) {
Expand All @@ -2956,6 +2991,8 @@ Actual: ${stringify(fullActual)}`);
assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + name));
}

assert.equal(item.hasAction, hasAction);

return;
}
}
Expand Down Expand Up @@ -3669,12 +3706,12 @@ namespace FourSlashInterface {

// Verifies the completion list contains the specified symbol. The
// completion list is brought up if necessary
public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) {
if (this.negative) {
this.state.verifyCompletionListDoesNotContain(symbol, text, documentation, kind, spanIndex);
}
else {
this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex);
this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex, hasAction);
}
}

Expand Down Expand Up @@ -3999,6 +4036,10 @@ namespace FourSlashInterface {
this.state.getAndApplyCodeActions(errorCode, index);
}

public applyCodeActionFromCompletion(markerName: string, options: VerifyCompletionActionOptions): void {
this.state.applyCodeActionFromCompletion(markerName, options);
}

public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void {
this.state.verifyImportFixAtPosition(expectedTextArray, errorCode);
}
Expand Down Expand Up @@ -4396,12 +4437,20 @@ namespace FourSlashInterface {
isNewIdentifierLocation?: boolean;
}

export interface VerifyCodeFixOptions {
description: string;
// One of these should be defined.
export interface NewContentOptions {
// Exactly one of these should be defined.
newFileContent?: string;
newRangeContent?: string;
}

export interface VerifyCodeFixOptions extends NewContentOptions {
description: string;
errorCode?: number;
index?: number;
}

export interface VerifyCompletionActionOptions extends NewContentOptions {
name: string;
description: string;
}
}
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ namespace Harness.LanguageService {
getCompletionsAtPosition(fileName: string, position: number): ts.CompletionInfo {
return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position));
}
getCompletionEntryDetails(fileName: string, position: number, entryName: string): ts.CompletionEntryDetails {
return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName));
getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: ts.FormatCodeOptions): ts.CompletionEntryDetails {
return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName, JSON.stringify(options)));
}
getCompletionEntrySymbol(): ts.Symbol {
throw new Error("getCompletionEntrySymbol not implemented across the shim layer.");
Expand Down
4 changes: 3 additions & 1 deletion src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ namespace ts.server {
const request = this.processRequest<protocol.CompletionDetailsRequest>(CommandNames.CompletionDetails, args);
const response = this.processResponse<protocol.CompletionDetailsResponse>(request);
Debug.assert(response.body.length === 1, "Unexpected length of completion details response body.");
return response.body[0];

const convertedCodeActions = map(response.body[0].codeActions, codeAction => this.convertCodeActions(codeAction, fileName));
return { ...response.body[0], codeActions: convertedCodeActions };
}

getCompletionEntrySymbol(_fileName: string, _position: number, _entryName: string): Symbol {
Expand Down
10 changes: 10 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1658,6 +1658,11 @@ namespace ts.server.protocol {
* this span should be used instead of the default one.
*/
replacementSpan?: TextSpan;
/**
* Indicates whether commiting this completion entry will require additional code actions to be
* made to avoid errors. The CompletionEntryDetails will have these actions.
*/
hasAction?: true;
}

/**
Expand Down Expand Up @@ -1690,6 +1695,11 @@ namespace ts.server.protocol {
* JSDoc tags for the symbol.
*/
tags: JSDocTagInfo[];

/**
* The associated code actions for this entry
*/
codeActions?: CodeAction[];
}

export interface CompletionsResponse extends Response {
Expand Down
23 changes: 17 additions & 6 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1178,11 +1178,12 @@ namespace ts.server {

const completions = project.getLanguageService().getCompletionsAtPosition(file, position);
if (simplifiedResult) {
return mapDefined(completions && completions.entries, entry => {
return mapDefined<CompletionEntry, protocol.CompletionEntry>(completions && completions.entries, entry => {
if (completions.isMemberCompletion || (entry.name.toLowerCase().indexOf(prefix.toLowerCase()) === 0)) {
const { name, kind, kindModifiers, sortText, replacementSpan } = entry;
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction } = entry;
const convertedSpan = replacementSpan ? this.decorateSpan(replacementSpan, scriptInfo) : undefined;
return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan };
// Use `hasAction || undefined` to avoid serializing `false`.
return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined };
}
}).sort((a, b) => compareStrings(a.name, b.name));
}
Expand All @@ -1193,10 +1194,20 @@ namespace ts.server {

private getCompletionEntryDetails(args: protocol.CompletionDetailsRequestArgs): ReadonlyArray<protocol.CompletionEntryDetails> {
const { file, project } = this.getFileAndProject(args);
const position = this.getPositionInFile(args, file);
const scriptInfo = this.projectService.getScriptInfoForNormalizedPath(file);
const position = this.getPosition(args, scriptInfo);
const formattingOptions = project.projectService.getFormatCodeOptions(file);

return mapDefined(args.entryNames, entryName =>
project.getLanguageService().getCompletionEntryDetails(file, position, entryName));
return mapDefined(args.entryNames, entryName => {
const details = project.getLanguageService().getCompletionEntryDetails(file, position, entryName, formattingOptions);
if (details) {
const mappedCodeActions = map(details.codeActions, action => this.mapCodeAction(action, scriptInfo));
return { ...details, codeActions: mappedCodeActions };
}
else {
return undefined;
}
});
}

private getCompileOnSaveAffectedFileList(args: protocol.FileRequestArgs): ReadonlyArray<protocol.CompileOnSaveAffectedFileListSingleProject> {
Expand Down
Loading

0 comments on commit 4c0f3d2

Please sign in to comment.