-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix indexing of excluded nested Ruff workspaces #27303
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| use anyhow::Result; | ||
| use insta::assert_json_snapshot; | ||
| use anyhow::{Context, Result}; | ||
| use insta::{assert_json_snapshot, assert_snapshot}; | ||
|
|
||
| use crate::TestServerBuilder; | ||
| use crate::{TestServer, TestServerBuilder}; | ||
|
|
||
| const SOURCE: &str = "value= \"hello\"\n"; | ||
|
|
||
| #[test] | ||
| fn selects_the_correct_workspace_settings_for_multi_root_workspaces() -> Result<()> { | ||
|
|
@@ -104,6 +106,166 @@ ignore = ["F401"] | |
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn nested_workspace_root_is_not_excluded_by_an_ancestor() -> Result<()> { | ||
| let mut server = nested_workspace_server(&["sub"], WorkspaceExclusion::Exclude)?; | ||
|
|
||
| assert_snapshot!( | ||
| open_and_format(&mut server, "sub/test.py", SOURCE) | ||
| .context("nested workspace should be formatted")?, | ||
| @"value = 'hello'" | ||
| ); | ||
| // Explicitly opening `sub` does not override its own exclusion of `foo`. | ||
| assert!(open_and_format(&mut server, "sub/foo/test.py", SOURCE).is_none()); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn nested_workspace_root_is_not_excluded_by_an_ancestor_in_a_multi_root_workspace() -> Result<()> { | ||
| const ISSUE_SOURCE: &str = r#"print("That's a pretty long long long long long long long long long long long long long long long long long long long long long long line") | ||
| "#; | ||
|
|
||
| let mut server = TestServerBuilder::new()? | ||
| .with_workspace(".")? | ||
| .with_workspace("sub")? | ||
| .with_file( | ||
| ".ruff.toml", | ||
| r#"target-version = "py312" | ||
| line-length = 120 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can we reduce the |
||
|
|
||
| extend-exclude = [ | ||
| "sub", | ||
| ] | ||
| "#, | ||
| )? | ||
| .with_file( | ||
| "sub/.ruff.toml", | ||
| r#"target-version = "py312" | ||
| line-length = 120 | ||
|
|
||
| extend-exclude = [ | ||
| "foo", | ||
| ] | ||
| "#, | ||
| )? | ||
| .with_file("test.py", ISSUE_SOURCE)? | ||
| .with_file("sub/test.py", ISSUE_SOURCE)? | ||
| .with_file("sub/foo/test.py", ISSUE_SOURCE)? | ||
| .build(); | ||
|
|
||
| assert_snapshot!( | ||
| open_and_format(&mut server, "test.py", ISSUE_SOURCE) | ||
| .context("parent workspace should be formatted")?, | ||
| @r#" | ||
| print( | ||
| "That's a pretty long long long long long long long long long long long long long long long long long long long long long long line" | ||
| ) | ||
| "# | ||
| ); | ||
| assert_snapshot!( | ||
| open_and_format(&mut server, "sub/test.py", ISSUE_SOURCE) | ||
| .context("nested workspace should be formatted")?, | ||
| @r#" | ||
| print( | ||
| "That's a pretty long long long long long long long long long long long long long long long long long long long long long long line" | ||
| ) | ||
| "# | ||
| ); | ||
| assert!(open_and_format(&mut server, "sub/foo/test.py", ISSUE_SOURCE).is_none()); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn nested_workspace_remains_excluded_without_explicit_registration() -> Result<()> { | ||
| let mut server = nested_workspace_server(&["."], WorkspaceExclusion::ExtendExclude)?; | ||
|
|
||
| assert!(open_and_format(&mut server, "sub/test.py", SOURCE).is_none()); | ||
| assert!(open_and_format(&mut server, "sub/foo/test.py", SOURCE).is_none()); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn unrelated_file_outside_workspace_uses_fallback_configuration() -> Result<()> { | ||
| let mut server = nested_workspace_server(&["sub"], WorkspaceExclusion::ExtendExclude)?; | ||
|
|
||
| assert_snapshot!( | ||
| open_and_format(&mut server, "unrelated/test.py", SOURCE) | ||
| .context("unrelated file should use fallback formatting")?, | ||
| @r#"value = "hello""# | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[test] | ||
| fn single_file_mode_does_not_index_nested_configuration() -> Result<()> { | ||
| let mut server = TestServerBuilder::new()? | ||
| .with_file("nested/.ruff.toml", "[format]\nquote-style = \"single\"\n")? | ||
| .with_file("nested/test.py", SOURCE)? | ||
| .with_file("unrelated/test.py", SOURCE)? | ||
| .build(); | ||
|
|
||
| assert_snapshot!( | ||
| open_and_format(&mut server, "nested/test.py", SOURCE) | ||
| .context("nested file should use fallback formatting")?, | ||
| @r#"value = "hello""# | ||
| ); | ||
| assert_snapshot!( | ||
| open_and_format(&mut server, "unrelated/test.py", SOURCE) | ||
| .context("unrelated file should use fallback formatting")?, | ||
| @r#"value = "hello""# | ||
| ); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| #[derive(Clone, Copy)] | ||
| enum WorkspaceExclusion { | ||
| Exclude, | ||
| ExtendExclude, | ||
| } | ||
|
|
||
| fn nested_workspace_server( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be useful to have the function doc include the file tree it's constructing otherwise it's a little bit hard to visualize the tree structure here 😅 Thanks to Codex: |
||
| workspaces: &[&str], | ||
| exclusion: WorkspaceExclusion, | ||
| ) -> Result<TestServer> { | ||
| let mut builder = TestServerBuilder::new()?; | ||
| for workspace in workspaces { | ||
| builder = builder.with_workspace(workspace)?; | ||
| } | ||
|
|
||
| let server = builder | ||
| .with_file( | ||
| ".ruff.toml", | ||
| match exclusion { | ||
| WorkspaceExclusion::Exclude => "exclude = [\"sub\"]\n", | ||
| WorkspaceExclusion::ExtendExclude => "extend-exclude = [\"sub\"]\n", | ||
| }, | ||
| )? | ||
| .with_file( | ||
| "sub/.ruff.toml", | ||
| "extend-exclude = [\"foo\"]\n[format]\nquote-style = \"single\"\n", | ||
| )? | ||
| .with_file("test.py", SOURCE)? | ||
| .with_file("sub/test.py", SOURCE)? | ||
| .with_file("sub/foo/test.py", SOURCE)? | ||
| .with_file("unrelated/test.py", SOURCE)? | ||
| .build(); | ||
|
|
||
| Ok(server) | ||
| } | ||
|
|
||
| fn open_and_format(server: &mut TestServer, path: &str, source: &str) -> Option<String> { | ||
| server.open_text_document(path, source, 1); | ||
| server | ||
| .format_request(path) | ||
| .and_then(|edits| edits.into_iter().next()) | ||
| .map(|edit| edit.new_text) | ||
| } | ||
|
|
||
| #[test] | ||
| fn unavailable_document_diagnostic_returns_empty_response() -> Result<()> { | ||
| let mut server = TestServerBuilder::new()?.with_workspace(".")?.build(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We skip workspace folders before where the ancestor configuration excluded them (the ancestor configuration is built above)