From fa0df6da1c2c6ab4394826c035183fdf1804f7e2 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Mon, 8 Sep 2025 15:15:17 -0400 Subject: [PATCH] python: Replace pyright with basedpyright (#35362) Follow-up to #35250. Let's experiment with having this by default on nightly. Release Notes: - Added built-in support for the basedpyright language server for Python code. basedpyright is now enabled by default, and pyright (previously the primary Python language server) remains available but is disabled by default. This supersedes the basedpyright extension, which can be uninstalled. Advantages of basedpyright over pyright include support for inlay hints, semantic highlighting, auto-import code actions, and stricter type checking. To switch back to pyright, add the following configuration to settings.json: ```json { "languages": { "Python": { "language_servers": ["pyright", "pylsp", "!basedpyright"] } } } ``` --------- Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Co-authored-by: Lukas Wirth --- Cargo.lock | 15 ++- Cargo.toml | 2 + crates/language/src/language.rs | 9 ++ crates/language/src/language_registry.rs | 43 ++++++--- .../src/extension_lsp_adapter.rs | 4 + crates/language_onboarding/Cargo.toml | 30 ++++++ crates/language_onboarding/LICENSE-GPL | 1 + crates/language_onboarding/src/python.rs | 95 +++++++++++++++++++ crates/languages/Cargo.toml | 1 - crates/languages/src/lib.rs | 46 ++------- crates/languages/src/python.rs | 44 ++++++--- crates/lsp/src/lsp.rs | 6 ++ crates/zed/Cargo.toml | 1 + crates/zed/src/zed.rs | 3 + 14 files changed, 233 insertions(+), 67 deletions(-) create mode 100644 crates/language_onboarding/Cargo.toml create mode 120000 crates/language_onboarding/LICENSE-GPL create mode 100644 crates/language_onboarding/src/python.rs diff --git a/Cargo.lock b/Cargo.lock index 09106277c77288f56d1bc348e7d42cfde9366355..47281f51fa7c80d5d04b0ff1e1ea40f3dd2ad4f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9181,6 +9181,19 @@ dependencies = [ "x_ai", ] +[[package]] +name = "language_onboarding" +version = "0.1.0" +dependencies = [ + "db", + "editor", + "gpui", + "project", + "ui", + "workspace", + "workspace-hack", +] + [[package]] name = "language_selector" version = "0.1.0" @@ -9243,7 +9256,6 @@ dependencies = [ "chrono", "collections", "dap", - "feature_flags", "futures 0.3.31", "gpui", "http_client", @@ -20428,6 +20440,7 @@ dependencies = [ "language_extension", "language_model", "language_models", + "language_onboarding", "language_selector", "language_tools", "languages", diff --git a/Cargo.toml b/Cargo.toml index c8d555a24278a77a9dbd0f649ee75d6d04efcba0..0f13835f0c87b94c5acf335a63b0067d50fff156 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -94,6 +94,7 @@ members = [ "crates/language_extension", "crates/language_model", "crates/language_models", + "crates/language_onboarding", "crates/language_selector", "crates/language_tools", "crates/languages", @@ -320,6 +321,7 @@ language = { path = "crates/language" } language_extension = { path = "crates/language_extension" } language_model = { path = "crates/language_model" } language_models = { path = "crates/language_models" } +language_onboarding = { path = "crates/language_onboarding" } language_selector = { path = "crates/language_selector" } language_tools = { path = "crates/language_tools" } languages = { path = "crates/languages" } diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 86faf2b9d316dd068c400c48c5b0b99196cfc191..256f6d45734ec068f1e038fe0d07049bb732e34b 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -590,6 +590,11 @@ pub trait LspAdapter: 'static + Send + Sync { "Not implemented for this adapter. This method should only be called on the default JSON language server adapter" ); } + + /// True for the extension adapter and false otherwise. + fn is_extension(&self) -> bool { + false + } } async fn try_fetch_server_binary( @@ -2270,6 +2275,10 @@ impl LspAdapter for FakeLspAdapter { let label_for_completion = self.label_for_completion.as_ref()?; label_for_completion(item, language) } + + fn is_extension(&self) -> bool { + false + } } fn get_capture_indices(query: &Query, captures: &mut [(&str, &mut Option)]) { diff --git a/crates/language/src/language_registry.rs b/crates/language/src/language_registry.rs index 4f07240e44599361bf92188fd410dd674745874a..5d9d5529c145a8769d142a1f943b6ae00aeaaeb8 100644 --- a/crates/language/src/language_registry.rs +++ b/crates/language/src/language_registry.rs @@ -374,14 +374,23 @@ impl LanguageRegistry { pub fn register_available_lsp_adapter( &self, name: LanguageServerName, - load: impl Fn() -> Arc + 'static + Send + Sync, + adapter: Arc, ) { - self.state.write().available_lsp_adapters.insert( + let mut state = self.state.write(); + + if adapter.is_extension() + && let Some(existing_adapter) = state.all_lsp_adapters.get(&name) + && !existing_adapter.adapter.is_extension() + { + log::warn!( + "not registering extension-provided language server {name:?}, since a builtin language server exists with that name", + ); + return; + } + + state.available_lsp_adapters.insert( name, - Arc::new(move || { - let lsp_adapter = load(); - CachedLspAdapter::new(lsp_adapter) - }), + Arc::new(move || CachedLspAdapter::new(adapter.clone())), ); } @@ -396,13 +405,21 @@ impl LanguageRegistry { Some(load_lsp_adapter()) } - pub fn register_lsp_adapter( - &self, - language_name: LanguageName, - adapter: Arc, - ) -> Arc { - let cached = CachedLspAdapter::new(adapter); + pub fn register_lsp_adapter(&self, language_name: LanguageName, adapter: Arc) { let mut state = self.state.write(); + + if adapter.is_extension() + && let Some(existing_adapter) = state.all_lsp_adapters.get(&adapter.name()) + && !existing_adapter.adapter.is_extension() + { + log::warn!( + "not registering extension-provided language server {:?} for language {language_name:?}, since a builtin language server exists with that name", + adapter.name(), + ); + return; + } + + let cached = CachedLspAdapter::new(adapter); state .lsp_adapters .entry(language_name) @@ -411,8 +428,6 @@ impl LanguageRegistry { state .all_lsp_adapters .insert(cached.name.clone(), cached.clone()); - - cached } /// Register a fake language server and adapter diff --git a/crates/language_extension/src/extension_lsp_adapter.rs b/crates/language_extension/src/extension_lsp_adapter.rs index 9b6e467f2f2dfddccaa96d7aaf5d5550d72fe904..c1bc058a344e02fd4830c9db89684579a9e7e045 100644 --- a/crates/language_extension/src/extension_lsp_adapter.rs +++ b/crates/language_extension/src/extension_lsp_adapter.rs @@ -398,6 +398,10 @@ impl LspAdapter for ExtensionLspAdapter { Ok(labels_from_extension(labels, language)) } + + fn is_extension(&self) -> bool { + true + } } fn labels_from_extension( diff --git a/crates/language_onboarding/Cargo.toml b/crates/language_onboarding/Cargo.toml new file mode 100644 index 0000000000000000000000000000000000000000..a437adf1191a3b76fbd828dacaa60b75b1f7df28 --- /dev/null +++ b/crates/language_onboarding/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "language_onboarding" +version = "0.1.0" +edition.workspace = true +publish.workspace = true +license = "GPL-3.0-or-later" + +[lints] +workspace = true + +[lib] +path = "src/python.rs" + +[features] +default = [] + +[dependencies] +db.workspace = true +editor.workspace = true +gpui.workspace = true +project.workspace = true +ui.workspace = true +workspace.workspace = true +workspace-hack.workspace = true + +# Uncomment other workspace dependencies as needed +# assistant.workspace = true +# client.workspace = true +# project.workspace = true +# settings.workspace = true diff --git a/crates/language_onboarding/LICENSE-GPL b/crates/language_onboarding/LICENSE-GPL new file mode 120000 index 0000000000000000000000000000000000000000..89e542f750cd3860a0598eff0dc34b56d7336dc4 --- /dev/null +++ b/crates/language_onboarding/LICENSE-GPL @@ -0,0 +1 @@ +../../LICENSE-GPL \ No newline at end of file diff --git a/crates/language_onboarding/src/python.rs b/crates/language_onboarding/src/python.rs new file mode 100644 index 0000000000000000000000000000000000000000..405a265db95c347547ab02215dfc74f226ca9555 --- /dev/null +++ b/crates/language_onboarding/src/python.rs @@ -0,0 +1,95 @@ +use db::kvp::Dismissable; +use editor::Editor; +use gpui::{Context, EventEmitter, Subscription}; +use ui::{ + Banner, Button, Clickable, Color, FluentBuilder as _, IconButton, IconName, + InteractiveElement as _, IntoElement, Label, LabelCommon, LabelSize, ParentElement as _, + Render, Styled as _, Window, div, h_flex, v_flex, +}; +use workspace::{ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace}; + +pub struct BasedPyrightBanner { + dismissed: bool, + have_basedpyright: bool, + _subscriptions: [Subscription; 1], +} + +impl Dismissable for BasedPyrightBanner { + const KEY: &str = "basedpyright-banner"; +} + +impl BasedPyrightBanner { + pub fn new(workspace: &Workspace, cx: &mut Context) -> Self { + let subscription = cx.subscribe(workspace.project(), |this, _, event, _| { + if let project::Event::LanguageServerAdded(_, name, _) = event + && name == "basedpyright" + { + this.have_basedpyright = true; + } + }); + let dismissed = Self::dismissed(); + Self { + dismissed, + have_basedpyright: false, + _subscriptions: [subscription], + } + } +} + +impl EventEmitter for BasedPyrightBanner {} + +impl Render for BasedPyrightBanner { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + div() + .id("basedpyright-banner") + .when(!self.dismissed && self.have_basedpyright, |el| { + el.child( + Banner::new() + .severity(ui::Severity::Info) + .child( + h_flex() + .gap_2() + .child(v_flex() + .child("Basedpyright is now the only default language server for Python") + .child(Label::new("We have disabled PyRight and pylsp by default. They can be re-enabled in your settings.").size(LabelSize::XSmall).color(Color::Muted)) + ) + .child( + Button::new("learn-more", "Learn More") + .icon(IconName::ArrowUpRight) + .on_click(|_, _, cx| { + cx.open_url("https://zed.dev/docs/languages/python") + }), + ), + ) + .action_slot(IconButton::new("dismiss", IconName::Close).on_click( + cx.listener(|this, _, _, cx| { + this.dismissed = true; + Self::set_dismissed(true, cx); + cx.notify(); + }), + )) + .into_any_element(), + ) + }) + } +} + +impl ToolbarItemView for BasedPyrightBanner { + fn set_active_pane_item( + &mut self, + active_pane_item: Option<&dyn workspace::ItemHandle>, + _window: &mut ui::Window, + cx: &mut Context, + ) -> ToolbarItemLocation { + if let Some(item) = active_pane_item + && let Some(editor) = item.act_as::(cx) + && let Some(path) = editor.update(cx, |editor, cx| editor.target_file_abs_path(cx)) + && let Some(file_name) = path.file_name() + && file_name.as_encoded_bytes().ends_with(".py".as_bytes()) + { + return ToolbarItemLocation::Secondary; + } + + ToolbarItemLocation::Hidden + } +} diff --git a/crates/languages/Cargo.toml b/crates/languages/Cargo.toml index 8e258180702626bb3dd32b28bfb0e82722a1f12f..e09b27d4742d4660cffb3d86905fb67268e617fa 100644 --- a/crates/languages/Cargo.toml +++ b/crates/languages/Cargo.toml @@ -42,7 +42,6 @@ async-trait.workspace = true chrono.workspace = true collections.workspace = true dap.workspace = true -feature_flags.workspace = true futures.workspace = true gpui.workspace = true http_client.workspace = true diff --git a/crates/languages/src/lib.rs b/crates/languages/src/lib.rs index 33fb2af0612a203b45276bb8e7f580c5a86a90b6..95fe1312183a3412509375050b1e1ff67642ef3e 100644 --- a/crates/languages/src/lib.rs +++ b/crates/languages/src/lib.rs @@ -1,5 +1,4 @@ use anyhow::Context as _; -use feature_flags::{FeatureFlag, FeatureFlagAppExt as _}; use gpui::{App, SharedString, UpdateGlobal}; use node_runtime::NodeRuntime; use python::PyprojectTomlManifestProvider; @@ -54,12 +53,6 @@ pub static LANGUAGE_GIT_COMMIT: std::sync::LazyLock> = )) }); -struct BasedPyrightFeatureFlag; - -impl FeatureFlag for BasedPyrightFeatureFlag { - const NAME: &'static str = "basedpyright"; -} - pub fn init(languages: Arc, node: NodeRuntime, cx: &mut App) { #[cfg(feature = "load-grammars")] languages.register_native_grammars([ @@ -174,7 +167,7 @@ pub fn init(languages: Arc, node: NodeRuntime, cx: &mut App) { }, LanguageInfo { name: "python", - adapters: vec![python_lsp_adapter, py_lsp_adapter], + adapters: vec![basedpyright_lsp_adapter], context: Some(python_context_provider), toolchain: Some(python_toolchain_provider), manifest_name: Some(SharedString::new_static("pyproject.toml").into()), @@ -240,17 +233,6 @@ pub fn init(languages: Arc, node: NodeRuntime, cx: &mut App) { ); } - let mut basedpyright_lsp_adapter = Some(basedpyright_lsp_adapter); - cx.observe_flag::({ - let languages = languages.clone(); - move |enabled, _| { - if enabled && let Some(adapter) = basedpyright_lsp_adapter.take() { - languages.register_available_lsp_adapter(adapter.name(), move || adapter.clone()); - } - } - }) - .detach(); - // Register globally available language servers. // // This will allow users to add support for a built-in language server (e.g., Tailwind) @@ -267,27 +249,19 @@ pub fn init(languages: Arc, node: NodeRuntime, cx: &mut App) { // ``` languages.register_available_lsp_adapter( LanguageServerName("tailwindcss-language-server".into()), - { - let adapter = tailwind_adapter.clone(); - move || adapter.clone() - }, + tailwind_adapter.clone(), ); - languages.register_available_lsp_adapter(LanguageServerName("eslint".into()), { - let adapter = eslint_adapter.clone(); - move || adapter.clone() - }); - languages.register_available_lsp_adapter(LanguageServerName("vtsls".into()), { - let adapter = vtsls_adapter; - move || adapter.clone() - }); + languages.register_available_lsp_adapter( + LanguageServerName("eslint".into()), + eslint_adapter.clone(), + ); + languages.register_available_lsp_adapter(LanguageServerName("vtsls".into()), vtsls_adapter); languages.register_available_lsp_adapter( LanguageServerName("typescript-language-server".into()), - { - let adapter = typescript_lsp_adapter; - move || adapter.clone() - }, + typescript_lsp_adapter, ); - + languages.register_available_lsp_adapter(python_lsp_adapter.name(), python_lsp_adapter); + languages.register_available_lsp_adapter(py_lsp_adapter.name(), py_lsp_adapter); // Register Tailwind for the existing languages that should have it by default. // // This can be driven by the `language_servers` setting once we have a way for diff --git a/crates/languages/src/python.rs b/crates/languages/src/python.rs index d1f40a8233a3590b382bc1e0edbe5dd69b3317d8..978f22d91c26c604ba670d712589199a64275950 100644 --- a/crates/languages/src/python.rs +++ b/crates/languages/src/python.rs @@ -35,7 +35,7 @@ use std::{ sync::Arc, }; use task::{ShellKind, TaskTemplate, TaskTemplates, VariableName}; -use util::ResultExt; +use util::{ResultExt, maybe}; pub(crate) struct PyprojectTomlManifestProvider; @@ -1619,23 +1619,37 @@ impl LspAdapter for BasedPyrightLspAdapter { } } - // Always set the python interpreter path - // Get or create the python section - let python = object + // Set both pythonPath and defaultInterpreterPath for compatibility + if let Some(python) = object .entry("python") .or_insert(Value::Object(serde_json::Map::default())) .as_object_mut() - .unwrap(); - - // Set both pythonPath and defaultInterpreterPath for compatibility - python.insert( - "pythonPath".to_owned(), - Value::String(interpreter_path.clone()), - ); - python.insert( - "defaultInterpreterPath".to_owned(), - Value::String(interpreter_path), - ); + { + python.insert( + "pythonPath".to_owned(), + Value::String(interpreter_path.clone()), + ); + python.insert( + "defaultInterpreterPath".to_owned(), + Value::String(interpreter_path), + ); + } + // Basedpyright by default uses `strict` type checking, we tone it down as to not surpris users + maybe!({ + let basedpyright = object + .entry("basedpyright") + .or_insert(Value::Object(serde_json::Map::default())); + let analysis = basedpyright + .as_object_mut()? + .entry("analysis") + .or_insert(Value::Object(serde_json::Map::default())); + if let serde_json::map::Entry::Vacant(v) = + analysis.as_object_mut()?.entry("typeCheckingMode") + { + v.insert(Value::String("standard".to_owned())); + } + Some(()) + }); } user_settings diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 943bdab5ff817da7819590679d19bbe522b47835..7af51ef6fff8bddefac993fb5eb40e10d054977c 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -166,6 +166,12 @@ impl<'a> From<&'a str> for LanguageServerName { } } +impl PartialEq for LanguageServerName { + fn eq(&self, other: &str) -> bool { + self.0 == other + } +} + /// Handle to a language server RPC activity subscription. pub enum Subscription { Notification { diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 6f1934456d747de7c27e0a0903fabbb083549fba..4341a77fc771f5d887847a3f091ce73c08464112 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -89,6 +89,7 @@ language.workspace = true language_extension.workspace = true language_model.workspace = true language_models.workspace = true +language_onboarding.workspace = true language_selector.workspace = true language_tools.workspace = true languages = { workspace = true, features = ["load-grammars"] } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index efe2d031a6f995f7cad36f383a4be5c2ee773d56..355925d2289daac0c6346b6b92d45942c4724357 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -32,6 +32,7 @@ use gpui::{ }; use image_viewer::ImageInfo; use language::Capability; +use language_onboarding::BasedPyrightBanner; use language_tools::lsp_button::{self, LspButton}; use language_tools::lsp_log_view::LspLogToolbarItemView; use migrate::{MigrationBanner, MigrationEvent, MigrationNotification, MigrationType}; @@ -1001,6 +1002,8 @@ fn initialize_pane( toolbar.add_item(project_diff_toolbar, window, cx); let agent_diff_toolbar = cx.new(AgentDiffToolbar::new); toolbar.add_item(agent_diff_toolbar, window, cx); + let basedpyright_banner = cx.new(|cx| BasedPyrightBanner::new(workspace, cx)); + toolbar.add_item(basedpyright_banner, window, cx); }) }); }