Reset ruff.configuration if it contains VS Code variables

[dhruvmanila]

Summary

fixes: #741

Test Plan

settings.json:

{
  "ruff.configuration": "${workspaceFolder}/formatter/ruff.toml"
}

"Ruff" output channel:

Note

I've only kept the necessary settings here but the actual log output includes everything

2025-05-01 10:38:43.029 [info] Workspace settings for /Users/dhruv/playground/ruff: {
    "nativeServer": "on",
    "cwd": "/Users/dhruv/playground/ruff",
    "workspace": "file:///Users/dhruv/playground/ruff",
    "path": [
        "/Users/dhruv/work/astral/ruff/target/debug/ruff"
    ],
    "configuration": "/Users/dhruv/playground/ruff/formatter/ruff.toml",
}
2025-05-01 10:38:43.030 [info] Resetting 'ruff.configuration' to null in global settings because it contains workspace specific variables
2025-05-01 10:38:43.030 [info] Global settings: {
    "nativeServer": "on",
    "cwd": "/",
    "workspace": "/",
    "path": [
        "/Users/dhruv/work/astral/ruff/target/debug/ruff"
    ],
    "configuration": null,
}

settings.json:

{
	"ruff.configuration": "${workspaceFolder:root}/formatter/ruff.toml"
}

"Ruff" output channel:

2025-05-01 10:40:42.200 [info] Workspace settings for /Users/dhruv/playground/ruff: {
    "nativeServer": "on",
    "cwd": "/Users/dhruv/playground/ruff",
    "workspace": "file:///Users/dhruv/playground/ruff",
    "path": [
        "/Users/dhruv/work/astral/ruff/target/debug/ruff"
    ],
    "configuration": "${workspaceFolder:root}/formatter/ruff.toml",
}
2025-05-01 10:40:42.200 [info] Resetting 'ruff.configuration' to null in global settings because it contains workspace specific variables
2025-05-01 10:40:42.200 [info] Global settings: {
    "nativeServer": "on",
    "cwd": "/",
    "workspace": "/",
    "path": [
        "/Users/dhruv/work/astral/ruff/target/debug/ruff"
    ],
    "configuration": null,
}

For this specific test case, you can see that the workspace settings also includes the ${workspaceFolder:root} because it doesn't exists. That would also create an error:

2025-05-01 10:42:14.245925000 ERROR Failed to load settings from `configuration`: error looking key 'workspaceFolder:root' up: environment variable not found

I think we should handle that as well.

[dhruvmanila]

Another thing I noted is that we could call resolveVariables even for global settings and pass null for the workspace parameter to resolve other variables like ${userHome}, ${cwd}, and ${env:...}.

[MichaReiser]

I'm wondering if the extension should fail in that case instead of reverting to a default configuration. Just to be safe and the log message will guide users towards taking the right action. This is, from what I understand, also the closest to the current behavior.

What was your motivation for returning none instead?

[MichaReiser]

Should we log this with warn or even error?

[dhruvmanila]

I don't think so, it's not a user error but just something to note of. Refer to my other comment.

[dhruvmanila]

I'm wondering if the extension should fail in that case instead of reverting to a default configuration. Just to be safe and the log message will guide users towards taking the right action. This is, from what I understand, also the closest to the current behavior.

I don't think the extension can fail because it's not a user error. We're passing two settings to the language server - extension settings and global settings. The extension settings are computed for each workspace in the VS Code session while the global settings are computed by just considering the global values. Now, when a user has just specified a single ruff.configuration value in the global VS Code settings, the workspace settings will use that and the global settings will also use that. Does that make sense?

[MichaReiser]

Now, when a user has just specified a single ruff.configuration value in the global VS Code settings, the workspace settings will use that and the global settings will also use that. Does that make sense?

I don't understand this part.

I guess the situation I'm concerned about is that we're ignoring a setting explicitly set by the user (in one of their configuration file). Or is this not the case because the settings.json is applied boht as workspace and extension settings?

[dhruvmanila]

I guess the situation I'm concerned about is that we're ignoring a setting explicitly set by the user (in one of their configuration file). Or is this not the case because the settings.json is applied boht as workspace and extension settings?

We're not ignoring any setting. The language server has access to settings specific to a workspace and the global settings. The global settings is used as a fallback if the workspace doesn't have a settings in the index. When this fallback happens, the server tries to resolve the settings which involves expanding any environment variables which is where the user sees this error. If the settings is used in the context of a workspace, the server will pickup the configuration to the path.

This is a bit confusing because for VS Code, the workspace settings and the default settings (used to get the global settings value) are the same for a VS Code session with the single session unless the user has a workspace specific settings.json. Basically, the global settings.json values are used for both workspace and global settings. But, if there's .vscode/settings.json in the workspace, the workspace settings would use the values from that while the global settings would use the values from the global settings.json.