Skip to content

Commit

Permalink
Expand auto-import to all package.json dependencies (#38923)
Browse files Browse the repository at this point in the history
* Start experiment

* Add logging

* Go back to a single program

* Fix forEachExternalModuleToImportFrom

* Move auxiliary program to language service

* Add logging

* Don’t use resolution cache

* Fix(?) containingProjects for ScriptInfo in auxiliary program

* Fix ScriptInfo project inclusion

* Add test for default project of auto-importable ScriptInfo

* Add fourslash server test

* Don’t create auto import provider inside node_modules

* Add monorepo-like test

* WIP

* Naively ensure autoImportProvider is up to date after package.json change

* Start limiting when auto update provider gets updated

* Respond to changes in node_modules

* Don’t create auto-import provider until a file is open that would use it

e.g., don’t create them during cross-project find-all-refs

* Clean up naming, @internal marking, and fix empty project creation bug

* Drop devDependencies, include peerDependencies

* Add additional compiler options

* Fix interaction with importSuggestionsCache

* Move option to UserPreferences, allow inclusion of devDependencies

* Don’t filter out peerDependencies

* Watch unparseable package.jsons

* But don’t filter packages out due to an invalid package.json

* Update test

* Don’t use autoImportProvider in codefixes where it can never be used (or any refactors)

* Add CompletionEntry property for telemetry

* Add assertion for isPackageJsonImport to fourslash

* Fix missing pushSymbol argument

* Add isPackageJsonImport to tests and API baselines

* Fix unit test

* Host auto import provider in new Project kind

* Fix InferredProject attaching on AutoImportProvider-included files, load eagerly

* Update Public APIs

* Simplify PackageJsonCache host

* Remove unneeded markAsDirty

* Defer project finished event until after AutoImportProvider is created

* Make AutoImportProviderProject always report isOrphan = true

* Close and remove AutoImportProviderProject when host project closes

* Don’t set pendingEnsureProjectForOpenFiles

* Use hasAddedOrRemovedFiles instead of hasNewProgram

* Use host-wide watchOptions for package.json watching

* Add to `printProjects`

* Clean up

* Get autoImportProvider directly from LanguageServiceHost

* Clean up

* Clean up

* Close auto import provider on disableLanguageService

* Move AutoImportProvider preload to project updateGraph

* Clear auto import suggestion cache when provider program changes

* Fix tests

* Revert yet-unneeded change

* Use projectService host for module resolution host

* Don’t re-resolve type directives if nothing has changed

* Update src/server/project.ts

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>

* Use ts.emptyArray

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
  • Loading branch information
andrewbranch and sheetalkamat authored Jun 22, 2020
1 parent d76e85d commit 086e00d
Show file tree
Hide file tree
Showing 36 changed files with 1,101 additions and 222 deletions.
4 changes: 2 additions & 2 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2361,7 +2361,7 @@ namespace ts {
}

const result = matchFileNames(filesSpecs, includeSpecs, excludeSpecs, configFileName ? directoryOfCombinedPath(configFileName, basePath) : basePath, options, host, errors, extraFileExtensions, sourceFile);
if (shouldReportNoInputFiles(result, canJsonReportNoInutFiles(raw), resolutionStack)) {
if (shouldReportNoInputFiles(result, canJsonReportNoInputFiles(raw), resolutionStack)) {
errors.push(getErrorForNoInputFiles(result.spec, configFileName));
}

Expand Down Expand Up @@ -2413,7 +2413,7 @@ namespace ts {
}

/*@internal*/
export function canJsonReportNoInutFiles(raw: any) {
export function canJsonReportNoInputFiles(raw: any) {
return !hasProperty(raw, "files") && !hasProperty(raw, "references");
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/tsbuildPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1187,7 +1187,7 @@ namespace ts {
else if (reloadLevel === ConfigFileProgramReloadLevel.Partial) {
// Update file names
const result = getFileNamesFromConfigSpecs(config.configFileSpecs!, getDirectoryPath(project), config.options, state.parseConfigFileHost);
updateErrorForNoInputFiles(result, project, config.configFileSpecs!, config.errors, canJsonReportNoInutFiles(config.raw));
updateErrorForNoInputFiles(result, project, config.configFileSpecs!, config.errors, canJsonReportNoInputFiles(config.raw));
config.fileNames = result.fileNames;
watchInputFiles(state, project, projectPath, config);
}
Expand Down Expand Up @@ -1371,7 +1371,7 @@ namespace ts {
}

// Container if no files are specified in the project
if (!project.fileNames.length && !canJsonReportNoInutFiles(project.raw)) {
if (!project.fileNames.length && !canJsonReportNoInputFiles(project.raw)) {
return {
type: UpToDateStatusType.ContainerOnly
};
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8024,6 +8024,7 @@ namespace ts {
readonly importModuleSpecifierEnding?: "auto" | "minimal" | "index" | "js";
readonly allowTextChangesInNewFiles?: boolean;
readonly providePrefixAndSuffixTextForRename?: boolean;
readonly includePackageJsonAutoImports?: "exclude-dev" | "all" | "none";
readonly provideRefactorNotApplicableReason?: boolean;
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/watchPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ namespace ts {
configFileSpecs = configFileParseResult.configFileSpecs!; // TODO: GH#18217
projectReferences = configFileParseResult.projectReferences;
configFileParsingDiagnostics = getConfigFileParsingDiagnostics(configFileParseResult).slice();
canConfigFileJsonReportNoInputFiles = canJsonReportNoInutFiles(configFileParseResult.raw);
canConfigFileJsonReportNoInputFiles = canJsonReportNoInputFiles(configFileParseResult.raw);
hasChangedConfigFileParsingErrors = true;
}

Expand Down
13 changes: 12 additions & 1 deletion src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,13 @@ namespace ts.server {
this.processResponse(request, /*expectEmptyBody*/ true);
}

/*@internal*/
setFormattingOptions(formatOptions: FormatCodeSettings) {
const args: protocol.ConfigureRequestArguments = { formatOptions };
const request = this.processRequest(CommandNames.Configure, args);
this.processResponse(request, /*expectEmptyBody*/ true);
}

openFile(file: string, fileContent?: string, scriptKindName?: "TS" | "JS" | "TSX" | "JSX"): void {
const args: protocol.OpenRequestArgs = { file, fileContent, scriptKindName };
this.processRequest(CommandNames.Open, args);
Expand Down Expand Up @@ -791,7 +798,11 @@ namespace ts.server {
}

getProgram(): Program {
throw new Error("SourceFile objects are not serializable through the server protocol.");
throw new Error("Program objects are not serializable through the server protocol.");
}

getAutoImportProvider(): Program | undefined {
throw new Error("Program objects are not serializable through the server protocol.");
}

getNonBoundSourceFile(_fileName: string): SourceFile {
Expand Down
53 changes: 25 additions & 28 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -871,53 +871,47 @@ namespace FourSlash {
}

private verifyCompletionEntry(actual: ts.CompletionEntry, expected: FourSlashInterface.ExpectedCompletionEntry) {
const { insertText, replacementSpan, hasAction, isRecommended, isFromUncheckedFile, kind, kindModifiers, text, documentation, tags, source, sourceDisplay, sortText } = typeof expected === "string"
? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, isFromUncheckedFile: undefined, kind: undefined, kindModifiers: undefined, text: undefined, documentation: undefined, tags: undefined, source: undefined, sourceDisplay: undefined, sortText: undefined }
: expected;
expected = typeof expected === "string" ? { name: expected } : expected;

if (actual.insertText !== insertText) {
this.raiseError(`Expected completion insert text to be ${insertText}, got ${actual.insertText}`);
if (actual.insertText !== expected.insertText) {
this.raiseError(`Expected completion insert text to be ${expected.insertText}, got ${actual.insertText}`);
}
const convertedReplacementSpan = replacementSpan && ts.createTextSpanFromRange(replacementSpan);
const convertedReplacementSpan = expected.replacementSpan && ts.createTextSpanFromRange(expected.replacementSpan);
try {
assert.deepEqual(actual.replacementSpan, convertedReplacementSpan);
}
catch {
this.raiseError(`Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`);
}

if (kind !== undefined || kindModifiers !== undefined) {
if (actual.kind !== kind) {
this.raiseError(`Unexpected kind for ${actual.name}: Expected '${kind}', actual '${actual.kind}'`);
}
if (actual.kindModifiers !== (kindModifiers || "")) {
this.raiseError(`Bad kindModifiers for ${actual.name}: Expected ${kindModifiers || ""}, actual ${actual.kindModifiers}`);
}
if (expected.kind !== undefined || expected.kindModifiers !== undefined) {
assert.equal(actual.kind, expected.kind, `Expected 'kind' for ${actual.name} to match`);
assert.equal(actual.kindModifiers, expected.kindModifiers || "", `Expected 'kindModifiers' for ${actual.name} to match`);
}

if (isFromUncheckedFile !== undefined) {
if (actual.isFromUncheckedFile !== isFromUncheckedFile) {
this.raiseError(`Expected 'isFromUncheckedFile' value '${actual.isFromUncheckedFile}' to equal '${isFromUncheckedFile}'`);
}
if (expected.isFromUncheckedFile !== undefined) {
assert.equal<boolean | undefined>(actual.isFromUncheckedFile, expected.isFromUncheckedFile, "Expected 'isFromUncheckedFile' properties to match");
}
if (expected.isPackageJsonImport !== undefined) {
assert.equal<boolean | undefined>(actual.isPackageJsonImport, expected.isPackageJsonImport, "Expected 'isPackageJsonImport' properties to match");
}

assert.equal(actual.hasAction, hasAction, `Expected 'hasAction' properties to match`);
assert.equal(actual.isRecommended, isRecommended, `Expected 'isRecommended' properties to match'`);
assert.equal(actual.source, source, `Expected 'source' values to match`);
assert.equal(actual.sortText, sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`));
assert.equal(actual.hasAction, expected.hasAction, `Expected 'hasAction' properties to match`);
assert.equal(actual.isRecommended, expected.isRecommended, `Expected 'isRecommended' properties to match'`);
assert.equal(actual.source, expected.source, `Expected 'source' values to match`);
assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`));

if (text !== undefined) {
if (expected.text !== undefined) {
const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source)!;
assert.equal(ts.displayPartsToString(actualDetails.displayParts), text, "Expected 'text' property to match 'displayParts' string");
assert.equal(ts.displayPartsToString(actualDetails.documentation), documentation || "", "Expected 'documentation' property to match 'documentation' display parts string");
assert.equal(ts.displayPartsToString(actualDetails.displayParts), expected.text, "Expected 'text' property to match 'displayParts' string");
assert.equal(ts.displayPartsToString(actualDetails.documentation), expected.documentation || "", "Expected 'documentation' property to match 'documentation' display parts string");
// TODO: GH#23587
// assert.equal(actualDetails.kind, actual.kind);
assert.equal(actualDetails.kindModifiers, actual.kindModifiers, "Expected 'kindModifiers' properties to match");
assert.equal(actualDetails.source && ts.displayPartsToString(actualDetails.source), sourceDisplay, "Expected 'sourceDisplay' property to match 'source' display parts string");
assert.deepEqual(actualDetails.tags, tags);
assert.equal(actualDetails.source && ts.displayPartsToString(actualDetails.source), expected.sourceDisplay, "Expected 'sourceDisplay' property to match 'source' display parts string");
assert.deepEqual(actualDetails.tags, expected.tags);
}
else {
assert(documentation === undefined && tags === undefined && sourceDisplay === undefined, "If specifying completion details, should specify 'text'");
assert(expected.documentation === undefined && expected.tags === undefined && expected.sourceDisplay === undefined, "If specifying completion details, should specify 'text'");
}
}

Expand Down Expand Up @@ -2122,6 +2116,9 @@ namespace FourSlash {
public setFormatOptions(formatCodeOptions: ts.FormatCodeOptions | ts.FormatCodeSettings): ts.FormatCodeSettings {
const oldFormatCodeOptions = this.formatCodeSettings;
this.formatCodeSettings = ts.toEditorSettings(formatCodeOptions);
if (this.testType === FourSlashTestType.Server) {
(this.languageService as ts.server.SessionClient).setFormattingOptions(this.formatCodeSettings);
}
return oldFormatCodeOptions;
}

Expand Down
3 changes: 2 additions & 1 deletion src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,7 @@ namespace FourSlashInterface {
}

public setOption(name: keyof ts.FormatCodeSettings, value: number | string | boolean): void {
this.state.formatCodeSettings = { ...this.state.formatCodeSettings, [name]: value };
this.state.setFormatOptions({ ...this.state.formatCodeSettings, [name]: value });
}
}

Expand Down Expand Up @@ -1495,6 +1495,7 @@ namespace FourSlashInterface {
readonly isRecommended?: boolean; // If not specified, will assert that this is false.
readonly isFromUncheckedFile?: boolean; // If not specified, won't assert about this
readonly kind?: string; // If not specified, won't assert about this
readonly isPackageJsonImport?: boolean; // If not specified, won't assert about this
readonly kindModifiers?: string; // Must be paired with 'kind'
readonly text?: string;
readonly documentation?: string;
Expand Down
3 changes: 3 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,9 @@ namespace Harness.LanguageService {
getProgram(): ts.Program {
throw new Error("Program can not be marshaled across the shim layer.");
}
getAutoImportProvider(): ts.Program | undefined {
throw new Error("Program can not be marshaled across the shim layer.");
}
getNonBoundSourceFile(): ts.SourceFile {
throw new Error("SourceFile can not be marshaled across the shim layer.");
}
Expand Down
Loading

0 comments on commit 086e00d

Please sign in to comment.