From 94a7352d96e0a7d512cde0de1e73ebb25347b7c7 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei-Da" <36730922+jasonlyu123@users.noreply.github.com> Date: Wed, 18 Sep 2024 22:45:59 +0800 Subject: [PATCH] fix: include files indirectly belonging to a project into correct project (#2488) Fixes #2486 Fixes #2485 --- .../plugins/typescript/DocumentSnapshot.ts | 7 +- .../src/plugins/typescript/service.ts | 100 ++++++++++++++---- packages/language-server/src/svelte-check.ts | 28 +++-- .../test/plugins/typescript/service.test.ts | 96 ++++++++++++++++- .../different-ts-service/tsconfig.json | 1 + 5 files changed, 194 insertions(+), 38 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts index 00c902554..aaf2a11e6 100644 --- a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts +++ b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts @@ -109,7 +109,7 @@ export namespace DocumentSnapshot { tsSystem: ts.System ) { if (isSvelteFilePath(filePath)) { - return DocumentSnapshot.fromSvelteFilePath(filePath, createDocument, options); + return DocumentSnapshot.fromSvelteFilePath(filePath, createDocument, options, tsSystem); } else { return DocumentSnapshot.fromNonSvelteFilePath(filePath, tsSystem); } @@ -173,9 +173,10 @@ export namespace DocumentSnapshot { export function fromSvelteFilePath( filePath: string, createDocument: (filePath: string, text: string) => Document, - options: SvelteSnapshotOptions + options: SvelteSnapshotOptions, + tsSystem: ts.System ) { - const originalText = ts.sys.readFile(filePath) ?? ''; + const originalText = tsSystem.readFile(filePath) ?? ''; return fromDocument(createDocument(filePath, originalText), options); } } diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index 8f5720bb0..f1b70aa09 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -101,6 +101,10 @@ export interface TsConfigInfo { extendedConfigPaths?: Set; } +enum TsconfigSvelteDiagnostics { + NO_SVELTE_INPUT = 100_001 +} + const maxProgramSizeForNonTsFiles = 20 * 1024 * 1024; // 20 MB const services = new FileMap>(); const serviceSizeMap = new FileMap(); @@ -173,12 +177,23 @@ export async function getService( return service; } + // First try to find a service whose includes config matches our file const defaultService = await findDefaultServiceForFile(service, triedTsConfig); if (defaultService) { configFileForOpenFiles.set(path, defaultService.tsconfigPath); return defaultService; } + // If no such service found, see if the file is part of any existing service indirectly. + // This can happen if the includes doesn't match the file but it was imported from one of the included files. + for (const configPath of triedTsConfig) { + const service = await getConfiguredService(configPath); + const ls = service.getService(); + if (ls.getProgram()?.getSourceFile(path)) { + return service; + } + } + tsconfigPath = ''; } @@ -217,6 +232,8 @@ export async function getService( return; } + triedTsConfig.add(service.tsconfigPath); + // TODO: maybe add support for ts 5.6's ancestor searching return findDefaultFromProjectReferences(service, triedTsConfig); } @@ -315,6 +332,8 @@ async function createLanguageService( const projectConfig = getParsedConfig(); const { options: compilerOptions, raw, errors: configErrors } = projectConfig; + const allowJs = compilerOptions.allowJs ?? !!compilerOptions.checkJs; + const virtualDocuments = new FileMap(tsSystem.useCaseSensitiveFileNames); const getCanonicalFileName = createGetCanonicalFileName(tsSystem.useCaseSensitiveFileNames); watchWildCardDirectories(projectConfig); @@ -360,6 +379,7 @@ async function createLanguageService( let languageServiceReducedMode = false; let projectVersion = 0; let dirty = projectConfig.fileNames.length > 0; + let skipSvelteInputCheck = !tsconfigPath; const host: ts.LanguageServiceHost = { log: (message) => Logger.debug(`[ts] ${message}`), @@ -529,12 +549,19 @@ async function createLanguageService( return prevSnapshot; } + const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig); + if (!prevSnapshot) { svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath); + if (configFileForOpenFiles.get(filePath) === '' && services.size > 1) { + configFileForOpenFiles.delete(filePath); + } + } else if (prevSnapshot.scriptKind !== newSnapshot.scriptKind && !allowJs) { + // if allowJs is false, we need to invalid the cache so that js svelte files can be loaded through module resolution + svelteModuleLoader.deleteFromModuleCache(filePath); + configFileForOpenFiles.delete(filePath); } - const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig); - snapshotManager.set(filePath, newSnapshot); return newSnapshot; @@ -640,14 +667,22 @@ async function createLanguageService( : snapshotManager.getProjectFileNames(); const canonicalProjectFileNames = new Set(projectFiles.map(getCanonicalFileName)); + // We only assign project files (i.e. those found through includes config) and virtual files to getScriptFileNames. + // We don't to include other client files otherwise they stay in the program and are never removed + const clientFiles = tsconfigPath + ? Array.from(virtualDocuments.values()) + .map((v) => v.getFilePath()) + .filter(isNotNullOrUndefined) + : snapshotManager.getClientFileNames(); + return Array.from( new Set([ ...projectFiles, // project file is read from the file system so it's more likely to have // the correct casing - ...snapshotManager - .getClientFileNames() - .filter((file) => !canonicalProjectFileNames.has(getCanonicalFileName(file))), + ...clientFiles.filter( + (file) => !canonicalProjectFileNames.has(getCanonicalFileName(file)) + ), ...svelteTsxFiles ]) ); @@ -736,20 +771,6 @@ async function createLanguageService( } } - const svelteConfigDiagnostics = checkSvelteInput(parsedConfig); - if (svelteConfigDiagnostics.length > 0) { - docContext.reportConfigError?.({ - uri: pathToUrl(tsconfigPath), - diagnostics: svelteConfigDiagnostics.map((d) => ({ - message: d.messageText as string, - range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, - severity: ts.DiagnosticCategory.Error, - source: 'svelte' - })) - }); - parsedConfig.errors.push(...svelteConfigDiagnostics); - } - return { ...parsedConfig, fileNames: parsedConfig.fileNames.map(normalizePath), @@ -758,22 +779,32 @@ async function createLanguageService( }; } - function checkSvelteInput(config: ts.ParsedCommandLine) { + function checkSvelteInput(program: ts.Program | undefined, config: ts.ParsedCommandLine) { if (!tsconfigPath || config.raw.references || config.raw.files) { return []; } - const svelteFiles = config.fileNames.filter(isSvelteFilePath); - if (svelteFiles.length > 0) { + const configFileName = basename(tsconfigPath); + // Only report to possible nearest config file since referenced project might not be a svelte project + if (configFileName !== 'tsconfig.json' && configFileName !== 'jsconfig.json') { + return []; + } + + const hasSvelteFiles = + config.fileNames.some(isSvelteFilePath) || + program?.getSourceFiles().some((file) => isSvelteFilePath(file.fileName)); + + if (hasSvelteFiles) { return []; } + const { include, exclude } = config.raw; const inputText = JSON.stringify(include); const excludeText = JSON.stringify(exclude); const svelteConfigDiagnostics: ts.Diagnostic[] = [ { category: ts.DiagnosticCategory.Error, - code: 0, + code: TsconfigSvelteDiagnostics.NO_SVELTE_INPUT, file: undefined, start: undefined, length: undefined, @@ -933,6 +964,28 @@ async function createLanguageService( dirty = false; compilerHost = undefined; + if (!skipSvelteInputCheck) { + const svelteConfigDiagnostics = checkSvelteInput(program, projectConfig); + const codes = svelteConfigDiagnostics.map((d) => d.code); + if (!svelteConfigDiagnostics.length) { + // stop checking once it passed once + skipSvelteInputCheck = true; + } + // report even if empty to clear previous diagnostics + docContext.reportConfigError?.({ + uri: pathToUrl(tsconfigPath), + diagnostics: svelteConfigDiagnostics.map((d) => ({ + message: d.messageText as string, + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } }, + severity: ts.DiagnosticCategory.Error, + source: 'svelte' + })) + }); + projectConfig.errors = projectConfig.errors + .filter((e) => !codes.includes(e.code)) + .concat(svelteConfigDiagnostics); + } + // https://github.com/microsoft/TypeScript/blob/23faef92703556567ddbcb9afb893f4ba638fc20/src/server/project.ts#L1624 // host.getCachedExportInfoMap will create the cache if it doesn't exist // so we need to check the property instead @@ -1135,6 +1188,7 @@ async function createLanguageService( if (!filePath) { return; } + virtualDocuments.set(filePath, document); configFileForOpenFiles.set(filePath, tsconfigPath || workspacePath); updateSnapshot(document); scheduleUpdate(filePath); diff --git a/packages/language-server/src/svelte-check.ts b/packages/language-server/src/svelte-check.ts index 652f2d7a7..2d3e11ff3 100644 --- a/packages/language-server/src/svelte-check.ts +++ b/packages/language-server/src/svelte-check.ts @@ -209,19 +209,14 @@ export class SvelteCheck { }; if (lsContainer.configErrors.length > 0) { - const grouped = groupBy( - lsContainer.configErrors, - (error) => error.file?.fileName ?? tsconfigPath - ); - - return Object.entries(grouped).map(([filePath, errors]) => ({ - filePath, - text: '', - diagnostics: errors.map((diagnostic) => map(diagnostic)) - })); + return reportConfigError(); } const lang = lsContainer.getService(); + if (lsContainer.configErrors.length > 0) { + return reportConfigError(); + } + const files = lang.getProgram()?.getSourceFiles() || []; const options = lang.getProgram()?.getCompilerOptions() || {}; @@ -318,6 +313,19 @@ export class SvelteCheck { } }) ); + + function reportConfigError() { + const grouped = groupBy( + lsContainer.configErrors, + (error) => error.file?.fileName ?? tsconfigPath + ); + + return Object.entries(grouped).map(([filePath, errors]) => ({ + filePath, + text: '', + diagnostics: errors.map((diagnostic) => map(diagnostic)) + })); + } } private async getDiagnosticsForFile(uri: string) { diff --git a/packages/language-server/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index 50c793c6f..71db7f064 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -108,7 +108,46 @@ describe('service', () => { assert.ok(called); }); - it('do not errors if referenced tsconfig matches no svelte files', async () => { + it('does not report no svelte files when loaded through import', async () => { + const dirPath = getRandomVirtualDirPath(testDir); + const { virtualSystem, lsDocumentContext, rootUris } = setup(); + + virtualSystem.readDirectory = () => [path.join(dirPath, 'random.ts')]; + + virtualSystem.writeFile( + path.join(dirPath, 'tsconfig.json'), + JSON.stringify({ + include: ['**/*.ts'] + }) + ); + + virtualSystem.writeFile( + path.join(dirPath, 'random.svelte'), + '' + ); + + virtualSystem.writeFile( + path.join(dirPath, 'random.ts'), + 'import {} from "./random.svelte"' + ); + + let called = false; + const service = await getService(path.join(dirPath, 'random.svelte'), rootUris, { + ...lsDocumentContext, + reportConfigError: (message) => { + called = true; + assert.deepStrictEqual([], message.diagnostics); + } + }); + + assert.equal( + normalizePath(path.join(dirPath, 'tsconfig.json')), + normalizePath(service.tsconfigPath) + ); + assert.ok(called); + }); + + it('does not errors if referenced tsconfig matches no svelte files', async () => { const dirPath = getRandomVirtualDirPath(testDir); const { virtualSystem, lsDocumentContext, rootUris } = setup(); @@ -565,10 +604,63 @@ describe('service', () => { sinon.assert.calledWith(watchDirectory.firstCall, []); }); + it('assigns files to service with the file in the program', async () => { + const dirPath = getRandomVirtualDirPath(testDir); + const { virtualSystem, lsDocumentContext, rootUris } = setup(); + + const tsconfigPath = path.join(dirPath, 'tsconfig.json'); + virtualSystem.writeFile( + tsconfigPath, + JSON.stringify({ + compilerOptions: { + noImplicitOverride: true + }, + include: ['src/*.ts'] + }) + ); + + const referencedFile = path.join(dirPath, 'anotherPackage', 'index.svelte'); + const tsFilePath = path.join(dirPath, 'src', 'random.ts'); + + virtualSystem.readDirectory = () => [tsFilePath]; + virtualSystem.writeFile( + referencedFile, + '' + ); + virtualSystem.writeFile(tsFilePath, 'import "../anotherPackage/index.svelte";'); + + const document = new Document( + pathToUrl(referencedFile), + virtualSystem.readFile(referencedFile)! + ); + document.openedByClient = true; + const ls = await getService(referencedFile, rootUris, lsDocumentContext); + ls.updateSnapshot(document); + + assert.equal(normalizePath(ls.tsconfigPath), normalizePath(tsconfigPath)); + + const noImplicitOverrideErrorCode = 4114; + const findError = (ls: LanguageServiceContainer) => + ls + .getService() + .getSemanticDiagnostics(referencedFile) + .find((f) => f.code === noImplicitOverrideErrorCode); + + assert.ok(findError(ls)); + + virtualSystem.writeFile(tsFilePath, ''); + ls.updateTsOrJsFile(tsFilePath); + + const ls2 = await getService(referencedFile, rootUris, lsDocumentContext); + ls2.updateSnapshot(document); + + assert.deepStrictEqual(findError(ls2), undefined); + }); + function getSemanticDiagnosticsMessages(ls: LanguageServiceContainer, filePath: string) { return ls .getService() .getSemanticDiagnostics(filePath) - .map((d) => d.messageText); + .map((d) => ts.flattenDiagnosticMessageText(d.messageText, '\n')); } }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/different-ts-service/tsconfig.json b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/different-ts-service/tsconfig.json index 6d3385d79..04d6ed3d5 100644 --- a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/different-ts-service/tsconfig.json +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/different-ts-service/tsconfig.json @@ -1,5 +1,6 @@ { "compilerOptions": { + "allowJs": true, /** This is actually not needed, but makes the tests faster because TS does not look up other types.