diff --git a/crates/ide-language/src/lib.rs b/crates/ide-language/src/lib.rs index a42587cc8..908e8315c 100644 --- a/crates/ide-language/src/lib.rs +++ b/crates/ide-language/src/lib.rs @@ -96,7 +96,9 @@ pub struct LanguageServerConfig { pub args: Vec, #[serde(default)] pub env: BTreeMap, - /// Files whose presence marks a workspace root, e.g. `Cargo.toml`. + /// Files whose presence marks a workspace root, e.g. `Cargo.toml`; + /// `file#table` requires the TOML file to contain that dotted table, e.g. + /// `pyproject.toml#tool.ruff`. #[serde(default)] pub root_markers: Vec, /// Serve only files below a root marker, not the whole project: a framework's diff --git a/crates/ide-language/src/markers.rs b/crates/ide-language/src/markers.rs index 832dc0c63..05eb9b0ff 100644 --- a/crates/ide-language/src/markers.rs +++ b/crates/ide-language/src/markers.rs @@ -19,9 +19,13 @@ pub fn marked_root(project_root: Option<&Path>, file: &Path, markers: &[String]) best } -/// `dir` contains `marker`: a relative path, or a file name pattern with one -/// `*` (`*.csproj`). +/// `dir` contains `marker`: a relative path, a file name pattern with one +/// `*` (`*.csproj`), or a TOML file name (no pattern) and a dotted table it +/// must contain (`pyproject.toml#tool.ruff`). fn has_marker(dir: &Path, marker: &str) -> bool { + if let Some((file, table)) = marker.split_once('#') { + return has_toml_table(&dir.join(file), table); + } let Some((prefix, suffix)) = marker.split_once('*') else { return dir.join(marker).exists(); }; @@ -30,9 +34,23 @@ fn has_marker(dir: &Path, marker: &str) -> bool { }) } +/// `file` parses as TOML and has the dotted `table` (`tool.ruff`). +fn has_toml_table(file: &Path, table: &str) -> bool { + let Ok(text) = std::fs::read_to_string(file) else { return false }; + let value = match toml::from_str::(&text) { + Ok(value) => value, + Err(e) => { + tracing::debug!("root marker {}: {e}", file.display()); + return false; + } + }; + table.split('.').try_fold(&value, |v, key| v.get(key)).is_some_and(toml::Value::is_table) +} + /// `path` is (or, after its creation or removal, was) a root marker of the /// directory above it. pub(crate) fn is_marker_path(path: &Path, marker: &str) -> bool { + let marker = marker.split_once('#').map_or(marker, |(file, _)| file); match marker.split_once('*') { None => path.ends_with(marker), Some((prefix, suffix)) => { @@ -74,6 +92,29 @@ mod tests { assert_eq!(marked_root(None, &file, &markers), Some(home.to_path_buf())); } + #[test] + fn toml_markers_need_their_table() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let file = root.join("svc/app/main.py"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + let markers = vec!["ruff.toml".to_string(), "pyproject.toml#tool.ruff".to_string()]; + std::fs::write(root.join("pyproject.toml"), "[project]\nname = \"x\"\n[tool.black]\n").unwrap(); + assert_eq!(marked_root(Some(root), &file, &markers), None); + std::fs::write(root.join("svc/pyproject.toml"), "[tool.ruff.lint]\nselect = [\"E\"]\n").unwrap(); + assert_eq!(marked_root(Some(root), &file, &markers), Some(root.join("svc"))); + // Outermost wins: an inline table counts; a value that is no table, or a + // file that does not parse, does not. + std::fs::write(root.join("pyproject.toml"), "tool = { ruff = { line-length = 100 } }\n").unwrap(); + assert_eq!(marked_root(Some(root), &file, &markers), Some(root.to_path_buf())); + std::fs::write(root.join("pyproject.toml"), "tool = { ruff = 1 }\n").unwrap(); + assert_eq!(marked_root(Some(root), &file, &markers), Some(root.join("svc"))); + std::fs::write(root.join("pyproject.toml"), "[tool.ruff]\n[broken").unwrap(); + assert_eq!(marked_root(Some(root), &file, &markers), Some(root.join("svc"))); + assert!(is_marker_path(&root.join("svc/pyproject.toml"), "pyproject.toml#tool.ruff")); + assert!(!is_marker_path(&root.join("svc/setup.cfg"), "pyproject.toml#tool.ruff")); + } + #[test] fn patterns_match_file_names() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/ide-language/src/registry.rs b/crates/ide-language/src/registry.rs index 584943590..d777ba370 100644 --- a/crates/ide-language/src/registry.rs +++ b/crates/ide-language/src/registry.rs @@ -604,6 +604,32 @@ mod tests { } } + #[test] + fn ruff_serves_python_files_only_below_its_config() { + let reg = reg(); + let cfg = reg.get(&LanguageId::new("python")).unwrap(); + let names: Vec<&str> = cfg.language_servers.iter().map(|s| s.name.as_str()).collect(); + // After the fallbacks: hover, rename and symbols stay with the type checker. + assert_eq!(names, ["basedpyright", "pyright", "ty", "ruff"]); + let ruff = cfg.language_servers.iter().find(|s| s.name == "ruff").unwrap(); + assert_eq!((ruff.command.as_str(), ruff.args.as_slice()), ("ruff", ["server".to_string()].as_slice())); + assert!(ruff.require_root_marker && !ruff.fallback); + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + for dir in ["lint", "dot", "pyproject", "plain"] { + std::fs::create_dir_all(root.join(dir)).unwrap(); + } + std::fs::write(root.join("lint/ruff.toml"), "").unwrap(); + std::fs::write(root.join("dot/.ruff.toml"), "").unwrap(); + std::fs::write(root.join("pyproject/pyproject.toml"), "[tool.ruff]\nline-length = 100\n").unwrap(); + std::fs::write(root.join("plain/pyproject.toml"), "[project]\nname = \"plain\"\n").unwrap(); + let served = |file: &str| crate::marked_root(Some(root), &root.join(file), &ruff.root_markers); + for dir in ["lint", "dot", "pyproject"] { + assert_eq!(served(&format!("{dir}/pkg/main.py")), Some(root.join(dir)), "{dir}"); + } + assert_eq!(served("plain/main.py"), None); + } + #[test] fn html_of_angular_projects_is_an_angular_template() { let reg = reg(); diff --git a/crates/ide-lsp/tests/real_servers.rs b/crates/ide-lsp/tests/real_servers.rs index f25f77745..3ce021f39 100644 --- a/crates/ide-lsp/tests/real_servers.rs +++ b/crates/ide-lsp/tests/real_servers.rs @@ -194,6 +194,62 @@ async fn ty_python_end_to_end() { .await; } +/// Ruff next to basedpyright in a project configured for Ruff: its lint +/// diagnostics, the only formatter and the only Organize Imports and Fix All. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "requires ruff and basedpyright on PATH"] +async fn ruff_next_to_basedpyright_end_to_end() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + std::fs::write(root.join("pyproject.toml"), "[tool.ruff.lint]\nselect = [\"F\", \"I\"]\n").unwrap(); + let path = root.join("example.py"); + let source = "import sys\nimport os\n\nx = os.getcwd( )\n"; + std::fs::write(&path, source).unwrap(); + let mut python = LanguageRegistry::load(None).unwrap().get(&LanguageId::new("python")).unwrap(); + python.language_servers.retain(|server| matches!(server.name.as_str(), "basedpyright" | "ruff")); + assert_eq!(python.language_servers.len(), 2); + let (mgr, events) = manager(&root, LanguageRegistry::from_configs(vec![python])); + let doc = DocId::next(); + mgr.did_open(doc, &path, &LanguageId::new("python"), 1, source); + for name in ["basedpyright", "ruff"] { + retry(name, Duration::from_secs(60), || { + let status = last_status(&events, name); + if let Some((ServerStatus::Crashed | ServerStatus::Missing | ServerStatus::Misconfigured, msg)) = &status { + panic!("{name} failed: {msg:?}"); + } + async move { matches!(status, Some((ServerStatus::Ready | ServerStatus::Busy, _))).then_some(()) } + }) + .await; + } + // F401 (`sys` unused) and I001 (unsorted imports). + let codes = retry("ruff diagnostics", Duration::from_secs(30), || { + let codes: Option> = events.lock().iter().rev().find_map(|e| match e { + IdeEvent::Diagnostics { source, items, .. } if source == "ruff" && !items.is_empty() => { + Some(items.iter().filter_map(|d| d.code.clone()).collect()) + } + _ => None, + }); + async move { codes } + }) + .await; + assert!(codes.iter().any(|c| c == "F401") && codes.iter().any(|c| c == "I001"), "{codes:?}"); + + let options = ide_lsp::types::FormatOptions { tab_size: 4, insert_spaces: true, on_save: false }; + let edits = mgr.format(doc, None, options).await.unwrap(); + assert!(!edits.is_empty() && edits.iter().all(|e| e.range.start.line == 3), "{edits:?}"); + + let whole = Range::new(Pos::new(0, 0), Pos::new(4, 0)); + for kind in ["source.organizeImports", "source.fixAll"] { + let actions = mgr.code_actions(doc, whole, vec![kind.into()]).await.unwrap(); + let kinds: Vec<&str> = actions.iter().filter_map(|a| a.kind.as_deref()).collect(); + assert_eq!(kinds, [format!("{kind}.ruff")], "{actions:?}"); + let action = mgr.resolve_code_action(doc, actions[0].clone()).await.unwrap(); + let edit = action.edit.expect("an edit"); + assert!(!edited_lines(&edit, &path).is_empty(), "{edit:?}"); + } + tokio::time::timeout(Duration::from_secs(10), mgr.shutdown_all()).await.unwrap(); +} + #[tokio::test(flavor = "multi_thread")] #[ignore = "requires rust-analyzer"] async fn rust_analyzer_end_to_end() { diff --git a/crates/ide-sdk/src/tests.rs b/crates/ide-sdk/src/tests.rs index dd4628b2e..3e121f241 100644 --- a/crates/ide-sdk/src/tests.rs +++ b/crates/ide-sdk/src/tests.rs @@ -254,10 +254,10 @@ fn language_server_settings_disable_add_and_override() { let mut s = Settings::default(); s.language_servers.insert("pylsp".into(), LanguageServerSettings { enabled: false, ..Default::default() }); s.language_servers.insert( - "ruff".into(), + "pylyzer".into(), LanguageServerSettings { - command: "ruff".into(), - args: Some(vec!["server".into()]), + command: "pylyzer".into(), + args: Some(vec!["--server".into()]), languages: vec!["python".into()], root_markers: vec!["pyproject.toml".into()], ..Default::default() @@ -265,17 +265,42 @@ fn language_server_settings_disable_add_and_override() { ); let defined = [server("basedpyright"), server("pylsp")]; let names = |v: Vec| v.into_iter().map(|c| c.name).collect::>(); - assert_eq!(names(language_servers("python", &defined, &s)), ["basedpyright", "ruff"]); + assert_eq!(names(language_servers("python", &defined, &s)), ["basedpyright", "pylyzer"]); assert_eq!(names(language_servers("rust", &[server("rust-analyzer")], &s)), ["rust-analyzer"]); // Listed with the tools even while disabled. let mut langs = ide_language::LanguageRegistry::load(None).unwrap().all(); with_custom_servers(&mut langs, &s); let python = langs.iter().find(|l| l.id.as_str() == "python").unwrap(); + let mut pylyzer = python.language_servers.iter().find(|c| c.name == "pylyzer").unwrap().clone(); + assert_eq!(pylyzer.root_markers, ["pyproject.toml"]); + server_launch_overrides(&mut pylyzer, &s); + assert_eq!((pylyzer.command.as_str(), pylyzer.args.as_slice()), ("pylyzer", &["--server".to_string()][..])); + + // A hand-defined server named like a built-in one only overrides how it + // launches: languages and root markers stay the built-in definition's. + s.language_servers.insert( + "ruff".into(), + LanguageServerSettings { + command: "/opt/ruff".into(), + args: Some(vec!["server".into(), "--preview".into()]), + languages: vec!["python".into()], + root_markers: vec!["pyproject.toml".into()], + ..Default::default() + }, + ); + let mut langs = ide_language::LanguageRegistry::load(None).unwrap().all(); + with_custom_servers(&mut langs, &s); + let python = langs.iter().find(|l| l.id.as_str() == "python").unwrap(); + assert_eq!(python.language_servers.iter().filter(|c| c.name == "ruff").count(), 1); let mut ruff = python.language_servers.iter().find(|c| c.name == "ruff").unwrap().clone(); - assert_eq!(ruff.root_markers, ["pyproject.toml"]); + assert_eq!(ruff.root_markers, ["ruff.toml", ".ruff.toml", "pyproject.toml#tool.ruff"]); + assert!(ruff.require_root_marker); server_launch_overrides(&mut ruff, &s); - assert_eq!((ruff.command.as_str(), ruff.args.as_slice()), ("ruff", &["server".to_string()][..])); + assert_eq!( + (ruff.command.as_str(), ruff.args.as_slice()), + ("/opt/ruff", &["server".to_string(), "--preview".to_string()][..]) + ); s.language_servers.insert( "rust-analyzer".into(), diff --git a/crates/ide-tools/src/registry.rs b/crates/ide-tools/src/registry.rs index 661a71705..755da795b 100644 --- a/crates/ide-tools/src/registry.rs +++ b/crates/ide-tools/src/registry.rs @@ -233,6 +233,17 @@ pub static TOOLS: &[ToolSpec] = &[ homepage: "https://docs.astral.sh/ty/", needs: &[], }, + ToolSpec { + name: "ruff", + display: "Ruff", + kind: LanguageServer, + commands: &["ruff"], + method: Method::PipVenv { package: "ruff" }, + fallback: None, + version: ver("ruff"), + homepage: "https://docs.astral.sh/ruff/", + needs: &[], + }, ToolSpec { name: "bash-language-server", display: "Bash Language Server", diff --git a/crates/ide-tools/src/tests.rs b/crates/ide-tools/src/tests.rs index 00a3c55ea..ccb35e2be 100644 --- a/crates/ide-tools/src/tests.rs +++ b/crates/ide-tools/src/tests.rs @@ -157,6 +157,7 @@ fn interchangeable_servers_list_their_alternatives() { assert_eq!(find(&list, "vtsls").alternatives, ["vtsls", "typescript-language-server"]); // A regular server after a fallback starts no group with it. assert!(find(&list, "angular-language-server").alternatives.is_empty()); + assert!(find(&list, "ruff").alternatives.is_empty()); assert!(find(&list, "rust-analyzer").alternatives.is_empty()); } diff --git a/languages/python.toml b/languages/python.toml index b450c3dd7..1d3b9504b 100644 --- a/languages/python.toml +++ b/languages/python.toml @@ -40,6 +40,16 @@ args = ["server"] root-markers = ["ty.toml", "pyproject.toml", "setup.py", "setup.cfg", "requirements.txt", "Pipfile"] install-hint = "uv tool install ty (or: pip install ty)" +# Lint, format, organize imports and fix all next to the type checker, in +# projects configured for Ruff (formatting is Ruff's alone: basedpyright has none). +[[language-servers]] +name = "ruff" +command = "ruff" +args = ["server"] +root-markers = ["ruff.toml", ".ruff.toml", "pyproject.toml#tool.ruff"] +require-root-marker = true +install-hint = "pip install ruff (or: uv tool install ruff)" + [debugger] adapter = "debugpy" command = "python3" diff --git a/src/features/tools/ServerSettings.tsx b/src/features/tools/ServerSettings.tsx index 2e9710e45..3f10ea893 100644 --- a/src/features/tools/ServerSettings.tsx +++ b/src/features/tools/ServerSettings.tsx @@ -553,7 +553,7 @@ export function NewServerDialog({ const [commandLine, setCommandLine] = useState(""); const [name, setName] = useState(null); const [languages, setLanguages] = useState([]); - // "ruff server": the executable, then its arguments. + // "pylyzer --server": the executable, then its arguments. let words: string[] = []; let quoteError: string | null = null; try { @@ -600,7 +600,7 @@ export function NewServerDialog({ autoFocus className={styles.grow} value={commandLine} - placeholder="ruff server" + placeholder="pylyzer --server" onChange={(e) => setCommandLine(e.target.value)} /> ([ "NAME=value; OTHER=value", "node_modules/typescript", "PascalCase", + "pylyzer --server", "pyproject.toml, .git", - "ruff server", "run pytest", "snake_case", "tests",