Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config file "extend" not working from language server #13261

Closed
jakeanq opened this issue Sep 6, 2024 · 3 comments · Fixed by #13285
Closed

Config file "extend" not working from language server #13261

jakeanq opened this issue Sep 6, 2024 · 3 comments · Fixed by #13285
Labels
bug Something isn't working great writeup A wonderful example of a quality contribution server Related to the LSP server

Comments

@jakeanq
Copy link

jakeanq commented Sep 6, 2024

I don't seem to be able to include a config file from another config file when using the language server, however it works fine from the command line.

Example

This is fairly arbitrary...

ruff_config1.toml:

[lint]
select = ["F401", "F821", "ANN"]

ruff_config2.toml:

extend = "./ruff_config1.toml"
[lint]
ignore = ["F401"]

ruff_test.py:

import typing

print(lemurs)

def x():
    return 10

When I run this with ruff check I get the expected output:

$ ruff check ruff_test.py --config ~/ruff_configs/ruff_config2.toml --output-format concise
ruff_test.py:3:7: F821 Undefined name `lemurs`
ruff_test.py:5:5: ANN201 Missing return type annotation for public function `x`
Found 2 errors.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).

(note that F821 is a default rule)

However with the following Neovim LSP config:

lc = require'lspconfig'
lc.ruff.setup{
  init_options = {
    settings = {
      configuration = "~/ruff_configs/ruff_config2.toml"
    }
  }
}

I see the F821 violation (this is enabled by default and indicates that checking is working) and I don't see the F401 violation (expected, indicating that ruff_config2.toml is being loaded, since that rule is enabled by default) but I don't see the ANN201 violation, indicating that ruff_config1.toml was not loaded.

I have also tried changing the extend path to use an absolute path or a path based on an environment variable, but it doesn't seem to make a difference.

Replicated with ruff 0.6.2 and 0.6.3.

@MichaReiser MichaReiser added bug Something isn't working server Related to the LSP server labels Sep 8, 2024
@MichaReiser
Copy link
Member

Thanks for reporting this.

I haven't reproduced it yet myself, but I'm pretty confident to say that this is a bug in the server. The server loads the configuration but it doesn't follow the extends chain here (not saying that we should add it here ;))

let editor_configuration = if let Some(config_file_path) = configuration {
match open_configuration_file(&config_file_path, project_root) {
Ok(config_from_file) => editor_configuration.combine(config_from_file),
Err(err) => {
tracing::error!("Unable to find editor-specified configuration file: {err}");
editor_configuration
}
}
} else {
editor_configuration
};

@MichaReiser
Copy link
Member

I think the fix is:

fn open_configuration_file(config_path: &Path) -> crate::Result<Configuration> {
    ruff_workspace::resolver::resolve_configuration(config_path, Relativity::Parent, &())
}

Need to find a good approach to test this change.

@jakeanq
Copy link
Author

jakeanq commented Sep 17, 2024

Thank you for the fix! It is working perfectly with the newly released ruff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working great writeup A wonderful example of a quality contribution server Related to the LSP server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants