Skip to content

Commit c226b66

Browse files
authored
fix incorrect error message surfacing for defaultInterpreterPath (#1344)
fixes #1316
1 parent f4021ca commit c226b66

File tree

2 files changed

+103
-9
lines changed

2 files changed

+103
-9
lines changed

src/features/interpreterSelection.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,15 +108,25 @@ async function resolvePriorityChainCore(
108108
if (userInterpreterPath) {
109109
const expandedInterpreterPath = resolveVariables(userInterpreterPath, scope);
110110
if (expandedInterpreterPath.includes('${')) {
111-
traceWarn(
112-
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' contains unresolved variables, falling back to auto-discovery`,
113-
);
114-
const error: SettingResolutionError = {
115-
setting: 'defaultInterpreterPath',
116-
configuredValue: userInterpreterPath,
117-
reason: l10n.t('Path contains unresolved variables'),
118-
};
119-
errors.push(error);
111+
if (scope) {
112+
// Workspace scope: unresolved variables are a genuine configuration error
113+
traceWarn(
114+
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' contains unresolved variables, falling back to auto-discovery`,
115+
);
116+
const error: SettingResolutionError = {
117+
setting: 'defaultInterpreterPath',
118+
configuredValue: userInterpreterPath,
119+
reason: l10n.t('Path contains unresolved variables'),
120+
};
121+
errors.push(error);
122+
} else {
123+
// Global scope: workspace-specific variables like ${workspaceFolder} can't resolve here.
124+
// This is expected when a workspace-level setting uses workspace variables —
125+
// the per-folder chain handles them correctly. Silently skip to auto-discovery.
126+
traceVerbose(
127+
`${logPrefix} defaultInterpreterPath '${userInterpreterPath}' contains workspace-specific variables, skipping for global scope`,
128+
);
129+
}
120130
} else {
121131
const resolved = await tryResolveInterpreterPath(nativeFinder, api, expandedInterpreterPath, envManagers);
122132
if (resolved) {

src/test/features/interpreterSelection.unit.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as path from 'path';
66
import * as sinon from 'sinon';
77
import { ConfigurationChangeEvent, Uri, WorkspaceConfiguration, WorkspaceFolder } from 'vscode';
88
import { PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api';
9+
import * as windowApis from '../../common/window.apis';
910
import * as workspaceApis from '../../common/workspace.apis';
1011
import {
1112
applyInitialEnvironmentSelection,
@@ -660,6 +661,62 @@ suite('Interpreter Selection - applyInitialEnvironmentSelection', () => {
660661
assert.strictEqual(callArgs[0], 'global', 'First arg should be "global"');
661662
assert.strictEqual(callArgs[2], false, 'shouldPersistSettings should be false');
662663
});
664+
665+
test('should not show warning when defaultInterpreterPath with ${workspaceFolder} is used in workspace scope (issue #1316)', async () => {
666+
// Scenario from issue #1316: workspace settings.json has
667+
// "python.defaultInterpreterPath": "${workspaceFolder}/python-embedded/python.exe"
668+
// The per-folder chain resolves it correctly, but the global chain cannot.
669+
// The global chain should silently skip — no warning popup should be shown.
670+
const workspaceFolder = { uri: testUri, name: 'test', index: 0 } as WorkspaceFolder;
671+
sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([workspaceFolder]);
672+
sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(workspaceFolder);
673+
sandbox.stub(workspaceApis, 'getConfiguration').returns(createMockConfig([]) as WorkspaceConfiguration);
674+
675+
const expandedPath = path.join(testUri.fsPath, 'python-embedded', 'python.exe');
676+
sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => {
677+
if (section === 'python' && key === 'defaultInterpreterPath') {
678+
return '${workspaceFolder}/python-embedded/python.exe';
679+
}
680+
return undefined;
681+
});
682+
683+
// For workspace scope: nativeFinder resolves the expanded path successfully
684+
mockNativeFinder.resolve.resolves({
685+
executable: expandedPath,
686+
version: '3.12.10',
687+
prefix: path.join(testUri.fsPath, 'python-embedded'),
688+
});
689+
const mockResolvedEnv: PythonEnvironment = {
690+
envId: { id: 'embedded-env', managerId: 'ms-python.python:system' },
691+
name: 'Embedded Python',
692+
displayName: 'Python 3.12.10',
693+
version: '3.12.10',
694+
displayPath: expandedPath,
695+
environmentPath: Uri.file(expandedPath),
696+
sysPrefix: path.join(testUri.fsPath, 'python-embedded'),
697+
execInfo: { run: { executable: expandedPath } },
698+
};
699+
mockApi.resolveEnvironment.resolves(mockResolvedEnv);
700+
701+
// Stub showWarningMessage to track if it's called
702+
const showWarnStub = sandbox.stub(windowApis, 'showWarningMessage').resolves(undefined);
703+
704+
await applyInitialEnvironmentSelection(
705+
mockEnvManagers as unknown as EnvironmentManagers,
706+
mockProjectManager as unknown as PythonProjectManager,
707+
mockNativeFinder as unknown as NativePythonFinder,
708+
mockApi as unknown as PythonEnvironmentApi,
709+
);
710+
711+
// The workspace folder should be set successfully
712+
assert.ok(mockEnvManagers.setEnvironment.called, 'setEnvironment should be called for workspace folder');
713+
714+
// No warning should be shown — the global chain should silently skip ${workspaceFolder}
715+
assert.ok(
716+
showWarnStub.notCalled,
717+
'showWarningMessage should not be called when ${workspaceFolder} is only unresolvable in global scope',
718+
);
719+
});
663720
});
664721

665722
suite('Interpreter Selection - resolveGlobalEnvironmentByPriority', () => {
@@ -850,6 +907,33 @@ suite('Interpreter Selection - resolveGlobalEnvironmentByPriority', () => {
850907
assert.strictEqual(result.source, 'autoDiscovery');
851908
assert.strictEqual(result.manager.id, 'ms-python.python:system');
852909
});
910+
911+
test('should silently skip ${workspaceFolder} in defaultInterpreterPath for global scope (issue #1316)', async () => {
912+
// When defaultInterpreterPath contains ${workspaceFolder}, the global priority chain
913+
// cannot resolve it (no workspace folder context). It should silently fall through
914+
// to auto-discovery without generating an error that triggers a user notification.
915+
sandbox.stub(workspaceApis, 'getWorkspaceFolders').returns([]);
916+
sandbox.stub(workspaceApis, 'getWorkspaceFolder').returns(undefined);
917+
sandbox.stub(helpers, 'getUserConfiguredSetting').callsFake((section: string, key: string) => {
918+
if (section === 'python' && key === 'defaultInterpreterPath') {
919+
return '${workspaceFolder}/python-embedded/python.exe';
920+
}
921+
return undefined;
922+
});
923+
924+
const result = await resolveGlobalEnvironmentByPriority(
925+
mockEnvManagers as unknown as EnvironmentManagers,
926+
mockNativeFinder as unknown as NativePythonFinder,
927+
mockApi as unknown as PythonEnvironmentApi,
928+
);
929+
930+
// Should fall through to auto-discovery without calling nativeFinder.resolve
931+
assert.strictEqual(result.source, 'autoDiscovery');
932+
assert.ok(
933+
mockNativeFinder.resolve.notCalled,
934+
'nativeFinder.resolve should not be called with unresolved variables',
935+
);
936+
});
853937
});
854938

855939
suite('Interpreter Selection - registerInterpreterSettingsChangeListener', () => {

0 commit comments

Comments
 (0)