Skip to content

Commit 364e793

Browse files
ericsnowcurrentlyericsnowcurrently
committed
Make PythonLocatorQuery.searchLocations more explicit about the non-rooted cases.
1 parent 5fb8ee3 commit 364e793

File tree

7 files changed

+113
-72
lines changed

7 files changed

+113
-72
lines changed

‎src/client/pythonEnvironments/base/locator.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,31 @@ export type BasicPythonLocatorQuery = {
8282
kinds?: PythonEnvKind[];
8383
};
8484

85+
/**
86+
* The portion of a query related to env search locations.
87+
*/
88+
export type SearchLocations = {
89+
/**
90+
* The locations under which to look for environments.
91+
*/
92+
roots: Uri[];
93+
/**
94+
* If true, also look for environments that do not have a search location.
95+
*/
96+
includeNonRooted?: boolean;
97+
};
98+
8599
/**
86100
* The full set of possible info to send to a locator when requesting environments.
87101
*
88102
* This is directly correlated with the `PythonEnvsChangedEvent`
89103
* emitted by watchers.
90-
*
91-
* @prop - searchLocations - if provided, results should be limited to
92-
* within these locations; `null` means "include envs that have
93-
* no search location"; if not provided, the search location
94-
* of each enviroment is not considered when filtering
95104
*/
96105
export type PythonLocatorQuery = BasicPythonLocatorQuery & {
97-
searchLocations?: (Uri | null)[] | null;
106+
/**
107+
* If provided, results should be limited to within these locations.
108+
*/
109+
searchLocations?: SearchLocations;
98110
};
99111

100112
type QueryForEvent<E> = E extends PythonEnvsChangedEvent ? PythonLocatorQuery : BasicPythonLocatorQuery;

‎src/client/pythonEnvironments/base/locatorUtils.ts

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,16 @@ export function getQueryFilter(query: PythonLocatorQuery): (env: PythonEnvInfo)
1818
const kinds = (query.kinds !== undefined && query.kinds.length > 0)
1919
? query.kinds
2020
: undefined;
21-
let includeGlobal = !query.searchLocations || query.searchLocations.length === 0;
22-
let locationFilters: ((u: Uri) => boolean)[] | undefined;
23-
if (!includeGlobal) {
24-
const candidates = query.searchLocations!.filter((u) => !!u);
25-
includeGlobal = candidates.length < query.searchLocations!.length;
26-
if (candidates.length > 0) {
27-
locationFilters = candidates.map((loc) => getURIFilter(loc!, {
28-
checkParent: true,
29-
checkExact: true,
30-
}));
21+
let includeNonRooted = true;
22+
if (query.searchLocations !== undefined) {
23+
if (query.searchLocations.includeNonRooted !== undefined) {
24+
includeNonRooted = query.searchLocations.includeNonRooted;
25+
} else {
26+
// We default to `false`.
27+
includeNonRooted = false;
3128
}
3229
}
30+
const locationFilters = getSearchLocationFilters(query);
3331
function checkKind(env: PythonEnvInfo): boolean {
3432
if (kinds === undefined) {
3533
return true;
@@ -39,20 +37,15 @@ export function getQueryFilter(query: PythonLocatorQuery): (env: PythonEnvInfo)
3937
function checkSearchLocation(env: PythonEnvInfo): boolean {
4038
if (env.searchLocation === undefined) {
4139
// It is not a "rooted" env.
42-
return includeGlobal;
43-
}
44-
45-
// It is a "rooted" env.
46-
if (locationFilters === undefined) {
47-
if (query.searchLocations === null) {
48-
// The caller explicitly refused rooted envs.
49-
return false;
40+
return includeNonRooted;
41+
} else {
42+
// It is a "rooted" env.
43+
if (locationFilters === undefined) {
44+
return query.searchLocations === undefined;
5045
}
51-
// `query.searchLocations` only had `null` in it.
52-
return !query.searchLocations || query.searchLocations.length === 0;
46+
// Check against the requested roots.
47+
return locationFilters.some((filter) => filter(env.searchLocation!));
5348
}
54-
// Check against the requested roots.
55-
return locationFilters.some((filter) => filter(env.searchLocation!));
5649
}
5750
return (env) => {
5851
if (!checkKind(env)) {
@@ -65,6 +58,20 @@ export function getQueryFilter(query: PythonLocatorQuery): (env: PythonEnvInfo)
6558
};
6659
}
6760

61+
function getSearchLocationFilters(query: PythonLocatorQuery): ((u: Uri) => boolean)[] | undefined {
62+
if (query.searchLocations === undefined) {
63+
return undefined;
64+
}
65+
const candidates: Uri[] = query.searchLocations.roots.filter((u) => !!u);
66+
if (candidates.length === 0) {
67+
return undefined;
68+
}
69+
return candidates.map((loc) => getURIFilter(loc, {
70+
checkParent: true,
71+
checkExact: true,
72+
}));
73+
}
74+
6875
/**
6976
* Unroll the given iterator into an array.
7077
*

‎src/client/pythonEnvironments/discovery/locators/index.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,12 @@ export class WorkspaceLocators extends Locator {
103103
return NOOP_ITERATOR;
104104
}
105105
const iterators = Object.keys(this.locators).map((key) => {
106-
if (query?.searchLocations) {
106+
if (query?.searchLocations !== undefined) {
107107
const root = this.roots[key];
108108
// Match any related search location.
109109
const filter = getURIFilter(root, { checkParent: true, checkChild: true, checkExact: true });
110110
// Ignore any requests for global envs.
111-
const candidates: unknown = query.searchLocations.filter((u) => u !== null);
112-
if (!(candidates as Uri[]).some(filter)) {
111+
if (!query.searchLocations.roots.some(filter)) {
113112
// This workspace folder did not match the query, so skip it!
114113
return NOOP_ITERATOR;
115114
}

‎src/client/pythonEnvironments/legacyIOC.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ class ComponentAdapter implements IComponentAdapter {
267267
if (resource !== undefined) {
268268
const wsFolder = vscode.workspace.getWorkspaceFolder(resource);
269269
if (wsFolder !== undefined) {
270-
query.searchLocations = [wsFolder.uri];
270+
query.searchLocations = { roots: [wsFolder.uri] };
271271
}
272272
}
273273

‎src/test/pythonEnvironments/base/locatorUtils.unit.test.ts

Lines changed: 56 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ suite('Python envs locator utils - getQueryFilter', () => {
7373
const queries: PythonLocatorQuery[] = [
7474
{},
7575
{ kinds: [] },
76-
{ searchLocations: [] },
77-
{ kinds: [], searchLocations: [] },
76+
// Any "defined" value for searchLocations causes filtering...
7877
];
7978
queries.forEach((query) => {
8079
test(`all envs kept (query ${query})`, () => {
@@ -144,7 +143,11 @@ suite('Python envs locator utils - getQueryFilter', () => {
144143

145144
suite('searchLocations', () => {
146145
test('match none', () => {
147-
const query: PythonLocatorQuery = { searchLocations: [doesNotExist] };
146+
const query: PythonLocatorQuery = {
147+
searchLocations: {
148+
roots: [doesNotExist],
149+
},
150+
};
148151

149152
const filter = getQueryFilter(query);
150153
const filtered = envs.filter(filter);
@@ -154,11 +157,13 @@ suite('Python envs locator utils - getQueryFilter', () => {
154157

155158
test('match one (multiple locations)', () => {
156159
const expected = [envSL4];
157-
const searchLocations = [
158-
envSL4.searchLocation!,
159-
doesNotExist,
160-
envSL4.searchLocation!, // repeated
161-
];
160+
const searchLocations = {
161+
roots: [
162+
envSL4.searchLocation!,
163+
doesNotExist,
164+
envSL4.searchLocation!, // repeated
165+
],
166+
};
162167
const query: PythonLocatorQuery = { searchLocations };
163168

164169
const filter = getQueryFilter(query);
@@ -169,9 +174,11 @@ suite('Python envs locator utils - getQueryFilter', () => {
169174

170175
test('match multiple (one location)', () => {
171176
const expected = [envS3, envSL2];
172-
const searchLocations = [
173-
workspaceRoot,
174-
];
177+
const searchLocations = {
178+
roots: [
179+
workspaceRoot,
180+
],
181+
};
175182
const query: PythonLocatorQuery = { searchLocations };
176183

177184
const filter = getQueryFilter(query);
@@ -182,8 +189,10 @@ suite('Python envs locator utils - getQueryFilter', () => {
182189

183190
test('match multiple (multiple locations)', () => {
184191
const expected = [envS3, ...rootedLocatedEnvs];
185-
const searchLocations = rootedLocatedEnvs.map((env) => env.searchLocation!);
186-
searchLocations.push(doesNotExist);
192+
const searchLocations = {
193+
roots: rootedLocatedEnvs.map((env) => env.searchLocation!),
194+
};
195+
searchLocations.roots.push(doesNotExist);
187196
const query: PythonLocatorQuery = { searchLocations };
188197

189198
const filter = getQueryFilter(query);
@@ -194,8 +203,11 @@ suite('Python envs locator utils - getQueryFilter', () => {
194203

195204
test('match multiple (include non-searched envs)', () => {
196205
const expected = [...plainEnvs, ...locatedEnvs, envS3, ...rootedLocatedEnvs];
197-
const searchLocations = [null, ...rootedLocatedEnvs.map((env) => env.searchLocation!)];
198-
searchLocations.push(doesNotExist);
206+
const searchLocations = {
207+
roots: rootedLocatedEnvs.map((env) => env.searchLocation!),
208+
includeNonRooted: true,
209+
};
210+
searchLocations.roots.push(doesNotExist);
199211
const query: PythonLocatorQuery = { searchLocations };
200212

201213
const filter = getQueryFilter(query);
@@ -206,7 +218,9 @@ suite('Python envs locator utils - getQueryFilter', () => {
206218

207219
test('match all searched', () => {
208220
const expected = [...rootedEnvs, ...rootedLocatedEnvs];
209-
const searchLocations = expected.map((env) => env.searchLocation!);
221+
const searchLocations = {
222+
roots: expected.map((env) => env.searchLocation!),
223+
};
210224
const query: PythonLocatorQuery = { searchLocations };
211225

212226
const filter = getQueryFilter(query);
@@ -217,7 +231,10 @@ suite('Python envs locator utils - getQueryFilter', () => {
217231

218232
test('match all (including non-searched)', () => {
219233
const expected = envs;
220-
const searchLocations = [null, ...expected.map((env) => env.searchLocation!)];
234+
const searchLocations = {
235+
roots: expected.map((env) => env.searchLocation!),
236+
includeNonRooted: true,
237+
};
221238
const query: PythonLocatorQuery = { searchLocations };
222239

223240
const filter = getQueryFilter(query);
@@ -228,7 +245,9 @@ suite('Python envs locator utils - getQueryFilter', () => {
228245

229246
test('match all searched under one root', () => {
230247
const expected = [envS1, envS2, envSL1, envSL3, envSL5];
231-
const searchLocations = [Uri.file(homeDir)];
248+
const searchLocations = {
249+
roots: [Uri.file(homeDir)],
250+
};
232251
const query: PythonLocatorQuery = { searchLocations };
233252

234253
const filter = getQueryFilter(query);
@@ -237,9 +256,12 @@ suite('Python envs locator utils - getQueryFilter', () => {
237256
assert.deepEqual(filtered, expected);
238257
});
239258

240-
test('match only non-searched envs (null location)', () => {
259+
test('match only non-searched envs (empty roots)', () => {
241260
const expected = [...plainEnvs, ...locatedEnvs];
242-
const searchLocations = [null];
261+
const searchLocations = {
262+
roots: [],
263+
includeNonRooted: true,
264+
};
243265
const query: PythonLocatorQuery = { searchLocations };
244266

245267
const filter = getQueryFilter(query);
@@ -250,32 +272,26 @@ suite('Python envs locator utils - getQueryFilter', () => {
250272

251273
test('match only non-searched envs (with unmatched location)', () => {
252274
const expected = [...plainEnvs, ...locatedEnvs];
253-
const searchLocations: (Uri | null)[] = [null];
254-
searchLocations.push(doesNotExist);
275+
const searchLocations = {
276+
roots: [doesNotExist],
277+
includeNonRooted: true,
278+
};
255279
const query: PythonLocatorQuery = { searchLocations };
256280

257281
const filter = getQueryFilter(query);
258282
const filtered = envs.filter(filter);
259283

260284
assert.deepEqual(filtered, expected);
261285
});
262-
263-
test('match only non-searched envs (explicit null)', () => {
264-
const expected = [...plainEnvs, ...locatedEnvs];
265-
const query: PythonLocatorQuery = { searchLocations: null };
266-
267-
const filter = getQueryFilter(query);
268-
const filtered = envs.filter(filter);
269-
270-
assert.deepEqual(filtered, expected);
271-
});
272286
});
273287

274288
suite('mixed query', () => {
275289
test('match none', () => {
276290
const query: PythonLocatorQuery = {
277291
kinds: [PythonEnvKind.OtherGlobal],
278-
searchLocations: [doesNotExist],
292+
searchLocations: {
293+
roots: [doesNotExist],
294+
},
279295
};
280296

281297
const filter = getQueryFilter(query);
@@ -287,8 +303,10 @@ suite('Python envs locator utils - getQueryFilter', () => {
287303
test('match some', () => {
288304
const expected = [envSL1, envSL4, envSL5];
289305
const kinds = [PythonEnvKind.Venv, PythonEnvKind.Custom];
290-
const searchLocations = rootedLocatedEnvs.map((env) => env.searchLocation!);
291-
searchLocations.push(doesNotExist);
306+
const searchLocations = {
307+
roots: rootedLocatedEnvs.map((env) => env.searchLocation!),
308+
};
309+
searchLocations.roots.push(doesNotExist);
292310
const query: PythonLocatorQuery = { kinds, searchLocations };
293311

294312
const filter = getQueryFilter(query);
@@ -300,7 +318,9 @@ suite('Python envs locator utils - getQueryFilter', () => {
300318
test('match all', () => {
301319
const expected = [...rootedEnvs, ...rootedLocatedEnvs];
302320
const kinds: PythonEnvKind[] = getEnumValues(PythonEnvKind);
303-
const searchLocations = expected.map((env) => env.searchLocation!);
321+
const searchLocations = {
322+
roots: expected.map((env) => env.searchLocation!),
323+
};
304324
const query: PythonLocatorQuery = { kinds, searchLocations };
305325

306326
const filter = getQueryFilter(query);

‎src/test/pythonEnvironments/base/locators.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ suite('Python envs locators - Locators', () => {
9696
test('with query', async () => {
9797
const expected: PythonLocatorQuery = {
9898
kinds: [PythonEnvKind.Venv],
99-
searchLocations: [Uri.file('???')]
99+
searchLocations: { roots: [Uri.file('???')] },
100100
};
101101
let query: PythonLocatorQuery | undefined;
102102
async function onQuery(q: PythonLocatorQuery | undefined, e: PythonEnvInfo[]) {

‎src/test/pythonEnvironments/discovery/locators/index.unit.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,9 @@ suite('WorkspaceLocators', () => {
392392
]);
393393
const folders = new WorkspaceFolders([root1, root2]);
394394
locators.activate(folders);
395+
const query = { searchLocations: { roots: [root1] } };
395396

396-
const iterators = locators.iterEnvs({ searchLocations: [root1] });
397+
const iterators = locators.iterEnvs(query);
397398
const envs = await getEnvs(iterators);
398399

399400
expect(envs).to.deep.equal(expected);
@@ -417,8 +418,9 @@ suite('WorkspaceLocators', () => {
417418
]);
418419
const folders = new WorkspaceFolders([root1, root2]);
419420
locators.activate(folders);
421+
const query = { searchLocations: { roots: [root1, root2] } };
420422

421-
const iterators = locators.iterEnvs({ searchLocations: [root1, root2] });
423+
const iterators = locators.iterEnvs(query);
422424
const envs = await getEnvs(iterators);
423425

424426
expect(envs).to.deep.equal(expected);
@@ -441,8 +443,9 @@ suite('WorkspaceLocators', () => {
441443
]);
442444
const folders = new WorkspaceFolders([root1, root2]);
443445
locators.activate(folders);
446+
const query = { searchLocations: { roots: [Uri.file('baz')] } };
444447

445-
const iterators = locators.iterEnvs({ searchLocations: [Uri.file('baz')] });
448+
const iterators = locators.iterEnvs(query);
446449
const envs = await getEnvs(iterators);
447450

448451
expect(envs).to.deep.equal([]);

0 commit comments

Comments
 (0)