From 9b610e20bfcef9869103dc26d40f443bf81eb647 Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Wed, 1 Apr 2026 21:37:28 +0530 Subject: [PATCH] languages: Update vscode-eslint to 3.0.24 and fix ESLint 8-10 breaking cases (#52886) Closes #29757 Closes #49387 This PR upgrades ESLint language server from `vscode-eslint 2.4.4` to upstream `microsoft/vscode-eslint 3.0.24`, and make the workspace configuration version-aware so ESLint 8, 9, and 10 take the correct config-mode path. The key part is that the 3.x bump alone is not enough. This PR keeps Zed out of that path except where it is still actually needed. Rest heavy-lifting is done by eslint server itself. Zed now only overrides ESLint settings in the two known broken cases: - ESLint 8.21-8.56 flat config: send `experimental.useFlatConfig = true` - ESLint 9 legacy config: send `useFlatConfig = false` All other cases defer to `vscode-eslint 3.x`'s own config and working-directory discovery. For testing, I created https://github.com/smitbarmase/eslint-repros, which contains a versioned ESLint repros covering root flat config, legacy config, and package-local monorepo cases across ESLint 8, 9, and 10. Here is compare between `zed/main`, a pure `vscode-eslint 3.x` upgrade and this branch with the config-mode fixes: ## zed main ```text eslint-v8_21-flat-root-single eslint-v8_56-flat-package-local-monorepo -> breaks on main eslint-v8_56-flat-root-single eslint-v8_56-legacy-root-single eslint-v8_57-flat-package-local-monorepo -> breaks on main eslint-v8_57-flat-root-single eslint-v8_57-legacy-root-single eslint-v9_0-flat-package-local-monorepo eslint-v9_0-flat-root-single eslint-v9_0-legacy-root-single -> breaks on main eslint-v10_0-flat-package-local-monorepo eslint-v10_0-flat-root-single -> breaks on main ``` ## vscode-eslint 3.x upgrade ```text eslint-v8_21-flat-root-single eslint-v8_56-flat-package-local-monorepo -> breaks on 3.x upgrade eslint-v8_56-flat-root-single eslint-v8_56-legacy-root-single eslint-v8_57-flat-package-local-monorepo eslint-v8_57-flat-root-single eslint-v8_57-legacy-root-single eslint-v9_0-flat-package-local-monorepo eslint-v9_0-flat-root-single eslint-v9_0-legacy-root-single -> breaks on 3.x upgrade eslint-v10_0-flat-package-local-monorepo eslint-v10_0-flat-root-single -> breaks on 3.x upgrade ``` ## vscode-eslint 3.x upgrade + use flat config fixes ```text eslint-v8_21-flat-root-single eslint-v8_56-flat-package-local-monorepo eslint-v8_56-flat-root-single eslint-v8_56-legacy-root-single eslint-v8_57-flat-package-local-monorepo eslint-v8_57-flat-root-single eslint-v8_57-legacy-root-single eslint-v9_0-flat-package-local-monorepo eslint-v9_0-flat-root-single eslint-v9_0-legacy-root-single eslint-v10_0-flat-package-local-monorepo eslint-v10_0-flat-root-single ``` Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed ESLint not reporting diagnostics in some cases for projects that use flat-config, legacy-config, and monorepo projects across ESLint 8, 9, and 10. --- crates/languages/src/eslint.rs | 420 +++++++++++++++++++++++++++++++-- crates/languages/src/lib.rs | 2 +- 2 files changed, 395 insertions(+), 27 deletions(-) diff --git a/crates/languages/src/eslint.rs b/crates/languages/src/eslint.rs index bf51636f60bb4e0eec6eebcd3efaab2996352c18..7ef55c64ef1b35fa42f35e779c4cf46b30a18ee5 100644 --- a/crates/languages/src/eslint.rs +++ b/crates/languages/src/eslint.rs @@ -7,8 +7,10 @@ use http_client::{ }; use language::{LspAdapter, LspAdapterDelegate, LspInstaller, Toolchain}; use lsp::{CodeActionKind, LanguageServerBinary, LanguageServerName, Uri}; -use node_runtime::NodeRuntime; +use node_runtime::{NodeRuntime, read_package_installed_version}; +use project::Fs; use project::lsp_store::language_server_settings_for; +use semver::Version; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; use settings::SettingsLocation; @@ -31,11 +33,12 @@ fn eslint_server_binary_arguments(server_path: &Path) -> Vec { pub struct EsLintLspAdapter { node: NodeRuntime, + fs: Arc, } impl EsLintLspAdapter { - const CURRENT_VERSION: &'static str = "2.4.4"; - const CURRENT_VERSION_TAG_NAME: &'static str = "release/2.4.4"; + const CURRENT_VERSION: &'static str = "3.0.24"; + const CURRENT_VERSION_TAG_NAME: &'static str = "release/3.0.24"; #[cfg(not(windows))] const GITHUB_ASSET_KIND: AssetKind = AssetKind::TarGz; @@ -45,7 +48,10 @@ impl EsLintLspAdapter { const SERVER_PATH: &'static str = "vscode-eslint/server/out/eslintServer.js"; const SERVER_NAME: LanguageServerName = LanguageServerName::new_static("eslint"); - const FLAT_CONFIG_FILE_NAMES: &'static [&'static str] = &[ + const FLAT_CONFIG_FILE_NAMES_V8_21: &'static [&'static str] = &["eslint.config.js"]; + const FLAT_CONFIG_FILE_NAMES_V8_57: &'static [&'static str] = + &["eslint.config.js", "eslint.config.mjs", "eslint.config.cjs"]; + const FLAT_CONFIG_FILE_NAMES_V10: &'static [&'static str] = &[ "eslint.config.js", "eslint.config.mjs", "eslint.config.cjs", @@ -53,9 +59,17 @@ impl EsLintLspAdapter { "eslint.config.cts", "eslint.config.mts", ]; + const LEGACY_CONFIG_FILE_NAMES: &'static [&'static str] = &[ + ".eslintrc", + ".eslintrc.js", + ".eslintrc.cjs", + ".eslintrc.yaml", + ".eslintrc.yml", + ".eslintrc.json", + ]; - pub fn new(node: NodeRuntime) -> Self { - EsLintLspAdapter { node } + pub fn new(node: NodeRuntime, fs: Arc) -> Self { + EsLintLspAdapter { node, fs } } fn build_destination_path(container_dir: &Path) -> PathBuf { @@ -73,7 +87,7 @@ impl LspInstaller for EsLintLspAdapter { _: &mut AsyncApp, ) -> Result { let url = build_asset_url( - "zed-industries/vscode-eslint", + "microsoft/vscode-eslint", Self::CURRENT_VERSION_TAG_NAME, Self::GITHUB_ASSET_KIND, )?; @@ -157,6 +171,42 @@ impl LspInstaller for EsLintLspAdapter { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum EslintConfigKind { + Flat, + Legacy, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq)] +struct EslintSettingsOverrides { + use_flat_config: Option, + experimental_use_flat_config: Option, +} + +impl EslintSettingsOverrides { + fn apply_to(self, workspace_configuration: &mut Value) { + if let Some(use_flat_config) = self.use_flat_config + && let Some(workspace_configuration) = workspace_configuration.as_object_mut() + { + workspace_configuration.insert("useFlatConfig".to_string(), json!(use_flat_config)); + } + + if let Some(experimental_use_flat_config) = self.experimental_use_flat_config + && let Some(workspace_configuration) = workspace_configuration.as_object_mut() + { + let experimental = workspace_configuration + .entry("experimental") + .or_insert_with(|| json!({})); + if let Some(experimental) = experimental.as_object_mut() { + experimental.insert( + "useFlatConfig".to_string(), + json!(experimental_use_flat_config), + ); + } + } + } +} + #[async_trait(?Send)] impl LspAdapter for EsLintLspAdapter { fn code_action_kinds(&self) -> Option> { @@ -174,9 +224,26 @@ impl LspAdapter for EsLintLspAdapter { cx: &mut AsyncApp, ) -> Result { let worktree_root = delegate.worktree_root_path(); - let use_flat_config = Self::FLAT_CONFIG_FILE_NAMES - .iter() - .any(|file| worktree_root.join(file).is_file()); + let requested_file_path = requested_uri + .as_ref() + .filter(|uri| uri.scheme() == "file") + .and_then(|uri| uri.to_file_path().ok()) + .filter(|path| path.starts_with(worktree_root)); + let eslint_version = find_eslint_version( + delegate.as_ref(), + worktree_root, + requested_file_path.as_deref(), + ) + .await?; + let config_kind = find_eslint_config_kind( + worktree_root, + requested_file_path.as_deref(), + eslint_version.as_ref(), + self.fs.as_ref(), + ) + .await; + let eslint_settings_overrides = + eslint_settings_overrides_for(eslint_version.as_ref(), config_kind); let mut default_workspace_configuration = json!({ "validate": "on", @@ -206,26 +273,13 @@ impl LspAdapter for EsLintLspAdapter { "showDocumentation": { "enable": true } - }, - "experimental": { - "useFlatConfig": use_flat_config, } }); + eslint_settings_overrides.apply_to(&mut default_workspace_configuration); - let file_path = requested_uri + let file_path = requested_file_path .as_ref() - .and_then(|uri| { - (uri.scheme() == "file") - .then(|| uri.to_file_path().ok()) - .flatten() - }) - .and_then(|abs_path| { - abs_path - .strip_prefix(&worktree_root) - .ok() - .map(ToOwned::to_owned) - }); - let file_path = file_path + .and_then(|abs_path| abs_path.strip_prefix(worktree_root).ok()) .and_then(|p| RelPath::unix(&p).ok().map(ToOwned::to_owned)) .unwrap_or_else(|| RelPath::empty().to_owned()); let override_options = cx.update(|cx| { @@ -272,6 +326,109 @@ impl LspAdapter for EsLintLspAdapter { } } +fn ancestor_directories<'a>( + worktree_root: &'a Path, + requested_file: Option<&'a Path>, +) -> impl Iterator + 'a { + let start = requested_file + .filter(|file| file.starts_with(worktree_root)) + .and_then(Path::parent) + .unwrap_or(worktree_root); + + start + .ancestors() + .take_while(move |dir| dir.starts_with(worktree_root)) +} + +fn flat_config_file_names(version: Option<&Version>) -> &'static [&'static str] { + match version { + Some(version) if version.major >= 10 => EsLintLspAdapter::FLAT_CONFIG_FILE_NAMES_V10, + Some(version) if version.major == 9 => EsLintLspAdapter::FLAT_CONFIG_FILE_NAMES_V8_57, + Some(version) if version.major == 8 && version.minor >= 57 => { + EsLintLspAdapter::FLAT_CONFIG_FILE_NAMES_V8_57 + } + Some(version) if version.major == 8 && version.minor >= 21 => { + EsLintLspAdapter::FLAT_CONFIG_FILE_NAMES_V8_21 + } + _ => &[], + } +} + +async fn find_eslint_config_kind( + worktree_root: &Path, + requested_file: Option<&Path>, + version: Option<&Version>, + fs: &dyn Fs, +) -> Option { + let flat_config_file_names = flat_config_file_names(version); + + for directory in ancestor_directories(worktree_root, requested_file) { + for file_name in flat_config_file_names { + if fs.is_file(&directory.join(file_name)).await { + return Some(EslintConfigKind::Flat); + } + } + + for file_name in EsLintLspAdapter::LEGACY_CONFIG_FILE_NAMES { + if fs.is_file(&directory.join(file_name)).await { + return Some(EslintConfigKind::Legacy); + } + } + } + + None +} + +fn eslint_settings_overrides_for( + version: Option<&Version>, + config_kind: Option, +) -> EslintSettingsOverrides { + // vscode-eslint 3.x already discovers config files and chooses a working + // directory from the active file on its own. Zed only overrides settings + // for the two cases where leaving everything unset is known to be wrong: + // + // - ESLint 8.21-8.56 flat config still needs experimental.useFlatConfig. + // - ESLint 9.x legacy config needs useFlatConfig = false. + // + // All other cases should defer to the server's own defaults and discovery. + let Some(version) = version else { + return EslintSettingsOverrides::default(); + }; + + match config_kind { + Some(EslintConfigKind::Flat) if version.major == 8 && (21..57).contains(&version.minor) => { + EslintSettingsOverrides { + use_flat_config: None, + experimental_use_flat_config: Some(true), + } + } + Some(EslintConfigKind::Legacy) if version.major == 9 => EslintSettingsOverrides { + use_flat_config: Some(false), + experimental_use_flat_config: None, + }, + _ => EslintSettingsOverrides::default(), + } +} + +async fn find_eslint_version( + delegate: &dyn LspAdapterDelegate, + worktree_root: &Path, + requested_file: Option<&Path>, +) -> Result> { + for directory in ancestor_directories(worktree_root, requested_file) { + if let Some(version) = + read_package_installed_version(directory.join("node_modules"), "eslint").await? + { + return Ok(Some(version)); + } + } + + Ok(delegate + .npm_package_installed_version("eslint") + .await? + .map(|(_, version)| version)) +} + /// On Windows, converts Unix-style separators (/) to Windows-style (\). /// On Unix, returns the path unchanged fn normalize_path_separators(path: &str) -> String { @@ -624,6 +781,217 @@ mod tests { } } + mod eslint_settings { + use super::*; + use ::fs::FakeFs; + use gpui::TestAppContext; + + #[test] + fn test_ancestor_directories_for_package_local_file() { + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform( + "/workspace/packages/web/src/index.js", + )); + + let directories: Vec<&Path> = + ancestor_directories(&worktree_root, Some(&requested_file)).collect(); + + assert_eq!( + directories, + vec![ + Path::new(&unix_path_to_platform("/workspace/packages/web/src")), + Path::new(&unix_path_to_platform("/workspace/packages/web")), + Path::new(&unix_path_to_platform("/workspace/packages")), + Path::new(&unix_path_to_platform("/workspace")), + ] + ); + } + + #[test] + fn test_eslint_8_flat_root_repo_uses_experimental_flag() { + let version = Version::parse("8.56.0").expect("valid ESLint version"); + let settings = + eslint_settings_overrides_for(Some(&version), Some(EslintConfigKind::Flat)); + + assert_eq!( + settings, + EslintSettingsOverrides { + use_flat_config: None, + experimental_use_flat_config: Some(true), + } + ); + } + + #[test] + fn test_eslint_8_57_flat_repo_uses_no_override() { + let version = Version::parse("8.57.0").expect("valid ESLint version"); + let settings = + eslint_settings_overrides_for(Some(&version), Some(EslintConfigKind::Flat)); + + assert_eq!(settings, EslintSettingsOverrides::default()); + } + + #[test] + fn test_eslint_9_legacy_repo_uses_use_flat_config_false() { + let version = Version::parse("9.0.0").expect("valid ESLint version"); + let settings = + eslint_settings_overrides_for(Some(&version), Some(EslintConfigKind::Legacy)); + + assert_eq!( + settings, + EslintSettingsOverrides { + use_flat_config: Some(false), + experimental_use_flat_config: None, + } + ); + } + + #[test] + fn test_eslint_10_repo_uses_no_override() { + let version = Version::parse("10.0.0").expect("valid ESLint version"); + let settings = + eslint_settings_overrides_for(Some(&version), Some(EslintConfigKind::Flat)); + + assert_eq!(settings, EslintSettingsOverrides::default()); + } + + #[gpui::test] + async fn test_eslint_8_56_does_not_treat_cjs_as_flat_config(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + unix_path_to_platform("/workspace"), + json!({ "eslint.config.cjs": "" }), + ) + .await; + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform("/workspace/src/index.js")); + let version = Version::parse("8.56.0").expect("valid ESLint version"); + + let config_kind = find_eslint_config_kind( + &worktree_root, + Some(&requested_file), + Some(&version), + fs.as_ref(), + ) + .await; + + assert_eq!(config_kind, None); + } + + #[gpui::test] + async fn test_eslint_8_57_treats_cjs_as_flat_config(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + unix_path_to_platform("/workspace"), + json!({ "eslint.config.cjs": "" }), + ) + .await; + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform("/workspace/src/index.js")); + let version = Version::parse("8.57.0").expect("valid ESLint version"); + + let config_kind = find_eslint_config_kind( + &worktree_root, + Some(&requested_file), + Some(&version), + fs.as_ref(), + ) + .await; + + assert_eq!(config_kind, Some(EslintConfigKind::Flat)); + } + + #[gpui::test] + async fn test_eslint_10_treats_typescript_config_as_flat_config(cx: &mut TestAppContext) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + unix_path_to_platform("/workspace"), + json!({ "eslint.config.ts": "" }), + ) + .await; + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform("/workspace/src/index.js")); + let version = Version::parse("10.0.0").expect("valid ESLint version"); + + let config_kind = find_eslint_config_kind( + &worktree_root, + Some(&requested_file), + Some(&version), + fs.as_ref(), + ) + .await; + + assert_eq!(config_kind, Some(EslintConfigKind::Flat)); + } + + #[gpui::test] + async fn test_package_local_flat_config_is_preferred_for_monorepo_file( + cx: &mut TestAppContext, + ) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + unix_path_to_platform("/workspace"), + json!({ + "eslint.config.js": "", + "packages": { + "web": { + "eslint.config.js": "" + } + } + }), + ) + .await; + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform( + "/workspace/packages/web/src/index.js", + )); + let version = Version::parse("8.56.0").expect("valid ESLint version"); + + let config_kind = find_eslint_config_kind( + &worktree_root, + Some(&requested_file), + Some(&version), + fs.as_ref(), + ) + .await; + + assert_eq!(config_kind, Some(EslintConfigKind::Flat)); + } + + #[gpui::test] + async fn test_package_local_legacy_config_is_detected_for_eslint_9( + cx: &mut TestAppContext, + ) { + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + unix_path_to_platform("/workspace"), + json!({ + "packages": { + "web": { + ".eslintrc.cjs": "" + } + } + }), + ) + .await; + let worktree_root = PathBuf::from(unix_path_to_platform("/workspace")); + let requested_file = PathBuf::from(unix_path_to_platform( + "/workspace/packages/web/src/index.js", + )); + let version = Version::parse("9.0.0").expect("valid ESLint version"); + + let config_kind = find_eslint_config_kind( + &worktree_root, + Some(&requested_file), + Some(&version), + fs.as_ref(), + ) + .await; + + assert_eq!(config_kind, Some(EslintConfigKind::Legacy)); + } + } + #[cfg(windows)] mod windows_style_paths { use super::*; diff --git a/crates/languages/src/lib.rs b/crates/languages/src/lib.rs index 9a0524dffd238b566931a4a612edd91b1e6361c3..9010bbde022e765b53ccceec042a075f85fc102b 100644 --- a/crates/languages/src/lib.rs +++ b/crates/languages/src/lib.rs @@ -59,7 +59,7 @@ pub fn init(languages: Arc, fs: Arc, node: NodeRuntime let c_lsp_adapter = Arc::new(c::CLspAdapter); let css_lsp_adapter = Arc::new(css::CssLspAdapter::new(node.clone())); - let eslint_adapter = Arc::new(eslint::EsLintLspAdapter::new(node.clone())); + let eslint_adapter = Arc::new(eslint::EsLintLspAdapter::new(node.clone(), fs.clone())); let go_context_provider = Arc::new(go::GoContextProvider); let go_lsp_adapter = Arc::new(go::GoLspAdapter); let json_context_provider = Arc::new(JsonTaskProvider);