From 60ca443819b383896f1e1631ca6242418c839a45 Mon Sep 17 00:00:00 2001 From: Lyu Jason Date: Fri, 6 Sep 2024 15:58:01 +0800 Subject: [PATCH 1/4] warn svelte input only when no svelte files at all --- .../plugins/typescript/DocumentSnapshot.ts | 7 +- .../src/plugins/typescript/service.ts | 90 ++++++++++++++----- packages/language-server/src/svelte-check.ts | 28 +++--- .../test/plugins/typescript/service.test.ts | 41 ++++++++- 4 files changed, 129 insertions(+), 37 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..b26c251e0 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(); @@ -167,18 +171,31 @@ export async function getService( * Prevent infinite loop when the project reference is circular */ const triedTsConfig = new Set(); + const possibleConfigPath = new Set(); const needAssign = !configFileForOpenFiles.has(path); let service = await getConfiguredService(tsconfigPath); if (!needAssign) { return service; } - const defaultService = await findDefaultServiceForFile(service, triedTsConfig); + const defaultService = await findDefaultServiceForFile( + service, + triedTsConfig, + possibleConfigPath + ); if (defaultService) { configFileForOpenFiles.set(path, defaultService.tsconfigPath); return defaultService; } + for (const configPath of possibleConfigPath) { + const service = await getConfiguredService(configPath); + service.getService(); + if (service.snapshotManager.has(path)) { + return service; + } + } + tsconfigPath = ''; } @@ -207,7 +224,8 @@ export async function getService( async function findDefaultServiceForFile( service: LanguageServiceContainer, - triedTsConfig: Set + triedTsConfig: Set, + possibleConfigPath: Set ): Promise { service.ensureProjectFileUpdates(); if (service.snapshotManager.isProjectFile(path)) { @@ -217,13 +235,16 @@ export async function getService( return; } + possibleConfigPath.add(service.tsconfigPath); + // TODO: maybe add support for ts 5.6's ancestor searching - return findDefaultFromProjectReferences(service, triedTsConfig); + return findDefaultFromProjectReferences(service, triedTsConfig, possibleConfigPath); } async function findDefaultFromProjectReferences( service: LanguageServiceContainer, - triedTsConfig: Set + triedTsConfig: Set, + possibleConfigPath: Set ) { const projectReferences = service.getResolvedProjectReferences(); if (projectReferences.length === 0) { @@ -243,7 +264,11 @@ export async function getService( for (const ref of possibleSubPaths) { const subService = await getConfiguredService(ref); - const defaultService = await findDefaultServiceForFile(subService, triedTsConfig); + const defaultService = await findDefaultServiceForFile( + subService, + triedTsConfig, + possibleConfigPath + ); if (defaultService) { return defaultService; } @@ -360,6 +385,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}`), @@ -736,20 +762,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 +770,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 +955,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 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..ae2f0ff25 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(); From 07691a6947e008792d402fb34073edcc5afe1343 Mon Sep 17 00:00:00 2001 From: Lyu Jason Date: Fri, 6 Sep 2024 18:01:36 +0800 Subject: [PATCH 2/4] wip --- .../src/plugins/typescript/service.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index b26c251e0..023b7ff46 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -113,6 +113,7 @@ const dependedConfigWatchers = new FileMap(); const configPathToDependedProject = new FileMap(); const configFileModifiedTime = new FileMap(); const configFileForOpenFiles = new FileMap(); +const containingServices = new FileMap(); const pendingReloads = new FileSet(); const documentRegistries = new Map(); const pendingForAllServices = new Set>(); @@ -190,8 +191,8 @@ export async function getService( for (const configPath of possibleConfigPath) { const service = await getConfiguredService(configPath); - service.getService(); - if (service.snapshotManager.has(path)) { + const ls = service.getService(); + if (ls.getProgram()?.getSourceFile(path)) { return service; } } @@ -340,6 +341,7 @@ async function createLanguageService( const projectConfig = getParsedConfig(); const { options: compilerOptions, raw, errors: configErrors } = projectConfig; + const allowJs = compilerOptions.allowJs ?? !!compilerOptions.checkJs; const getCanonicalFileName = createGetCanonicalFileName(tsSystem.useCaseSensitiveFileNames); watchWildCardDirectories(projectConfig); @@ -555,12 +557,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; From 318f56d64b8c171891bf2d40cc878738cc266a6a Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Mon, 16 Sep 2024 14:39:16 +0800 Subject: [PATCH 3/4] don't treat client files as root in configured service so it can be removed from the service --- .../src/plugins/typescript/service.ts | 42 +++++++------- .../test/plugins/typescript/service.test.ts | 55 ++++++++++++++++++- .../different-ts-service/tsconfig.json | 1 + 3 files changed, 75 insertions(+), 23 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index 023b7ff46..d0c7776f0 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -113,7 +113,6 @@ const dependedConfigWatchers = new FileMap(); const configPathToDependedProject = new FileMap(); const configFileModifiedTime = new FileMap(); const configFileForOpenFiles = new FileMap(); -const containingServices = new FileMap(); const pendingReloads = new FileSet(); const documentRegistries = new Map(); const pendingForAllServices = new Set>(); @@ -172,24 +171,19 @@ export async function getService( * Prevent infinite loop when the project reference is circular */ const triedTsConfig = new Set(); - const possibleConfigPath = new Set(); const needAssign = !configFileForOpenFiles.has(path); let service = await getConfiguredService(tsconfigPath); if (!needAssign) { return service; } - const defaultService = await findDefaultServiceForFile( - service, - triedTsConfig, - possibleConfigPath - ); + const defaultService = await findDefaultServiceForFile(service, triedTsConfig); if (defaultService) { configFileForOpenFiles.set(path, defaultService.tsconfigPath); return defaultService; } - for (const configPath of possibleConfigPath) { + for (const configPath of triedTsConfig) { const service = await getConfiguredService(configPath); const ls = service.getService(); if (ls.getProgram()?.getSourceFile(path)) { @@ -225,8 +219,7 @@ export async function getService( async function findDefaultServiceForFile( service: LanguageServiceContainer, - triedTsConfig: Set, - possibleConfigPath: Set + triedTsConfig: Set ): Promise { service.ensureProjectFileUpdates(); if (service.snapshotManager.isProjectFile(path)) { @@ -236,16 +229,15 @@ export async function getService( return; } - possibleConfigPath.add(service.tsconfigPath); + triedTsConfig.add(service.tsconfigPath); // TODO: maybe add support for ts 5.6's ancestor searching - return findDefaultFromProjectReferences(service, triedTsConfig, possibleConfigPath); + return findDefaultFromProjectReferences(service, triedTsConfig); } async function findDefaultFromProjectReferences( service: LanguageServiceContainer, - triedTsConfig: Set, - possibleConfigPath: Set + triedTsConfig: Set ) { const projectReferences = service.getResolvedProjectReferences(); if (projectReferences.length === 0) { @@ -265,11 +257,7 @@ export async function getService( for (const ref of possibleSubPaths) { const subService = await getConfiguredService(ref); - const defaultService = await findDefaultServiceForFile( - subService, - triedTsConfig, - possibleConfigPath - ); + const defaultService = await findDefaultServiceForFile(subService, triedTsConfig); if (defaultService) { return defaultService; } @@ -342,6 +330,7 @@ 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); @@ -675,14 +664,22 @@ async function createLanguageService( : snapshotManager.getProjectFileNames(); const canonicalProjectFileNames = new Set(projectFiles.map(getCanonicalFileName)); + // We only assign project files or files already in the program to the language service + // so don't need to include other client files otherwise it will stay in the program and not be 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 ]) ); @@ -1188,6 +1185,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/test/plugins/typescript/service.test.ts b/packages/language-server/test/plugins/typescript/service.test.ts index ae2f0ff25..71db7f064 100644 --- a/packages/language-server/test/plugins/typescript/service.test.ts +++ b/packages/language-server/test/plugins/typescript/service.test.ts @@ -604,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. From 2a4ff9e553a7fc79435982ddb5502ad0d070c12c Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 18 Sep 2024 16:40:57 +0200 Subject: [PATCH 4/4] tweak comments --- packages/language-server/src/plugins/typescript/service.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index d0c7776f0..f1b70aa09 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -177,12 +177,15 @@ 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(); @@ -664,8 +667,8 @@ async function createLanguageService( : snapshotManager.getProjectFileNames(); const canonicalProjectFileNames = new Set(projectFiles.map(getCanonicalFileName)); - // We only assign project files or files already in the program to the language service - // so don't need to include other client files otherwise it will stay in the program and not be removed + // 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())