astral-sh / ruff-vscode

A Visual Studio Code extension with support for the Ruff linter.
Other
1.1k stars 53 forks source link

VS Code setting ruff.interpreter does no longer accept relative paths #551

Closed ni-co-la closed 3 months ago

ni-co-la commented 3 months ago

With the latest version of the VS Code extension (v2024.34.0 and v2024.32.0), relative paths for "ruff.interpreter" are no longer accepted (in .vscode/settings.json).

In our repository, this settings file is shared between for all developers and it points to the venv Python installation inside the repository. For example:

    "ruff.interpreter": [
        ".venv/bin/python"
    ],

This was working just fine in previous versions and is still working fine with other extension like pylint and flake8.

Output:

2024-07-23 10:35:41.066 [error] Python version undefined.undefined is not supported.
2024-07-23 10:35:41.066 [error] Selected python path: undefined
2024-07-23 10:35:41.066 [error] Supported versions are 3.7 and above.

Setup:

ni-co-la commented 3 months ago

Neither does the following setting work:

    "ruff.interpreter": [
        "${workspaceFolder}/.venv/bin/python"
    ],
charliermarsh commented 3 months ago

Thanks, will fix!

charliermarsh commented 3 months ago

I am surprised that "${workspaceFolder}/.venv/bin/python" doesn't work.

dhruvmanila commented 3 months ago

Can you tell me which language server are you using? Are you using the native server or ruff-lsp i.e., have you set the ruff.nativeServer setting?

I'm asking because there shouldn't be any change if you're using ruff-lsp. The meaning of the setting is only changed for the native server where it's only used to find out the ruff binary.

If it's the native server then I think it might be related to https://github.com/astral-sh/ruff-vscode/commit/e1777e138865453199626bb51cc6b2842e16360c or https://github.com/astral-sh/ruff-vscode/commit/564a37a0128cf8e55b79e260bb12f74d470eeb0d because earlier we would just pass the interpreter path as ServerOptions to create the LanguageClient in VS Code, as is still being done for ruff-lsp:

https://github.com/astral-sh/ruff-vscode/blob/7896831184d161ca4ab93c93b9db57eaf1609a6c/src/common/server.ts#L210-L263

charliermarsh commented 3 months ago

We have to expand settings.interpreter[0].

ni-co-la commented 3 months ago

I am surprised that "${workspaceFolder}/.venv/bin/python" doesn't work.

Me too. I expected that this should not make a difference for ruff, if I enter the absolute path or if I use the ${workspaceFolder}.

Can you tell me which language server are you using? Are you using the native server or ruff-lsp i.e., have you set the ruff.nativeServer setting?

ruff.nativeServer is not set.

For the VS Code extension I have the following settings:

    "ruff.importStrategy": "fromEnvironment",
    "ruff.path": [
        "${workspaceFolder}/.venv/bin/ruff"
    ],
    "ruff.lint.args": [
        "--config=${workspaceFolder}/config/ruff.toml"
    ],
    "ruff.format.args": [
        "--config=${workspaceFolder}/config/ruff.toml"
    ],
charliermarsh commented 3 months ago

I think I see at least the non-expansion issue.

charliermarsh commented 3 months ago

@dhruvmanila - Do you want to take it? getInterpreterFromSetting just calls config.get<string[]>("interpreter"), but we need to run resolveVariables on it like we do in getWorkspaceSettings.

dhruvmanila commented 3 months ago

Yeah, I can take it on.

charliermarsh commented 3 months ago

Thanks. We probably need to expand relative paths too before passing to execFile.

dhruvmanila commented 3 months ago

I think there might be a bigger issue here. We do resolve the variables but only when it's a workspace setting:

https://github.com/astral-sh/ruff-vscode/blob/7896831184d161ca4ab93c93b9db57eaf1609a6c/src/common/settings.ts#L139-L139

And, we don't resolve any variables for the global settings:

https://github.com/astral-sh/ruff-vscode/blob/7896831184d161ca4ab93c93b9db57eaf1609a6c/src/common/settings.ts#L183-L223

Which, I guess, makes sense because it's not tied to any specific workspace.

Black doesn't seem to be resolving the variables from global settings either:

https://github.com/microsoft/vscode-black-formatter/blob/458ed8f03f03c6e021361b7dcf4a8c7ab7c3d5fb/src/common/settings.ts#L149-L171

dhruvmanila commented 3 months ago

Ok, I can reproduce this, let me check what's actually going on.

dhruvmanila commented 3 months ago

I found the problem, it's bug I introduced in https://github.com/astral-sh/ruff-vscode/commit/4b03f7c4266a8b6d2f0047c7fbe231ec0cd408a4

dhruvmanila commented 3 months ago

Hi @ni-co-la, can you try out the pre-release version which contains the bug fix. You can go to the Ruff extension page and click on "Switch to Pre-Release Version":

Screenshot 2024-07-24 at 10 28 16
ni-co-la commented 3 months ago

I just tested it with the pre-release version and it is working fine with "ruff.interpreter": [ "${workspaceFolder}/.venv/bin/python" ].

"ruff.interpreter": [ "./.venv/bin/python" ] is still not working, which was working in a previous version.

Which is fine for me, as it is more explicit to use the ${workspaceFolder}.

Thanks a lot for the fix.

dhruvmanila commented 3 months ago

Thanks for testing it!

"ruff.interpreter": [ "./.venv/bin/python" ] is still not working, which was working in a previous version.

Yes, I explicitly decided not to support this. It wasn't working before either, and you'll probably get the same log messages as in the PR description for earlier versions. It's just that a recent change (https://github.com/astral-sh/ruff-vscode/commit/e665ec7be83b72f7586b30387cd87c8faf62b9e1) made it explicitly visible by removing an unintended fall back mechanism.

I'll make a stable release later today.