Merge pull request #20048 from RyanCavanaugh/fixSafeList

Apply the safe list to projects which didn't specify an upfront include
This commit is contained in:
Ryan Cavanaugh 2017-11-16 14:18:11 -08:00 committed by GitHub
commit 1ea1ad4a37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 118 additions and 72 deletions

View File

@ -1482,7 +1482,7 @@ namespace ts.projectSystem {
it("ignores files excluded by a custom safe type list", () => {
const file1 = {
path: "/a/b/f1.ts",
path: "/a/b/f1.js",
content: "export let x = 5"
};
const office = {
@ -1503,7 +1503,7 @@ namespace ts.projectSystem {
it("ignores files excluded by the default type list", () => {
const file1 = {
path: "/a/b/f1.ts",
path: "/a/b/f1.js",
content: "export let x = 5"
};
const minFile = {

View File

@ -304,35 +304,35 @@ namespace ts.projectSystem {
// 1. react typings are installed for .jsx
// 2. loose files names are matched against safe list for typings if
// this is a JS project (only js, jsx, d.ts files are present)
const file1 = {
const lodashJs = {
path: "/a/b/lodash.js",
content: ""
};
const file2 = {
const file2Jsx = {
path: "/a/b/file2.jsx",
content: ""
};
const file3 = {
const file3dts = {
path: "/a/b/file3.d.ts",
content: ""
};
const react = {
const reactDts = {
path: "/a/data/node_modules/@types/react/index.d.ts",
content: "declare const react: { x: number }"
};
const lodash = {
const lodashDts = {
path: "/a/data/node_modules/@types/lodash/index.d.ts",
content: "declare const lodash: { x: number }"
};
const host = createServerHost([file1, file2, file3, customTypesMap]);
const host = createServerHost([lodashJs, file2Jsx, file3dts, customTypesMap]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("lodash", "react") });
}
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings = ["@types/lodash", "@types/react"];
const typingFiles = [lodash, react];
const typingFiles = [lodashDts, reactDts];
executeCommand(this, host, installedTypings, typingFiles, cb);
}
})();
@ -342,35 +342,31 @@ namespace ts.projectSystem {
projectService.openExternalProject({
projectFileName,
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)],
typeAcquisition: {}
rootFiles: [toExternalFile(lodashJs.path), toExternalFile(file2Jsx.path), toExternalFile(file3dts.path)],
typeAcquisition: { }
});
const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [file1.path, file2.path, file3.path]);
checkProjectActualFiles(p, [file2Jsx.path, file3dts.path]);
installer.installAll(/*expectedCount*/ 1);
checkNumberOfProjects(projectService, { externalProjects: 1 });
host.checkTimeoutQueueLengthAndRun(2);
checkNumberOfProjects(projectService, { externalProjects: 1 });
checkProjectActualFiles(p, [file1.path, file2.path, file3.path, lodash.path, react.path]);
checkProjectActualFiles(p, [file2Jsx.path, file3dts.path, lodashDts.path, reactDts.path]);
});
it("external project - no type acquisition, with js & ts files", () => {
it("external project - type acquisition with enable: false", () => {
// Tests:
// 1. No typings are included for JS projects when the project contains ts files
const file1 = {
// Exclude
const jqueryJs = {
path: "/a/b/jquery.js",
content: ""
};
const file2 = {
path: "/a/b/file2.ts",
content: ""
};
const host = createServerHost([file1, file2]);
const host = createServerHost([jqueryJs]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("jquery") });
@ -390,18 +386,62 @@ namespace ts.projectSystem {
projectService.openExternalProject({
projectFileName,
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path)],
rootFiles: [toExternalFile(jqueryJs.path)],
typeAcquisition: { enable: false }
});
const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [jqueryJs.path]);
installer.checkPendingCommands(/*expectedCount*/ 0);
});
it("external project - no type acquisition, with js & ts files", () => {
// Tests:
// 1. No typings are included for JS projects when the project contains ts files
const jqueryJs = {
path: "/a/b/jquery.js",
content: ""
};
const file2Ts = {
path: "/a/b/file2.ts",
content: ""
};
const host = createServerHost([jqueryJs, file2Ts]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("jquery") });
}
enqueueInstallTypingsRequest(project: server.Project, typeAcquisition: TypeAcquisition, unresolvedImports: server.SortedReadonlyArray<string>) {
super.enqueueInstallTypingsRequest(project, typeAcquisition, unresolvedImports);
}
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings: string[] = [];
const typingFiles: FileOrFolder[] = [];
executeCommand(this, host, installedTypings, typingFiles, cb);
}
})();
const projectFileName = "/a/app/test.csproj";
const projectService = createProjectService(host, { typingsInstaller: installer });
projectService.openExternalProject({
projectFileName,
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
rootFiles: [toExternalFile(jqueryJs.path), toExternalFile(file2Ts.path)],
typeAcquisition: {}
});
const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [file2.path]);
checkProjectActualFiles(p, [jqueryJs.path, file2Ts.path]);
installer.checkPendingCommands(/*expectedCount*/ 0);
checkNumberOfProjects(projectService, { externalProjects: 1 });
checkProjectActualFiles(p, [file2.path]);
checkProjectActualFiles(p, [jqueryJs.path, file2Ts.path]);
});
it("external project - with type acquisition, with only js, d.ts files", () => {
@ -409,15 +449,15 @@ namespace ts.projectSystem {
// 1. Safelist matching, type acquisition includes/excludes and package.json typings are all acquired
// 2. Types for safelist matches are not included when they also appear in the type acquisition exclude list
// 3. Multiple includes and excludes are respected in type acquisition
const file1 = {
const lodashJs = {
path: "/a/b/lodash.js",
content: ""
};
const file2 = {
const commanderJs = {
path: "/a/b/commander.js",
content: ""
};
const file3 = {
const file3dts = {
path: "/a/b/file3.d.ts",
content: ""
};
@ -448,7 +488,7 @@ namespace ts.projectSystem {
content: "declare const moment: { x: number }"
};
const host = createServerHost([file1, file2, file3, packageJson, customTypesMap]);
const host = createServerHost([lodashJs, commanderJs, file3dts, packageJson, customTypesMap]);
const installer = new (class extends Installer {
constructor() {
super(host, { typesRegistry: createTypesRegistry("jquery", "commander", "moment", "express") });
@ -465,20 +505,25 @@ namespace ts.projectSystem {
projectService.openExternalProject({
projectFileName,
options: { allowJS: true, moduleResolution: ModuleResolutionKind.NodeJs },
rootFiles: [toExternalFile(file1.path), toExternalFile(file2.path), toExternalFile(file3.path)],
typeAcquisition: { include: ["jquery", "moment"], exclude: ["lodash"] }
rootFiles: [toExternalFile(lodashJs.path), toExternalFile(commanderJs.path), toExternalFile(file3dts.path)],
typeAcquisition: { enable: true, include: ["jquery", "moment"], exclude: ["lodash"] }
});
const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [file1.path, file2.path, file3.path]);
checkProjectActualFiles(p, [file3dts.path]);
installer.installAll(/*expectedCount*/ 1);
checkNumberOfProjects(projectService, { externalProjects: 1 });
host.checkTimeoutQueueLengthAndRun(2);
checkNumberOfProjects(projectService, { externalProjects: 1 });
checkProjectActualFiles(p, [file1.path, file2.path, file3.path, commander.path, express.path, jquery.path, moment.path]);
// Commander: Existed as a JS file
// JQuery: Specified in 'include'
// Moment: Specified in 'include'
// Express: Specified in package.json
// lodash: Excluded (not present)
checkProjectActualFiles(p, [file3dts.path, commander.path, jquery.path, moment.path, express.path]);
});
it("Throttle - delayed typings to install", () => {
@ -548,7 +593,7 @@ namespace ts.projectSystem {
const p = projectService.externalProjects[0];
projectService.checkNumberOfProjects({ externalProjects: 1 });
checkProjectActualFiles(p, [lodashJs.path, commanderJs.path, file3.path]);
checkProjectActualFiles(p, [file3.path]);
installer.checkPendingCommands(/*expectedCount*/ 1);
installer.executePendingCommands();
// expected all typings file to exist
@ -557,7 +602,7 @@ namespace ts.projectSystem {
}
host.checkTimeoutQueueLengthAndRun(2);
checkNumberOfProjects(projectService, { externalProjects: 1 });
checkProjectActualFiles(p, [lodashJs.path, commanderJs.path, file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path]);
checkProjectActualFiles(p, [file3.path, commander.path, express.path, jquery.path, moment.path, lodash.path]);
});
it("Throttle - delayed run install requests", () => {
@ -648,7 +693,7 @@ namespace ts.projectSystem {
const p1 = projectService.externalProjects[0];
const p2 = projectService.externalProjects[1];
projectService.checkNumberOfProjects({ externalProjects: 2 });
checkProjectActualFiles(p1, [lodashJs.path, commanderJs.path, file3.path]);
checkProjectActualFiles(p1, [file3.path]);
checkProjectActualFiles(p2, [file3.path]);
installer.executePendingCommands();
@ -659,7 +704,7 @@ namespace ts.projectSystem {
installer.executePendingCommands();
host.checkTimeoutQueueLengthAndRun(3); // for 2 projects and 1 refreshing inferred project
checkProjectActualFiles(p1, [lodashJs.path, commanderJs.path, file3.path, commander.path, jquery.path, lodash.path, cordova.path]);
checkProjectActualFiles(p1, [file3.path, commander.path, jquery.path, lodash.path, cordova.path]);
checkProjectActualFiles(p2, [file3.path, grunt.path, gulp.path]);
});
@ -973,7 +1018,7 @@ namespace ts.projectSystem {
}
};
session.executeCommand(changeRequest);
host.checkTimeoutQueueLengthAndRun(2); // This enqueues the updategraph and refresh inferred projects
host.checkTimeoutQueueLengthAndRun(0); // This enqueues the updategraph and refresh inferred projects
const version2 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
assert.equal(version1, version2, "set of unresolved imports should not change");
});

View File

@ -144,9 +144,9 @@ interface Array<T> {}`
}
export function checkFileNames(caption: string, actualFileNames: ReadonlyArray<string>, expectedFileNames: string[]) {
assert.equal(actualFileNames.length, expectedFileNames.length, `${caption}: incorrect actual number of files, expected ${expectedFileNames}, got ${actualFileNames}`);
assert.equal(actualFileNames.length, expectedFileNames.length, `${caption}: incorrect actual number of files, expected:\r\n${expectedFileNames.join("\r\n")}\r\ngot: ${actualFileNames.join("\r\n")}`);
for (const f of expectedFileNames) {
assert.isTrue(contains(actualFileNames, f), `${caption}: expected to find ${f} in ${actualFileNames}`);
assert.equal(true, contains(actualFileNames, f), `${caption}: expected to find ${f} in ${actualFileNames}`);
}
}

View File

@ -2193,8 +2193,13 @@ namespace ts.server {
applySafeList(proj: protocol.ExternalProject): NormalizedPath[] {
const { rootFiles, typeAcquisition } = proj;
const types = (typeAcquisition && typeAcquisition.include) || [];
Debug.assert(!!typeAcquisition, "proj.typeAcquisition should be set by now");
// If type acquisition has been explicitly disabled, do not exclude anything from the project
if (typeAcquisition.enable === false) {
return [];
}
const typeAcqInclude = typeAcquisition.include || (typeAcquisition.include = []);
const excludeRules: string[] = [];
const normalizedNames = rootFiles.map(f => normalizeSlashes(f.fileName)) as NormalizedPath[];
@ -2209,8 +2214,10 @@ namespace ts.server {
// If the file matches, collect its types packages and exclude rules
if (rule.types) {
for (const type of rule.types) {
if (types.indexOf(type) < 0) {
types.push(type);
// Best-effort de-duping here - doesn't need to be unduplicated but
// we don't want the list to become a 400-element array of just 'kendo'
if (typeAcqInclude.indexOf(type) < 0) {
typeAcqInclude.push(type);
}
}
}
@ -2248,12 +2255,6 @@ namespace ts.server {
}
}
}
// Copy back this field into the project if needed
if (types.length > 0) {
proj.typeAcquisition = proj.typeAcquisition || {};
proj.typeAcquisition.include = types;
}
}
const excludeRegexes = excludeRules.map(e => new RegExp(e, "i"));
@ -2264,7 +2265,7 @@ namespace ts.server {
}
else {
let exclude = false;
if (typeAcquisition && (typeAcquisition.enable || typeAcquisition.enableAutoDiscovery)) {
if (typeAcquisition.enable || typeAcquisition.enableAutoDiscovery) {
const baseName = getBaseFileName(normalizedNames[i].toLowerCase());
if (fileExtensionIs(baseName, "js")) {
const inferredTypingName = removeFileExtension(baseName);
@ -2272,7 +2273,14 @@ namespace ts.server {
if (this.legacySafelist[cleanedTypingName]) {
this.logger.info(`Excluded '${normalizedNames[i]}' because it matched ${cleanedTypingName} from the legacy safelist`);
excludedFiles.push(normalizedNames[i]);
// *exclude* it from the project...
exclude = true;
// ... but *include* it in the list of types to acquire
const typeName = this.legacySafelist[cleanedTypingName];
// Same best-effort dedupe as above
if (typeAcqInclude.indexOf(typeName) < 0) {
typeAcqInclude.push(typeName);
}
}
}
}
@ -2292,6 +2300,12 @@ namespace ts.server {
const typeAcquisition = convertEnableAutoDiscoveryToEnable(proj.typingOptions);
proj.typeAcquisition = typeAcquisition;
}
proj.typeAcquisition = proj.typeAcquisition || {};
proj.typeAcquisition.include = proj.typeAcquisition.include || [];
proj.typeAcquisition.exclude = proj.typeAcquisition.exclude || [];
if (proj.typeAcquisition.enable === undefined) {
proj.typeAcquisition.enable = hasNoTypeScriptSource(proj.rootFiles.map(f => f.fileName));
}
const excludedFiles = this.applySafeList(proj);

View File

@ -52,6 +52,11 @@ namespace ts.server {
return counts.ts === 0 && counts.tsx === 0;
}
/* @internal */
export function hasNoTypeScriptSource(fileNames: string[]): boolean {
return !fileNames.some(fileName => (fileExtensionIs(fileName, Extension.Ts) && !fileExtensionIs(fileName, Extension.Dts)) || fileExtensionIs(fileName, Extension.Tsx));
}
/* @internal */
export interface ProjectFilesWithTSDiagnostics extends protocol.ProjectFiles {
projectErrors: ReadonlyArray<Diagnostic>;
@ -1436,26 +1441,10 @@ namespace ts.server {
}
setTypeAcquisition(newTypeAcquisition: TypeAcquisition): void {
if (!newTypeAcquisition) {
// set default typings options
newTypeAcquisition = {
enable: allRootFilesAreJsOrDts(this),
include: [],
exclude: []
};
}
else {
if (newTypeAcquisition.enable === undefined) {
// if autoDiscovery was not specified by the caller - set it based on the content of the project
newTypeAcquisition.enable = allRootFilesAreJsOrDts(this);
}
if (!newTypeAcquisition.include) {
newTypeAcquisition.include = [];
}
if (!newTypeAcquisition.exclude) {
newTypeAcquisition.exclude = [];
}
}
Debug.assert(!!newTypeAcquisition, "newTypeAcquisition may not be null/undefined");
Debug.assert(!!newTypeAcquisition.include, "newTypeAcquisition.include may not be null/undefined");
Debug.assert(!!newTypeAcquisition.exclude, "newTypeAcquisition.exclude may not be null/undefined");
Debug.assert(typeof newTypeAcquisition.enable === "boolean", "newTypeAcquisition.enable may not be null/undefined");
this.typeAcquisition = newTypeAcquisition;
}
}

View File

@ -123,9 +123,6 @@ namespace ts.server.typingsInstaller {
this.log.writeLine(`Finished typings discovery: ${JSON.stringify(discoverTypingsResult)}`);
}
// respond with whatever cached typings we have now
this.sendResponse(this.createSetTypings(req, discoverTypingsResult.cachedTypingPaths));
// start watching files
this.watchFiles(req.projectName, discoverTypingsResult.filesToWatch);
@ -134,6 +131,7 @@ namespace ts.server.typingsInstaller {
this.installTypings(req, req.cachePath || this.globalCachePath, discoverTypingsResult.cachedTypingPaths, discoverTypingsResult.newTypingNames);
}
else {
this.sendResponse(this.createSetTypings(req, discoverTypingsResult.cachedTypingPaths));
if (this.log.isEnabled()) {
this.log.writeLine(`No new typings were requested as a result of typings discovery`);
}