diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 9579ba80d0fb1..bab0af265971f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -311,6 +311,8 @@ namespace ts { node = getParseTreeNode(node, isTypeNode); return node && getTypeArgumentConstraint(node); }, + + getSuggestionDiagnostics: file => suggestionDiagnostics.get(file.fileName) || emptyArray, }; const tupleTypes: GenericType[] = []; @@ -449,6 +451,19 @@ namespace ts { const awaitedTypeStack: number[] = []; const diagnostics = createDiagnosticCollection(); + // Suggestion diagnostics must have a file. Keyed by source file name. + const suggestionDiagnostics = createMultiMap(); + function addSuggestionDiagnostic(diag: Diagnostic): void { + suggestionDiagnostics.add(diag.file.fileName, { ...diag, category: DiagnosticCategory.Suggestion }); + } + function addErrorOrSuggestionDiagnostic(isError: boolean, diag: Diagnostic): void { + if (isError) { + diagnostics.add(diag); + } + else { + addSuggestionDiagnostic(diag); + } + } const enum TypeFacts { None = 0, @@ -2072,6 +2087,9 @@ namespace ts { const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName); if (sourceFile) { if (sourceFile.symbol) { + if (resolvedModule.isExternalLibraryImport && !extensionIsTypeScript(resolvedModule.extension)) { + addSuggestionDiagnostic(createModuleImplicitlyAnyDiagnostic(errorNode, resolvedModule, moduleReference)); + } // merged symbol is module declaration symbol combined with all augmentations return getMergedSymbol(sourceFile.symbol); } @@ -2095,15 +2113,8 @@ namespace ts { const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented; error(errorNode, diag, moduleReference, resolvedModule.resolvedFileName); } - else if (noImplicitAny && moduleNotFoundError) { - let errorInfo = resolvedModule.packageId && chainDiagnosticMessages(/*details*/ undefined, - Diagnostics.Try_npm_install_types_Slash_0_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0, - getMangledNameForScopedPackage(resolvedModule.packageId.name)); - errorInfo = chainDiagnosticMessages(errorInfo, - Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, - moduleReference, - resolvedModule.resolvedFileName); - diagnostics.add(createDiagnosticForNodeFromMessageChain(errorNode, errorInfo)); + else { + addErrorOrSuggestionDiagnostic(noImplicitAny && !!moduleNotFoundError, createModuleImplicitlyAnyDiagnostic(errorNode, resolvedModule, moduleReference)); } // Failed imports and untyped modules are both treated in an untyped manner; only difference is whether we give a diagnostic first. return undefined; @@ -2128,6 +2139,18 @@ namespace ts { return undefined; } + function createModuleImplicitlyAnyDiagnostic(errorNode: Node, { packageId, resolvedFileName }: ResolvedModuleFull, moduleReference: string): Diagnostic { + const errorInfo = packageId && chainDiagnosticMessages( + /*details*/ undefined, + Diagnostics.Try_npm_install_types_Slash_0_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0, + getMangledNameForScopedPackage(packageId.name)); + return createDiagnosticForNodeFromMessageChain(errorNode, chainDiagnosticMessages( + errorInfo, + Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type, + moduleReference, + resolvedFileName)); + } + // An external module with an 'export =' declaration resolves to the target of the 'export =' declaration, // and an external module with no 'export =' declaration resolves to the module itself. function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index cc78623382eab..5a8d159a628d0 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2954,6 +2954,12 @@ namespace ts { /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */ /* @internal */ tryGetThisTypeAt(node: Node): Type | undefined; /* @internal */ getTypeArgumentConstraint(node: TypeNode): Type | undefined; + + /** + * Does *not* get *all* suggestion diagnostics, just the ones that were convenient to report in the checker. + * Others are added in computeSuggestionDiagnostics. + */ + /* @internal */ getSuggestionDiagnostics(file: SourceFile): ReadonlyArray; } /* @internal */ diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index f5efa233a613d..880d1f7459c1c 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -505,11 +505,11 @@ namespace FourSlash { return "\nMarker: " + this.lastKnownMarker + "\nChecking: " + msg + "\n\n"; } - private getDiagnostics(fileName: string): ts.Diagnostic[] { + private getDiagnostics(fileName: string, includeSuggestions = false): ts.Diagnostic[] { return [ ...this.languageService.getSyntacticDiagnostics(fileName), ...this.languageService.getSemanticDiagnostics(fileName), - ...this.languageService.getSuggestionDiagnostics(fileName), + ...(includeSuggestions ? this.languageService.getSuggestionDiagnostics(fileName) : ts.emptyArray), ]; } @@ -583,7 +583,8 @@ namespace FourSlash { public verifyNoErrors() { ts.forEachKey(this.inputFiles, fileName => { - if (!ts.isAnySupportedFileExtension(fileName)) return; + if (!ts.isAnySupportedFileExtension(fileName) + || !this.getProgram().getCompilerOptions().allowJs && !ts.extensionIsTypeScript(ts.extensionFromPath(fileName))) return; const errors = this.getDiagnostics(fileName).filter(e => e.category !== ts.DiagnosticCategory.Suggestion); if (errors.length) { this.printErrorLog(/*expectErrors*/ false, errors); @@ -2522,7 +2523,7 @@ Actual: ${stringify(fullActual)}`); * @param fileName Path to file where error should be retrieved from. */ private getCodeFixes(fileName: string, errorCode?: number): ts.CodeFixAction[] { - const diagnosticsForCodeFix = this.getDiagnostics(fileName).map(diagnostic => ({ + const diagnosticsForCodeFix = this.getDiagnostics(fileName, /*includeSuggestions*/ true).map(diagnostic => ({ start: diagnostic.start, length: diagnostic.length, code: diagnostic.code diff --git a/src/services/codefixes/fixCannotFindModule.ts b/src/services/codefixes/fixCannotFindModule.ts index a28a46caf1b16..41563e6ca9077 100644 --- a/src/services/codefixes/fixCannotFindModule.ts +++ b/src/services/codefixes/fixCannotFindModule.ts @@ -31,7 +31,7 @@ namespace ts.codefix { return host.isKnownTypesPackageName(packageName) ? getTypesPackageName(packageName) : undefined; } - export function tryGetCodeActionForInstallPackageTypes(host: LanguageServiceHost, fileName: string, moduleName: string): CodeAction | undefined { + function tryGetCodeActionForInstallPackageTypes(host: LanguageServiceHost, fileName: string, moduleName: string): CodeAction | undefined { const packageName = getTypesPackageNameToInstall(host, moduleName); return packageName === undefined ? undefined : { description: formatStringFromArgs(getLocaleSpecificMessage(Diagnostics.Install_0), [packageName]), diff --git a/src/services/refactors/installTypesForPackage.ts b/src/services/refactors/installTypesForPackage.ts deleted file mode 100644 index 4e1d71daf668e..0000000000000 --- a/src/services/refactors/installTypesForPackage.ts +++ /dev/null @@ -1,67 +0,0 @@ -/* @internal */ -namespace ts.refactor.installTypesForPackage { - const refactorName = "Install missing types package"; - const actionName = "install"; - const description = "Install missing types package"; - registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); - - function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { - if (getStrictOptionValue(context.program.getCompilerOptions(), "noImplicitAny")) { - // Then it will be available via `fixCannotFindModule`. - return undefined; - } - - const action = getAction(context); - return action && [ - { - name: refactorName, - description, - actions: [ - { - description: action.description, - name: actionName, - }, - ], - }, - ]; - } - - function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined { - Debug.assertEqual(actionName, _actionName); - const action = getAction(context)!; // Should be defined if we said there was an action available. - return { - edits: [], - renameFilename: undefined, - renameLocation: undefined, - commands: action.commands, - }; - } - - function getAction(context: RefactorContext): CodeAction | undefined { - const { file, startPosition } = context; - const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); - if (!isStringLiteral(node) || !isModuleIdentifier(node)) { - return undefined; - } - - const resolvedTo = getResolvedModule(file, node.text); - // Still offer to install types if it resolved to e.g. a ".js" file. - // `tryGetCodeActionForInstallPackageTypes` will verify that we're looking for a valid package name, - // so the fix won't trigger for imports of ".js" files that couldn't be better replaced by typings. - if (resolvedTo && extensionIsTypeScript(resolvedTo.extension)) { - return undefined; - } - - return codefix.tryGetCodeActionForInstallPackageTypes(context.host, file.fileName, node.text); - } - - function isModuleIdentifier(node: StringLiteral): boolean { - switch (node.parent.kind) { - case SyntaxKind.ImportDeclaration: - case SyntaxKind.ExternalModuleReference: - return true; - default: - return false; - } - } -} diff --git a/src/services/refactors/refactors.ts b/src/services/refactors/refactors.ts index 3858b1987434e..bc82b2a24732f 100644 --- a/src/services/refactors/refactors.ts +++ b/src/services/refactors/refactors.ts @@ -1,5 +1,4 @@ /// /// /// -/// /// diff --git a/src/services/services.ts b/src/services/services.ts index e29633a92e038..34af8423e9756 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1422,7 +1422,7 @@ namespace ts { function getSuggestionDiagnostics(fileName: string): Diagnostic[] { synchronizeHostData(); - return computeSuggestionDiagnostics(getValidSourceFile(fileName)); + return computeSuggestionDiagnostics(getValidSourceFile(fileName), program); } function getCompilerOptionsDiagnostics() { diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 8cb26f74a01ee..2588dee2726bc 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -1,8 +1,14 @@ /* @internal */ namespace ts { - export function computeSuggestionDiagnostics(sourceFile: SourceFile): Diagnostic[] { - return sourceFile.commonJsModuleIndicator - ? [createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)] - : emptyArray; + export function computeSuggestionDiagnostics(sourceFile: SourceFile, program: Program): Diagnostic[] { + program.getSemanticDiagnostics(sourceFile); + const checker = program.getDiagnosticsProducingTypeChecker(); + const diags: Diagnostic[] = []; + + if (sourceFile.commonJsModuleIndicator) { + diags.push(createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)); + } + + return diags.concat(checker.getSuggestionDiagnostics(sourceFile)); } } diff --git a/tests/cases/fourslash/codeFixCannotFindModule_suggestion.ts b/tests/cases/fourslash/codeFixCannotFindModule_suggestion.ts new file mode 100644 index 0000000000000..778ac040c1897 --- /dev/null +++ b/tests/cases/fourslash/codeFixCannotFindModule_suggestion.ts @@ -0,0 +1,29 @@ +/// + +// @moduleResolution: node + +// @Filename: /node_modules/abs/subModule.js +////export const x = 0; + +// @Filename: /a.ts +////import * as abs from [|"abs/subModule"|]; + +test.setTypesRegistry({ + "abs": undefined, +}); + +verify.noErrors(); +goTo.file("/a.ts"); +verify.getSuggestionDiagnostics([{ + message: "Could not find a declaration file for module 'abs/subModule'. '/node_modules/abs/subModule.js' implicitly has an 'any' type.", + code: 7016, +}]); + +verify.codeFixAvailable([{ + description: "Install '@types/abs'", + commands: [{ + type: "install package", + file: "/a.ts", + packageName: "@types/abs", + }], +}]); diff --git a/tests/cases/fourslash/codeFixCannotFindModule_suggestion_js.ts b/tests/cases/fourslash/codeFixCannotFindModule_suggestion_js.ts new file mode 100644 index 0000000000000..a8ca9156cf158 --- /dev/null +++ b/tests/cases/fourslash/codeFixCannotFindModule_suggestion_js.ts @@ -0,0 +1,32 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: /node_modules/abs/index.js +////export default function abs() {} + +// @Filename: /a.js +////import abs from [|"abs"|]; + +test.setTypesRegistry({ "abs": undefined }); + +verify.noErrors(); +goTo.file("/a.js"); +verify.getSuggestionDiagnostics([{ + message: "Could not find a declaration file for module 'abs'. '/node_modules/abs/index.js' implicitly has an 'any' type.", + code: 7016, +}]); + +verify.codeFixAvailable([ + { + description: "Install '@types/abs'", + commands: [{ + type: "install package", + file: "/a.js", + packageName: "@types/abs", + }], + }, + { description: "Ignore this error message" }, + { description: "Disable checking for this file" }, +]); diff --git a/tests/cases/fourslash/refactorInstallTypesForPackage.ts b/tests/cases/fourslash/refactorInstallTypesForPackage.ts deleted file mode 100644 index edcdc2ba6e0c9..0000000000000 --- a/tests/cases/fourslash/refactorInstallTypesForPackage.ts +++ /dev/null @@ -1,25 +0,0 @@ -/// - -////import * as abs from "/*a*/abs/subModule/*b*/"; - -test.setTypesRegistry({ - "abs": undefined, -}); - -goTo.select("a", "b"); -verify.refactor({ - name: "Install missing types package", - actionName: "install", - refactors: [ - { - name: "Install missing types package", - description: "Install missing types package", - actions: [ - { - description: "Install '@types/abs'", - name: "install", - } - ] - } - ], -}); diff --git a/tests/cases/fourslash/refactorInstallTypesForPackage_importEquals.ts b/tests/cases/fourslash/refactorInstallTypesForPackage_importEquals.ts deleted file mode 100644 index 18793e4b353dd..0000000000000 --- a/tests/cases/fourslash/refactorInstallTypesForPackage_importEquals.ts +++ /dev/null @@ -1,25 +0,0 @@ -/// - -////import abs = require("/*a*/abs/subModule/*b*/"); - -test.setTypesRegistry({ - "abs": undefined, -}); - -goTo.select("a", "b"); -verify.refactor({ - name: "Install missing types package", - actionName: "install", - refactors: [ - { - name: "Install missing types package", - description: "Install missing types package", - actions: [ - { - description: "Install '@types/abs'", - name: "install", - } - ] - } - ], -}); diff --git a/tests/cases/fourslash/refactorInstallTypesForPackage_js.ts b/tests/cases/fourslash/refactorInstallTypesForPackage_js.ts deleted file mode 100644 index 1cce0d3352603..0000000000000 --- a/tests/cases/fourslash/refactorInstallTypesForPackage_js.ts +++ /dev/null @@ -1,29 +0,0 @@ -/// - -// @allowJs: true - -// @Filename: /node_modules/abs/index.js -////not read - -// @Filename: /a.js -////import abs = require("/*a*/abs/*b*/"); - -test.setTypesRegistry({ "abs": undefined }); - -goTo.select("a", "b"); -verify.refactor({ - name: "Install missing types package", - actionName: "install", - refactors: [ - { - name: "Install missing types package", - description: "Install missing types package", - actions: [ - { - description: "Install '@types/abs'", - name: "install", - } - ] - } - ], -});