From 88ba2fcedc20d37c0b2ede43d058c4e5e1f64b68 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 18 Dec 2025 15:41:25 -0500 Subject: [PATCH 1/8] Validate setting types at runtime Because settings can be arbitrary JSON we should validate their type when reading them in. TypeScript provides a bit of a false sense of security here since at runtime they can be whatever the user wants, and then the extension falls over in wierd and wonderful ways. Add some basic type checking methods to make sure we're working with booleans, constrained strings/strings, or arrays of strings. Issue: #2007 --- src/configuration.ts | 291 ++++++++++++++++++++++++++++++------------- 1 file changed, 203 insertions(+), 88 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 9d77c7a12..b2cdf87a4 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -125,37 +125,51 @@ const configuration = { return { get serverPath(): string { return substituteVariablesInString( - vscode.workspace - .getConfiguration("swift.sourcekit-lsp") - .get("serverPath", "") + validateStringSetting( + vscode.workspace + .getConfiguration("swift.sourcekit-lsp") + .get("serverPath", ""), + "swift.sourcekit-lsp.serverPath" + ) ); }, get serverArguments(): string[] { - return vscode.workspace - .getConfiguration("swift.sourcekit-lsp") - .get("serverArguments", []) - .map(substituteVariablesInString); + return validateStringArraySettings( + vscode.workspace + .getConfiguration("swift.sourcekit-lsp") + .get("serverArguments", []), + "swift.sourcekit-lsp.serverArguments" + ).map(substituteVariablesInString); }, get inlayHintsEnabled(): boolean { - return vscode.workspace - .getConfiguration("sourcekit-lsp") - .get("inlayHints.enabled", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("sourcekit-lsp") + .get("inlayHints.enabled", true), + "swift.sourcekit-lsp.inlayHints.enabled" + ); }, get supportCFamily(): CFamilySupportOptions { - return vscode.workspace - .getConfiguration("sourcekit-lsp") - .get("support-c-cpp", "cpptools-inactive"); + return validateStringSetting( + vscode.workspace + .getConfiguration("sourcekit-lsp") + .get("support-c-cpp", "cpptools-inactive"), + "swift.sourcekit-lsp.support-c-cpp" + ); }, get supportedLanguages() { - return vscode.workspace - .getConfiguration("swift.sourcekit-lsp") - .get("supported-languages", [ - "swift", - "c", - "cpp", - "objective-c", - "objective-cpp", - ]); + return validateStringArraySettings( + vscode.workspace + .getConfiguration("swift.sourcekit-lsp") + .get("supported-languages", [ + "swift", + "c", + "cpp", + "objective-c", + "objective-cpp", + ]), + "swift.sourcekit-lsp.supported-languages" + ); }, get disable(): boolean { return vscode.workspace @@ -294,9 +308,12 @@ const configuration = { ); }, get disable(): boolean { - return vscode.workspace - .getConfiguration("swift.debugger") - .get("disable", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift.debugger") + .get("disable", false), + "swift.debugger.disable" + ); }, get setupCodeLLDB(): SetupCodeLLDBOptions { return vscode.workspace @@ -324,26 +341,38 @@ const configuration = { }, /** Files and directories to exclude from the Package Dependencies view. */ get excludePathsFromPackageDependencies(): string[] { - return vscode.workspace - .getConfiguration("swift") - .get("excludePathsFromPackageDependencies", []); + return validateStringArraySettings( + vscode.workspace + .getConfiguration("swift") + .get("excludePathsFromPackageDependencies", []), + "swift.excludePathsFromPackageDependencies" + ); }, /** Path to folder that include swift executable */ get path(): string { return substituteVariablesInString( - vscode.workspace.getConfiguration("swift").get("path", "") + validateStringSetting( + vscode.workspace.getConfiguration("swift").get("path", ""), + "swift.path" + ) ); }, /** Path to folder that include swift runtime */ get runtimePath(): string { return substituteVariablesInString( - vscode.workspace.getConfiguration("swift").get("runtimePath", "") + validateStringSetting( + vscode.workspace.getConfiguration("swift").get("runtimePath", ""), + "swift.runtimePath" + ) ); }, /** Path to custom --sdk */ get sdk(): string { return substituteVariablesInString( - vscode.workspace.getConfiguration("swift").get("SDK", "") + validateStringSetting( + vscode.workspace.getConfiguration("swift").get("SDK", ""), + "swift.SDK" + ) ); }, set sdk(value: string | undefined) { @@ -356,7 +385,10 @@ const configuration = { }, /** Path to custom --swift-sdk */ get swiftSDK(): string { - return vscode.workspace.getConfiguration("swift").get("swiftSDK", ""); + return validateStringSetting( + vscode.workspace.getConfiguration("swift").get("swiftSDK", ""), + "swift.swiftSDK" + ); }, set swiftSDK(value: string | undefined) { void vscode.workspace @@ -368,15 +400,18 @@ const configuration = { }, /** swift build arguments */ get buildArguments(): string[] { - return vscode.workspace - .getConfiguration("swift") - .get("buildArguments", []) - .map(substituteVariablesInString); + return validateStringArraySettings( + vscode.workspace.getConfiguration("swift").get("buildArguments", []), + "swift.buildArguments" + ).map(substituteVariablesInString); }, scriptSwiftLanguageVersion(toolchain: SwiftToolchain): string { - const version = vscode.workspace - .getConfiguration("swift") - .get("scriptSwiftLanguageVersion", toolchain.swiftVersion.major.toString()); + const version = validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("scriptSwiftLanguageVersion", toolchain.swiftVersion.major.toString()), + "swift.scriptSwiftLanguageVersion" + ); if (version.length === 0) { return toolchain.swiftVersion.major.toString(); } @@ -384,24 +419,33 @@ const configuration = { }, /** swift package arguments */ get packageArguments(): string[] { - return vscode.workspace - .getConfiguration("swift") - .get("packageArguments", []) - .map(substituteVariablesInString); + return validateStringArraySettings( + vscode.workspace.getConfiguration("swift").get("packageArguments", []), + "swift.packageArguments" + ).map(substituteVariablesInString); }, /** thread/address sanitizer */ get sanitizer(): string { - return vscode.workspace.getConfiguration("swift").get("sanitizer", "off"); + return validateStringSetting( + vscode.workspace.getConfiguration("swift").get("sanitizer", "off"), + "swift.sanitizer" + ); }, get buildPath(): string { return substituteVariablesInString( - vscode.workspace.getConfiguration("swift").get("buildPath", "") + validateStringSetting( + vscode.workspace.getConfiguration("swift").get("buildPath", ""), + "swift.buildPath" + ) ); }, get disableSwiftPMIntegration(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("disableSwiftPackageManagerIntegration", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("disableSwiftPackageManagerIntegration", false), + "swift.disableSwiftPackageManagerIntegration" + ); }, /** Environment variables to set when building */ get swiftEnvironmentVariables(): { [key: string]: string } { @@ -423,15 +467,21 @@ const configuration = { }, /** where to show the build progress for the running task */ get showBuildStatus(): ShowBuildStatusOptions { - return vscode.workspace - .getConfiguration("swift") - .get("showBuildStatus", "swiftStatus"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("showBuildStatus", "swiftStatus"), + "swift.showBuildStatus" + ); }, /** create build tasks for the library products of the package(s) */ get createTasksForLibraryProducts(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("createTasksForLibraryProducts", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("createTasksForLibraryProducts", false), + "swift.createTasksForLibraryProducts" + ); }, /** background compilation */ get backgroundCompilation(): BackgroundCompilationConfiguration { @@ -465,52 +515,79 @@ const configuration = { }, /** focus on problems view whenever there is a build error */ get actionAfterBuildError(): ActionAfterBuildError { - return vscode.workspace - .getConfiguration("swift") - .get("actionAfterBuildError", "Focus Terminal"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("actionAfterBuildError", "Focus Terminal"), + "swift.actionAfterBuildError" + ); }, /** output additional diagnostics */ get diagnostics(): boolean { - return vscode.workspace.getConfiguration("swift").get("diagnostics", false); + return validateBooleanSetting( + vscode.workspace.getConfiguration("swift").get("diagnostics", false), + "swift.diagnostics" + ); }, /** * Test coverage settings */ /** Should test coverage report be displayed after running test coverage */ get displayCoverageReportAfterRun(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.displayReportAfterRun", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.displayReportAfterRun", true), + "swift.coverage.displayReportAfterRun" + ); }, get alwaysShowCoverageStatusItem(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.alwaysShowStatusItem", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.alwaysShowStatusItem", true), + "swift.coverage.alwaysShowStatusItem" + ); }, get coverageHitColorLightMode(): string { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.colors.lightMode.hit", "#c0ffc0"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.colors.lightMode.hit", "#c0ffc0"), + "swift.coverage.colors.lightMode.hit" + ); }, get coverageMissColorLightMode(): string { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.colors.lightMode.miss", "#ffc0c0"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.colors.lightMode.miss", "#ffc0c0"), + "swift.coverage.colors.lightMode.miss" + ); }, get coverageHitColorDarkMode(): string { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.colors.darkMode.hit", "#003000"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.colors.darkMode.hit", "#003000"), + "swift.coverage.colors.darkMode.hit" + ); }, get coverageMissColorDarkMode(): string { - return vscode.workspace - .getConfiguration("swift") - .get("coverage.colors.darkMode.miss", "#400000"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("coverage.colors.darkMode.miss", "#400000"), + "swift.coverage.colors.darkMode.miss" + ); }, get openAfterCreateNewProject(): OpenAfterCreateNewProjectOptions { - return vscode.workspace - .getConfiguration("swift") - .get("openAfterCreateNewProject", "prompt"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("openAfterCreateNewProject", "prompt"), + "swift.openAfterCreateNewProject" + ); }, /** Whether or not the extension should warn about being unable to create symlinks on Windows */ get warnAboutSymlinkCreation(): boolean { @@ -528,13 +605,19 @@ const configuration = { }, /** Whether or not the extension will contribute Swift environment variables to the integrated terminal */ get enableTerminalEnvironment(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("enableTerminalEnvironment", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("enableTerminalEnvironment", true), + "swift.enableTerminalEnvironment" + ); }, /** Whether or not to disable SwiftPM sandboxing */ get disableSandbox(): boolean { - return vscode.workspace.getConfiguration("swift").get("disableSandbox", false); + return validateBooleanSetting( + vscode.workspace.getConfiguration("swift").get("disableSandbox", false), + "swift.disableSandbox" + ); }, /** Workspace folder glob patterns to exclude */ get excludePathsFromActivation(): Record { @@ -543,12 +626,18 @@ const configuration = { .get>("excludePathsFromActivation", {}); }, get lspConfigurationBranch(): string { - return vscode.workspace.getConfiguration("swift").get("lspConfigurationBranch", ""); + return validateStringSetting( + vscode.workspace.getConfiguration("swift").get("lspConfigurationBranch", ""), + "swift.lspConfigurationBranch" + ); }, get checkLspConfigurationSchema(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("checkLspConfigurationSchema", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("checkLspConfigurationSchema", true), + "swift.checkLspConfigurationSchema" + ); }, set checkLspConfigurationSchema(value: boolean) { void vscode.workspace @@ -559,7 +648,10 @@ const configuration = { }); }, get outputChannelLogLevel(): string { - return vscode.workspace.getConfiguration("swift").get("outputChannelLogLevel", "info"); + return validateStringSetting( + vscode.workspace.getConfiguration("swift").get("outputChannelLogLevel", "info"), + "swift.outputChannelLogLevel" + ); }, parameterHintsEnabled(documentUri: vscode.Uri): boolean { const enabled = vscode.workspace @@ -581,6 +673,29 @@ export function substituteVariablesInString(val: string): string { ); } +function validateBooleanSetting(val: boolean, settingName: string): boolean { + if (typeof val !== "boolean") { + throw new Error(`The setting ${settingName} must be a boolean`); + } + return val; +} + +function validateStringSetting(val: string, settingName: string): T { + if (typeof val !== "string") { + throw new Error(`The setting ${settingName} must be a string`); + } + return val as T; +} + +function validateStringArraySettings(arr: string[], settingName: string): string[] { + for (const v of arr) { + if (typeof v !== "string") { + throw new Error(`The setting ${settingName} must be an array of strings`); + } + } + return arr; +} + function computeVscodeVar(varName: string): string | null { const workspaceFolder = () => { const activeEditor = vscode.window.activeTextEditor; From 559a5f31e94eb2103fc16740482dec78d6365555 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 18 Dec 2025 16:01:49 -0500 Subject: [PATCH 2/8] Show a dialog that opens the UI to change the bad setting --- src/configuration.ts | 28 +++++++++++++++++++++++++--- src/extension.ts | 27 ++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index b2cdf87a4..8dc5be22d 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -19,6 +19,19 @@ import { WorkspaceContext } from "./WorkspaceContext"; import { SwiftToolchain } from "./toolchain/toolchain"; import { showReloadExtensionNotification } from "./ui/ReloadExtension"; +/** + * Custom error type for configuration validation errors that includes the setting name + */ +export class ConfigurationValidationError extends Error { + constructor( + public readonly settingName: string, + message: string + ) { + super(message); + this.name = "ConfigurationValidationError"; + } +} + export type DebugAdapters = "auto" | "lldb-dap" | "CodeLLDB"; export type SetupCodeLLDBOptions = | "prompt" @@ -675,14 +688,20 @@ export function substituteVariablesInString(val: string): string { function validateBooleanSetting(val: boolean, settingName: string): boolean { if (typeof val !== "boolean") { - throw new Error(`The setting ${settingName} must be a boolean`); + throw new ConfigurationValidationError( + settingName, + `The setting \`${settingName}\` must be a boolean` + ); } return val; } function validateStringSetting(val: string, settingName: string): T { if (typeof val !== "string") { - throw new Error(`The setting ${settingName} must be a string`); + throw new ConfigurationValidationError( + settingName, + `The setting \`${settingName}\` must be a string` + ); } return val as T; } @@ -690,7 +709,10 @@ function validateStringSetting(val: string, settingNa function validateStringArraySettings(arr: string[], settingName: string): string[] { for (const v of arr) { if (typeof v !== "string") { - throw new Error(`The setting ${settingName} must be an array of strings`); + throw new ConfigurationValidationError( + settingName, + `The setting \`${settingName}\` must be an array of strings` + ); } } return arr; diff --git a/src/extension.ts b/src/extension.ts index a3415ab7d..5ca1ce617 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -22,7 +22,10 @@ import { FolderEvent, FolderOperation, WorkspaceContext } from "./WorkspaceConte import * as commands from "./commands"; import { resolveFolderDependencies } from "./commands/dependencies/resolve"; import { registerSourceKitSchemaWatcher } from "./commands/generateSourcekitConfiguration"; -import configuration, { handleConfigurationChangeEvent } from "./configuration"; +import configuration, { + ConfigurationValidationError, + handleConfigurationChangeEvent, +} from "./configuration"; import { ContextKeys, createContextKeys } from "./contextKeys"; import { registerDebugger } from "./debugger/debugAdapterFactory"; import * as debug from "./debugger/launch"; @@ -190,10 +193,24 @@ export async function activate(context: vscode.ExtensionContext): Promise { }, }; } catch (error) { - const errorMessage = getErrorDescription(error); - // show this error message as the VS Code error message only shows when running - // the extension through the debugger - void vscode.window.showErrorMessage(`Activating Swift extension failed: ${errorMessage}`); + // Handle configuration validation errors with UI that points the user to the poorly configured setting + if (error instanceof ConfigurationValidationError) { + void vscode.window.showErrorMessage(error.message, "Open Settings").then(selection => { + if (selection === "Open Settings") { + void vscode.commands.executeCommand( + "workbench.action.openSettings", + error.settingName + ); + } + }); + } else { + const errorMessage = getErrorDescription(error); + // show this error message as the VS Code error message only shows when running + // the extension through the debugger + void vscode.window.showErrorMessage( + `Activating Swift extension failed: ${errorMessage}` + ); + } throw error; } } From 531b13a740d13aab7d52b9a6f7d59e18b4f23ceb Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 11:26:09 -0500 Subject: [PATCH 3/8] Add more validation for settings --- src/configuration.ts | 194 +++++++++++++++++++++++++++++-------------- 1 file changed, 131 insertions(+), 63 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 8dc5be22d..6d311fb20 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -185,9 +185,12 @@ const configuration = { ); }, get disable(): boolean { - return vscode.workspace - .getConfiguration("swift.sourcekit-lsp") - .get("disable", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift.sourcekit-lsp") + .get("disable", false), + "swift.sourcekit-lsp.disable" + ); }, }; }, @@ -232,49 +235,68 @@ const configuration = { return { /** Environment variables to set when running tests */ get testEnvironmentVariables(): { [key: string]: string } { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get<{ [key: string]: string }>("testEnvironmentVariables", {}); + return validateObjectSetting( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get<{ [key: string]: string }>("testEnvironmentVariables", {}), + "swift.testEnvironmentVariables" + ); }, /** Extra arguments to pass to swift test and swift build when running and debugging tests. */ get additionalTestArguments(): string[] { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get("additionalTestArguments", []) - .map(substituteVariablesInString); + return validateStringArraySettings( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get("additionalTestArguments", []), + "swift.additionalTestArguments" + ).map(substituteVariablesInString); }, /** auto-generate launch.json configurations */ get autoGenerateLaunchConfigurations(): boolean { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get("autoGenerateLaunchConfigurations", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get("autoGenerateLaunchConfigurations", true), + "swift.autoGenerateLaunchConfigurations" + ); }, /** disable automatic running of swift package resolve */ get disableAutoResolve(): boolean { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get("disableAutoResolve", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get("disableAutoResolve", false), + "swift.disableAutoResolve" + ); }, /** search sub-folder of workspace folder for Swift Packages */ get searchSubfoldersForPackages(): boolean { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get("searchSubfoldersForPackages", false); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get("searchSubfoldersForPackages", false), + "swift.searchSubfoldersForPackages" + ); }, /** Folders to ignore when searching for Swift Packages */ get ignoreSearchingForPackagesInSubfolders(): string[] { - return vscode.workspace - .getConfiguration("swift", workspaceFolder) - .get< - string[] - >("ignoreSearchingForPackagesInSubfolders", [".", ".build", "Packages", "out", "bazel-out", "bazel-bin"]) - .map(substituteVariablesInString); - }, - get attachmentsPath(): string { - return substituteVariablesInString( + return validateStringArraySettings( vscode.workspace .getConfiguration("swift", workspaceFolder) - .get("attachmentsPath", "./.build/attachments") + .get< + string[] + >("ignoreSearchingForPackagesInSubfolders", [".", ".build", "Packages", "out", "bazel-out", "bazel-bin"]), + "swift.ignoreSearchingForPackagesInSubfolders" + ).map(substituteVariablesInString); + }, + get attachmentsPath(): string { + return validateStringSetting( + substituteVariablesInString( + vscode.workspace + .getConfiguration("swift", workspaceFolder) + .get("attachmentsPath", "./.build/attachments") + ), + "swift.attachmentsPath" ); }, pluginPermissions(pluginId?: string): PluginPermissionConfiguration { @@ -297,14 +319,23 @@ const configuration = { let useDebugAdapterFromToolchain = inspectUseDebugAdapterFromToolchain?.workspaceValue ?? inspectUseDebugAdapterFromToolchain?.globalValue; + + validateBooleanSetting( + !!useDebugAdapterFromToolchain, + "swift.debugger.useDebugAdapterFromToolchain" + ); + // On Windows arm64 we enable swift.debugger.useDebugAdapterFromToolchain by default since CodeLLDB does // not support this platform and gives an awful error message. if (process.platform === "win32" && process.arch === "arm64") { useDebugAdapterFromToolchain = useDebugAdapterFromToolchain ?? true; } - const selectedAdapter = vscode.workspace - .getConfiguration("swift.debugger") - .get("debugAdapter", "auto"); + const selectedAdapter = validateStringSetting( + vscode.workspace + .getConfiguration("swift.debugger") + .get("debugAdapter", "auto"), + "swift.debugger.debugAdapter" + ); switch (selectedAdapter) { case "auto": if (useDebugAdapterFromToolchain !== undefined) { @@ -316,8 +347,11 @@ const configuration = { } }, get customDebugAdapterPath(): string { - return substituteVariablesInString( - vscode.workspace.getConfiguration("swift.debugger").get("path", "") + return validateStringSetting( + substituteVariablesInString( + vscode.workspace.getConfiguration("swift.debugger").get("path", "") + ), + "swift.debugger.path" ); }, get disable(): boolean { @@ -329,18 +363,21 @@ const configuration = { ); }, get setupCodeLLDB(): SetupCodeLLDBOptions { - return vscode.workspace - .getConfiguration("swift.debugger") - .get("setupCodeLLDB", "prompt"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift.debugger") + .get("setupCodeLLDB", "prompt"), + "swift.debugger.setupCodeLLDB" + ); }, }; }, /** Files and directories to exclude from the code coverage. */ get excludeFromCodeCoverage(): string[] { - return vscode.workspace - .getConfiguration("swift") - .get("excludeFromCodeCoverage", []) - .map(substituteVariablesInString); + return validateStringArraySettings( + vscode.workspace.getConfiguration("swift").get("excludeFromCodeCoverage", []), + "swift.excludeFromCodeCoverage" + ).map(substituteVariablesInString); }, /** Whether to show inline code lenses for running and debugging tests. */ get showTestCodeLenses(): boolean | ValidCodeLens[] { @@ -350,7 +387,10 @@ const configuration = { }, /** Whether to record the duration of tests in the Test Explorer. */ get recordTestDuration(): boolean { - return vscode.workspace.getConfiguration("swift").get("recordTestDuration", true); + return validateBooleanSetting( + vscode.workspace.getConfiguration("swift").get("recordTestDuration", true), + "swift.recordTestDuration" + ); }, /** Files and directories to exclude from the Package Dependencies view. */ get excludePathsFromPackageDependencies(): string[] { @@ -462,21 +502,30 @@ const configuration = { }, /** Environment variables to set when building */ get swiftEnvironmentVariables(): { [key: string]: string } { - return vscode.workspace - .getConfiguration("swift") - .get<{ [key: string]: string }>("swiftEnvironmentVariables", {}); + return validateObjectSetting( + vscode.workspace + .getConfiguration("swift") + .get<{ [key: string]: string }>("swiftEnvironmentVariables", {}), + "swift.swiftEnvironmentVariables" + ); }, /** include build errors in problems view */ get diagnosticsCollection(): DiagnosticCollectionOptions { - return vscode.workspace - .getConfiguration("swift") - .get("diagnosticsCollection", "keepSourceKit"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("diagnosticsCollection", "keepSourceKit"), + "swift.diagnosticsCollection" + ); }, /** set the -diagnostic-style option when running `swift` tasks */ get diagnosticsStyle(): DiagnosticStyle { - return vscode.workspace - .getConfiguration("swift") - .get("diagnosticsStyle", "default"); + return validateStringSetting( + vscode.workspace + .getConfiguration("swift") + .get("diagnosticsStyle", "default"), + "swift.diagnosticsStyle" + ); }, /** where to show the build progress for the running task */ get showBuildStatus(): ShowBuildStatusOptions { @@ -515,9 +564,12 @@ const configuration = { }, /** background indexing */ get backgroundIndexing(): "on" | "off" | "auto" { - const value = vscode.workspace - .getConfiguration("swift.sourcekit-lsp") - .get("backgroundIndexing", "auto"); + const value = validateStringSetting<"on" | "off" | "auto">( + vscode.workspace + .getConfiguration("swift.sourcekit-lsp") + .get("backgroundIndexing", "auto"), + "swift.sourcekit-lsp.backgroundIndexing" + ); // Legacy versions of this setting were a boolean, convert to the new string version. if (typeof value === "boolean") { @@ -604,9 +656,12 @@ const configuration = { }, /** Whether or not the extension should warn about being unable to create symlinks on Windows */ get warnAboutSymlinkCreation(): boolean { - return vscode.workspace - .getConfiguration("swift") - .get("warnAboutSymlinkCreation", true); + return validateBooleanSetting( + vscode.workspace + .getConfiguration("swift") + .get("warnAboutSymlinkCreation", true), + "swift.warnAboutSymlinkCreation" + ); }, set warnAboutSymlinkCreation(value: boolean) { void vscode.workspace @@ -667,12 +722,15 @@ const configuration = { ); }, parameterHintsEnabled(documentUri: vscode.Uri): boolean { - const enabled = vscode.workspace - .getConfiguration("editor.parameterHints", { - uri: documentUri, - languageId: "swift", - }) - .get("enabled"); + const enabled = validateBooleanSetting( + vscode.workspace + .getConfiguration("editor.parameterHints", { + uri: documentUri, + languageId: "swift", + }) + .get("enabled", false), + "editor.parameterHints.enabled" + ); return enabled === true; }, @@ -718,6 +776,16 @@ function validateStringArraySettings(arr: string[], settingName: string): string return arr; } +function validateObjectSetting(obj: T, settingName: string): T { + if (typeof obj !== "object" || obj === null) { + throw new ConfigurationValidationError( + settingName, + `The setting \`${settingName}\` must be an object` + ); + } + return obj; +} + function computeVscodeVar(varName: string): string | null { const workspaceFolder = () => { const activeEditor = vscode.window.activeTextEditor; From b33ef79555620925831464f1848aa4b039622418 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 11:26:40 -0500 Subject: [PATCH 4/8] Add tests --- .../utilities/configuration.test.ts | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 test/unit-tests/utilities/configuration.test.ts diff --git a/test/unit-tests/utilities/configuration.test.ts b/test/unit-tests/utilities/configuration.test.ts new file mode 100644 index 000000000..28ad73b60 --- /dev/null +++ b/test/unit-tests/utilities/configuration.test.ts @@ -0,0 +1,86 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2025 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// +import * as assert from "assert"; +import { afterEach } from "mocha"; +import { match, restore } from "sinon"; +import * as vscode from "vscode"; + +import configuration from "@src/configuration"; + +import { instance, mockFn, mockGlobalObject, mockObject } from "../../MockUtils"; + +suite.only("Configuration/Settings Test Suite", () => { + const mockWorkspace = mockGlobalObject(vscode, "workspace"); + + afterEach(() => { + restore(); + }); + + function mockSetting(settingName: string, value: T) { + const [lead, ...rest] = settingName.split("."); + const mockSwiftConfig = mockObject({ + get: mockFn(s => s.withArgs(rest.join("."), match.any).returns(value)), + }); + mockWorkspace.getConfiguration.withArgs(lead).returns(instance(mockSwiftConfig)); + } + + test("returns a string configuration value", () => { + mockSetting("swift.path", "foo"); + assert.equal(configuration.path, "foo"); + }); + + test("throws when a string setting is not a string", () => { + mockSetting("swift.path", 42); + assert.throws(() => { + configuration.path; + }); + }); + + test("returns a boolean configuration value", () => { + mockSetting("swift.recordTestDuration", false); + assert.equal(configuration.recordTestDuration, false); + }); + + test("throws when a boolean setting is not a boolean", () => { + mockSetting("swift.recordTestDuration", "notaboolean"); + assert.throws(() => { + configuration.recordTestDuration; + }); + }); + + test("returns a string array configuration value", () => { + mockSetting("swift.excludeFromCodeCoverage", ["foo", "bar"]); + assert.deepEqual(configuration.excludeFromCodeCoverage, ["foo", "bar"]); + }); + + test("throws when a string array setting is not a string array", () => { + mockSetting("swift.excludeFromCodeCoverage", [42, true]); + assert.throws(() => { + configuration.excludeFromCodeCoverage; + }); + }); + + test("returns an object configuration value", () => { + const obj = { FOO: "BAR" }; + mockSetting("swift.swiftEnvironmentVariables", obj); + assert.deepEqual(configuration.swiftEnvironmentVariables, obj); + }); + + test("throws when an object setting is not an object", () => { + mockSetting("swift.swiftEnvironmentVariables", "notanobject"); + assert.throws(() => { + configuration.swiftEnvironmentVariables; + }); + }); +}); From ad818a6a67fd3749164f22fd58e74751fa703241 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 11:39:53 -0500 Subject: [PATCH 5/8] Attempt to open to the file where the offending setting is defined --- src/configuration.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++ src/extension.ts | 6 ++---- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/configuration.ts b/src/configuration.ts index 6d311fb20..6c4523db9 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -865,4 +865,51 @@ export function handleConfigurationChangeEvent( }; } +/** + * Opens the appropriate settings JSON file based on where the setting is configured + */ +export async function openSettingsJsonForSetting(settingName: string): Promise { + try { + const config = vscode.workspace.getConfiguration(); + const inspection = config.inspect(settingName); + + if (!inspection) { + // If we can't inspect the setting, fall back to global settings + await vscode.commands.executeCommand("workbench.action.openSettingsJson"); + return; + } + + // Determine the most specific scope where the setting is defined + if (inspection.workspaceFolderValue !== undefined) { + const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; + if (workspaceFolder) { + const settingsUri = vscode.Uri.joinPath( + workspaceFolder.uri, + ".vscode", + "settings.json" + ); + try { + await vscode.window.showTextDocument(settingsUri); + return; + } catch { + // If the file doesn't exist, create it or fall back + await vscode.commands.executeCommand( + "workbench.action.openWorkspaceSettingsFile" + ); + return; + } + } + } + + if (inspection.workspaceValue !== undefined) { + await vscode.commands.executeCommand("workbench.action.openWorkspaceSettingsFile"); + return; + } + + await vscode.commands.executeCommand("workbench.action.openSettingsJson"); + } catch (error) { + await vscode.commands.executeCommand("workbench.action.openSettingsJson"); + } +} + export default configuration; diff --git a/src/extension.ts b/src/extension.ts index 5ca1ce617..8283cff05 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -25,6 +25,7 @@ import { registerSourceKitSchemaWatcher } from "./commands/generateSourcekitConf import configuration, { ConfigurationValidationError, handleConfigurationChangeEvent, + openSettingsJsonForSetting, } from "./configuration"; import { ContextKeys, createContextKeys } from "./contextKeys"; import { registerDebugger } from "./debugger/debugAdapterFactory"; @@ -197,10 +198,7 @@ export async function activate(context: vscode.ExtensionContext): Promise { if (error instanceof ConfigurationValidationError) { void vscode.window.showErrorMessage(error.message, "Open Settings").then(selection => { if (selection === "Open Settings") { - void vscode.commands.executeCommand( - "workbench.action.openSettings", - error.settingName - ); + void openSettingsJsonForSetting(error.settingName); } }); } else { From 55a571920150fd29b67f6c9364fa08a8f7532532 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 11:40:02 -0500 Subject: [PATCH 6/8] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cd79b7ea..5487a8d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Fix the wrong toolchain being shown as selected when using swiftly v1.0.1 ([#2014](https://github.com/swiftlang/vscode-swift/pull/2014)) - Fix extension displaying SwiftPM's project view and automatic build tasks even when `disableSwiftPMIntegration` was true ([#2011](https://github.com/swiftlang/vscode-swift/pull/2011)) +- Validate extension settings and warn if they are invalid ([#2016](https://github.com/swiftlang/vscode-swift/pull/2016)) ## 2.14.3 - 2025-12-15 From d1d5a47110ede3af24a4fc40fcdebcf162283b34 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 14:24:57 -0500 Subject: [PATCH 7/8] Remove errant .only --- test/unit-tests/utilities/configuration.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit-tests/utilities/configuration.test.ts b/test/unit-tests/utilities/configuration.test.ts index 28ad73b60..6e609bfcb 100644 --- a/test/unit-tests/utilities/configuration.test.ts +++ b/test/unit-tests/utilities/configuration.test.ts @@ -20,7 +20,7 @@ import configuration from "@src/configuration"; import { instance, mockFn, mockGlobalObject, mockObject } from "../../MockUtils"; -suite.only("Configuration/Settings Test Suite", () => { +suite("Configuration/Settings Test Suite", () => { const mockWorkspace = mockGlobalObject(vscode, "workspace"); afterEach(() => { From 802428bc351823e00e805d9790fa0c1400c823cf Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 19 Dec 2025 17:48:11 -0500 Subject: [PATCH 8/8] Fix mock cleanup --- test/MockUtils.ts | 75 ++++++++++--- .../debugger/debugAdapterFactory.test.ts | 6 ++ test/unit-tests/ui/ToolchainSelection.test.ts | 9 +- .../utilities/configuration.test.ts | 100 +++++++++--------- 4 files changed, 123 insertions(+), 67 deletions(-) diff --git a/test/MockUtils.ts b/test/MockUtils.ts index 66c44d568..4b63a78df 100644 --- a/test/MockUtils.ts +++ b/test/MockUtils.ts @@ -242,15 +242,25 @@ export function mockGlobalObject>( property: K ): MockedObject { let realMock: MockedObject; + let originalDescriptor: PropertyDescriptor | undefined; const originalValue: T[K] = obj[property]; // Create the mock at setup setup(() => { + originalDescriptor = Object.getOwnPropertyDescriptor(obj, property); realMock = mockObject(obj[property]); - Object.defineProperty(obj, property, { value: realMock }); + Object.defineProperty(obj, property, { + value: realMock, + writable: true, + configurable: true, + }); }); - // Restore original value at teardown + // Restore original property descriptor at teardown teardown(() => { - Object.defineProperty(obj, property, { value: originalValue }); + if (originalDescriptor) { + Object.defineProperty(obj, property, originalDescriptor); + } else { + delete (obj as any)[property]; + } }); // Return the proxy to the real mock return new Proxy(originalValue, { @@ -301,32 +311,46 @@ function shallowClone(obj: T): T { */ export function mockGlobalModule(mod: T): MockedObject { let realMock: MockedObject; + const originalDescriptors = new Map(); const originalValue: T = shallowClone(mod); // Create the mock at setup setup(() => { realMock = mockObject(mod); for (const property of Object.getOwnPropertyNames(realMock)) { try { + const originalDescriptor = Object.getOwnPropertyDescriptor(mod, property); + if (originalDescriptor) { + originalDescriptors.set(property, originalDescriptor); + } Object.defineProperty(mod, property, { value: (realMock as any)[property], writable: true, + configurable: true, }); } catch { // Some properties of a module just can't be mocked and that's fine } } }); - // Restore original value at teardown + // Restore original property descriptors at teardown teardown(() => { for (const property of Object.getOwnPropertyNames(originalValue)) { try { - Object.defineProperty(mod, property, { - value: (originalValue as any)[property], - }); + const originalDescriptor = originalDescriptors.get(property); + if (originalDescriptor) { + Object.defineProperty(mod, property, originalDescriptor); + } else { + Object.defineProperty(mod, property, { + value: (originalValue as any)[property], + writable: true, + configurable: true, + }); + } } catch { // Some properties of a module just can't be mocked and that's fine } } + originalDescriptors.clear(); }); // Return the proxy to the real mock return new Proxy(originalValue, { @@ -374,15 +398,19 @@ export interface MockedValue { */ export function mockGlobalValue(obj: T, property: K): MockedValue { let setupComplete: boolean = false; - let originalValue: T[K]; - // Grab the original value during setup + let originalDescriptor: PropertyDescriptor | undefined; + // Grab the original property descriptor during setup setup(() => { - originalValue = obj[property]; + originalDescriptor = Object.getOwnPropertyDescriptor(obj, property); setupComplete = true; }); - // Restore the original value on teardown + // Restore the original property descriptor on teardown teardown(() => { - Object.defineProperty(obj, property, { value: originalValue }); + if (originalDescriptor) { + Object.defineProperty(obj, property, originalDescriptor); + } else { + delete (obj as any)[property]; + } setupComplete = false; }); // Return a ValueMock that allows for easy mocking of the value @@ -391,7 +419,11 @@ export function mockGlobalValue(obj: T, property: K): Mock if (!setupComplete) { throw new Error("Mocks cannot be accessed outside of test functions"); } - Object.defineProperty(obj, property, { value: value }); + Object.defineProperty(obj, property, { + value: value, + writable: true, + configurable: true, + }); }, }; } @@ -441,15 +473,24 @@ export function mockGlobalEvent>( property: K ): AsyncEventEmitter> { let eventEmitter: vscode.EventEmitter>; - const originalValue: T[K] = obj[property]; + let originalDescriptor: PropertyDescriptor | undefined; // Create the mock at setup setup(() => { + originalDescriptor = Object.getOwnPropertyDescriptor(obj, property); eventEmitter = new vscode.EventEmitter(); - Object.defineProperty(obj, property, { value: eventEmitter.event }); + Object.defineProperty(obj, property, { + value: eventEmitter.event, + writable: true, + configurable: true, + }); }); - // Restore original value at teardown + // Restore original property descriptor at teardown teardown(() => { - Object.defineProperty(obj, property, { value: originalValue }); + if (originalDescriptor) { + Object.defineProperty(obj, property, originalDescriptor); + } else { + delete (obj as any)[property]; + } }); // Return the proxy to the EventEmitter return new Proxy(new AsyncEventEmitter(), { diff --git a/test/unit-tests/debugger/debugAdapterFactory.test.ts b/test/unit-tests/debugger/debugAdapterFactory.test.ts index 712766bf7..2336771e4 100644 --- a/test/unit-tests/debugger/debugAdapterFactory.test.ts +++ b/test/unit-tests/debugger/debugAdapterFactory.test.ts @@ -193,6 +193,12 @@ suite("LLDBDebugConfigurationProvider Tests", () => { get: mockFn(s => { s.withArgs("library").returns("/path/to/liblldb.dyLib"); s.withArgs("launch.expressions").returns("native"); + // Add defaults for swift configuration properties + s.withArgs("path").returns(""); + s.withArgs("runtimePath").returns(""); + s.withArgs("swiftEnvironmentVariables").returns({}); + // Default fallback + s.returns(undefined); }), update: mockFn(), }); diff --git a/test/unit-tests/ui/ToolchainSelection.test.ts b/test/unit-tests/ui/ToolchainSelection.test.ts index 5c7ee7067..7be5a6c64 100644 --- a/test/unit-tests/ui/ToolchainSelection.test.ts +++ b/test/unit-tests/ui/ToolchainSelection.test.ts @@ -69,7 +69,14 @@ suite("ToolchainSelection Unit Test Suite", () => { mockedConfiguration = mockObject({ update: mockFn(), inspect: mockFn(s => s.returns({})), - get: mockFn(), + get: mockFn(s => { + // Return appropriate defaults for configuration properties + s.withArgs("path", match.any).returns(""); + s.withArgs("runtimePath", match.any).returns(""); + s.withArgs("swiftEnvironmentVariables", match.any).returns({}); + // Default fallback + s.returns(undefined); + }), has: mockFn(s => s.returns(false)), }); mockedVSCodeWorkspace.getConfiguration.returns(instance(mockedConfiguration)); diff --git a/test/unit-tests/utilities/configuration.test.ts b/test/unit-tests/utilities/configuration.test.ts index 6e609bfcb..1e11d261a 100644 --- a/test/unit-tests/utilities/configuration.test.ts +++ b/test/unit-tests/utilities/configuration.test.ts @@ -12,8 +12,8 @@ // //===----------------------------------------------------------------------===// import * as assert from "assert"; -import { afterEach } from "mocha"; -import { match, restore } from "sinon"; +import { setup } from "mocha"; +import { match } from "sinon"; import * as vscode from "vscode"; import configuration from "@src/configuration"; @@ -21,66 +21,68 @@ import configuration from "@src/configuration"; import { instance, mockFn, mockGlobalObject, mockObject } from "../../MockUtils"; suite("Configuration/Settings Test Suite", () => { - const mockWorkspace = mockGlobalObject(vscode, "workspace"); + suite("Type validation", () => { + const mockWorkspace = mockGlobalObject(vscode, "workspace"); - afterEach(() => { - restore(); - }); - - function mockSetting(settingName: string, value: T) { - const [lead, ...rest] = settingName.split("."); - const mockSwiftConfig = mockObject({ - get: mockFn(s => s.withArgs(rest.join("."), match.any).returns(value)), + setup(() => { + mockWorkspace.getConfiguration.reset(); }); - mockWorkspace.getConfiguration.withArgs(lead).returns(instance(mockSwiftConfig)); - } - test("returns a string configuration value", () => { - mockSetting("swift.path", "foo"); - assert.equal(configuration.path, "foo"); - }); + function mockSetting(settingName: string, value: T) { + const [, ...rest] = settingName.split("."); + const mockSwiftConfig = mockObject({ + get: mockFn(s => s.withArgs(rest.join("."), match.any).returns(value)), + }); + mockWorkspace.getConfiguration.returns(instance(mockSwiftConfig)); + } - test("throws when a string setting is not a string", () => { - mockSetting("swift.path", 42); - assert.throws(() => { - configuration.path; + test("returns a string configuration value", () => { + mockSetting("swift.path", "foo"); + assert.equal(configuration.path, "foo"); }); - }); - test("returns a boolean configuration value", () => { - mockSetting("swift.recordTestDuration", false); - assert.equal(configuration.recordTestDuration, false); - }); + test("throws when a string setting is not a string", () => { + mockSetting("swift.path", 42); + assert.throws(() => { + configuration.path; + }); + }); - test("throws when a boolean setting is not a boolean", () => { - mockSetting("swift.recordTestDuration", "notaboolean"); - assert.throws(() => { - configuration.recordTestDuration; + test("returns a boolean configuration value", () => { + mockSetting("swift.recordTestDuration", false); + assert.equal(configuration.recordTestDuration, false); }); - }); - test("returns a string array configuration value", () => { - mockSetting("swift.excludeFromCodeCoverage", ["foo", "bar"]); - assert.deepEqual(configuration.excludeFromCodeCoverage, ["foo", "bar"]); - }); + test("throws when a boolean setting is not a boolean", () => { + mockSetting("swift.recordTestDuration", "notaboolean"); + assert.throws(() => { + configuration.recordTestDuration; + }); + }); - test("throws when a string array setting is not a string array", () => { - mockSetting("swift.excludeFromCodeCoverage", [42, true]); - assert.throws(() => { - configuration.excludeFromCodeCoverage; + test("returns a string array configuration value", () => { + mockSetting("swift.excludeFromCodeCoverage", ["foo", "bar"]); + assert.deepEqual(configuration.excludeFromCodeCoverage, ["foo", "bar"]); }); - }); - test("returns an object configuration value", () => { - const obj = { FOO: "BAR" }; - mockSetting("swift.swiftEnvironmentVariables", obj); - assert.deepEqual(configuration.swiftEnvironmentVariables, obj); - }); + test("throws when a string array setting is not a string array", () => { + mockSetting("swift.excludeFromCodeCoverage", [42, true]); + assert.throws(() => { + configuration.excludeFromCodeCoverage; + }); + }); + + test("returns an object configuration value", () => { + const obj = { FOO: "BAR" }; + mockSetting("swift.swiftEnvironmentVariables", obj); + assert.deepEqual(configuration.swiftEnvironmentVariables, obj); + }); - test("throws when an object setting is not an object", () => { - mockSetting("swift.swiftEnvironmentVariables", "notanobject"); - assert.throws(() => { - configuration.swiftEnvironmentVariables; + test("throws when an object setting is not an object", () => { + mockSetting("swift.swiftEnvironmentVariables", "notanobject"); + assert.throws(() => { + configuration.swiftEnvironmentVariables; + }); }); }); });