From 11fd64f399227da655daa12e646c6a80a9352bad Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 5 Nov 2026 08:50:59 +0800 Subject: [PATCH] feat(java): add the "Serializable class without 'serialVersionUID'" inspection (#1401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings › Editor › Inspections › Java gets the inspection, off by default. When on, jdtls's compiler reports the missing field and offers the add default/generated ID quick fixes. Built-in inspections can now be off by default, the backend resolves the profile in effect, the compiler-defaults file is named after the variant so jdtls restarts with it, and saving inspection settings reconfigures language servers. Fixes #1199 --- crates/ide-core/src/inspection_settings.rs | 33 ++++++++ crates/ide-host/src/ops/project.rs | 7 +- crates/ide-host/src/project.rs | 1 + crates/ide-sdk/src/apply.rs | 79 +++++++++++++++++-- crates/ide-sdk/src/java.rs | 1 - .../settings/__tests__/inspections.test.ts | 16 +++- .../settings/pages/editor/inspectionsModel.ts | 10 ++- src/platform/inspections.ts | 22 +++++- src/platform/locales/text/zh.json | 2 + 9 files changed, 156 insertions(+), 15 deletions(-) diff --git a/crates/ide-core/src/inspection_settings.rs b/crates/ide-core/src/inspection_settings.rs index f4522a2e..41dfb8f2 100644 --- a/crates/ide-core/src/inspection_settings.rs +++ b/crates/ide-core/src/inspection_settings.rs @@ -36,6 +36,23 @@ impl Default for InspectionSettings { } } +impl InspectionSettings { + /// Overrides of the profile in effect; `Default`'s when `profile` names none that exists. + pub fn active(&self) -> &BTreeMap { + let custom = |level: &str, key: &str| match level { + "ide" => self.ide_profiles.get(key), + "project" => self.project_profiles.get(key), + _ => None, + }; + match self.profile.as_str() { + "project" => &self.project, + id => { + id.split_once(':').and_then(|(level, key)| custom(level, key)).map_or(&self.default, |p| &p.overrides) + } + } + } +} + /// A named profile made with Copy. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize, Type)] #[serde(default, rename_all = "camelCase")] @@ -111,4 +128,20 @@ mod tests { let back: InspectionSettings = serde_json::from_str(&serde_json::to_string(&s).unwrap()).unwrap(); assert_eq!(back, s); } + + #[test] + fn active_profile_falls_back_to_default() { + let off = |id: &str| { + BTreeMap::from([(id.to_owned(), InspectionOverride { enabled: Some(false), ..Default::default() })]) + }; + let mut s = InspectionSettings { default: off("a"), project: off("b"), ..Default::default() }; + s.ide_profiles.insert("strict".into(), CustomInspectionProfile { name: "Strict".into(), overrides: off("c") }); + assert!(s.active().contains_key("a")); + s.profile = "project".into(); + assert!(s.active().contains_key("b")); + s.profile = "ide:strict".into(); + assert!(s.active().contains_key("c")); + s.profile = "project:gone".into(); + assert!(s.active().contains_key("a")); + } } diff --git a/crates/ide-host/src/ops/project.rs b/crates/ide-host/src/ops/project.rs index 9895b0aa..459a96d5 100644 --- a/crates/ide-host/src/ops/project.rs +++ b/crates/ide-host/src/ops/project.rs @@ -175,7 +175,12 @@ pub fn set_settings( host.apply_proxy(); } // A new interpreter / toolchain / server executable restarts the language servers it affects. - let code_style = key == "editor" || key == "editor.codeStyle" || key.starts_with("editor.codeStyle."); + // So do inspections that are compiler options (`ide-sdk` `configure_java_compiler`). + let code_style = key == "editor" + || key == "editor.codeStyle" + || key.starts_with("editor.codeStyle.") + || key == "inspections" + || key.starts_with("inspections."); let toolchain = ["languages", "tools", "languageServers", "projectStructure"] .iter() .any(|k| key == *k || key.starts_with(&format!("{k}."))); diff --git a/crates/ide-host/src/project.rs b/crates/ide-host/src/project.rs index 093326ab..1213db96 100644 --- a/crates/ide-host/src/project.rs +++ b/crates/ide-host/src/project.rs @@ -209,6 +209,7 @@ impl Host { cfg.launch_error = Some(e); None }); + ide_sdk::apply::configure_java_compiler(cfg, &data_dir, &effective.inspections); ide_sdk::java::configure_jdtls( cfg, server_root, diff --git a/crates/ide-sdk/src/apply.rs b/crates/ide-sdk/src/apply.rs index fb77ab2e..bfa33139 100644 --- a/crates/ide-sdk/src/apply.rs +++ b/crates/ide-sdk/src/apply.rs @@ -5,7 +5,7 @@ use crate::discover::{project_venv_python, workspace_tsdk}; use crate::exec::{path_with, search_path}; use crate::{angular, godot, pinned, vue}; use crate::{bin_dir, exe, venv_of}; -use ide_core::settings::{LanguageServerSettings, Settings}; +use ide_core::settings::{InspectionLevel, InspectionSettings, LanguageServerSettings, Settings}; use ide_core::version_managers::Tool; use ide_language::{LanguageConfig, LanguageServerConfig}; use ide_run::RunConfig; @@ -442,15 +442,41 @@ pub(crate) fn merge_init_options(cfg: &mut LanguageServerConfig, layer: Value) { } } +/// Editor › Inspections › Java: "Serializable class without 'serialVersionUID'" (off by default, +/// as in IntelliJ IDEA). ECJ reports it, so it is a compiler option rather than a filter. +pub const SERIAL_VERSION_INSPECTION: &str = "java:missing-serial-version"; + +/// ECJ's level for a missing `serialVersionUID`, from the inspection profile in effect. +fn missing_serial_version(inspections: &InspectionSettings) -> Option<&'static str> { + let o = inspections.active().get(SERIAL_VERSION_INSPECTION)?; + if o.enabled != Some(true) { + return None; + } + Some(match o.severity { + Some(InspectionLevel::Error) => "error", + None | Some(InspectionLevel::Warning) => "warning", + Some(_) => "info", + }) +} + /// Workspace compiler defaults; Eclipse project preferences still take precedence. -pub(crate) fn configure_java_compiler(cfg: &mut LanguageServerConfig, data_dir: &Path) { - let file = data_dir.join("lsp/jdtls/compiler-defaults.prefs"); +/// Each variant has its own file, so changing an inspection restarts the server with it. +pub fn configure_java_compiler(cfg: &mut LanguageServerConfig, data_dir: &Path, inspections: &InspectionSettings) { const DEFAULTS: &str = "org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning\n\ org.eclipse.jdt.core.compiler.problem.discouragedReference=warning\n"; + let serial = missing_serial_version(inspections); + let (name, content) = match serial { + None => ("compiler-defaults.prefs".to_owned(), DEFAULTS.to_owned()), + Some(level) => ( + format!("compiler-defaults-serial-{level}.prefs"), + format!("{DEFAULTS}org.eclipse.jdt.core.compiler.problem.missingSerialVersion={level}\n"), + ), + }; + let file = data_dir.join("lsp/jdtls").join(name); let write = || -> ide_core::IdeResult<()> { std::fs::create_dir_all(file.parent().expect("compiler preferences directory"))?; - if std::fs::read_to_string(&file).ok().as_deref() != Some(DEFAULTS) { - ide_vfs::write_atomic(&file, DEFAULTS.as_bytes())?; + if std::fs::read_to_string(&file).ok().as_deref() != Some(content.as_str()) { + ide_vfs::write_atomic(&file, content.as_bytes())?; } Ok(()) }; @@ -1095,13 +1121,50 @@ mod java_compiler_tests { serde_json::to_value(cfg.initialization_options.as_ref().unwrap()).unwrap() } + #[test] + fn serial_version_inspection_sets_the_compiler_option() { + let tmp = tempfile::tempdir().unwrap(); + let url = |inspections: &InspectionSettings| { + let mut cfg = custom_server("jdtls", &LanguageServerSettings::default()); + configure_java_compiler(&mut cfg, tmp.path(), inspections); + options(&cfg).pointer("/settings/java/settings/url").unwrap().as_str().unwrap().to_owned() + }; + let set = |o: ide_core::settings::InspectionOverride| { + let mut s = InspectionSettings::default(); + s.default.insert(SERIAL_VERSION_INSPECTION.into(), o); + s + }; + let off = url(&InspectionSettings::default()); + assert!(!std::fs::read_to_string(&off).unwrap().contains("missingSerialVersion")); + let on = url(&set(ide_core::settings::InspectionOverride { enabled: Some(true), ..Default::default() })); + assert_ne!(on, off, "a new file, so the server restarts with it"); + assert!(std::fs::read_to_string(&on).unwrap().ends_with("compiler.problem.missingSerialVersion=warning\n")); + let error = url(&set(ide_core::settings::InspectionOverride { + enabled: Some(true), + severity: Some(InspectionLevel::Error), + ..Default::default() + })); + assert!(std::fs::read_to_string(error).unwrap().ends_with("missingSerialVersion=error\n")); + let weak = url(&set(ide_core::settings::InspectionOverride { + enabled: Some(true), + severity: Some(InspectionLevel::WeakWarning), + ..Default::default() + })); + assert!(std::fs::read_to_string(weak).unwrap().ends_with("missingSerialVersion=info\n")); + let severity_only = url(&set(ide_core::settings::InspectionOverride { + severity: Some(InspectionLevel::Error), + ..Default::default() + })); + assert_eq!(severity_only, off, "off by default"); + } + #[test] fn compiler_defaults_live_in_the_cache_and_preserve_other_options() { let tmp = tempfile::tempdir().unwrap(); let cache = tmp.path().join("cache with spaces 测试"); let mut cfg = custom_server("jdtls", &LanguageServerSettings::default()); merge_init_options(&mut cfg, json!({"settings": {"java": {"format": {"tabSize": 4}}}})); - configure_java_compiler(&mut cfg, &cache); + configure_java_compiler(&mut cfg, &cache, &InspectionSettings::default()); let options = options(&cfg); let path = PathBuf::from(options.pointer("/settings/java/settings/url").unwrap().as_str().unwrap()); assert!(path.starts_with(&cache)); @@ -1112,7 +1175,7 @@ mod java_compiler_tests { } assert_eq!(options.pointer("/settings/java/format/tabSize"), Some(&json!(4))); let modified = std::fs::metadata(&path).unwrap().modified().unwrap(); - configure_java_compiler(&mut cfg, &cache); + configure_java_compiler(&mut cfg, &cache, &InspectionSettings::default()); assert_eq!(std::fs::metadata(path).unwrap().modified().unwrap(), modified); assert_eq!(self::options(&cfg), options); } @@ -1122,7 +1185,7 @@ mod java_compiler_tests { let tmp = tempfile::tempdir().unwrap(); let mut cfg = custom_server("jdtls", &LanguageServerSettings::default()); merge_init_options(&mut cfg, json!({"settings": {"java": {"settings": {"url": "own.prefs"}}}})); - configure_java_compiler(&mut cfg, tmp.path()); + configure_java_compiler(&mut cfg, tmp.path(), &InspectionSettings::default()); assert_eq!(options(&cfg).pointer("/settings/java/settings/url"), Some(&json!("own.prefs"))); for settings in [ diff --git a/crates/ide-sdk/src/java.rs b/crates/ide-sdk/src/java.rs index b1b51273..6de7d667 100644 --- a/crates/ide-sdk/src/java.rs +++ b/crates/ide-sdk/src/java.rs @@ -369,7 +369,6 @@ pub fn configure_jdtls( if cfg.name != "jdtls" { return; } - crate::apply::configure_java_compiler(cfg, data_dir); // A Lombok agent from the user's JVM arguments wins. let own_agent = crate::lombok::agent_in_args(cfg.args.iter().map(String::as_str)); let agent = own_agent.as_deref().or(lombok); diff --git a/src/features/settings/__tests__/inspections.test.ts b/src/features/settings/__tests__/inspections.test.ts index e7eea28c..417626d1 100644 --- a/src/features/settings/__tests__/inspections.test.ts +++ b/src/features/settings/__tests__/inspections.test.ts @@ -15,7 +15,7 @@ import { type InspectionNode, } from "../pages/editor/inspectionsModel"; import { splitPageTarget } from "../model/staging"; -import { validateInspections } from "@/platform/inspections"; +import { JAVA_SERIAL_VERSION_INSPECTION, validateInspections } from "@/platform/inspections"; import { NATURAL_LANGUAGES_PAGE, SPELLING_PAGE } from "../pages/editor/spelling"; import { INSPECTIONS_PAGE } from "../pages/editor/inspections"; import { EDITOR_PAGES } from "../pages/editor"; @@ -69,6 +69,7 @@ describe("inspections tree", () => { expect(tree.map((g) => g.name)).toEqual([ "Cargo", "General", + "Java", "JavaScript", "Other", "Proofreading", @@ -109,6 +110,19 @@ describe("inspections tree", () => { expect(checkState(o, python)).toBe("on"); }); + it("keeps an inspection that is off by default off until turned on", () => { + const serial = find(tree, `Java/${JAVA_SERIAL_VERSION_INSPECTION}`)!; + expect(checkState({}, serial)).toBe("off"); + let o = setEnabled({}, serial, true); + expect(o).toEqual({ [JAVA_SERIAL_VERSION_INSPECTION]: { enabled: true } }); + expect(checkState(o, serial)).toBe("on"); + o = setEnabled(o, serial, false); + expect(o).toEqual({}); + // Turning a whole group on or off only records what differs from each default. + const java = find(tree, "group:Java")!; + expect(setEnabled({}, java, true)).toEqual({ [JAVA_SERIAL_VERSION_INSPECTION]: { enabled: true } }); + }); + it("keeps severity and enabled overrides independent", () => { let o = setLevel({}, "lsp:ruff", "error"); o = setEnabled(o, find(tree, "Python/lsp:ruff")!, false); diff --git a/src/features/settings/pages/editor/inspectionsModel.ts b/src/features/settings/pages/editor/inspectionsModel.ts index bf029bac..85293a6f 100644 --- a/src/features/settings/pages/editor/inspectionsModel.ts +++ b/src/features/settings/pages/editor/inspectionsModel.ts @@ -8,6 +8,7 @@ import { INSPECTION_LEVELS, codeInspectionId, customProfileId, + enabledByDefault, newProfileKey, overridesOf, producerInspectionId, @@ -159,7 +160,7 @@ export function inspectionIds(node: InspectionNode): string[] { return [...(node.id ? [node.id] : []), ...node.children.flatMap(inspectionIds)]; } -export const isEnabled = (o: ProfileOverrides, id: string) => o[id]?.enabled !== false; +export const isEnabled = (o: ProfileOverrides, id: string) => o[id]?.enabled ?? enabledByDefault(id); export type CheckState = "on" | "off" | "mixed"; @@ -181,9 +182,12 @@ function withOverride(o: ProfileOverrides, id: string, patch: Partial withOverride(acc, id, { enabled: enabled ? undefined : false }), o); + return inspectionIds(node).reduce( + (acc, id) => withOverride(acc, id, { enabled: enabled === enabledByDefault(id) ? undefined : enabled }), + o, + ); } /** `undefined` restores the default ("As reported"). */ diff --git a/src/platform/inspections.ts b/src/platform/inspections.ts index d2269c93..5dce0afd 100644 --- a/src/platform/inspections.ts +++ b/src/platform/inspections.ts @@ -216,9 +216,14 @@ export interface BuiltinInspection { description: string; /** Highlighting level when the profile sets none (else as reported). */ defaultLevel?: InspectionLevel; + /** `false`: off until a profile turns it on. */ + defaultEnabled?: boolean; options?: readonly InspectionOptionSpec[]; } +/** Turned on, the Java language server's compiler reports it (`ide-sdk` `SERIAL_VERSION_INSPECTION`). */ +export const JAVA_SERIAL_VERSION_INSPECTION = "java:missing-serial-version"; + /** Producer key of the Typo inspection's diagnostics. */ export const SPELLING_PRODUCER = "spelling"; export const SPELLING_INSPECTION = "builtin:spelling"; @@ -283,6 +288,16 @@ export const BUILTIN_INSPECTIONS: readonly BuiltinInspection[] = [ group: "General", description: "Compiler errors and warnings of the last Build Project, shown at their source positions.", }, + { + id: JAVA_SERIAL_VERSION_INSPECTION, + producer: "jdt-serial-version", + name: "Serializable class without 'serialVersionUID'", + group: "Java", + description: + "Reports classes that implement Serializable but declare no serialVersionUID field. Alt+Enter on the class name adds a default or a generated ID. The Java language server checks it, and restarts when this is changed.", + defaultLevel: "warning", + defaultEnabled: false, + }, { id: "builtin:sql", producer: "sql-inspections", @@ -310,6 +325,11 @@ export const BUILTIN_INSPECTIONS: readonly BuiltinInspection[] = [ const BUILTIN_BY_PRODUCER = new Map(BUILTIN_INSPECTIONS.map((b) => [b.producer, b])); const BUILTIN_BY_ID = new Map(BUILTIN_INSPECTIONS.map((b) => [b.id, b])); +/** Whether inspection `id` is on when a profile doesn't say. */ +export function enabledByDefault(id: string): boolean { + return BUILTIN_BY_ID.get(id)?.defaultEnabled ?? true; +} + /** Option `key` of inspection `id` under `profile` (its default when unset). */ export function inspectionOption(profile: ProfileOverrides, id: string, key: string): boolean { const value = profile[id]?.options?.[key]; @@ -363,7 +383,7 @@ export function applyProfile( ): readonly D[] { const base = producerInspectionId(producer); const own = profile[base]; - if (own?.enabled === false) return []; + if (!(own?.enabled ?? enabledByDefault(base))) return []; const fallback = BUILTIN_BY_PRODUCER.get(producer)?.defaultLevel; let changed = false; const out: D[] = []; diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index 62cf3f0c..076cec07 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -32,6 +32,8 @@ "Not found": "未找到", "Not found on PATH": "未在 PATH 中找到", "Reports newer crate versions that Cargo.toml requirements do not already allow.": "报告 Cargo.toml 版本要求尚未允许的较新 crate 版本。", + "Serializable class without 'serialVersionUID'": "没有 'serialVersionUID' 的可序列化类", + "Reports classes that implement Serializable but declare no serialVersionUID field. Alt+Enter on the class name adds a default or a generated ID. The Java language server checks it, and restarts when this is changed.": "报告实现了 Serializable 但没有声明 serialVersionUID 字段的类。在类名上按 Alt+Enter 可添加默认 ID 或生成的 ID。此检查由 Java 语言服务器执行，更改后语言服务器会重启。", "Run `auggie login` in a terminal.": "在终端中运行 `auggie login`。", "Run `claude` in a terminal and sign in.": "在终端中运行 `claude` 并登录。", "Run `codex login` in a terminal.": "在终端中运行 `codex login`。",