Skip to content

Commit

Permalink
fix: include files indirectly belonging to a project into correct pro…
Browse files Browse the repository at this point in the history
…ject (#2488)

Fixes #2486
Fixes #2485
  • Loading branch information
jasonlyu123 authored Sep 18, 2024
1 parent 73125a6 commit 94a7352
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
100 changes: 77 additions & 23 deletions packages/language-server/src/plugins/typescript/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ export interface TsConfigInfo {
extendedConfigPaths?: Set<string>;
}

enum TsconfigSvelteDiagnostics {
NO_SVELTE_INPUT = 100_001
}

const maxProgramSizeForNonTsFiles = 20 * 1024 * 1024; // 20 MB
const services = new FileMap<Promise<LanguageServiceContainer>>();
const serviceSizeMap = new FileMap<number>();
Expand Down Expand Up @@ -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 = '';
}

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<Document>(tsSystem.useCaseSensitiveFileNames);

const getCanonicalFileName = createGetCanonicalFileName(tsSystem.useCaseSensitiveFileNames);
watchWildCardDirectories(projectConfig);
Expand Down Expand Up @@ -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}`),
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
])
);
Expand Down Expand Up @@ -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),
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1135,6 +1188,7 @@ async function createLanguageService(
if (!filePath) {
return;
}
virtualDocuments.set(filePath, document);
configFileForOpenFiles.set(filePath, tsconfigPath || workspacePath);
updateSnapshot(document);
scheduleUpdate(filePath);
Expand Down
28 changes: 18 additions & 10 deletions packages/language-server/src/svelte-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() || {};

Expand Down Expand Up @@ -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) {
Expand Down
96 changes: 94 additions & 2 deletions packages/language-server/test/plugins/typescript/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
'<script lang="ts">const a: number = null;</script>'
);

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();

Expand Down Expand Up @@ -565,10 +604,63 @@ describe('service', () => {
sinon.assert.calledWith(watchDirectory.firstCall, <RelativePattern[]>[]);
});

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: <ts.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,
'<script lang="ts">class A { a =1 }; class B extends A { a =2 };</script>'
);
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'));
}
});
Original file line number Diff line number Diff line change
@@ -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.
Expand Down

0 comments on commit 94a7352

Please sign in to comment.