Skip to content

Commit

Permalink
fix(typescript-estree): use simpler absolutify behavior for project s…
Browse files Browse the repository at this point in the history
…ervice client file paths (#8520)

* fix(typescript-estree): use simpler absolutify behavior for project service client file paths

* A bit of internal cleanup, and added unit tests

* Lol Windows

* Standardize type specifier style

* oopity

* Correct unit test for undefined

* Update packages/typescript-estree/src/useProgramFromProjectService.ts

Co-authored-by: Brad Zacher <brad.zacher@gmail.com>

* cspell

---------

Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
  • Loading branch information
JoshuaKGoldberg and bradzacher committed Feb 24, 2024
1 parent 1807d55 commit 5e7ec8f
Show file tree
Hide file tree
Showing 3 changed files with 187 additions and 16 deletions.
1 change: 1 addition & 0 deletions .cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"blurple",
"bradzacher",
"camelcase",
"canonicalize",
"Cena",
"codebases",
"Codecov",
Expand Down
50 changes: 34 additions & 16 deletions packages/typescript-estree/src/useProgramFromProjectService.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import debug from 'debug';
import { minimatch } from 'minimatch';
import path from 'path';

import { createProjectProgram } from './create-program/createProjectProgram';
import type { ProjectServiceSettings } from './create-program/createProjectService';
import type { ASTAndDefiniteProgram } from './create-program/shared';
import {
ensureAbsolutePath,
getCanonicalFileName,
} from './create-program/shared';
import type { MutableParseSettings } from './parseSettings';

const log = debug(
Expand All @@ -19,11 +16,17 @@ export function useProgramFromProjectService(
parseSettings: Readonly<MutableParseSettings>,
hasFullTypeInformation: boolean,
): ASTAndDefiniteProgram | undefined {
const filePath = getCanonicalFileName(parseSettings.filePath);
log('Opening project service file for: %s', filePath);
// We don't canonicalize the filename because it caused a performance regression.
// See https://github.com/typescript-eslint/typescript-eslint/issues/8519
const filePathAbsolute = absolutify(parseSettings.filePath);
log(
'Opening project service file for: %s at absolute path %s',
parseSettings.filePath,
filePathAbsolute,
);

const opened = service.openClientFile(
ensureAbsolutePath(filePath, service.host.getCurrentDirectory()),
filePathAbsolute,
parseSettings.codeFullText,
/* scriptKind */ undefined,
parseSettings.tsconfigRootDir,
Expand All @@ -36,36 +39,51 @@ export function useProgramFromProjectService(
'Project service type information enabled; checking for file path match on: %o',
allowDefaultProjectForFiles,
);
const isDefaultProjectAllowedPath = filePathMatchedBy(
parseSettings.filePath,
allowDefaultProjectForFiles,
);

log(
'Default project allowed path: %s, based on config file: %s',
isDefaultProjectAllowedPath,
opened.configFileName,
);

if (opened.configFileName) {
if (filePathMatchedBy(filePath, allowDefaultProjectForFiles)) {
if (isDefaultProjectAllowedPath) {
throw new Error(
`${filePath} was included by allowDefaultProjectForFiles but also was found in the project service. Consider removing it from allowDefaultProjectForFiles.`,
`${parseSettings.filePath} was included by allowDefaultProjectForFiles but also was found in the project service. Consider removing it from allowDefaultProjectForFiles.`,
);
}
} else if (!filePathMatchedBy(filePath, allowDefaultProjectForFiles)) {
} else if (!isDefaultProjectAllowedPath) {
throw new Error(
`${filePath} was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProjectForFiles.`,
`${parseSettings.filePath} was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProjectForFiles.`,
);
}
}
log('Retrieving script info and then program for: %s', filePathAbsolute);

log('Retrieving script info and then program for: %s', filePath);

const scriptInfo = service.getScriptInfo(filePath);
const scriptInfo = service.getScriptInfo(filePathAbsolute);
const program = service
.getDefaultProjectForFile(scriptInfo!.fileName, true)!
.getLanguageService(/*ensureSynchronized*/ true)
.getProgram();

if (!program) {
log('Could not find project service program for: %s', filePath);
log('Could not find project service program for: %s', filePathAbsolute);
return undefined;
}

log('Found project service program for: %s', filePath);
log('Found project service program for: %s', filePathAbsolute);

return createProjectProgram(parseSettings, [program]);

function absolutify(filePath: string): string {
return path.isAbsolute(filePath)
? filePath
: path.join(service.host.getCurrentDirectory(), filePath);
}
}

function filePathMatchedBy(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/* eslint-disable @typescript-eslint/explicit-function-return-type -- Fancy mocks */
import path from 'path';

import type { TypeScriptProjectService } from '../../src/create-program/createProjectService';
import type { ParseSettings } from '../../src/parseSettings';
import { useProgramFromProjectService } from '../../src/useProgramFromProjectService';

const mockCreateProjectProgram = jest.fn();

jest.mock('../../src/create-program/createProjectProgram', () => ({
get createProjectProgram() {
return mockCreateProjectProgram;
},
}));

const mockGetProgram = jest.fn();

const currentDirectory = '/repos/repo';

function createMockProjectService() {
const openClientFile = jest.fn();
const service = {
getDefaultProjectForFile: () => ({
getLanguageService: () => ({
getProgram: mockGetProgram,
}),
}),
getScriptInfo: () => ({}),
host: {
getCurrentDirectory: () => currentDirectory,
},
openClientFile,
};

return {
service: service as typeof service & TypeScriptProjectService,
openClientFile,
};
}

const mockParseSettings = {
filePath: 'path/PascalCaseDirectory/camelCaseFile.ts',
} as ParseSettings;

describe('useProgramFromProjectService', () => {
it('passes an absolute, case-matching file path to service.openClientFile', () => {
const { service } = createMockProjectService();

useProgramFromProjectService(
{ allowDefaultProjectForFiles: undefined, service },
mockParseSettings,
false,
);

expect(service.openClientFile).toHaveBeenCalledWith(
path.normalize('/repos/repo/path/PascalCaseDirectory/camelCaseFile.ts'),
undefined,
undefined,
undefined,
);
});

it('throws an error when hasFullTypeInformation is enabled and the file is both in the project service and allowDefaultProjectForFiles', () => {
const { service } = createMockProjectService();

service.openClientFile.mockReturnValueOnce({
configFileName: 'tsconfig.json',
});

expect(() =>
useProgramFromProjectService(
{ allowDefaultProjectForFiles: [mockParseSettings.filePath], service },
mockParseSettings,
true,
),
).toThrow(
`${mockParseSettings.filePath} was included by allowDefaultProjectForFiles but also was found in the project service. Consider removing it from allowDefaultProjectForFiles.`,
);
});

it('throws an error when hasFullTypeInformation is enabled and the file is neither in the project service nor allowDefaultProjectForFiles', () => {
const { service } = createMockProjectService();

service.openClientFile.mockReturnValueOnce({});

expect(() =>
useProgramFromProjectService(
{ allowDefaultProjectForFiles: [], service },
mockParseSettings,
true,
),
).toThrow(
`${mockParseSettings.filePath} was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProjectForFiles.`,
);
});

it('returns undefined when hasFullTypeInformation is disabled, the file is both in the project service and allowDefaultProjectForFiles, and the service does not have a matching program', () => {
const { service } = createMockProjectService();

mockGetProgram.mockReturnValue(undefined);

service.openClientFile.mockReturnValueOnce({
configFileName: 'tsconfig.json',
});

const actual = useProgramFromProjectService(
{ allowDefaultProjectForFiles: [mockParseSettings.filePath], service },
mockParseSettings,
false,
);

expect(actual).toBeUndefined();
});

it('returns a created program when hasFullTypeInformation is disabled, the file is both in the project service and allowDefaultProjectForFiles, and the service has a matching program', () => {
const { service } = createMockProjectService();
const program = { getSourceFile: jest.fn() };

mockGetProgram.mockReturnValue(program);

service.openClientFile.mockReturnValueOnce({
configFileName: 'tsconfig.json',
});
mockCreateProjectProgram.mockReturnValueOnce(program);

const actual = useProgramFromProjectService(
{ allowDefaultProjectForFiles: [mockParseSettings.filePath], service },
mockParseSettings,
false,
);

expect(actual).toBe(program);
});

it('returns a created program when hasFullTypeInformation is disabled, the file is neither in the project service nor allowDefaultProjectForFiles, and the service has a matching program', () => {
const { service } = createMockProjectService();
const program = { getSourceFile: jest.fn() };

mockGetProgram.mockReturnValue(program);

service.openClientFile.mockReturnValueOnce({});
mockCreateProjectProgram.mockReturnValueOnce(program);

const actual = useProgramFromProjectService(
{ allowDefaultProjectForFiles: [], service },
mockParseSettings,
false,
);

expect(actual).toBe(program);
});
});

0 comments on commit 5e7ec8f

Please sign in to comment.