Skip to content

Commit

Permalink
Code fix for missing imports (#11768)
Browse files Browse the repository at this point in the history
* Add codefix for missing imports + tests

* Re-order and cleanup

* refactor

* make tests pass

* Make import specifier for new imports more comprehensive

* Fix existing import cases

* refactor

* Fix multiple import statement case

* add multiple code fixes and code action filtering and polishing

* not using the generic verify method for import fixes.

* Correct insert position for new imports

* improve the code action filtering logic

* Fix line ending issue

* cache where we can
  • Loading branch information
paulvanbrenk authored and zhengbli committed Nov 17, 2016
1 parent 0b0b68e commit 52ec508
Show file tree
Hide file tree
Showing 46 changed files with 1,265 additions and 9 deletions.
9 changes: 8 additions & 1 deletion src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ namespace ts {
getEmitResolver,
getExportsOfModule: getExportsOfModuleAsArray,
getAmbientModules,

getJsxElementAttributesType,
getJsxIntrinsicTagNames,
isOptionalParameter,
tryGetMemberInModuleExports,
tryFindAmbientModuleWithoutAugmentations: moduleName => {
// we deliberately exclude augmentations
// since we are only interested in declarations of the module itself
Expand Down Expand Up @@ -1483,6 +1483,13 @@ namespace ts {
return symbolsToArray(getExportsOfModule(moduleSymbol));
}

function tryGetMemberInModuleExports(memberName: string, moduleSymbol: Symbol): Symbol | undefined {
const symbolTable = getExportsOfModule(moduleSymbol);
if (symbolTable) {
return symbolTable[memberName];
}
}

function getExportsOfSymbol(symbol: Symbol): SymbolTable {
return symbol.flags & SymbolFlags.Module ? getExportsOfModule(symbol) : symbol.exports || emptySymbols;
}
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,14 @@ namespace ts {
getEmitScriptTarget(compilerOptions) >= ScriptTarget.ES2015 ? ModuleKind.ES2015 : ModuleKind.CommonJS;
}

export function getEmitModuleResolutionKind(compilerOptions: CompilerOptions) {
let moduleResolution = compilerOptions.moduleResolution;
if (moduleResolution === undefined) {
moduleResolution = getEmitModuleKind(compilerOptions) === ModuleKind.CommonJS ? ModuleResolutionKind.NodeJs : ModuleResolutionKind.Classic;
}
return moduleResolution;
}

/* @internal */
export function hasZeroOrOneAsteriskCharacter(str: string): boolean {
let seenAsterisk = false;
Expand Down
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3190,5 +3190,17 @@
"Type '{0}' is not assignable to type '{1}'. Two different types with this name exist, but they are unrelated.": {
"category": "Error",
"code": 90010
},
"Import {0} from {1}": {
"category": "Message",
"code": 90013
},
"Change {0} to {1}": {
"category": "Message",
"code": 90014
},
"Add {0} to existing import declaration from {1}": {
"category": "Message",
"code": 90015
}
}
2 changes: 1 addition & 1 deletion src/compiler/moduleNameResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace ts {
return { resolvedModule: resolved && resolvedModuleFromResolved(resolved, isExternalLibraryImport), failedLookupLocations };
}

function moduleHasNonRelativeName(moduleName: string): boolean {
export function moduleHasNonRelativeName(moduleName: string): boolean {
return !(isRootedDiskPath(moduleName) || isExternalModuleNameRelative(moduleName));
}

Expand Down
4 changes: 3 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,8 @@ namespace ts {
isOptionalParameter(node: ParameterDeclaration): boolean;
getAmbientModules(): Symbol[];

tryGetMemberInModuleExports(memberName: string, moduleSymbol: Symbol): Symbol | undefined;

/* @internal */ tryFindAmbientModuleWithoutAugmentations(moduleName: string): Symbol;

// Should not be called directly. Should only be accessed through the Program instance.
Expand Down Expand Up @@ -3190,7 +3192,7 @@ namespace ts {
target?: ScriptTarget;
traceResolution?: boolean;
types?: string[];
/** Paths used to used to compute primary types search locations */
/** Paths used to compute primary types search locations */
typeRoots?: string[];
/*@internal*/ version?: boolean;
/*@internal*/ watch?: boolean;
Expand Down
38 changes: 37 additions & 1 deletion src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2046,6 +2046,34 @@ namespace FourSlash {
}
}

public verifyImportFixAtPosition(expectedTextArray: string[], errorCode?: number) {
const ranges = this.getRanges();
if (ranges.length == 0) {
this.raiseError("At least one range should be specified in the testfile.");
}

const codeFixes = this.getCodeFixes(errorCode);

if (!codeFixes || codeFixes.length == 0) {
this.raiseError("No codefixes returned.");
}

const actualTextArray: string[] = [];
const scriptInfo = this.languageServiceAdapterHost.getScriptInfo(codeFixes[0].changes[0].fileName);
const originalContent = scriptInfo.content;
for (const codeFix of codeFixes) {
this.applyEdits(codeFix.changes[0].fileName, codeFix.changes[0].textChanges, /*isFormattingEdit*/ false);
actualTextArray.push(this.normalizeNewlines(this.rangeText(ranges[0])));
scriptInfo.updateContent(originalContent);
}
const sortedExpectedArray = ts.map(expectedTextArray, str => this.normalizeNewlines(str)).sort();
const sortedActualArray = actualTextArray.sort();
if (!ts.arrayIsEqualTo(sortedExpectedArray, sortedActualArray)) {
this.raiseError(
`Actual text array doesn't match expected text array. \nActual: \n"${sortedActualArray.join("\n\n")}"\n---\nExpected: \n'${sortedExpectedArray.join("\n\n")}'`);
}
}

public verifyDocCommentTemplate(expected?: ts.TextInsertion) {
const name = "verifyDocCommentTemplate";
const actual = this.languageService.getDocCommentTemplateAtPosition(this.activeFile.fileName, this.currentCaretPosition);
Expand Down Expand Up @@ -2079,6 +2107,10 @@ namespace FourSlash {
});
}

private normalizeNewlines(str: string) {
return str.replace(/\r?\n/g, "\n");
}

public verifyBraceCompletionAtPosition(negative: boolean, openingBrace: string) {

const openBraceMap = ts.createMap<ts.CharacterCodes>({
Expand Down Expand Up @@ -2606,7 +2638,7 @@ ${code}
resetLocalData();
}

currentFileName = basePath + "/" + value;
currentFileName = ts.isRootedDiskPath(value) ? value : basePath + "/" + value;
currentFileOptions[key] = value;
}
else {
Expand Down Expand Up @@ -3303,6 +3335,10 @@ namespace FourSlashInterface {
this.state.verifyCodeFixAtPosition(expectedText, errorCode);
}

public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void {
this.state.verifyImportFixAtPosition(expectedTextArray, errorCode);
}

public navigationBar(json: any) {
this.state.verifyNavigationBar(json);
}
Expand Down
4 changes: 3 additions & 1 deletion src/services/codefixes/codeFixProvider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* @internal */
/* @internal */
namespace ts {
export interface CodeFix {
errorCodes: number[];
Expand All @@ -11,6 +11,8 @@ namespace ts {
span: TextSpan;
program: Program;
newLineCharacter: string;
host: LanguageServiceHost;
cancellationToken: CancellationToken;
}

export namespace codefix {
Expand Down
3 changes: 2 additions & 1 deletion src/services/codefixes/fixes.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
///<reference path='superFixes.ts' />
///<reference path='unusedIdentifierFixes.ts' />
///<reference path='importFixes.ts' />
///<reference path='unusedIdentifierFixes.ts' />
Loading

1 comment on commit 52ec508

@olmobrutall
Copy link

Choose a reason for hiding this comment

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

This is going to be awesome 👍 👍

Please sign in to comment.