From 4959b430b13dc0d4e2b0756385454bd51b749f83 Mon Sep 17 00:00:00 2001 From: Jacob Kjeldahl Date: Thu, 5 Nov 2026 14:36:13 +0100 Subject: [PATCH 001/185] feat(coverage): Run with Coverage and report import for Ruby (SimpleCov) (#1535) Ruby had no coverage runner. Run with Coverage now works for ruby, bundle, rake, rspec and rails test configurations through SimpleCov, read from its .resultset.json (lines and branches). A project that starts SimpleCov itself keeps its own setup; otherwise a small preload starts it and writes the report to the IDE's run directory, so nothing is written into the project. The preload is loaded through RUBYLIB and a bare -r in RUBYOPT, because RUBYOPT can't take the spaces in the macOS data directory, and it steps aside in the bundle exec launcher, which loads it before Bundler puts the project's gems on the load path. A CI .resultset.json can be imported through Import a Coverage Report. Co-authored-by: kjeldahl <19719+kjeldahl@users.noreply.github.com> --- crates/ide-coverage/src/import.rs | 31 +++++- crates/ide-coverage/src/lib.rs | 8 ++ crates/ide-coverage/src/plan.rs | 128 +++++++++++++++++++++++++ crates/ide-coverage/src/simplecov.rs | 74 ++++++++++++++ crates/ide-host/src/ops/coverage.rs | 2 +- crates/ide-run/src/lib.rs | 1 + crates/ide-run/src/ruby_preload.rs | 61 ++++++++++++ src/bindings.ts | 2 + src/features/coverage/coverage.test.ts | 3 + src/features/coverage/model.ts | 9 +- src/features/coverage/reports.tsx | 4 +- 11 files changed, 316 insertions(+), 7 deletions(-) create mode 100644 crates/ide-coverage/src/simplecov.rs create mode 100644 crates/ide-run/src/ruby_preload.rs diff --git a/crates/ide-coverage/src/import.rs b/crates/ide-coverage/src/import.rs index 669f1b8cd..e35c24c1c 100644 --- a/crates/ide-coverage/src/import.rs +++ b/crates/ide-coverage/src/import.rs @@ -1,8 +1,8 @@ //! "Import a report collected in CI from disk": the format is sniffed from the -//! content (lcov, Go cover profile, JaCoCo XML) and paths recorded on another +//! content (lcov, Go cover profile, JaCoCo XML, SimpleCov resultset) and paths recorded on another //! machine are mapped into the project by their longest existing suffix. -use crate::{Accumulator, CoverageSuite, goprofile, jacoco, lcov}; +use crate::{Accumulator, CoverageSuite, goprofile, jacoco, lcov, simplecov}; use ide_core::{IdeError, IdeResult}; use std::path::{Component, Path, PathBuf}; @@ -11,6 +11,7 @@ pub(crate) enum Format { Lcov, GoProfile, Jacoco, + SimpleCov, } pub(crate) fn detect(text: &str) -> Option { @@ -18,6 +19,9 @@ pub(crate) fn detect(text: &str) -> Option { if head.starts_with('<') { return head.contains(" IdeResult { let text = std::fs::read_to_string(path)?; let mut suite = match detect(&text) { Some(Format::Lcov) => lcov::parse(&text, root).finish("lcov"), + Some(Format::SimpleCov) => simplecov::parse(&text, root)?.finish("SimpleCov"), Some(Format::GoProfile) => { let (module, module_dir) = go_module_for(root, &text); goprofile::parse(&text, &module, &module_dir).finish("go test -cover") @@ -50,7 +55,7 @@ pub fn import(path: &Path, root: &Path) -> IdeResult { } None => { return Err(IdeError::invalid(format!( - "{} is not an lcov tracefile, Go cover profile or JaCoCo XML report", + "{} is not an lcov tracefile, Go cover profile, JaCoCo XML report or SimpleCov resultset", path.display() ))); } @@ -125,9 +130,29 @@ mod tests { Some(Format::Jacoco) ); assert_eq!(detect(""), None); + assert_eq!(detect("{\"RSpec\": {\"coverage\": {}}}"), Some(Format::SimpleCov)); + assert_eq!(detect("{\"name\": \"package.json\"}"), None); assert_eq!(detect("hello"), None); } + #[test] + fn imports_ci_simplecov_with_foreign_paths() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("app"); + std::fs::create_dir_all(root.join("lib")).unwrap(); + std::fs::write(root.join("lib/a.rb"), "").unwrap(); + let report = tmp.path().join(".resultset.json"); + std::fs::write( + &report, + r#"{"RSpec":{"coverage":{"/home/runner/work/app/app/lib/a.rb":{"lines":[null,1,0]}}}}"#, + ) + .unwrap(); + let suite = import(&report, &root).unwrap(); + assert_eq!(suite.runner, "SimpleCov"); + assert_eq!(suite.files[0].path.as_ref() as &Path, root.join("lib/a.rb")); + assert_eq!((suite.files[0].lines_total, suite.files[0].lines_covered), (2, 1)); + } + #[test] fn imports_ci_lcov_with_foreign_paths() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/ide-coverage/src/lib.rs b/crates/ide-coverage/src/lib.rs index 757812036..707f936da 100644 --- a/crates/ide-coverage/src/lib.rs +++ b/crates/ide-coverage/src/lib.rs @@ -10,6 +10,7 @@ //! | JS / TS (vitest, jest scripts) | the runner's `--coverage` | lcov | //! | Node.js scripts | `c8` | lcov | //! | Python (pytest, scripts) | coverage.py (`coverage run`, then `coverage lcov`) | lcov | +//! | Ruby (rspec, minitest, rake, scripts) | SimpleCov (`coverage/.resultset.json`) | SimpleCov resultset | //! | Go (`go test`) | `-coverprofile` | Go cover profile | //! | Maven / Gradle tests | JaCoCo (plugin goals / an init script) | JaCoCo XML | //! @@ -20,6 +21,7 @@ mod import; mod jacoco; mod lcov; mod plan; +mod simplecov; pub use import::import; pub use plan::prepare; @@ -83,6 +85,8 @@ pub enum CoverageReportSpec { Lcov { runner: String, path: IpcPath, base: IpcPath }, /// `go test -coverprofile`; import paths under `module` map to `module_dir`. GoProfile { path: IpcPath, module: String, module_dir: IpcPath }, + /// SimpleCov's `.resultset.json`; relative paths resolve against `base`. + SimpleCov { path: IpcPath, base: IpcPath }, /// Every JaCoCo XML report (Maven `target/site/jacoco`, Gradle `build/reports/jacoco`) under `dir`. Jacoco { dir: IpcPath }, /// The tool's raw data is exported to lcov by `argv` first (coverage.py: `… -m coverage lcov -o `). @@ -185,6 +189,10 @@ pub fn collect(spec: &CoverageReportSpec) -> IdeResult { let text = read_report(path)?; Ok(lcov::parse(&text, base).finish(runner)) } + CoverageReportSpec::SimpleCov { path, base } => { + let text = read_report(path)?; + Ok(simplecov::parse(&text, base)?.finish("SimpleCov")) + } CoverageReportSpec::GoProfile { path, module, module_dir } => { let text = read_report(path)?; Ok(goprofile::parse(&text, module, module_dir).finish("go test -cover")) diff --git a/crates/ide-coverage/src/plan.rs b/crates/ide-coverage/src/plan.rs index 2368aaaa4..5355ffd51 100644 --- a/crates/ide-coverage/src/plan.rs +++ b/crates/ide-coverage/src/plan.rs @@ -67,6 +67,7 @@ pub fn prepare(root: &Path, config: &RunConfig, out: &Path) -> CoveragePrepared "gradle" | "gradlew" => gradle(config, &dir, out), "npm" | "pnpm" | "yarn" | "bun" => npm(root, config, &dir, out), "node" | "npx" | "tsx" | "ts-node" => c8(root, config, &dir, out), + "ruby" | "bundle" | "rake" | "rspec" | "rails" => ruby(root, config, &dir, out), "deno" => unsupported("Coverage of Deno tasks is not supported yet."), "java" => unsupported( "Coverage of Java applications needs the JaCoCo agent; run a Maven or Gradle test configuration with coverage.", @@ -234,6 +235,65 @@ fn c8(root: &Path, config: &RunConfig, dir: &Path, out: &Path) -> CoveragePrepar ready("c8", config, path(&c8), args, report) } +/// Loaded first (`ide_run::ruby_preload`) when the project never starts SimpleCov itself. +const SIMPLECOV_PRELOAD: &str = r#"# Written by OpenIDEA for Run with Coverage. +begin + require "simplecov" +rescue LoadError + # `bundle exec` is a Ruby process too and loads this before Bundler puts the project's gems on + # the load path; the Ruby it starts loads bundler/setup first and finds SimpleCov. + return +end +SimpleCov.coverage_dir(ENV.fetch("OPENIDEA_COVERAGE_DIR")) +SimpleCov.start do + add_filter %r{^/(test|spec|features)/} + enable_coverage :branch +end +"#; + +/// Files where a project conventionally starts SimpleCov. +const SIMPLECOV_STARTERS: &[&str] = + &[".simplecov", "spec/spec_helper.rb", "spec/rails_helper.rb", "test/test_helper.rb", "test/minitest_helper.rb"]; + +fn ruby(root: &Path, config: &RunConfig, dir: &Path, out: &Path) -> CoveragePrepared { + let bundled = [dir, root].iter().any(|d| d.join("Gemfile").is_file()); + let mentions = |file: &str| { + [dir, root].iter().any(|d| std::fs::read_to_string(d.join(file)).is_ok_and(|t| t.contains("simplecov"))) + }; + if bundled && !mentions("Gemfile.lock") && !mentions("Gemfile") { + return missing( + "simplecov", + "Ruby coverage needs the simplecov gem in the project's Gemfile.", + Some((vec![s("bundle"), s("add"), s("simplecov"), s("--group"), s("test")], dir)), + ); + } + let starts = |d: &Path| { + SIMPLECOV_STARTERS.iter().any(|f| { + std::fs::read_to_string(d.join(f)) + .is_ok_and(|t| t.contains("SimpleCov.start") || f.starts_with(".simplecov")) + }) + }; + let mut config = config.clone(); + let resultset = if [dir, root].iter().any(|d| starts(d)) { + // The project's own configuration decides where the report goes (default `coverage/`). + // Drop the previous run's data: SimpleCov merges into it. + let results = dir.join("coverage"); + let _ = std::fs::remove_file(results.join(".resultset.json")); + results.join(".resultset.json") + } else { + let preload = out.join("openidea_simplecov.rb"); + if let Err(e) = std::fs::write(&preload, SIMPLECOV_PRELOAD) { + return unsupported(format!("Cannot write the SimpleCov preload: {e}")); + } + config.env.insert(s("OPENIDEA_COVERAGE_DIR"), path(out)); + // `out` lives under the IDE's data directory, which may have spaces (RUBYOPT cannot take them). + ide_run::ruby_preload::add(&mut config.env, out, "openidea_simplecov", true); + out.join(".resultset.json") + }; + let report = CoverageReportSpec::SimpleCov { path: resultset.into(), base: dir.to_path_buf().into() }; + ready("SimpleCov", &config, config.command.clone(), config.args.clone(), report) +} + fn python(config: &RunConfig, dir: &Path, out: &Path, py: wrap::PythonCommand) -> CoveragePrepared { if py.target.is_empty() { return unsupported("Coverage needs a Python script or module to run."); @@ -398,6 +458,74 @@ mod tests { )); } + /// `bundle exec` loads the preload before Bundler sets up the project's gems: without SimpleCov + /// on the load path it must step aside instead of failing the launcher. + #[test] + fn simplecov_preload_tolerates_a_ruby_without_the_gem() { + let dir = tempfile::tempdir().unwrap(); + let out = dir.path().join("out dir"); + std::fs::create_dir_all(&out).unwrap(); + std::fs::write(out.join("openidea_simplecov.rb"), SIMPLECOV_PRELOAD).unwrap(); + let status = std::process::Command::new("ruby") + .args(["-e", "exit 0"]) + .env("RUBYLIB", &out) + .env("RUBYOPT", "-ropenidea_simplecov") + .env("OPENIDEA_COVERAGE_DIR", &out) + .env("GEM_HOME", dir.path().join("empty gems")) + .env("GEM_PATH", dir.path().join("empty gems")) + .status(); + // No Ruby on this machine: nothing to check. + let Ok(status) = status else { return }; + assert!(status.success()); + } + + #[test] + fn ruby_runs_get_simplecov() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + // The IDE's data directory has spaces on macOS (`Application Support`). + let out = root.join("out dir"); + std::fs::create_dir_all(&out).unwrap(); + std::fs::write(root.join("Gemfile"), "source 'https://rubygems.org'\n").unwrap(); + let config = cfg("rspec", "bundle", &["exec", "rspec"]); + // Bundler project without the gem: install hint. + match prepare(root, &config, &out) { + CoveragePrepared::MissingTool { tool, install: Some(i), .. } => { + assert_eq!(tool, "simplecov"); + assert_eq!(i.args, ["add", "simplecov", "--group", "test"]); + } + other => panic!("{other:?}"), + } + // Gem present, never started: preloaded through RUBYOPT, report in `out`. + std::fs::write(root.join("Gemfile"), "gem 'simplecov'\n").unwrap(); + let p = prepare(root, &config, &out); + let (l, report) = launch(&p); + assert_eq!( + (l.command.as_str(), l.args.as_slice()), + ("bundle", ["exec".to_owned(), "rspec".to_owned()].as_slice()) + ); + assert!(l.env["RUBYOPT"].contains("-ropenidea_simplecov")); + assert!(std::env::split_paths(&l.env["RUBYLIB"]).any(|d| d == out)); + assert_eq!(l.env["OPENIDEA_COVERAGE_DIR"], out.to_string_lossy()); + assert!(out.join("openidea_simplecov.rb").is_file()); + assert!( + matches!(report, CoverageReportSpec::SimpleCov { path, .. } if (path.as_ref() as &Path).starts_with(&out)) + ); + // Started by the project: untouched environment, stale data dropped, report in `coverage/`. + std::fs::create_dir_all(root.join("test")).unwrap(); + std::fs::create_dir_all(root.join("coverage")).unwrap(); + std::fs::write(root.join("coverage/.resultset.json"), "{}").unwrap(); + std::fs::write(root.join("test/test_helper.rb"), "require 'simplecov'\nSimpleCov.start\n").unwrap(); + let p = prepare(root, &cfg("ruby", "ruby", &["-Ilib", "test/a_test.rb"]), &out); + let (l, report) = launch(&p); + assert!(!l.env.contains_key("RUBYOPT")); + assert_eq!(l.args, ["-Ilib", "test/a_test.rb"]); + assert!(!root.join("coverage/.resultset.json").exists()); + assert!( + matches!(report, CoverageReportSpec::SimpleCov { path, .. } if (path.as_ref() as &Path).ends_with("coverage/.resultset.json")) + ); + } + #[test] fn unknown_commands_are_unsupported() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/ide-coverage/src/simplecov.rs b/crates/ide-coverage/src/simplecov.rs new file mode 100644 index 000000000..f93c41759 --- /dev/null +++ b/crates/ide-coverage/src/simplecov.rs @@ -0,0 +1,74 @@ +//! SimpleCov's `coverage/.resultset.json`: +//! +//! ```json +//! { "RSpec": { "timestamp": 1, "coverage": { +//! "/abs/lib/a.rb": { "lines": [null, 1, 0], +//! "branches": { "[:if, 0, 3, 4, 3, 20]": { "[:then, 1, 4, 6, 4, 9]": 1, "[:else, 2, 6, 6, 6, 9]": 0 } } }, +//! "/abs/lib/legacy.rb": [null, 1] } } } +//! ``` +//! +//! `lines[i]` is line `i + 1` (`null` = not relevant), so the index is already 0-based. +//! A branch belongs to the line its condition starts on (third number of its key). + +use crate::Accumulator; +use ide_core::{IdeError, IdeResult}; +use serde_json::Value; +use std::path::Path; + +/// Start line (1-based) of a `"[:if, 0, 3, 4, 3, 20]"` branch key. +fn start_line(key: &str) -> Option { + key.trim_matches(['[', ']']).split(',').nth(2)?.trim().parse().ok() +} + +pub(crate) fn parse(text: &str, base: &Path) -> IdeResult { + let json: Value = serde_json::from_str(text).map_err(|e| IdeError::invalid(format!("SimpleCov resultset: {e}")))?; + let mut acc = Accumulator::default(); + // One entry per test command ("RSpec", "Minitest", …); their coverage is merged. + for suite in json.as_object().into_iter().flat_map(|o| o.values()) { + let Some(files) = suite.get("coverage").and_then(Value::as_object) else { continue }; + for (name, data) in files { + let p = Path::new(name); + let path = if p.is_absolute() { p.to_path_buf() } else { base.join(p) }; + acc.file(&path); + let lines = data.get("lines").or(Some(data)).and_then(Value::as_array); + for (i, hits) in lines.into_iter().flatten().enumerate() { + if let Some(hits) = hits.as_u64() { + acc.line(&path, i as u32, hits); + } + } + for (cond, branches) in data.get("branches").and_then(Value::as_object).into_iter().flatten() { + let Some(line) = start_line(cond) else { continue }; + for (branch, hits) in branches.as_object().into_iter().flatten() { + acc.has_branches = true; + acc.branch(&path, line.saturating_sub(1), format!("{cond}/{branch}"), hits.as_u64() > Some(0)); + } + } + } + } + Ok(acc) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reads_lines_branches_and_merges_suites() { + let text = r#"{ + "RSpec": {"coverage": {"/p/lib/a.rb": {"lines": [null, 1, 0], + "branches": {"[:if, 0, 2, 4, 2, 20]": {"[:then, 1, 2, 6, 2, 9]": 1, "[:else, 2, 2, 6, 2, 9]": 0}}}}}, + "Minitest": {"coverage": {"/p/lib/a.rb": {"lines": [null, 2, 3]}, "lib/old.rb": [1, null]}} + }"#; + let suite = parse(text, Path::new("/p")).unwrap().finish("SimpleCov"); + assert_eq!(suite.files.len(), 2); + let a = suite.files.iter().find(|f| (f.path.as_ref() as &Path).ends_with("a.rb")).unwrap(); + assert_eq!(a.lines.iter().map(|l| (l.line, l.hits)).collect::>(), [(1, 3), (2, 3)]); + assert_eq!((a.branches_total, a.branches_covered), (2, 1)); + assert_eq!(a.lines[0].branches_total, 2); + let old = suite.files.iter().find(|f| (f.path.as_ref() as &Path).ends_with("lib/old.rb")).unwrap(); + assert_eq!(old.path.as_ref() as &Path, Path::new("/p/lib/old.rb")); + assert_eq!(old.lines[0].hits, 1); + assert!(suite.has_branches); + assert!(parse("nope", Path::new("/p")).is_err()); + } +} diff --git a/crates/ide-host/src/ops/coverage.rs b/crates/ide-host/src/ops/coverage.rs index 007d701db..f57b538f4 100644 --- a/crates/ide-host/src/ops/coverage.rs +++ b/crates/ide-host/src/ops/coverage.rs @@ -32,7 +32,7 @@ pub async fn coverage_collect(host: &Host, window: &str, report: CoverageReportS blocking(move || ide_coverage::collect(&report)).await } -/// Reads a report collected elsewhere (CI): lcov, a Go cover profile or JaCoCo XML, +/// Reads a report collected elsewhere (CI): lcov, a Go cover profile, JaCoCo XML or a SimpleCov resultset, /// with paths mapped into this project. pub async fn coverage_import(host: &Host, window: &str, path: IpcPath) -> IdeResult { let root = host.project(window)?.root.clone(); diff --git a/crates/ide-run/src/lib.rs b/crates/ide-run/src/lib.rs index 7e85a1643..97e313423 100644 --- a/crates/ide-run/src/lib.rs +++ b/crates/ide-run/src/lib.rs @@ -12,6 +12,7 @@ pub mod java_cmdline; pub mod junit; mod process; pub mod python_path; +pub mod ruby_preload; pub mod tomcat; mod utf8; pub mod which; diff --git a/crates/ide-run/src/ruby_preload.rs b/crates/ide-run/src/ruby_preload.rs new file mode 100644 index 000000000..f42937633 --- /dev/null +++ b/crates/ide-run/src/ruby_preload.rs @@ -0,0 +1,61 @@ +//! Loads a Ruby file into a launched process without touching the project: the directory goes on +//! `RUBYLIB` and `RUBYOPT` gets the bare feature name (`-ropenidea_hook`). +//! +//! `RUBYOPT` is split at spaces with no quoting, so an absolute path in it (`-r/…/Application +//! Support/…/hook.rb`, where the IDE keeps its data) makes Ruby fail with `invalid switch in RUBYOPT`. +//! `RUBYLIB` is a path list and takes spaces. +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// Makes `feature` (a file `/.rb`) load first in every Ruby the process starts. +/// `inherit`: the process also sees the IDE's own environment, whose `RUBYLIB` / `RUBYOPT` stay. +pub fn add(env: &mut BTreeMap, dir: &Path, feature: &str, inherit: bool) { + let mut inherited = if inherit { crate::system_env() } else { BTreeMap::new() }; + let mut existing = |name: &str| env.get(name).cloned().or_else(|| inherited.remove(name)).filter(|v| !v.is_empty()); + + let mut dirs: Vec = vec![dir.to_path_buf()]; + if let Some(lib) = existing("RUBYLIB") { + for d in std::env::split_paths(&lib) { + if !dirs.contains(&d) { + dirs.push(d); + } + } + } + let rubyopt = existing("RUBYOPT"); + if let Ok(joined) = std::env::join_paths(dirs) { + env.insert("RUBYLIB".into(), joined.to_string_lossy().into_owned()); + } + let option = format!("-r{feature}"); + let value = match rubyopt { + Some(e) if e.split_whitespace().any(|o| o == option) => e, + Some(e) => format!("{e} {option}"), + None => option, + }; + env.insert("RUBYOPT".into(), value); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_directory_with_spaces_goes_on_rubylib_not_into_rubyopt() { + let dir = Path::new("/Users/me/Library/Application Support/ide/ruby"); + let mut env = BTreeMap::new(); + add(&mut env, dir, "openidea_hook", false); + assert_eq!(env["RUBYLIB"], dir.to_string_lossy()); + assert_eq!(env["RUBYOPT"], "-ropenidea_hook"); + assert!(!env["RUBYOPT"].contains(' ')); + } + + #[test] + fn keeps_existing_settings_and_adds_once() { + // Path lists use the platform's separator (`;` on Windows). + let list = |dirs: &[&str]| std::env::join_paths(dirs).unwrap().to_string_lossy().into_owned(); + let mut env = BTreeMap::from([("RUBYLIB".into(), list(&["/a", "/b"])), ("RUBYOPT".into(), "-w".into())]); + add(&mut env, Path::new("/hook"), "h", false); + add(&mut env, Path::new("/hook"), "h", false); + assert_eq!(env["RUBYLIB"], list(&["/hook", "/a", "/b"])); + assert_eq!(env["RUBYOPT"], "-w -rh"); + } +} diff --git a/src/bindings.ts b/src/bindings.ts index 39dfd5848..ffe154510 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -2426,6 +2426,8 @@ export type CoverageReportSpec = { kind: "lcov"; runner: string; path: IpcPath; base: IpcPath } | /** `go test -coverprofile`; import paths under `module` map to `module_dir`. */ { kind: "goProfile"; path: IpcPath; module: string; moduleDir: IpcPath } | +/** SimpleCov's `.resultset.json`; relative paths resolve against `base`. */ +{ kind: "simpleCov"; path: IpcPath; base: IpcPath } | /** Every JaCoCo XML report (Maven `target/site/jacoco`, Gradle `build/reports/jacoco`) under `dir`. */ { kind: "jacoco"; dir: IpcPath } | /** The tool's raw data is exported to lcov by `argv` first (coverage.py: `… -m coverage lcov -o `). */ diff --git a/src/features/coverage/coverage.test.ts b/src/features/coverage/coverage.test.ts index 22257cc20..dff972c7b 100644 --- a/src/features/coverage/coverage.test.ts +++ b/src/features/coverage/coverage.test.ts @@ -57,6 +57,9 @@ describe("coverageSupports", () => { expect(coverageSupports(cfg("pnpm", ["run", "dev"]))).toBe(false); expect(coverageSupports(cfg("/v/bin/python3", ["-m", "pytest"]))).toBe(true); expect(coverageSupports(cfg("uv", ["run", "pytest"]))).toBe(true); + expect(coverageSupports(cfg("/v/bin/ruby", ["-Ilib", "test/a_test.rb"]))).toBe(true); + expect(coverageSupports(cfg("bundle", ["exec", "rspec"], { isTest: true }))).toBe(true); + expect(coverageSupports(cfg("bundle", ["exec", "rake", "db:migrate"]))).toBe(false); expect(coverageSupports(cfg("make", ["test"]))).toBe(false); }); }); diff --git a/src/features/coverage/model.ts b/src/features/coverage/model.ts index e36d8ceac..472537edd 100644 --- a/src/features/coverage/model.ts +++ b/src/features/coverage/model.ts @@ -16,7 +16,7 @@ const runsJvmTests = (args: readonly string[]) => /** * Whether a coverage runner is known for `config` (cheap; the backend decides for real): * cargo test / run / nextest, go test, Maven / Gradle tests, package.json test scripts, - * Node.js programs and Python. + * Node.js programs, Python and Ruby (scripts, rspec, test rake tasks). */ export function coverageSupports(config: Pick): boolean { const program = programName(config.command); @@ -43,6 +43,13 @@ export function coverageSupports(config: Pick { filters: [ { name: "Coverage reports (lcov, JaCoCo XML, Go cover profile)", - extensions: ["info", "lcov", "xml", "out", "txt", "cov", "coverprofile"], + extensions: ["info", "lcov", "xml", "out", "txt", "cov", "coverprofile", "json"], }, { name: "All files", extensions: ["*"] }, ], From f6a18231211d74a39bae5d7c8afe7e8dde96b008 Mon Sep 17 00:00:00 2001 From: Jacob Kjeldahl Date: Thu, 5 Nov 2026 14:37:08 +0100 Subject: [PATCH 002/185] feat(run): run Ruby tests from the editor and Project view (-I roots, test-root folders) (#1545) A Ruby file run from the context menu had no project load path, so require "foo" for lib/foo.rb failed: Sources roots are now passed as -I. Folders under a test root get Run Tests in '': RSpec on the folder, rake test with TEST= (two comma-separated globs, since Rake splits TEST at commas), or a detected minitest configuration that loads every *_test.rb / test_*.rb under the folder when the project has neither. Co-authored-by: kjeldahl <19719+kjeldahl@users.noreply.github.com> --- crates/ide-run/src/detect.rs | 14 +++++ crates/ide-run/tests/detect.rs | 19 +++++++ src/features/run/producers.ruby.test.ts | 75 +++++++++++++++++++++++++ src/features/run/producers.ts | 37 +++++++++++- 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 src/features/run/producers.ruby.test.ts diff --git a/crates/ide-run/src/detect.rs b/crates/ide-run/src/detect.rs index b7bc88027..7e7d49b0d 100644 --- a/crates/ide-run/src/detect.rs +++ b/crates/ide-run/src/detect.rs @@ -1059,6 +1059,9 @@ fn ruby_config( c } +/// Loads the test files of the directories (or files) in `ARGV`, default `test`: the way `rake test` does. +const MINITEST_RUNNER: &str = r#"dirs = ARGV.empty? ? ["test"] : ARGV.dup; ARGV.clear; dirs.flat_map { |d| File.file?(d) ? [d] : Dir[File.join(d, "**", "{*_test,test_*}.rb")] }.sort.each { |f| require File.expand_path(f) }"#; + fn uses_rspec(root: &Path) -> bool { root.join("spec").is_dir() || root.join(".rspec").is_file() @@ -1101,6 +1104,17 @@ fn detect_ruby(root: &Path, out: &mut Vec) { } if uses_rspec(root) { out.push(ruby_config("rspec".into(), "rspec".into(), "rspec", ruby_tool(root, "rspec"), &[]).test()); + } else if root.join("test").is_dir() && !out.iter().any(|c| c.id == "rake:test") { + // Minitest / Test::Unit without a `test` Rake task: load every test file under `test/`. + let mut args = Vec::new(); + for dir in ["lib", "test"] { + if root.join(dir).is_dir() { + args.push(format!("-I{dir}")); + } + } + args.extend(["-e".to_owned(), MINITEST_RUNNER.to_owned(), "test".to_owned()]); + let extra: Vec<&str> = args.iter().map(String::as_str).collect(); + out.push(ruby_config("minitest".into(), "minitest".into(), "ruby", ruby_tool(root, "ruby"), &extra).test()); } } diff --git a/crates/ide-run/tests/detect.rs b/crates/ide-run/tests/detect.rs index aebe520c2..5cbb58187 100644 --- a/crates/ide-run/tests/detect.rs +++ b/crates/ide-run/tests/detect.rs @@ -820,6 +820,25 @@ task default: :build assert_eq!(find(&c, "rake:build").extras.folder.as_deref(), Some("Rake")); } +#[test] +fn ruby_minitest_projects_get_a_test_runner() { + let dir = project(&[("Gemfile", "source 'https://rubygems.org'\n"), ("lib/a.rb", ""), ("test/a_test.rb", "")]); + let c = detect(dir.path()); + let m = find(&c, "minitest"); + assert!(m.is_test); + assert_eq!(m.kind, "ruby"); + assert_eq!( + (m.command.as_str(), &m.args[..4]), + ("bundle", &["exec", "ruby", "-Ilib", "-Itest"].map(String::from)[..]) + ); + assert_eq!(m.args.last().map(String::as_str), Some("test")); + // A `test` Rake task or RSpec already runs the tests. + let dir = project(&[("Rakefile", "task :test\n"), ("test/a_test.rb", "")]); + assert!(!ids(&detect(dir.path())).contains(&"minitest")); + let dir = project(&[("Gemfile", "gem 'rspec'\n"), ("test/a_test.rb", "")]); + assert!(!ids(&detect(dir.path())).contains(&"minitest")); +} + #[test] fn ruby_without_bundler_runs_bare_tools() { let dir = project(&[("Rakefile", "task :test\n")]); diff --git a/src/features/run/producers.ruby.test.ts b/src/features/run/producers.ruby.test.ts new file mode 100644 index 000000000..bcf37acb1 --- /dev/null +++ b/src/features/run/producers.ruby.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/platform/projectRoots", () => ({ + currentRoots: () => [ + { path: "lib", kind: "source", detectedFrom: null }, + { path: "test", kind: "test", detectedFrom: null }, + { path: "spec", kind: "test", detectedFrom: null }, + { path: "", kind: "source", detectedFrom: null }, + ], +})); + +import type { RunConfig } from "@/platform/ipc"; +import { produceContext, produceDirectory, type ToolEnv } from "./producers"; + +const env: ToolEnv = { + tool: (n) => (n === "ruby" ? { path: "/bin/ruby", version: null } : null), + compileDir: "/tmp/openidea-run", +}; +const input = (path: string) => ({ path, root: "/work/app", configs: [], content: undefined, line: undefined }); + +describe("Ruby file run", () => { + it("puts Sources roots on the load path", () => { + const p = produceContext(input("/work/app/test/integration/a_test.rb"), env); + expect(p?.config?.args).toEqual(["-Ilib", "test/integration/a_test.rb"]); + }); + + it("adds none for files outside the project", () => { + const p = produceContext(input("/elsewhere/a.rb"), env); + expect(p?.config?.args).toEqual(["a.rb"]); + }); +}); + +describe("Ruby directory run", () => { + const cfg = (over: Partial & { id: string; kind: string }): RunConfig => ({ + name: over.id, + command: "bundle", + args: [], + cwd: null, + env: {}, + detected: true, + isTest: true, + debugAdapter: null, + ...over, + }); + const dir = (path: string, configs: RunConfig[]) => + produceDirectory({ path: `/work/app/${path}`, root: "/work/app", configs }); + + it("RSpec on a nested folder", () => { + const p = dir("spec/models/user", [cfg({ id: "rspec", kind: "rspec", args: ["exec", "rspec"] })]); + expect(p?.config?.args).toEqual(["exec", "rspec", "spec/models/user"]); + expect(p?.name).toBe("Tests in 'user'"); + }); + + it("rake test with a TEST glob", () => { + const p = dir("test/unit", [cfg({ id: "rake:test", kind: "rake", args: ["exec", "rake", "test"] })]); + expect(p?.config?.args).toEqual(["exec", "rake", "test", "TEST=test/unit/**/*_test.rb,test/unit/**/test_*.rb"]); + }); + + it("the Minitest runner with Sources roots and the folder", () => { + const base = cfg({ id: "minitest", kind: "ruby", args: ["exec", "ruby", "-Itest", "-e", "SCRIPT", "test"] }); + const p = dir("test/unit", [base]); + expect(p?.config?.args).toEqual(["exec", "ruby", "-Itest", "-Ilib", "-e", "SCRIPT", "test/unit"]); + }); + + it("the test root itself", () => { + const p = dir("test", [cfg({ id: "rspec", kind: "rspec", args: ["exec", "rspec"] })]); + expect(p?.config?.args).toEqual(["exec", "rspec", "test"]); + }); + + it("nothing for non-test folders or projects without a Ruby runner", () => { + expect(dir("lib/unit", [cfg({ id: "rspec", kind: "rspec" })])).toBeNull(); + expect(dir("other/unit", [cfg({ id: "rspec", kind: "rspec" })])).toBeNull(); + expect(dir("test/unit", [])).toBeNull(); + }); +}); diff --git a/src/features/run/producers.ts b/src/features/run/producers.ts index cfca6c5bb..ce9347c72 100644 --- a/src/features/run/producers.ts +++ b/src/features/run/producers.ts @@ -596,6 +596,10 @@ function simple( return { name: s.file, scope: "file", config: fileConfig(s, kind, t.command, args(t.command, t.name), debugAdapter) }; } +/** `-I` for each Sources root of the project (the project root itself is already the cwd). */ +const rubyLoadPath = (roots: readonly { path: string; kind: string }[]): string[] => + roots.filter((r) => r.kind === "source" && r.path !== "").map((r) => `-I${r.path}`); + const hasGoMain = (content?: string) => content === undefined || /^\s*package\s+main\b/m.test(content); const hasRustMain = (content?: string) => content === undefined || /^(?:pub\s+)?(?:async\s+)?fn\s+main\s*\(/m.test(content); @@ -798,6 +802,8 @@ export function produceFile(input: ContextInput, env: ToolEnv): Produced | null }; } case "ruby": + // Sources roots go on the load path (`-Ilib`), as `rake test` does; paths are project-relative. + return simple(lang, s, env, lang, () => [...(s.inProject ? rubyLoadPath(currentRoots()) : []), s.arg]); case "php": case "perl": case "lua": @@ -1021,6 +1027,33 @@ function cmakeTargets(input: ContextInput): RunConfig[] { const TEST_DIR = /(^|\/)(tests?|__tests__|specs?|test_?\w*)$/; +/** + * The tests of a Ruby folder: RSpec on the folder, `rake test` with `TEST=`, or the + * detected Minitest runner given the folder (Sources roots on its load path). + */ +function rubyDirectory( + closest: (kinds: string[]) => RunConfig | undefined, + relToCwd: (c: RunConfig) => string, + make: (base: RunConfig, label: string, args: string[]) => Produced, + label: string, +): Produced | null { + const rspec = closest(["rspec"]); + if (rspec) return make(rspec, label, [...rspec.args, relToCwd(rspec)]); + const rake = closest(["rake"]); + if (rake && rake.args.at(-1) === "test") { + // Rake splits `TEST` at commas into patterns, so no braces: two globs. + const dir = relToCwd(rake); + return make(rake, label, [...rake.args, `TEST=${dir}/**/*_test.rb,${dir}/**/test_*.rb`]); + } + const minitest = closest(["ruby"]); + if (!minitest) return null; + const at = minitest.args.indexOf("-e"); + const flags = rubyLoadPath(currentRoots()).filter((f) => !minitest.args.includes(f)); + const args = [...minitest.args.slice(0, -1)]; + args.splice(at < 0 ? args.length : at, 0, ...flags); + return make(minitest, label, [...args, relToCwd(minitest)]); +} + export function produceDirectory(input: ContextInput): Produced | null { const { root } = input; if (!root || !isProjectPath(input) || input.path === root) return null; @@ -1029,8 +1062,8 @@ export function produceDirectory(input: ContextInput): Produced | null { const jvmRoot = /^(?:(.*)\/)?src\/test(?:\/(java|kotlin|scala|groovy))?$/.exec(rel); const jvmPackage = /^(?:(.*)\/)?src\/test\/(?:java|kotlin|scala|groovy)\/(.+)$/.exec(rel); const marked = isTestPath(currentRoots(), rel); - if (!TEST_DIR.test(rel) && !jvmRoot && !jvmPackage && !marked && !/(^|\/)src\/test(\/|$)/.test(rel)) return null; const tests = input.configs.filter((c) => c.isTest); + if (!TEST_DIR.test(rel) && !jvmRoot && !jvmPackage && !marked && !/(^|\/)src\/test(\/|$)/.test(rel)) return null; const closest = (kinds: string[]) => { const cands = tests.filter((c) => kinds.includes(c.kind)); const score = (c: RunConfig) => { @@ -1071,6 +1104,8 @@ export function produceDirectory(input: ContextInput): Produced | null { } } const js = closest(["npm", "bun", "deno"]); + const ruby = marked && !js && rubyDirectory(closest, relToCwd, make, testsIn); + if (ruby) return ruby; const py = closest(["pytest"]); const django = closest(["django-test"]); const go = closest(["go"]); From 087cb4cbaaf1923bb51a48022cc1f9541c9808f2 Mon Sep 17 00:00:00 2001 From: Jacob Kjeldahl Date: Thu, 5 Nov 2026 14:38:12 +0100 Subject: [PATCH 003/185] feat(run): Ruby test tree, jump to source and gutter actions (#1546) Ruby test runs showed only console text: there was no Ruby output parser and test_*.rb was not a test file. A Minitest parser now builds the test tree (class, test, status, time, failure text) from verbose output (-v / TESTOPTS=-v), test_*.rb counts as a test file, jump to source finds the class and then its def, and the gutter gets a run action per test method (coloured by the last result) and per test class and module that holds test classes; one test, class or module runs with --name=.... The streaming Find in Files helper the jump to source uses moved from the search feature to src/platform, so the run feature doesn't import another feature. Co-authored-by: kjeldahl <19719+kjeldahl@users.noreply.github.com> --- crates/ide-run/src/detect.rs | 5 +- src/features/run/context.ts | 9 +- src/features/run/links.ts | 6 +- src/features/run/navigate.ts | 39 +++++ src/features/run/producers.ruby.test.ts | 45 +++++- src/features/run/producers.ts | 4 +- src/features/run/rubyTargets.test.ts | 101 ++++++++++++ src/features/run/rubyTests.test.ts | 67 ++++++++ src/features/run/rubyTests.ts | 47 ++++++ src/features/run/targets.ts | 68 +++++++- src/features/run/testing/index.ts | 28 +++- src/features/run/testing/ruby.test.ts | 127 +++++++++++++++ src/features/run/testing/ruby.ts | 147 ++++++++++++++++++ src/features/run/testing/types.ts | 4 +- src/features/search/everywhere/deps.ts | 2 +- src/features/search/find/windowStore.ts | 2 +- .../search/common => platform}/textSearch.ts | 0 17 files changed, 683 insertions(+), 18 deletions(-) create mode 100644 src/features/run/rubyTargets.test.ts create mode 100644 src/features/run/rubyTests.test.ts create mode 100644 src/features/run/rubyTests.ts create mode 100644 src/features/run/testing/ruby.test.ts create mode 100644 src/features/run/testing/ruby.ts rename src/{features/search/common => platform}/textSearch.ts (100%) diff --git a/crates/ide-run/src/detect.rs b/crates/ide-run/src/detect.rs index 7e7d49b0d..9b1e98bc9 100644 --- a/crates/ide-run/src/detect.rs +++ b/crates/ide-run/src/detect.rs @@ -1059,8 +1059,9 @@ fn ruby_config( c } -/// Loads the test files of the directories (or files) in `ARGV`, default `test`: the way `rake test` does. -const MINITEST_RUNNER: &str = r#"dirs = ARGV.empty? ? ["test"] : ARGV.dup; ARGV.clear; dirs.flat_map { |d| File.file?(d) ? [d] : Dir[File.join(d, "**", "{*_test,test_*}.rb")] }.sort.each { |f| require File.expand_path(f) }"#; +/// Loads the test files of the directories (or files) named in `ARGV`, default `test`, the way `rake test` +/// does; options such as `-v` stay in `ARGV` for Minitest. +const MINITEST_RUNNER: &str = r#"paths, opts = ARGV.partition { |a| !a.start_with?("-") }; ARGV.replace(opts); (paths.empty? ? ["test"] : paths).flat_map { |d| File.file?(d) ? [d] : Dir[File.join(d, "**", "{*_test,test_*}.rb")] }.sort.each { |f| require File.expand_path(f) }"#; fn uses_rspec(root: &Path) -> bool { root.join("spec").is_dir() diff --git a/src/features/run/context.ts b/src/features/run/context.ts index fe8c40b9c..26ba771fa 100644 --- a/src/features/run/context.ts +++ b/src/features/run/context.ts @@ -21,7 +21,7 @@ export function fileRole(relPath: string, content?: string, windows = isWindows( (django && name === "tests.py") || /_test\.go$/.test(name) || /Test\.php$/.test(name) || - /_(spec|test)\.rb$/.test(name) || + /_(spec|test)\.rb$|^test_.*\.rb$/.test(name) || /(Test|Tests|Spec|IT)\.(kt|java|scala|groovy)$/.test(name) || (/(^|\/)tests\/[^/]+\.rs$/.test(relPath) && name !== "mod.rs") ) @@ -43,8 +43,8 @@ function testKinds(relPath: string): string[] { if (/\.[cm]?[jt]sx?$/.test(relPath)) return ["npm", "deno", "bun"]; if (/\.py$/.test(relPath)) return ["pytest", "python", "django-test"]; if (/\.php$/.test(relPath)) return ["phpunit"]; - if (/_spec\.rb$/.test(relPath)) return ["rspec", "rake"]; - if (/_test\.rb$/.test(relPath)) return ["rake", "rspec"]; + if (/_spec\.rb$/.test(relPath)) return ["rspec", "rake", "ruby"]; + if (/(^|\/)test_[^/]*\.rb$|_test\.rb$/.test(relPath)) return ["rake", "rspec", "ruby"]; if (/\.go$/.test(relPath)) return ["go"]; if (/\.rs$/.test(relPath)) return ["cargo"]; if (/\.(kt|java|scala|groovy)$/.test(relPath)) return ["gradle", "maven"]; @@ -180,6 +180,9 @@ export function testConfigFor(relPath: string, configs: RunConfig[], base?: RunC const key = /(^|:)spec$/.test(task) ? "SPEC" : /(^|:)test$/.test(task) ? "TEST" : null; return key ? derived(base, relPath, { args: [...base.args, `${key}=${file}`] }) : base; } + case "ruby": + // The detected Minitest runner (`ruby … -e test`): its last argument is what to load. + return derived(base, relPath, { args: [...base.args.slice(0, -1), file] }); case "django-test": { const tests = djangoTestsOf(base, [djangoLabel(file)]); return tests ? derived(base, relPath, tests) : null; diff --git a/src/features/run/links.ts b/src/features/run/links.ts index 563118bb2..cd195ecb5 100644 --- a/src/features/run/links.ts +++ b/src/features/run/links.ts @@ -12,9 +12,11 @@ export interface FileTarget { /** * `path`: resolve against the run directory; `name`: a file name from a JVM * stack frame — look it up in the project (`path` then is the file under its - * package directory, `com/acme/OrderController.kt`, when the frame names the class). + * package directory, `com/acme/OrderController.kt`, when the frame names the class); + * `rubyTest`: a Minitest test with no file in its output — `className` is its class, `path` its + * method name — found by searching the project (`rubyTests.ts`). */ - resolve: "path" | "name"; + resolve: "path" | "name" | "rubyTest"; /** JVM frames: the class's binary name (`com.acme.OrderController$Inner`). */ className?: string; } diff --git a/src/features/run/navigate.ts b/src/features/run/navigate.ts index a81dcfae1..3d4eee6ac 100644 --- a/src/features/run/navigate.ts +++ b/src/features/run/navigate.ts @@ -2,7 +2,9 @@ import { call, commands, isTauri, nextRequestId } from "@/platform/ipc"; import { useProject } from "@/platform/project"; import { editorService, notifications } from "@/workbench/contributions"; +import { streamSearch } from "@/platform/textSearch"; import { joinPath, type FileTarget } from "./links"; +import { findRubyTest } from "./rubyTests"; import { tx } from "@/platform/i18n"; async function exists(path: string): Promise { @@ -90,8 +92,45 @@ export async function resolveTarget(target: FileTarget, cwd: string): Promise out.push(f), + ); + return out.sort((a, b) => a.relPath.localeCompare(b.relPath)); +} + export async function openTarget(target: FileTarget, cwd: string, focus = true): Promise { if (!isTauri()) return; + if (target.resolve === "rubyTest") { + const site = target.className + ? await findRubyTest(searchRuby, target.className, target.path).catch(() => null) + : null; + if (!site) { + notifications().notify({ + group: "run", + level: "warning", + title: "File not found", + body: `${target.className ?? ""}#${target.path}`, + key: "run-link", + }); + return; + } + await editorService().openFile(site.path, { selection: { line: site.line - 1, col: 0 }, focus }); + return; + } const path = await resolveTarget(target, cwd); if (!path && target.className) { // A library or JDK frame without sources: nothing to open, which is no error. diff --git a/src/features/run/producers.ruby.test.ts b/src/features/run/producers.ruby.test.ts index bcf37acb1..d37ff00c2 100644 --- a/src/features/run/producers.ruby.test.ts +++ b/src/features/run/producers.ruby.test.ts @@ -10,7 +10,7 @@ vi.mock("@/platform/projectRoots", () => ({ })); import type { RunConfig } from "@/platform/ipc"; -import { produceContext, produceDirectory, type ToolEnv } from "./producers"; +import { produceContext, produceDirectory, produceFile, type ToolEnv } from "./producers"; const env: ToolEnv = { tool: (n) => (n === "ruby" ? { path: "/bin/ruby", version: null } : null), @@ -73,3 +73,46 @@ describe("Ruby directory run", () => { expect(dir("test/unit", [])).toBeNull(); }); }); + +describe("Ruby test files", () => { + const rake = { + id: "rake:test", + name: "rake test", + kind: "rake", + command: "bundle", + args: ["exec", "rake", "test"], + cwd: null, + env: {}, + detected: true, + isTest: true, + debugAdapter: null, + } as RunConfig; + const minitest = { + ...rake, + id: "minitest", + name: "minitest", + kind: "ruby", + command: "ruby", + args: ["-Ilib", "-Itest", "-e", "RUNNER", "test"], + } as RunConfig; + const file = (path: string, configs: RunConfig[]) => + produceFile({ path: `/work/app/${path}`, root: "/work/app", configs, content: "" }, env); + + it("test_*.rb is a test file: rake test with TEST=, reported per test", () => { + const p = file("test/unit/test_decide.rb", [rake]); + expect(p?.config).toMatchObject({ isTest: true, args: ["exec", "rake", "test", "TEST=test/unit/test_decide.rb"] }); + }); + + it("without a rake test task the detected Minitest runner loads the file", () => { + const p = file("test/unit/test_decide.rb", [minitest]); + expect(p?.config).toMatchObject({ + isTest: true, + kind: "ruby", + args: ["-Ilib", "-Itest", "-e", "RUNNER", "test/unit/test_decide.rb"], + }); + }); + + it("other scripts stay plain runs", () => { + expect(file("lib/tool.rb", [rake])?.config?.isTest).toBe(false); + }); +}); diff --git a/src/features/run/producers.ts b/src/features/run/producers.ts index ce9347c72..aef877345 100644 --- a/src/features/run/producers.ts +++ b/src/features/run/producers.ts @@ -972,7 +972,7 @@ function elementProduction(input: ContextInput, env: ToolEnv, target: RunTarget) export function produceContext(input: ContextInput, env: ToolEnv): Produced | null { if (input.content !== undefined && input.line !== undefined) { const targets = fileTargets(input, input.content.slice(0, 400_000)); - const inner = innermostTarget(input.content, targets, input.line, /\.py$/.test(input.path)); + const inner = innermostTarget(input.content, targets, input.line, /\.(py|rb)$/.test(input.path)); if (inner) { const p = elementProduction(input, env, inner); if (p) return p; @@ -997,7 +997,7 @@ export function produceContextChoices(input: ContextInput, env: ToolEnv): Produc let element: RunTarget | null = null; if (produced.scope === "element") { const targets = fileTargets(input, (input.content ?? "").slice(0, 400_000)); - element = innermostTarget(input.content ?? "", targets, input.line ?? 0, /\.py$/.test(input.path)); + element = innermostTarget(input.content ?? "", targets, input.line ?? 0, /\.(py|rb)$/.test(input.path)); if (!element || element.kind === "main" || element.kind === "generate") return [produced]; } else if (!produced.config.isTest) { return [produced]; diff --git a/src/features/run/rubyTargets.test.ts b/src/features/run/rubyTargets.test.ts new file mode 100644 index 000000000..d65f06a4f --- /dev/null +++ b/src/features/run/rubyTargets.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from "vitest"; +import type { RunConfig } from "@/platform/ipc"; +import { detectRunTargets, targetConfig, targetLanguage } from "./targets"; + +const SRC = `require_relative "../test_helper" + +module Outer + class InnerTest < Minitest::Test + def test_one = assert true + + def test_two? + assert true + end + + def helper = 1 + end + + class Helper + def test_not_a_test_class = 1 + end +end + +class TopTest < ActiveSupport::TestCase + test "does a thing" do + assert true + end +end + +module Empty + class Plain + end +end +`; + +const cfg = (over: Partial & { id: string; kind: string }): RunConfig => ({ + name: over.id, + command: "bundle", + args: [], + cwd: null, + env: {}, + detected: true, + isTest: true, + debugAdapter: null, + ...over, +}); + +describe("Ruby run targets", () => { + const targets = detectRunTargets(SRC, "test/unit/test_outer.rb"); + const find = (name: string) => targets.find((t) => t.name === name)!; + + it("is a target language", () => expect(targetLanguage("a.rb")).toBe("ruby")); + + it("finds test classes, the modules holding them, and their test methods", () => { + expect(targets.map((t) => [t.kind, t.name, t.line])).toEqual([ + ["suite", "Outer", 2], + ["suite", "Outer::InnerTest", 3], + ["test", "Outer::InnerTest#test_one", 4], + ["test", "Outer::InnerTest#test_two?", 6], + ["suite", "TopTest", 18], + ["test", "TopTest#test_does_a_thing", 19], + ]); + expect(find("Outer").rubyModule).toBe(true); + expect(find("Outer::InnerTest").path).toEqual(["Outer::InnerTest"]); + expect(find("Outer::InnerTest#test_one").path).toEqual(["Outer::InnerTest", "test_one"]); + }); + + it("leaves modules without test classes, non-test classes and non-test files alone", () => { + expect(targets.some((t) => t.name === "Empty" || t.name.includes("Helper"))).toBe(false); + expect(detectRunTargets(SRC, "lib/outer.rb")).toEqual([]); + expect(detectRunTargets(SRC, "spec/outer_spec.rb")).toEqual([]); + }); + + describe("run configurations", () => { + const rake = cfg({ id: "rake:test", kind: "rake", args: ["exec", "rake", "test"] }); + const minitest = cfg({ id: "minitest", kind: "ruby", command: "ruby", args: ["-Ilib", "-e", "RUNNER", "test"] }); + const run = (t: string, base: RunConfig) => targetConfig(find(t), "test/unit/test_outer.rb", [base], SRC)?.args; + + it("one test by its exact name, through rake and through the Minitest runner", () => { + expect(run("Outer::InnerTest#test_one", rake)).toEqual([ + "exec", + "rake", + "test", + "TEST=test/unit/test_outer.rb", + "TESTOPTS=--name=Outer::InnerTest#test_one", + ]); + expect(run("Outer::InnerTest#test_one", minitest)).toEqual([ + "-Ilib", + "-e", + "RUNNER", + "test/unit/test_outer.rb", + "--name=Outer::InnerTest#test_one", + ]); + }); + + it("a class runs its own tests, a module the tests of the classes inside it", () => { + expect(run("Outer::InnerTest", minitest)!.at(-1)).toBe("--name=/^Outer::InnerTest#/"); + expect(run("Outer", minitest)!.at(-1)).toBe("--name=/^Outer::/"); + expect(run("Outer", rake)!.at(-1)).toBe("TESTOPTS=--name=/^Outer::/"); + }); + }); +}); diff --git a/src/features/run/rubyTests.test.ts b/src/features/run/rubyTests.test.ts new file mode 100644 index 000000000..99b7f9b3a --- /dev/null +++ b/src/features/run/rubyTests.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import type { FileMatches } from "@/platform/ipc"; +import { findRubyTest, type RubySearch } from "./rubyTests"; + +const file = (path: string, ...lines: number[]): FileMatches => ({ + path, + relPath: path.slice(1), + matches: lines.map((line) => ({ line, text: "", ranges: [], textOffset: 0 })), +}); + +/** A search over fixed hits: the first matching pattern fragment wins. */ +function fake(hits: Record): RubySearch & { calls: [string, string[] | undefined][] } { + const calls: [string, string[] | undefined][] = []; + const fn = (async (pattern: string, files?: string[]) => { + calls.push([pattern, files]); + const key = Object.keys(hits).find((k) => pattern.includes(k)); + const found = key ? hits[key]! : []; + return files ? found.filter((f) => files.includes(f.path)) : found; + }) as RubySearch & { calls: [string, string[] | undefined][] }; + fn.calls = calls; + return fn; +} + +describe("finding a Minitest test's source", () => { + it("the def inside the file that declares the class", async () => { + const search = fake({ + "(class|module)": [file("/p/test/a_test.rb", 0), file("/p/test/b_test.rb", 0)], + "def\\s+test_init": [file("/p/test/a_test.rb", 11), file("/p/test/b_test.rb", 4)], + }); + // Only b declares TestB: its def wins even though a's file sorts first. + const owners = fake({ + "(class|module)": [file("/p/test/b_test.rb", 0)], + "def\\s+test_init": [file("/p/test/a_test.rb", 11), file("/p/test/b_test.rb", 4)], + }); + expect(await findRubyTest(owners, "TestB", "test_initialize")).toEqual({ path: "/p/test/b_test.rb", line: 5 }); + expect(await findRubyTest(search, "TestA", "test_initialize")).toEqual({ path: "/p/test/a_test.rb", line: 12 }); + }); + + it("looks for the last segment of a namespaced class", async () => { + const search = fake({ + InnerTest: [file("/p/test/n_test.rb", 3)], + "def\\s+test_nested": [file("/p/test/n_test.rb", 4)], + }); + expect(await findRubyTest(search, "Outer::InnerTest", "test_nested")).toEqual({ + path: "/p/test/n_test.rb", + line: 5, + }); + expect(search.calls[0]![0]).not.toContain("Outer"); + }); + + it("spec-style tests: the it block of the describe, underscores for spaces", async () => { + const search = fake({ + describe: [file("/p/spec/x_test.rb", 0)], + "does[ _]a[ _]thing": [file("/p/spec/x_test.rb", 6)], + }); + expect(await findRubyTest(search, "Spec style", "test_0001_does_a_thing")).toEqual({ + path: "/p/spec/x_test.rb", + line: 7, + }); + }); + + it("falls back to the class when the method is not found, and to nothing when nothing is", async () => { + const onlyClass = fake({ "(class|module)": [file("/p/test/a_test.rb", 2)] }); + expect(await findRubyTest(onlyClass, "TestA", "test_dynamic")).toEqual({ path: "/p/test/a_test.rb", line: 3 }); + expect(await findRubyTest(fake({}), "TestA", "test_x")).toBeNull(); + }); +}); diff --git a/src/features/run/rubyTests.ts b/src/features/run/rubyTests.ts new file mode 100644 index 000000000..093941bda --- /dev/null +++ b/src/features/run/rubyTests.ts @@ -0,0 +1,47 @@ +/** + * Where a Minitest test is defined. Minitest's output names `Class#test_name` and no file for tests that + * pass, so the tree finds the source by searching the project: the class, then the `def` in its file. + * Spec-style tests (`describe "Thing" do it "does x"`) are reported as `Thing#test_0001_does_x`. + */ +import type { FileMatches } from "@/platform/ipc"; + +/** Runs a regular-expression search over the project's Ruby files, or over `files` when given. */ +export type RubySearch = (pattern: string, files?: string[]) => Promise; + +export interface RubyTestSite { + path: string; + /** 1-based. */ + line: number; +} + +const escape = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +const SPEC_TEST = /^test_\d+_(.+)$/; + +const first = (hits: FileMatches[], file?: string) => { + const hit = file ? hits.find((h) => h.path === file) : hits[0]; + return hit?.matches[0] ? { path: hit.path, line: hit.matches[0].line + 1 } : null; +}; + +export async function findRubyTest(search: RubySearch, className: string, test: string): Promise { + const spec = SPEC_TEST.exec(test); + // `Outer::InnerTest` is declared as `class InnerTest` inside `module Outer`, or as written. + const last = className.split("::").pop()!; + const declared = spec + ? `^\\s*(describe|context)\\s*\\(?\\s*(["'])${escape(className)}\\2` + : `^\\s*(class|module)\\s+([A-Z]\\w*::)*${escape(last)}\\b`; + const owners = await search(declared); + const files = owners.map((o) => o.path); + const inOwners = (pattern: string) => (files.length ? search(pattern, files) : search(pattern)); + const body = spec + ? // Spec descriptions lose their spaces to underscores in the method name. + `^\\s*it\\s*\\(?\\s*(["'])${escape(spec[1]!).replace(/_/g, "[ _]")}\\1` + : `^\\s*def\\s+${escape(test)}\\b`; + const hits = await inOwners(body); + // The same method name is common (`test_initialize`): prefer the file that declares the class. + const site = + first( + hits, + files.find((f) => hits.some((h) => h.path === f)), + ) ?? first(hits); + return site ?? first(owners); +} diff --git a/src/features/run/targets.ts b/src/features/run/targets.ts index 11bd6d4df..ef7573120 100644 --- a/src/features/run/targets.ts +++ b/src/features/run/targets.ts @@ -2,7 +2,7 @@ * Run targets inside a file, for the gutter ▶ icons and "Run test" code lenses: * `fn main`, `#[test]` functions and `mod tests`, JS/TS `test`/`it`/`describe`, * pytest `def test_*` / `class Test*`, unittest `TestCase` subclasses, Go `func TestX(t *testing.T)`, - * JUnit `@Test`. + * JUnit `@Test`, Minitest / Test::Unit classes, modules holding them and `def test_*`. * Line-based regex detection (cheap, no parser), plus the run configuration for * one target derived from the project's detected test configurations. */ @@ -34,12 +34,14 @@ export interface RunTarget { spring?: boolean; /** Python: a unittest `TestCase` subclass or one of its methods (what Django's runner can run). */ unittest?: boolean; + /** Ruby: a `module` suite, which runs the tests of every class declared inside it. */ + rubyModule?: boolean; } /** Above this size we skip detection (generated / minified files). */ export const MAX_SCAN = 400_000; -type Lang = "rust" | "js" | "python" | "php" | "go" | "jvm" | null; +type Lang = "rust" | "js" | "python" | "php" | "go" | "jvm" | "ruby" | null; export function targetLanguage(relPath: string): Lang { if (/\.rs$/.test(relPath)) return "rust"; @@ -48,6 +50,7 @@ export function targetLanguage(relPath: string): Lang { if (/\.php$/.test(relPath)) return "php"; if (/\.go$/.test(relPath)) return "go"; if (/\.(java|kt|kts|scala|groovy)$/.test(relPath)) return "jvm"; + if (/\.rb$/.test(relPath)) return "ruby"; return null; } @@ -211,6 +214,53 @@ function pythonTargets(lines: string[]): RunTarget[] { return out; } +/** A class that holds Minitest / Test::Unit tests: by its superclass or its name. */ +const isRubyTestClass = (name: string, superclass: string | undefined) => + /(Test|TestCase|Spec)$/.test(superclass ?? "") || /Tests?(Case)?$/.test(name.split("::").pop()!); + +/** + * Minitest / Test::Unit: test classes, the modules that hold them (run together) and their + * `def test_*` methods (ActiveSupport's `test "does x" do` is `test_does_x`). Frames are tracked + * by indentation, as Python's are. Results name a class by its full name (`Outer::InnerTest`). + */ +function rubyTargets(lines: string[]): RunTarget[] { + const frames: Array<{ indent: number; name: string; test: boolean; at: number; hasTests: boolean }> = []; + const out: Array = []; + const fullName = () => frames.map((f) => f.name).join("::"); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + if (!line.trim() || /^\s*#/.test(line)) continue; + const indent = indentOf(line); + while (frames.length && frames[frames.length - 1]!.indent >= indent) frames.pop(); + const decl = /^\s*(class|module)\s+([A-Z]\w*(?:::[A-Z]\w*)*)(?:\s*<\s*([\w:]+))?/.exec(line); + if (decl) { + const test = decl[1] === "class" && isRubyTestClass(decl[2]!, decl[3]); + const frame = { indent, name: decl[2]!, test, at: i, hasTests: false }; + frames.push(frame); + out.push({ + line: i, + kind: "suite", + name: fullName(), + path: [fullName()], + ...(decl[1] === "module" ? { rubyModule: true } : {}), + frame, + }); + continue; + } + const owner = frames[frames.length - 1]; + if (!owner?.test) continue; + const method = + /^\s*def\s+(test_\w*[?!]?)/.exec(line)?.[1] ?? + (/^\s*test\s*\(?\s*(["'])(.+?)\1/.exec(line)?.[2] ?? "").replace(/\s+/g, "_").replace(/^(?=.)/, "test_"); + if (method && method !== "test_") { + for (const f of frames) f.hasTests = true; + out.push({ line: i, kind: "test", name: `${fullName()}#${method}`, path: [fullName(), method] }); + } + } + // A class or module that holds no tests is noise. + return out.filter((t) => t.kind !== "suite" || t.frame?.hasTests).map(({ frame: _frame, ...t }) => t); +} + /** `go test` reports subtest names with spaces as `_` (`t.Run("zero case")` → `TestX/zero_case`). */ export const goSubtestName = (name: string) => name.replace(/\s/g, "_"); @@ -367,6 +417,9 @@ export function detectRunTargets(text: string, relPath: string, django = false): return goTargets(lines).filter((t) => t.kind === "main" || t.kind === "generate" || /_test\.go$/.test(relPath)); case "jvm": return /\.(java|kts?)$/.test(relPath) ? javaKotlinTargets(text, relPath) : jvmTargets(lines, relPath); + case "ruby": + // RSpec files (`spec/`) are not Minitest. + return /(^|\/)test_[^/]*\.rb$|_test\.rb$|(^|\/)tests?\//.test(relPath) ? rubyTargets(lines) : []; default: return []; } @@ -456,6 +509,17 @@ export function targetConfig( }; return result; } + case "rake": + case "ruby": { + // Minitest's `--name=`: `Class#test_x` names one test exactly, a regexp (no `$`, which a shell would + // eat) the rest. The `=` form is one argument: rake's test loader would `require` the value of `-n x`. + const full = target.path[0]!; + const pattern = + target.kind === "test" ? `${full}#${target.path[1]}` : target.rubyModule ? `/^${full}::/` : `/^${full}#/`; + return file.kind === "rake" + ? named([...file.args, `TESTOPTS=--name=${pattern}`]) + : named([...file.args, `--name=${pattern}`]); + } case "cargo": { const path = target.path.join("::"); return named(target.kind === "suite" ? [...file.args, "--", `${path}::`] : [...file.args, "--", path, "--exact"]); diff --git a/src/features/run/testing/index.ts b/src/features/run/testing/index.ts index 889e49099..33704129f 100644 --- a/src/features/run/testing/index.ts +++ b/src/features/run/testing/index.ts @@ -6,13 +6,14 @@ import { jsParser } from "./js"; import { jvmParser } from "./jvm"; import { phpunitParser, withTeamcity } from "./phpunit"; import { pytestParser } from "./pytest"; +import { rubyParser } from "./ruby"; import { unittestParser } from "./unittest"; import type { TestEvent, TestOutputParser } from "./types"; export type { TestEvent, TestOutputParser, TestStatus } from "./types"; export { parseDuration } from "./types"; export { TestResults, nodeStatus, type TestNode } from "./results"; -export { cargoParser, goParser, jsParser, jvmParser, phpunitParser, pytestParser, unittestParser }; +export { cargoParser, goParser, jsParser, jvmParser, phpunitParser, pytestParser, rubyParser, unittestParser }; /** Runs several parsers side by side (unknown tool: e.g. `make test`). */ export function combinedParser(parsers: TestOutputParser[]): TestOutputParser { @@ -22,7 +23,7 @@ export function combinedParser(parsers: TestOutputParser[]): TestOutputParser { }; } -export type Framework = "cargo" | "js" | "pytest" | "unittest" | "phpunit" | "go" | "jvm" | "auto"; +export type Framework = "cargo" | "js" | "pytest" | "unittest" | "phpunit" | "go" | "jvm" | "ruby" | "auto"; export function frameworkFor(config: Pick): Framework { const cmd = [config.command, ...config.args].join(" "); @@ -47,6 +48,10 @@ export function frameworkFor(config: Pick>(config: T): T { + if (config.kind === "rspec") return config; + if (config.kind === "rake") { + const at = config.args.findIndex((a) => a.startsWith("TESTOPTS=")); + if (at < 0) return { ...config, args: [...config.args, "TESTOPTS=-v"] }; + // Options already given (`--name=Class#test_x`): add ours unless verbosity was chosen. + return /(^|\s)(-v|--verbose)(\s|$)/.test(config.args[at]!) + ? config + : { ...config, args: config.args.map((a, i) => (i === at ? `${a} -v` : a)) }; + } + const chosen = config.args.some((a) => a === "-v" || a === "--verbose"); + return chosen ? config : { ...config, args: [...config.args, "-v"] }; +} + /** * Launch arguments that make a test run reportable per test: pytest and * Django print only progress dots by default, so passed tests would never @@ -79,6 +99,7 @@ export function withTestReporting /^(-v|--verbosity)(=?\d)?$/.test(a)); return chosen ? config : { ...config, args: [...config.args, "-v", "2"] }; } + if (frameworkFor(config) === "ruby") return rubyVerbose(config); if (frameworkFor(config) !== "pytest") return config; const cmd = [config.command, ...config.args]; const pytestAt = cmd.findIndex((a) => /(^|[/\\])pytest(\.exe)?$/.test(a)); @@ -103,6 +124,8 @@ export function createTestParser(config: Pick { + const events = parseAll(rubyParser(), MINITEST); + const results = new TestResults(); + results.apply(events); + + it("reports every test under its class, with its status", () => { + const status = (cls: string, name: string) => nodeStatus(results.get([cls, name].join("\u0000"))!); + expect(results.roots.map((r) => r.name).sort()).toEqual(["MixedTest", "Outer::InnerTest", "Spec style"]); + expect(status("MixedTest", "test_pass")).toBe("passed"); + expect(status("MixedTest", "test_fail")).toBe("failed"); + expect(status("MixedTest", "test_error")).toBe("failed"); + expect(status("MixedTest", "test_skip")).toBe("skipped"); + expect(status("MixedTest", "test_prints")).toBe("passed"); + expect(status("Spec style", "test_0001_works")).toBe("passed"); + expect(status("Outer::InnerTest", "test_nested")).toBe("passed"); + }); + + it("keeps a test's own output with it, and the result printed after it", () => { + expect(events).toContainEqual({ type: "output", path: ["MixedTest", "test_prints"], line: "hello from test" }); + expect(events.filter((e) => e.type === "result" && e.path[1] === "test_prints")).toHaveLength(1); + }); + + it("attaches the failure text and locates it", () => { + const out = events + .filter((e) => e.type === "output" && e.path[1] === "test_fail") + .map((e) => (e as { line: string }).line); + expect(out).toEqual(expect.arrayContaining(["Expected: 4", " Actual: 3"])); + expect(events).toContainEqual({ + type: "location", + path: ["MixedTest", "test_fail"], + location: { path: "test/mixed_test.rb", line: 5, resolve: "path" }, + }); + // An error has no location in its header: the first frame gives it. + expect(events).toContainEqual({ + type: "location", + path: ["MixedTest", "test_error"], + location: { path: "test/mixed_test.rb", line: 6, resolve: "path" }, + }); + }); + + it("locates every test by class and name, once, for jumping to its source", () => { + const located = events.filter((e) => e.type === "location" && e.location.resolve === "rubyTest"); + expect(located).toHaveLength(7); + expect(located).toContainEqual({ + type: "location", + path: ["MixedTest", "test_pass"], + location: { path: "test_pass", className: "MixedTest", resolve: "rubyTest" }, + }); + // The first location wins in the tree: passing tests have no other. + expect(results.get(["MixedTest", "test_pass"].join("\u0000"))!.location?.resolve).toBe("rubyTest"); + }); + + it("summarises the run", () => { + expect(events.at(-1)).toEqual({ type: "summary", passed: 4, failed: 2, skipped: 1, durationMs: 0.893 }); + }); +}); + +describe("Ruby test reporting", () => { + const cfg = (kind: string, command: string, args: string[]) => ({ kind, command, args }); + + it("asks Minitest for one line per test", () => { + expect(withTestReporting(cfg("ruby", "ruby", ["-Ilib", "test/a_test.rb"])).args).toEqual([ + "-Ilib", + "test/a_test.rb", + "-v", + ]); + expect(withTestReporting(cfg("rake", "bundle", ["exec", "rake", "test"])).args).toEqual([ + "exec", + "rake", + "test", + "TESTOPTS=-v", + ]); + expect(withTestReporting(cfg("ruby", "ruby", ["a_test.rb", "-v"])).args).toEqual(["a_test.rb", "-v"]); + expect(withTestReporting(cfg("rake", "rake", ["test", "TESTOPTS=--seed 1"])).args).toEqual([ + "test", + "TESTOPTS=--seed 1 -v", + ]); + expect(withTestReporting(cfg("rake", "rake", ["test", "TESTOPTS=--name=Foo#test_a"])).args).toEqual([ + "test", + "TESTOPTS=--name=Foo#test_a -v", + ]); + // RSpec has no parser yet: untouched. + expect(withTestReporting(cfg("rspec", "rspec", ["spec"])).args).toEqual(["spec"]); + }); + + it("is the framework of Ruby configurations", () => { + expect(frameworkFor(cfg("ruby", "ruby", []))).toBe("ruby"); + expect(frameworkFor(cfg("rake", "bundle", ["exec", "rake", "test"]))).toBe("ruby"); + }); +}); diff --git a/src/features/run/testing/ruby.ts b/src/features/run/testing/ruby.ts new file mode 100644 index 000000000..a73fe6cbf --- /dev/null +++ b/src/features/run/testing/ruby.ts @@ -0,0 +1,147 @@ +/** + * Minitest's verbose output (`-v`; Test::Unit's minitest shim prints the same): one + * `Class#test_name = 0.00 s = .` line per test (`.` passed, `F` failed, `E` error, `S` skipped), + * then the failure sections and the summary. Tests are reported as `[Class, test_name]`. + * + * ```text + * MixedTest#test_prints = hello from test <- the test's own output comes first, + * 0.00 s = . the result follows on a line of its own + * + * 1) Failure: + * MixedTest#test_fail [test/mixed_test.rb:5]: + * Expected: 4 + * Actual: 3 + * + * 7 runs, 5 assertions, 1 failures, 1 errors, 1 skips + * ``` + */ +import { parseDuration, type TestEvent, type TestOutputParser, type TestStatus } from "./types"; + +// A class may be a spec description with spaces (`Spec style#test_0001_works`). +const RESULT_RE = /^(.+?)#(\S+) = ([\d.]+ s) = ([.FES])$/; +const STARTED_RE = /^(.+?)#(\S+) = (.*)$/; +/** The result of a test that printed output first. */ +const LATE_RESULT_RE = /^([\d.]+ s) = ([.FES])$/; +const SECTION_RE = /^\s*\d+\) (Failure|Error|Skipped):$/; +const SECTION_TEST_RE = /^(.+?)#(\S+?)(?: \[(.+?):(\d+)\])?:$/; +const FRAME_RE = /^\s+(\S+?):(\d+):in /; +const SUMMARY_RE = /^(\d+) runs, \d+ assertions, (\d+) failures, (\d+) errors, (\d+) skips/; +const FINISHED_RE = /^Finished in ([\d.]+s)/; +/** Gems and the standard library never locate a failure. */ +const LIBRARY_RE = /[/\\](?:gems|rubygems|lib[/\\]ruby)[/\\]/; + +const STATUS: Record> = { + ".": "passed", + F: "failed", + E: "failed", + S: "skipped", +}; + +/** Minitest names no file for a passing test: its location is searched for when the user jumps to it. */ +const located = (path: string[]): TestEvent => ({ + type: "location", + path, + location: { path: path[1]!, className: path[0], resolve: "rubyTest" }, +}); + +export function rubyParser(): TestOutputParser { + const seen = new Set(); + /** The test's own location, once. */ + const locate = (path: string[]): TestEvent[] => { + const id = path.join("\0"); + if (seen.has(id)) return []; + seen.add(id); + return [located(path)]; + }; + /** A test whose result line has not been printed yet: its own output came first. */ + let pending: string[] | null = null; + let section: { path: string[] | null; located: boolean; expectTest: boolean } | null = null; + let finished: number | undefined; + + return { + feed(line) { + const events: TestEvent[] = []; + let m: RegExpExecArray | null; + + if ((m = SUMMARY_RE.exec(line))) { + section = null; + const [runs, failures, errors, skips] = [m[1], m[2], m[3], m[4]].map(Number) as [ + number, + number, + number, + number, + ]; + events.push({ + type: "summary", + passed: Math.max(0, runs - failures - errors - skips), + failed: failures + errors, + skipped: skips, + durationMs: finished, + }); + return events; + } + if ((m = FINISHED_RE.exec(line))) { + finished = parseDuration(m[1]); + pending = null; + return events; + } + if (SECTION_RE.test(line)) { + section = { path: null, located: false, expectTest: true }; + return events; + } + if (section) { + if (section.expectTest) { + const t = SECTION_TEST_RE.exec(line); + section.expectTest = false; + if (t) { + section.path = [t[1]!, t[2]!]; + events.push({ type: "output", path: section.path, line }); + if (t[3]) { + section.located = true; + events.push({ + type: "location", + path: section.path, + location: { path: t[3], line: Number(t[4]), resolve: "path" }, + }); + } + return events; + } + } + if (section.path) { + events.push({ type: "output", path: section.path, line }); + const frame = FRAME_RE.exec(line); + // An error has no `[file:line]` in its header: its first frame outside the libraries is the place. + if (frame && !section.located && !LIBRARY_RE.test(frame[1]!)) { + section.located = true; + events.push({ + type: "location", + path: section.path, + location: { path: frame[1]!, line: Number(frame[2]), resolve: "path" }, + }); + } + } + return events; + } + + if ((m = RESULT_RE.exec(line))) { + const path = [m[1]!, m[2]!]; + events.push({ type: "result", path, status: STATUS[m[4]!]!, durationMs: parseDuration(m[3]) }, ...locate(path)); + pending = null; + return events; + } + if (pending && (m = LATE_RESULT_RE.exec(line))) { + events.push({ type: "result", path: pending, status: STATUS[m[2]!]!, durationMs: parseDuration(m[1]) }); + pending = null; + return events; + } + if ((m = STARTED_RE.exec(line)) && /^(test|\w*_\d+)/.test(m[2]!)) { + pending = [m[1]!, m[2]!]; + events.push({ type: "start", path: pending }, ...locate(pending)); + if (m[3]) events.push({ type: "output", path: pending, line: m[3] }); + return events; + } + if (pending) events.push({ type: "output", path: pending, line }); + return events; + }, + }; +} diff --git a/src/features/run/testing/types.ts b/src/features/run/testing/types.ts index a54f25005..0985ff4b7 100644 --- a/src/features/run/testing/types.ts +++ b/src/features/run/testing/types.ts @@ -10,8 +10,8 @@ export interface TestLocation { /** 1-based, as printed. */ line?: number; col?: number; - /** `name`: a file name to look up in the project (JVM / Go frames). */ - resolve: "path" | "name"; + /** `name`: a file name to look up in the project (JVM / Go frames); `rubyTest`: a test to search for (`rubyTests.ts`). */ + resolve: "path" | "name" | "rubyTest"; /** JVM frames: the class's binary name; `path` is then the file under its package directory. */ className?: string; } diff --git a/src/features/search/everywhere/deps.ts b/src/features/search/everywhere/deps.ts index f3c5c9e39..237cb78f6 100644 --- a/src/features/search/everywhere/deps.ts +++ b/src/features/search/everywhere/deps.ts @@ -4,7 +4,7 @@ import { call, commands, nextRequestId, type LanguageInfo } from "@/platform/ipc import { inFileSet } from "@/platform/projectRoots"; import { searchEndpoints, searchableSettings } from "@/workbench/contributions"; import { projectRoot } from "../common/open"; -import { cancelRequest, streamSearch } from "../common/textSearch"; +import { cancelRequest, streamSearch } from "@/platform/textSearch"; import type { SeDeps } from "./controller"; import { recentActions } from "./store"; diff --git a/src/features/search/find/windowStore.ts b/src/features/search/find/windowStore.ts index 1a8e03aaa..a1b12cd50 100644 --- a/src/features/search/find/windowStore.ts +++ b/src/features/search/find/windowStore.ts @@ -8,7 +8,7 @@ import type { FindResultsSource } from "@/workbench/contributions"; import type { MenuItem } from "@/ui"; import { loadJson, saveJson } from "../common/open"; import { maybeToolWindowService } from "../common/services"; -import { cancelRequest, streamSearch } from "../common/textSearch"; +import { cancelRequest, streamSearch } from "@/platform/textSearch"; import { FindSession, IDLE, countFileMatches, type FindDeps, type FindState } from "./session"; export interface FindTab { diff --git a/src/features/search/common/textSearch.ts b/src/platform/textSearch.ts similarity index 100% rename from src/features/search/common/textSearch.ts rename to src/platform/textSearch.ts From 9fb38769ada4a64f160a046d085b93c281fb63ce Mon Sep 17 00:00:00 2001 From: Jacob Kjeldahl Date: Thu, 5 Nov 2026 14:39:36 +0100 Subject: [PATCH 004/185] feat(run): load a Minitest reporter into Ruby test runs (#1547) Parsing Minitest's console output needs -v, gives no file or line for passing tests and depends on how output interleaves. Ruby test runs (ruby and rake test configurations in the console) now load a small Minitest reporter through RUBYLIB/RUBYOPT that prints one record per test on stdout with its status, time, failure text and real file and line. The console strips the records and the test tree is built from them, so jump to source gets exact locations; the text parser stays as the fallback. It works on Minitest 5 and 6 (6 no longer loads plugins on its own, so a small hook registers the reporter once Minitest is loaded). Co-authored-by: kjeldahl <19719+kjeldahl@users.noreply.github.com> --- crates/ide-host/src/ops/run.rs | 16 +++ crates/ide-run/src/lib.rs | 1 + crates/ide-run/src/process.rs | 6 ++ crates/ide-run/src/ruby_reporter.rs | 52 ++++++++++ .../src/ruby_reporter/minitest_reporter.rb | 78 +++++++++++++++ .../src/ruby_reporter/openidea_test_hook.rb | 32 ++++++ crates/ide-run/tests/process.rs | 30 ++++++ src/features/run/store.ts | 10 +- src/features/run/testing/results.ts | 4 +- src/features/run/testing/ruby.test.ts | 97 ++++++++++++++++++- src/features/run/testing/ruby.ts | 82 +++++++++++++++- 11 files changed, 402 insertions(+), 6 deletions(-) create mode 100644 crates/ide-run/src/ruby_reporter.rs create mode 100644 crates/ide-run/src/ruby_reporter/minitest_reporter.rb create mode 100644 crates/ide-run/src/ruby_reporter/openidea_test_hook.rb diff --git a/crates/ide-host/src/ops/run.rs b/crates/ide-host/src/ops/run.rs index a71a8da5e..7409f0a32 100644 --- a/crates/ide-host/src/ops/run.rs +++ b/crates/ide-host/src/ops/run.rs @@ -198,6 +198,7 @@ pub async fn start(host: &Host, window: &str, config: RunConfig, on_output: Emit let options = StartOptions { encoding: Some(encoding).filter(|e| !e.trim().is_empty()), python_paths: if *root != project.root { vec![root.clone()] } else { super::python::paths(&project) }, + ruby_reporter: ruby_reporter_dir(host, &config), }; project.require_trust()?; let mut sink = project.sink.clone(); @@ -214,6 +215,21 @@ pub async fn start(host: &Host, window: &str, config: RunConfig, on_output: Emit host.processes.start(&project.root, &config, &options, sink, Box::new(move |c| on_output(c))) } +/// The Ruby test reporter's directory, installed on demand, for runs that take it. +fn ruby_reporter_dir(host: &Host, config: &RunConfig) -> Option { + if !ide_run::ruby_reporter::applies(config) { + return None; + } + let dir = host.local_data_dir.join("ruby-reporter"); + match ide_run::ruby_reporter::install(&dir) { + Ok(()) => Some(dir), + Err(e) => { + tracing::warn!("cannot install the Ruby test reporter in {}: {e}", dir.display()); + None + } + } +} + /// Stops a run (interrupt, then kill after Settings › Run's termination timeout). pub fn stop(host: &Host, window: &str, id: RunId) -> IdeResult<()> { let settings = match host.project(window) { diff --git a/crates/ide-run/src/lib.rs b/crates/ide-run/src/lib.rs index 97e313423..4b2493041 100644 --- a/crates/ide-run/src/lib.rs +++ b/crates/ide-run/src/lib.rs @@ -13,6 +13,7 @@ pub mod junit; mod process; pub mod python_path; pub mod ruby_preload; +pub mod ruby_reporter; pub mod tomcat; mod utf8; pub mod which; diff --git a/crates/ide-run/src/process.rs b/crates/ide-run/src/process.rs index 2899c026a..9deb76d46 100644 --- a/crates/ide-run/src/process.rs +++ b/crates/ide-run/src/process.rs @@ -80,6 +80,8 @@ pub struct StartOptions { pub encoding: Option, /// Project content and source roots added to Python launch environments. pub python_paths: Vec, + /// Where the Ruby test reporter is installed (see [`crate::ruby_reporter`]); Ruby test runs load it. + pub ruby_reporter: Option, } #[derive(Default)] @@ -355,6 +357,10 @@ impl ProcessManager { if matches!(config.kind.as_str(), "python" | "pytest" | "django-test") { crate::python_path::extend(&mut env.vars, &options.python_paths, !env.clear)?; } + if let Some(dir) = options.ruby_reporter.as_deref().filter(|_| crate::ruby_reporter::applies(config)) { + crate::ruby_preload::add(&mut env.vars, dir, crate::ruby_reporter::FEATURE, !env.clear); + env.vars.insert("OPENIDEA_TEST_REPORT".into(), "1".into()); + } let on_output: OutputFn = Arc::from(on_output); let on_output = match extras.output_file.as_deref() { Some(file) => tee_output(&resolve_cwd(root, Some(Path::new(file))), on_output)?, diff --git a/crates/ide-run/src/ruby_reporter.rs b/crates/ide-run/src/ruby_reporter.rs new file mode 100644 index 000000000..c689d4ddd --- /dev/null +++ b/crates/ide-run/src/ruby_reporter.rs @@ -0,0 +1,52 @@ +//! A Minitest reporter the IDE loads into test runs (see `ruby_reporter/`): it prints one record per +//! test on stdout, which the IDE strips from the console and turns into the test tree. Tests then +//! report their real source file and line, and the output no longer needs `-v` to be parsed. +use crate::RunConfig; +use std::io; +use std::path::Path; + +/// The Ruby file `RUBYOPT` loads first (`/openidea_test_hook.rb`). +pub const FEATURE: &str = "openidea_test_hook"; + +const FILES: [(&str, &str); 2] = [ + ("openidea_test_hook.rb", include_str!("ruby_reporter/openidea_test_hook.rb")), + ("openidea/minitest_reporter.rb", include_str!("ruby_reporter/minitest_reporter.rb")), +]; + +/// Whether a run takes the reporter: Minitest-style test runs through `ruby` or `rake`, with their output +/// in the console (a terminal would show the records). +pub fn applies(config: &RunConfig) -> bool { + config.is_test && matches!(config.kind.as_str(), "ruby" | "rake") && !config.extras.emulate_terminal +} + +/// Writes the reporter files under `dir` (unchanged files stay). +pub fn install(dir: &Path) -> io::Result<()> { + for (name, text) in FILES { + let path = dir.join(name); + if std::fs::read_to_string(&path).is_ok_and(|current| current == text) { + continue; + } + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + std::fs::write(&path, text)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn installs_the_files_once_and_only_for_ruby_test_runs() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path().join("ruby reporter"); + install(&dir).unwrap(); + install(&dir).unwrap(); + for (name, text) in FILES { + assert_eq!(std::fs::read_to_string(dir.join(name)).unwrap(), text); + } + assert!(FILES[0].1.contains("OPENIDEA_TEST_REPORT") && FILES[1].1.contains("@@OPENIDEA@@")); + } +} diff --git a/crates/ide-run/src/ruby_reporter/minitest_reporter.rb b/crates/ide-run/src/ruby_reporter/minitest_reporter.rb new file mode 100644 index 000000000..a657ad4e9 --- /dev/null +++ b/crates/ide-run/src/ruby_reporter/minitest_reporter.rb @@ -0,0 +1,78 @@ +# Written by OpenIDEA for test runs; registered by `openidea_test_hook.rb` once Minitest is loaded. +# Nothing in the project is changed. +# +# One record per line on stdout, which the IDE strips from the console again: +# @@OPENIDEA@@{"t":"start","c":"MyTest","n":"test_x","f":"/abs/test.rb","l":12} +# @@OPENIDEA@@{"t":"result","c":"MyTest","n":"test_x","s":"passed","d":0.001,"f":...,"l":...,"m":"failure text"} +# @@OPENIDEA@@{"t":"summary","runs":7,"failures":1,"errors":1,"skips":1,"d":0.4} +require "json" + +module Minitest + class OpenIdeaReporter < AbstractReporter + MARK = "@@OPENIDEA@@" + + def initialize(io) + super() + @io = io + @cwd = Dir.pwd + @lock = Mutex.new + @runs = @failures = @errors = @skips = 0 + @started = nil + end + + def start + @started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + + def prerecord(klass, name) + emit(t: "start", c: klass.to_s, n: name.to_s, **where(klass, name)) + end + + def record(result) + @runs += 1 + status = result.skipped? ? "skipped" : result.passed? ? "passed" : "failed" + @skips += 1 if status == "skipped" + if status == "failed" + result.failures.any? { |f| f.is_a?(UnexpectedError) } ? @errors += 1 : @failures += 1 + end + loc = result.source_location + emit(t: "result", c: result.klass.to_s, n: result.name.to_s, s: status, d: result.time, + f: loc && absolute(loc[0]), l: loc && loc[1], m: result.failures.map(&:message).join("\n")) + end + + def report + d = @started ? Process.clock_gettime(Process::CLOCK_MONOTONIC) - @started : nil + emit(t: "summary", runs: @runs, failures: @failures, errors: @errors, skips: @skips, d: d) + end + + private + + # Where the test is defined, known before it runs (so the tree can show it while running). + def where(klass, name) + f, l = klass.instance_method(name).source_location + { f: absolute(f), l: l } + rescue StandardError + {} + end + + # Test files are often loaded by a path relative to the working directory. + def absolute(file) + File.expand_path(file, @cwd) + end + + def emit(record) + line = "#{MARK}#{JSON.generate(record)}\n" + @lock.synchronize { @io.write(line) } + rescue StandardError + nil # reporting must never fail a test run + end + end + + def self.plugin_openidea_init(options) + return unless ENV["OPENIDEA_TEST_REPORT"] + + io = options[:io] || $stdout + io.sync = true if io.respond_to?(:sync=) + reporter << OpenIdeaReporter.new(io) + end +end diff --git a/crates/ide-run/src/ruby_reporter/openidea_test_hook.rb b/crates/ide-run/src/ruby_reporter/openidea_test_hook.rb new file mode 100644 index 000000000..57a72084e --- /dev/null +++ b/crates/ide-run/src/ruby_reporter/openidea_test_hook.rb @@ -0,0 +1,32 @@ +# Written by OpenIDEA for test runs. Loaded first in every Ruby the run starts +# (RUBYLIB=, RUBYOPT=-ropenidea_test_hook). +# +# Registers the test reporter once a test framework is loaded. Minitest 5 would find a plugin on +# its own, Minitest 6 does not load plugins any more, so both are registered the same way. +if ENV["OPENIDEA_TEST_REPORT"] + module OpenIdeaTestHook + @registered = false + + def self.register + return if @registered + return unless defined?(::Minitest) && ::Minitest.respond_to?(:extensions) && defined?(::Minitest::AbstractReporter) + + @registered = true + require "openidea/minitest_reporter" + ::Minitest.extensions << "openidea" unless ::Minitest.extensions.include?("openidea") + rescue StandardError, LoadError + nil # reporting must never break a test run + end + + # `require "minitest/autorun"` loads Minitest through require_relative, so look after every require. + module Watch + def require(path) + result = super + OpenIdeaTestHook.register + result + end + end + end + + Kernel.prepend(OpenIdeaTestHook::Watch) +end diff --git a/crates/ide-run/tests/process.rs b/crates/ide-run/tests/process.rs index 176be79c9..cbb2ba914 100644 --- a/crates/ide-run/tests/process.rs +++ b/crates/ide-run/tests/process.rs @@ -479,6 +479,36 @@ fn python_scripts_import_sibling_packages_and_preserve_env_file_paths() { } } +#[test] +fn ruby_test_runs_load_the_reporter_and_other_runs_do_not() { + let reporter = std::env::temp_dir().join("ruby reporter"); + let run = |kind: &str, is_test: bool| { + let h = harness(); + let mut config = + cfg("ruby-env", "/bin/sh", &["-c", r#"echo "lib=$RUBYLIB opt=$RUBYOPT report=$OPENIDEA_TEST_REPORT""#]); + config.kind = kind.into(); + config.is_test = is_test; + let options = StartOptions { ruby_reporter: Some(reporter.clone()), ..Default::default() }; + let chunks: Chunks = Arc::default(); + let sink = chunks.clone(); + let info = + h.pm.start(h.root.path(), &config, &options, h.sink.clone(), Box::new(move |c| sink.lock().push(c))) + .unwrap(); + assert_eq!(wait_exit(&h, info.id, Duration::from_secs(10)), Some(0)); + text(&chunks, stdout) + }; + for kind in ["ruby", "rake"] { + let out = run(kind, true); + // The directory has a space: it is on RUBYLIB and only the bare feature name is in RUBYOPT. + assert!(out.contains(&format!("lib={}", reporter.display())), "{out}"); + assert!(out.contains("-ropenidea_test_hook") && !out.contains(&format!("-r{}", reporter.display())), "{out}"); + assert!(out.contains("report=1"), "{out}"); + } + for (kind, is_test) in [("ruby", false), ("rspec", true), ("pytest", true)] { + assert!(run(kind, is_test).contains("report=\n"), "{kind} {is_test}"); + } +} + #[test] fn revoking_one_project_stops_only_its_runs() { let h = harness(); diff --git a/src/features/run/store.ts b/src/features/run/store.ts index 6c542050c..5ed72249f 100644 --- a/src/features/run/store.ts +++ b/src/features/run/store.ts @@ -43,6 +43,7 @@ import { } from "./testing"; import { GoTestOutput } from "./testing/go"; import { moveToReportedClasses } from "./testing/jvm"; +import { RubyTestOutput } from "./testing/ruby"; import { TeamcityOutput } from "./testing/teamcity"; import type { TestStdout } from "./testing/types"; import { recordTestStates } from "./testing/states"; @@ -87,7 +88,7 @@ interface SessionRuntime { console: ConsoleModel; tests: TestResults | null; parser: TestOutputParser | null; - /** Machine-readable stdout (`go test -json`, `--teamcity`): fed to the parser, shown decoded. */ + /** Machine-readable stdout (`go test -json`, `--teamcity`, the Ruby reporter): fed to the parser, shown decoded. */ stdout: TestStdout | null; queue: OutputChunk[]; /** rAF id while the page is visible, timeout id while hidden (rAF pauses in background tabs). */ @@ -567,13 +568,16 @@ function newRuntime(config: RunConfig): SessionRuntime { if (tests && parser) tests.apply(parser.feed(line)); }; const framework = tests && parser ? frameworkFor(config) : null; - // PHPUnit `--teamcity` and Gradle's test init script report through service messages. + // PHPUnit `--teamcity` and Gradle's test init script report through service messages, Minitest through + // the IDE's own reporter (RSpec has none: its progress dots must show as they come). const stdout = framework === "go" ? new GoTestOutput(feed) : framework === "phpunit" || framework === "jvm" ? new TeamcityOutput(feed) - : null; + : framework === "ruby" && config.kind !== "rspec" + ? new RubyTestOutput(feed) + : null; const runtime: SessionRuntime = { stdout, tests, diff --git a/src/features/run/testing/results.ts b/src/features/run/testing/results.ts index e0c75aca7..9b67583f9 100644 --- a/src/features/run/testing/results.ts +++ b/src/features/run/testing/results.ts @@ -128,7 +128,9 @@ export class TestResults { } case "location": { const n = this.node(e.path); - n.location ??= e.location; + // The first location wins, except that a place to search for (Ruby tests) gives way to a real one. + if (!n.location || (n.location.resolve === "rubyTest" && e.location.resolve !== "rubyTest")) + n.location = e.location; break; } case "output": { diff --git a/src/features/run/testing/ruby.test.ts b/src/features/run/testing/ruby.test.ts index 769ce3f27..1139bc54d 100644 --- a/src/features/run/testing/ruby.test.ts +++ b/src/features/run/testing/ruby.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { parseAll, withTestReporting, frameworkFor } from "./index"; import { TestResults, nodeStatus } from "./results"; -import { rubyParser } from "./ruby"; +import { RubyTestOutput, rubyParser } from "./ruby"; // Captured from `ruby -Ilib test/mixed_test.rb -v --seed 1` (Minitest 6.0.6). const MINITEST = `Run options: -v --seed 1 @@ -125,3 +125,98 @@ describe("Ruby test reporting", () => { expect(frameworkFor(cfg("rake", "bundle", ["exec", "rake", "test"]))).toBe("ruby"); }); }); + +// Captured from a run with the IDE's reporter (Minitest 6.0.6): records end with their own newline, so the +// progress dots stay on one line once they are taken out. +const M = "@@OPENIDEA@@"; +const F = "/p/test/mixed_test.rb"; +const REPORTER_STREAM = [ + "# Running:", + "", + `${M}{"t":"start","c":"MixedTest","n":"test_pass","f":"${F}","l":4}`, + `.${M}{"t":"result","c":"MixedTest","n":"test_pass","s":"passed","d":0.000063,"f":"${F}","l":4,"m":""}`, + `${M}{"t":"start","c":"MixedTest","n":"test_fail","f":"${F}","l":5}`, + `F${M}{"t":"result","c":"MixedTest","n":"test_fail","s":"failed","d":0.000054,"f":"${F}","l":5,"m":"Expected: 4\\n Actual: 3"}`, + `${M}{"t":"start","c":"MixedTest","n":"test_prints","f":"${F}","l":8}`, + "hello from test", + `.${M}{"t":"result","c":"MixedTest","n":"test_prints","s":"passed","d":0.000007,"f":"${F}","l":8,"m":""}`, + `${M}{"t":"start","c":"MixedTest","n":"test_skip","f":"${F}","l":7}`, + `S${M}{"t":"result","c":"MixedTest","n":"test_skip","s":"skipped","d":0.000006,"f":"${F}","l":7,"m":"later"}`, + "", + `${M}{"t":"summary","runs":4,"failures":1,"errors":0,"skips":1,"d":0.0011}`, + "Finished in 0.001124s", + "", +].join("\n"); + +describe("the IDE's Minitest reporter", () => { + const collect = (chunks: string[]) => { + const lines: string[] = []; + const out = new RubyTestOutput((line) => lines.push(line)); + const text = chunks.map((c) => out.feed(c)).join("") + out.end(); + return { text, records: lines.filter((l) => l.includes(M)), lines }; + }; + + it("takes the records out of the console, exactly as Minitest prints them without it", () => { + const { text, records } = collect([REPORTER_STREAM]); + expect(text).toBe("# Running:\n\n.Fhello from test\n.S\nFinished in 0.001124s\n"); + expect(records).toHaveLength(9); + expect(records.every((r) => r.includes(M))).toBe(true); + }); + + it("is the same however the output is cut into chunks", () => { + const whole = collect([REPORTER_STREAM]); + const chars = collect(REPORTER_STREAM.split("")); + const odd = collect(REPORTER_STREAM.match(/[\s\S]{1,7}/g)!); + expect(chars).toEqual(whole); + expect(odd).toEqual(whole); + }); + + it("drops a record cut off by the end of the process, not the text before it", () => { + expect(collect([`abc${M}{"t":"sta`]).text).toBe("abc"); + expect(collect(["no marker @@OPEN"]).text).toBe("no marker @@OPEN\n"); + }); + + describe("results", () => { + const parser = rubyParser(); + const results = new TestResults(); + for (const l of collect([REPORTER_STREAM]).lines) results.apply(parser.feed(l)); + // The console's own text arrives too: the same results, printed. + for (const l of ["MixedTest#test_pass = 0.00 s = .", "1 runs, 1 assertions, 0 failures, 0 errors, 0 skips"]) + results.apply(parser.feed(l)); + const node = (name: string) => results.get(["MixedTest", name].join("\u0000"))!; + + it("builds the tree with statuses, times and failure text", () => { + expect(nodeStatus(node("test_pass"))).toBe("passed"); + expect(nodeStatus(node("test_fail"))).toBe("failed"); + expect(nodeStatus(node("test_skip"))).toBe("skipped"); + expect(node("test_fail").output).toEqual(["Expected: 4", " Actual: 3"]); + expect(node("test_pass").durationMs).toBeCloseTo(0.063, 3); + }); + + it("gives every test its real file and line, passed or not", () => { + for (const [name, line] of [ + ["test_pass", 4], + ["test_fail", 5], + ["test_prints", 8], + ["test_skip", 7], + ] as const) + expect(node(name).location).toEqual({ path: F, line, resolve: "path" }); + }); + + it("summarises from the reporter, and ignores the repeated text", () => { + expect(results.summary).toMatchObject({ passed: 2, failed: 1, skipped: 1 }); + expect(results.get("MixedTest")!.children).toHaveLength(4); + }); + }); + + it("a real location replaces the place to search for", () => { + const results = new TestResults(); + results.apply(parseAll(rubyParser(), "MixedTest#test_pass = 0.00 s = .")); + const id = ["MixedTest", "test_pass"].join("\u0000"); + expect(results.get(id)!.location?.resolve).toBe("rubyTest"); + results.apply([ + { type: "location", path: ["MixedTest", "test_pass"], location: { path: F, line: 4, resolve: "path" } }, + ]); + expect(results.get(id)!.location).toEqual({ path: F, line: 4, resolve: "path" }); + }); +}); diff --git a/src/features/run/testing/ruby.ts b/src/features/run/testing/ruby.ts index a73fe6cbf..bc7d49ce2 100644 --- a/src/features/run/testing/ruby.ts +++ b/src/features/run/testing/ruby.ts @@ -15,7 +15,7 @@ * 7 runs, 5 assertions, 1 failures, 1 errors, 1 skips * ``` */ -import { parseDuration, type TestEvent, type TestOutputParser, type TestStatus } from "./types"; +import { parseDuration, TestStdout, type TestEvent, type TestOutputParser, type TestStatus } from "./types"; // A class may be a spec description with spaces (`Spec style#test_0001_works`). const RESULT_RE = /^(.+?)#(\S+) = ([\d.]+ s) = ([.FES])$/; @@ -44,7 +44,81 @@ const located = (path: string[]): TestEvent => ({ location: { path: path[1]!, className: path[0], resolve: "rubyTest" }, }); +/** Starts a record of the IDE's own Minitest reporter (`ide-run/src/ruby_reporter`); a JSON object follows. */ +export const RECORD_MARK = "@@OPENIDEA@@"; + +interface ReporterRecord { + t: "start" | "result" | "summary"; + c?: string; + n?: string; + s?: "passed" | "failed" | "skipped"; + /** Seconds. */ + d?: number | null; + f?: string | null; + l?: number | null; + m?: string; + runs?: number; + failures?: number; + errors?: number; + skips?: number; +} + +/** What a reporter record says: the test's real file and line, its status, time and failure text. */ +function fromRecord(json: string): TestEvent[] { + let r: ReporterRecord; + try { + r = JSON.parse(json) as ReporterRecord; + } catch { + return []; + } + if (r.t === "summary") { + const failed = (r.failures ?? 0) + (r.errors ?? 0); + const skipped = r.skips ?? 0; + return [ + { + type: "summary", + passed: Math.max(0, (r.runs ?? 0) - failed - skipped), + failed, + skipped, + durationMs: typeof r.d === "number" ? Math.round(r.d * 1000 * 1000) / 1000 : undefined, + }, + ]; + } + if (!r.c || !r.n) return []; + const path = [r.c, r.n]; + const events: TestEvent[] = []; + if (r.t === "start") events.push({ type: "start", path }); + if (r.t === "result" && r.s) { + events.push({ + type: "result", + path, + status: r.s, + durationMs: typeof r.d === "number" ? Math.round(r.d * 1000 * 1000) / 1000 : undefined, + }); + for (const line of (r.m ?? "").split("\n")) if (line) events.push({ type: "output", path, line }); + } + if (r.f && r.l) { + events.push({ type: "location", path, location: { path: r.f, line: r.l, resolve: "path" } }); + } + return events; +} + +/** + * The console side of the reporter: a record ends with its own newline, so what the line holds before the + * marker (a progress dot) is all the console shows of it. + */ +export class RubyTestOutput extends TestStdout { + constructor(feedLine: (line: string) => void) { + super(feedLine, (line) => { + const at = line.indexOf(RECORD_MARK); + return at < 0 ? line + "\n" : line.slice(0, at); + }); + } +} + export function rubyParser(): TestOutputParser { + /** The reporter's records are the source of results; the text is then ignored (it would repeat them). */ + let records = false; const seen = new Set(); /** The test's own location, once. */ const locate = (path: string[]): TestEvent[] => { @@ -60,6 +134,12 @@ export function rubyParser(): TestOutputParser { return { feed(line) { + const mark = line.indexOf(RECORD_MARK); + if (mark >= 0) { + records = true; + return fromRecord(line.slice(mark + RECORD_MARK.length)); + } + if (records) return []; const events: TestEvent[] = []; let m: RegExpExecArray | null; From 36fd117b295de183185493513ef165757df3699c Mon Sep 17 00:00:00 2001 From: Hacbit <107751081+hacbit@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:42:08 +0800 Subject: [PATCH 005/185] feat(vcs): stage Unity metadata with new assets (#1509) In a Unity project, Add File to Git for a new asset under Assets/ now takes its .meta along: an untracked .meta that already exists is listed in the same confirmation, and one Unity writes later is added once the asset's addition has succeeded. The ask / silently / never setting applies as before; pending metadata is dropped when the asset is unstaged or deleted, or the project or repository changes. File-operation paths are compared in IPC form, so they match the status paths on Windows. Closes #1429 Co-authored-by: hacbit <107751081+hacbit@users.noreply.github.com> --- .../vcs/__tests__/confirmations.test.ts | 454 ++++++++++++++++++ src/features/vcs/confirmations.tsx | 35 +- src/features/vcs/unityMetadata.ts | 172 +++++++ 3 files changed, 651 insertions(+), 10 deletions(-) create mode 100644 src/features/vcs/__tests__/confirmations.test.ts create mode 100644 src/features/vcs/unityMetadata.ts diff --git a/src/features/vcs/__tests__/confirmations.test.ts b/src/features/vcs/__tests__/confirmations.test.ts new file mode 100644 index 000000000..181403923 --- /dev/null +++ b/src/features/vcs/__tests__/confirmations.test.ts @@ -0,0 +1,454 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { FileStatus, ProjectInfo } from "@/bindings"; + +const mocks = vi.hoisted(() => ({ stage: vi.fn(), ask: vi.fn(), refresh: vi.fn(), notify: vi.fn(), mode: "ask" })); +vi.mock("@/platform/ipc", async (original) => { + const real = await original(); + return { + ...real, + isTauri: () => true, + commands: { ...real.commands, vcsStage: mocks.stage }, + }; +}); +vi.mock("@/workbench/messages", () => ({ ask: mocks.ask })); +vi.mock("@/platform/workflowSettings", () => ({ + workflowSettings: () => ({ vcs: { confirmation: { onCreate: mocks.mode, onDelete: "never" } } }), +})); +vi.mock("../store", async () => { + const { create } = await import("zustand"); + return { useVcs: create(() => ({})), refreshNow: mocks.refresh }; +}); +import { notifyFileOperation } from "@/platform/fileOps"; +import { useProject } from "@/platform/project"; +import { provideNotificationService } from "@/workbench/contributions"; +import { installFileConfirmations } from "../confirmations"; +import { clearConsole } from "../ops"; +import { useVcs } from "../store"; + +const asset = "/p/Assets/New.cs"; +const meta = asset + ".meta"; +const file = ( + path: string, + staged: FileStatus["staged"] = null, + unstaged: FileStatus["unstaged"] = "untracked", +): FileStatus => ({ path, relPath: path.slice(3), staged, unstaged, renamedFrom: null }); +const publish = (...status: FileStatus[]) => useVcs.setState({ status }); +const createFile = (path = asset) => notifyFileOperation({ kind: "created", paths: [path] }); +const settle = async () => { + for (let i = 0; i < 30; i++) await Promise.resolve(); +}; +let dispose: () => void; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.mode = "ask"; + mocks.ask.mockResolvedValue({ button: "ok", checked: false }); + mocks.refresh.mockResolvedValue(undefined); + mocks.stage.mockImplementation(async (paths: string[]) => { + publish( + ...useVcs + .getState() + .status.map((f) => (paths.includes(f.path) ? { ...f, staged: "added" as const, unstaged: null } : f)), + ); + return { status: "ok", data: null }; + }); + clearConsole(); + provideNotificationService({ notify: mocks.notify }); + useProject.setState({ project: { root: "/p", unity: {} } as ProjectInfo, opening: false, openingPath: null }); + useVcs.setState({ + available: true, + root: "/p", + roots: ["/p"], + repo: "/p", + statusLoaded: true, + loading: false, + error: null, + status: [file(asset)], + }); + dispose = installFileConfirmations(); +}); +afterEach(() => dispose()); + +describe("Unity new-file Git confirmations", () => { + it("includes existing metadata in the confirmation and stages both files", async () => { + publish(file(asset), file(meta)); + createFile(); + await settle(); + expect(mocks.ask.mock.calls[0]![0].body).toContain("Assets/New.cs.meta"); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset, meta]); + }); + + it("adds delayed metadata only, preserving later unstaged script edits", async () => { + createFile(); + await settle(); + publish(file(asset, "added", "modified"), file(meta), file("/p/Assets/Other.cs.meta")); + await settle(); + expect(mocks.stage.mock.calls.map(([paths]) => paths)).toEqual([[asset], [meta]]); + expect(useVcs.getState().status[0]!.unstaged).toBe("modified"); + expect(mocks.ask).toHaveBeenCalledTimes(1); + }); + + it("finds metadata generated while the confirmation is open", async () => { + mocks.ask.mockImplementationOnce(async () => { + publish(file(asset), file(meta)); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset, meta]); + }); + + it("also pairs metadata in silent mode", async () => { + mocks.mode = "silently"; + createFile(); + await settle(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.ask).not.toHaveBeenCalled(); + expect(mocks.stage).toHaveBeenLastCalledWith([meta]); + }); + + it("waits for a completed status refresh before adding delayed metadata", async () => { + createFile(); + await settle(); + useVcs.setState({ loading: true }); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + useVcs.setState({ loading: false }); + await settle(); + expect(mocks.stage).toHaveBeenLastCalledWith([meta]); + }); + + it("finds metadata generated during the initial staging operation", async () => { + mocks.stage.mockImplementationOnce(async () => { + publish(file(asset, "added", null), file(meta)); + return { status: "ok", data: null }; + }); + createFile(); + await settle(); + expect(mocks.stage.mock.calls.map(([paths]) => paths)).toEqual([[asset], [meta]]); + }); + + it.each(["declined", "never", "failure"])("does not follow metadata after %s", async (reason) => { + if (reason === "declined") mocks.ask.mockResolvedValue({ button: "cancel", checked: false }); + if (reason === "never") mocks.mode = "never"; + if (reason === "failure") mocks.stage.mockRejectedValueOnce(new Error("stage failed")); + createFile(); + await settle(); + const calls = mocks.stage.mock.calls.length; + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(calls); + }); + + it.each(["unstage", "delete", "switch", "opening", "repository", "dispose"])( + "cancels pending metadata after %s", + async (reason) => { + createFile(); + await settle(); + if (reason === "unstage") publish(file(asset)); + if (reason === "delete") publish(file(asset, "added", "deleted")); + if (reason === "switch") useProject.setState({ project: { root: "/other", unity: {} } as ProjectInfo }); + if (reason === "opening") { + useProject.setState({ opening: true, openingPath: "/other" }); + useProject.setState({ opening: false, openingPath: null }); + } + if (reason === "repository") { + useVcs.setState({ repo: "/p/other" }); + useVcs.setState({ repo: "/p" }); + } + if (reason === "dispose") dispose(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + }, + ); + + it.each(["normal project", "outside Assets", "metadata itself"])("does not pair files in %s", async (reason) => { + if (reason === "normal project") useProject.setState({ project: { root: "/p" } as ProjectInfo }); + const path = reason === "outside Assets" ? "/p/New.cs" : reason === "metadata itself" ? meta : asset; + publish(file(path), file(path + ".meta")); + createFile(path); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([path]); + }); + + it.each(["modified", "ignored"] as const)("does not stage %s metadata", async (kind) => { + createFile(); + await settle(); + publish(file(asset, "added", null), file(meta, null, kind)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + }); + + it("does not repeatedly retry a failed metadata addition", async () => { + createFile(); + await settle(); + mocks.stage.mockRejectedValueOnce(new Error("index locked")); + publish(file(asset, "added", null), file(meta)); + await settle(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(2); + expect(mocks.notify).toHaveBeenCalledWith(expect.objectContaining({ level: "error", body: "index locked" })); + }); + + it("does not stage files if the project changed while the dialog was open", async () => { + mocks.ask.mockImplementationOnce(async () => { + useProject.setState({ project: { root: "/other" } as ProjectInfo }); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it.each(["repository", "opening", "dispose"])("discards an open confirmation after %s", async (reason) => { + mocks.ask.mockImplementationOnce(async () => { + if (reason === "repository") useVcs.setState({ repo: "/p/nested" }); + if (reason === "opening") useProject.setState({ opening: true, openingPath: "/other" }); + if (reason === "dispose") dispose(); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it("does not stage a source or metadata staged by the user while the dialog was open", async () => { + publish(file(asset), file(meta)); + mocks.ask.mockImplementationOnce(async () => { + publish(file(asset, "added", "modified"), file(meta, "added", "modified")); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it("preserves manually staged metadata even when it was explicitly included in the creation event", async () => { + publish(file(asset), file(meta)); + mocks.ask.mockImplementationOnce(async () => { + publish(file(asset), file(meta, "added", "modified")); + return { button: "ok", checked: false }; + }); + notifyFileOperation({ kind: "created", paths: [asset, meta] }); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset]); + expect(useVcs.getState().status[1]!.unstaged).toBe("modified"); + }); + + it.each(["added", "modified", "ignored", "missing"] as const)( + "rechecks metadata that becomes %s while the dialog is open", + async (kind) => { + publish(file(asset), file(meta)); + mocks.ask.mockImplementationOnce(async () => { + publish( + file(asset), + ...(kind === "missing" + ? [] + : [file(meta, kind === "added" ? "added" : null, kind === "added" ? "modified" : kind)]), + ); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset]); + }, + ); + + it("waits for successful staging before following metadata", async () => { + let complete!: () => void; + mocks.stage.mockImplementationOnce( + () => + new Promise((resolve) => { + complete = () => resolve({ status: "ok", data: null }); + }), + ); + createFile(); + await settle(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + complete(); + await settle(); + expect(mocks.stage.mock.calls.map(([paths]) => paths)).toEqual([[asset], [meta]]); + }); + + it("remembers an unstage while the initial staging response is still in flight", async () => { + let complete!: () => void; + mocks.stage.mockImplementationOnce( + () => + new Promise((resolve) => { + complete = () => resolve({ status: "ok", data: null }); + }), + ); + createFile(); + await settle(); + publish(file(asset, "added", null)); + publish(file(asset)); + publish(file(asset, "added", null), file(meta)); + complete(); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset]); + }); + + it("does not re-add metadata already included in the successful initial operation", async () => { + publish(file(asset), file(meta)); + let complete!: () => void; + mocks.stage.mockImplementationOnce( + () => + new Promise((resolve) => { + complete = () => resolve({ status: "ok", data: null }); + }), + ); + createFile(); + await settle(); + publish(file(asset, "added", null), file(meta, "added", null)); + publish(file(asset, "added", null), file(meta)); + complete(); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset, meta]); + }); + + it("does not follow metadata after an in-flight addition outlives its project", async () => { + let complete!: () => void; + mocks.stage.mockImplementationOnce( + () => + new Promise((resolve) => { + complete = () => resolve({ status: "ok", data: null }); + }), + ); + createFile(); + await settle(); + useProject.setState({ opening: true, openingPath: "/other" }); + complete(); + await settle(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + }); + + it("ignores incomplete or failed status snapshots", async () => { + createFile(); + await settle(); + useVcs.setState({ statusLoaded: false }); + publish(file(asset)); + useVcs.setState({ statusLoaded: true, error: "refresh failed" }); + publish(file(asset, "added", "modified"), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + useVcs.setState({ error: null }); + await settle(); + expect(mocks.stage).toHaveBeenLastCalledWith([meta]); + }); + + it("does not cancel an addition when the first refresh discovers the repository", async () => { + useVcs.setState({ repo: null, roots: [], statusLoaded: false }); + mocks.refresh.mockImplementationOnce(async () => { + useVcs.setState({ roots: ["/p"], repo: "/p", statusLoaded: true }); + }); + createFile(); + await settle(); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage.mock.calls.map(([paths]) => paths)).toEqual([[asset], [meta]]); + }); + + it("does not stage from a failed pre-addition refresh", async () => { + mocks.ask.mockImplementationOnce(async () => { + useVcs.setState({ error: "refresh failed" }); + return { button: "ok", checked: false }; + }); + createFile(); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it("rechecks the project after the pre-staging promise resolves", async () => { + mocks.refresh.mockResolvedValueOnce(undefined).mockImplementationOnce(async () => { + queueMicrotask(() => queueMicrotask(() => useProject.setState({ opening: true, openingPath: "/other" }))); + }); + createFile(); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it("does not infer metadata consent for children of a copied directory", async () => { + createFile("/p/Assets"); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset]); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + }); + + it("keeps a batch limited to the explicitly created files", async () => { + const other = "/p/Assets/Other.cs"; + publish(file(asset), file(meta), file(other), file(`${other}.meta`)); + notifyFileOperation({ kind: "created", paths: [asset, asset] }); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([asset, meta]); + }); + + it.each([ + ["D:/Unity Project", "D:\\Unity Project\\Assets\\New.cs", "D:/Unity Project/Assets/New.cs"], + ["//server/share/project", "\\\\server\\share\\project\\Assets\\New.cs", "//server/share/project/Assets/New.cs"], + ["/p", "/p/Assets/Name\\Part.cs", "/p/Assets/Name\\Part.cs"], + ["/project space", "/project space/Assets/角色.cs", "/project space/Assets/角色.cs"], + ])("pairs files with native or literal path characters under %s", async (root, input, path) => { + useProject.setState({ project: { root, unity: {} } as ProjectInfo }); + useVcs.setState({ root, roots: [root], repo: root }); + publish(file(path), file(`${path}.meta`)); + createFile(input); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([path, `${path}.meta`]); + }); + + it.each(["/p/AssetsOther/New.cs", "/p/Packages/New.cs"])("does not pair %s outside Assets", async (path) => { + publish(file(path), file(`${path}.meta`)); + createFile(path); + await settle(); + expect(mocks.stage).toHaveBeenCalledExactlyOnceWith([path]); + }); + + it("does not add files from another project's similarly named path", async () => { + const other = "/project/Assets/New.cs"; + publish(file(other), file(`${other}.meta`)); + createFile(other); + await settle(); + expect(mocks.stage).not.toHaveBeenCalled(); + }); + + it("cancels metadata when the asset's repository disappears", async () => { + useVcs.setState({ roots: ["/p", "/p/Assets"] }); + createFile(); + await settle(); + useVcs.setState({ roots: ["/p"] }); + publish(file(asset, "added", null), file(meta)); + await settle(); + expect(mocks.stage).toHaveBeenCalledTimes(1); + }); + + it("still cancels other pending additions while metadata staging is in flight", async () => { + const other = "/p/Assets/Other.cs"; + publish(file(asset), file(other)); + notifyFileOperation({ kind: "created", paths: [asset, other] }); + await settle(); + let complete!: () => void; + mocks.stage.mockImplementationOnce( + () => + new Promise((resolve) => { + complete = () => resolve({ status: "ok", data: null }); + }), + ); + publish(file(asset, "added", null), file(meta), file(other, "added", null)); + await settle(); + publish(file(asset, "added", null), file(meta), file(other)); + publish(file(asset, "added", null), file(meta, "added", null), file(other, "added", null), file(`${other}.meta`)); + complete(); + await settle(); + expect(mocks.stage.mock.calls.map(([paths]) => paths)).toEqual([[asset, other], [meta]]); + }); +}); diff --git a/src/features/vcs/confirmations.tsx b/src/features/vcs/confirmations.tsx index b0a0ab7cb..b9145ad81 100644 --- a/src/features/vcs/confirmations.tsx +++ b/src/features/vcs/confirmations.tsx @@ -6,20 +6,20 @@ import { onFileOperation, type FileOperation } from "@/platform/fileOps"; import { tx } from "@/platform/i18n"; import { call, commands, isTauri } from "@/platform/ipc"; +import { ipcPath, isSameOrInside } from "@/platform/paths"; import { loadSettings } from "@/platform/project"; import { workflowSettings } from "@/platform/workflowSettings"; import { ask } from "@/workbench/messages"; import { effectiveKind, relTo, stripSlash } from "./model"; import { runOp } from "./ops"; import { refreshNow, useVcs } from "./store"; - -const inside = (root: string, p: string) => p === root || p.startsWith(root.endsWith("/") ? root : root + "/"); +import { createUnityMetadataStaging } from "./unityMetadata"; /** Status paths affected by `paths` (a deleted folder covers its content). */ function matching(paths: readonly string[], kind: "untracked" | "deleted"): string[] { const out = new Set(); for (const f of useVcs.getState().status) { - const p = stripSlash(f.path); + const p = stripSlash(ipcPath(f.path)); if (effectiveKind(f) !== kind || f.staged) continue; for (const q of paths) { if (p === q || p.startsWith(q + "/")) out.add(p); @@ -59,27 +59,33 @@ async function confirmFiles(op: FileOperation["kind"], files: string[]): Promise return { ok: r.button === "ok", remember: r.checked }; } -async function handle(op: FileOperation) { +async function handle(op: FileOperation, metadata: ReturnType) { const vcs = useVcs.getState(); if (!vcs.available || !vcs.root || !isTauri()) return; const conf = workflowSettings().vcs.confirmation; const mode = op.kind === "created" ? conf.onCreate : conf.onDelete; if (mode === "never") return; - const paths = op.paths.map(stripSlash).filter((p) => inside(vcs.root!, p)); + const paths = op.paths.map((p) => stripSlash(ipcPath(p))).filter((p) => isSameOrInside(p, vcs.root!)); if (!paths.length) return; + const addition = metadata.begin(op.kind === "created" ? paths : []); await refreshNow(); + if (!addition.current()) return; const files = matching(paths, op.kind === "created" ? "untracked" : "deleted"); if (!files.length) return; + const confirmed = addition.include(files); if (mode === "ask") { - const { ok, remember } = await confirmFiles(op.kind, files); + const { ok, remember } = await confirmFiles(op.kind, confirmed); + if (!addition.current()) return; if (remember) { const key = op.kind === "created" ? "vcs.confirmation.onCreate" : "vcs.confirmation.onDelete"; await rememberSetting(key, ok ? "silently" : "never"); } if (!ok) return; } - const n = files.length; - await runOp( + const stageFiles = await addition.beforeStage(files); + if (!addition.current() || !stageFiles.length) return; + const n = stageFiles.length; + const ok = await runOp( op.kind === "created" ? n === 1 ? tx("Add 1 file to Git") @@ -87,10 +93,19 @@ async function handle(op: FileOperation) { : n === 1 ? tx("Remove 1 file from Git") : tx`Remove ${n} files from Git`, - () => call(commands.vcsStage(files)), + async () => { + await call(commands.vcsStage(stageFiles)); + return true; + }, ); + await addition.finished(!!ok); } export function installFileConfirmations(): () => void { - return onFileOperation((op) => void handle(op)); + const metadata = createUnityMetadataStaging(); + const stopFiles = onFileOperation((op) => void handle(op, metadata)); + return () => { + stopFiles(); + metadata.dispose(); + }; } diff --git a/src/features/vcs/unityMetadata.ts b/src/features/vcs/unityMetadata.ts new file mode 100644 index 000000000..8bef662cf --- /dev/null +++ b/src/features/vcs/unityMetadata.ts @@ -0,0 +1,172 @@ +import type { FileStatus } from "@/bindings"; +import { tx } from "@/platform/i18n"; +import { call, commands } from "@/platform/ipc"; +import { ipcPath, isSameOrInside } from "@/platform/paths"; +import { useProject } from "@/platform/project"; +import { rootOf, stripSlash } from "./model"; +import { runOp } from "./ops"; +import { refreshNow, useVcs, type VcsState } from "./store"; + +const pathKey = (path: string) => stripSlash(ipcPath(path)); +const untracked = (file: FileStatus | undefined) => + file?.unstaged === "untracked" && !file.staged && !file.path.endsWith("/"); +const ready = (vcs: VcsState) => vcs.available && vcs.statusLoaded && !vcs.loading && !vcs.error; +const selectedRepo = (vcs: VcsState) => vcs.repo ?? vcs.roots[0] ?? null; +const statusByPath = () => new Map(useVcs.getState().status.map((file) => [pathKey(file.path), file])); + +interface PendingMetadata { + repo: string; + confirmed: boolean; + seenAdded: boolean; +} + +/** Pair only IDE-created files whose addition succeeded, for this project/repository session. */ +export function createUnityMetadataStaging() { + const pending = new Map(); + let generation = 0; + let disposed = false; + let staging = false; + const clear = () => { + generation++; + pending.clear(); + }; + + const drain = () => { + const vcs = useVcs.getState(); + if (disposed || !pending.size || !ready(vcs)) return; + const status = statusByPath(); + const files: string[] = []; + for (const [asset, entry] of pending) { + const source = status.get(asset); + const meta = status.get(`${asset}.meta`); + if (source?.staged === "added") entry.seenAdded = true; + if ( + ((entry.confirmed || entry.seenAdded) && source?.staged !== "added") || + source?.unstaged === "deleted" || + rootOf(asset, vcs.roots) !== entry.repo || + rootOf(`${asset}.meta`, vcs.roots) !== entry.repo || + (meta && !untracked(meta)) + ) { + pending.delete(asset); + } else if (meta && entry.confirmed && !staging) { + // Consume consent before invoking: a failed addition must not retry on every refresh. + pending.delete(asset); + files.push(meta.path); + } + } + if (!files.length) return; + staging = true; + void runOp(files.length === 1 ? tx("Add 1 file to Git") : tx`Add ${files.length} files to Git`, () => + call(commands.vcsStage(files)), + ).finally(() => { + staging = false; + drain(); + }); + }; + + const stopVcs = useVcs.subscribe((s, prev) => { + const before = selectedRepo(prev); + // The first refresh discovers the primary repository; that is not a user switch. + if (s.root !== prev.root || !s.available || (before && selectedRepo(s) !== before)) clear(); + else drain(); + }); + const stopProject = useProject.subscribe((s, prev) => { + if ( + s.project?.root !== prev.project?.root || + !!s.project?.unity !== !!prev.project?.unity || + (s.opening && !prev.opening) + ) + clear(); + }); + + return { + begin(paths: readonly string[]) { + const started = generation; + const current = () => !disposed && generation === started && !useProject.getState().opening; + const project = useProject.getState().project; + const assetsDir = project?.unity ? `${pathKey(project.root)}/Assets` : null; + const requested = new Set(paths.map(pathKey)); + const attempt = new Map(); + let assets: Map | undefined; + + const include = (files: readonly string[]) => { + const vcs = useVcs.getState(); + const status = statusByPath(); + const paired = (assets ??= new Map( + files.flatMap((file) => { + const path = pathKey(file); + const source = status.get(path); + const repo = rootOf(path, vcs.roots); + return assetsDir && + requested.has(path) && + path !== assetsDir && + isSameOrInside(path, assetsDir) && + !path.toLowerCase().endsWith(".meta") && + source && + untracked(source) && + repo && + rootOf(`${path}.meta`, vcs.roots) === repo + ? [[path, repo] as const] + : []; + }), + )); + const out = new Set( + files.filter((file) => { + const path = pathKey(file); + return !(paired.has(path) || paired.has(path.replace(/\.meta$/, ""))) || untracked(status.get(path)); + }), + ); + for (const [asset, repo] of paired) { + if (!untracked(status.get(asset)) || rootOf(asset, vcs.roots) !== repo) { + out.delete(asset); + out.delete(`${asset}.meta`); + paired.delete(asset); + continue; + } + const meta = status.get(`${asset}.meta`); + if (meta && untracked(meta) && rootOf(pathKey(meta.path), vcs.roots) === repo) out.add(meta.path); + } + return [...out]; + }; + + return { + current, + include, + async beforeStage(files: readonly string[]) { + if (assets?.size) { + await refreshNow(); + if (!ready(useVcs.getState())) return []; + } + if (!current()) return []; + const out = include(files); + const selected = new Set(out.map(pathKey)); + for (const [asset, repo] of assets ?? []) { + // Observe cancellation while the initial IPC is in flight, but do not + // add metadata until it succeeds. Already included metadata is finished. + if (selected.has(`${asset}.meta`)) continue; + const entry = { repo, confirmed: false, seenAdded: false }; + attempt.set(asset, entry); + pending.set(asset, entry); + } + return out; + }, + async finished(ok: boolean) { + for (const [asset, entry] of attempt) { + if (pending.get(asset) !== entry) continue; + if (ok && current()) entry.confirmed = true; + else pending.delete(asset); + } + if (!ok || !current() || !attempt.size) return; + await refreshNow(); + if (current()) drain(); + }, + }; + }, + dispose() { + disposed = true; + clear(); + stopVcs(); + stopProject(); + }, + }; +} From 324dded5c18b30a9326d38eb5a2493655ad6fc8e Mon Sep 17 00:00:00 2001 From: Jaw <53564781+jaw52@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:43:55 +0800 Subject: [PATCH 006/185] feat(vcs): mark diff and merge changes on the scrollbar (#1519) The diff and merge viewers had no overview lane, so changes outside the viewport were invisible. The scrollbar now marks every change, coloured by type (modified, inserted, deleted, conflict), in the side-by-side view (both panes), the aligned view (one stripe over the shared scroller), the unified view and the three-way merge editor; clicking a mark jumps to the change. Marks span the lane's width from the first to the last changed line with a 2px minimum. The colours are new --diff-*-stripe theme tokens (Darcula, Light and High-contrast values), mapped from ERROR_STRIPE_COLOR on .icls import and set in the Harbor Night theme template. Part of #1505 Co-authored-by: jaw52 <53564781+jaw52@users.noreply.github.com> --- docs/plugins/themes.md | 6 +- .../themes/harbor-night.theme.json | 4 + src/editor/api.ts | 8 +- src/editor/core/cm/lanes.ts | 70 +++++++--- src/editor/core/cm/theme.ts | 98 +++++++------- src/features/plugins/intellijTheme.ts | 18 ++- src/features/vcs/DiffEditor.module.css | 3 + src/features/vcs/DiffEditor.tsx | 6 +- src/features/vcs/MergeEditor.tsx | 3 + src/features/vcs/__tests__/diffStripe.test.ts | 121 ++++++++++++++++++ src/features/vcs/cm.ts | 63 ++++++++- src/features/vcs/mergeGutters.ts | 41 ++++++ src/styles/global.css | 4 + src/styles/tokens.css | 17 +++ 14 files changed, 393 insertions(+), 69 deletions(-) create mode 100644 src/features/vcs/__tests__/diffStripe.test.ts diff --git a/docs/plugins/themes.md b/docs/plugins/themes.md index 06f620a9d..96b639c2a 100644 --- a/docs/plugins/themes.md +++ b/docs/plugins/themes.md @@ -324,7 +324,7 @@ a token. -224 themeable tokens: 117 in the UI theme file, 107 in the editor colour scheme file. +228 themeable tokens: 121 in the UI theme file, 107 in the editor colour scheme file. ### Surfaces (UI theme) @@ -407,6 +407,10 @@ a token. | `--diff-modified` | `#385570` | `#c2d8f2` | | `--diff-inserted` | `#294436` | `#bee6be` | | `--diff-deleted` | `#484a4a` | `#d6d6d6` | +| `--diff-modified-stripe` | `#43698d` | `#b6d2f2` | +| `--diff-inserted-stripe` | `#447152` | `#aadeaa` | +| `--diff-deleted-stripe` | `#656e76` | `#c8c8c8` | +| `--diff-conflict-stripe` | `#8f5247` | `#ffc8bd` | ### Syntax (editor scheme) diff --git a/plugins/official/harbor-night/themes/harbor-night.theme.json b/plugins/official/harbor-night/themes/harbor-night.theme.json index a8f90cf3a..ed7d0886a 100644 --- a/plugins/official/harbor-night/themes/harbor-night.theme.json +++ b/plugins/official/harbor-night/themes/harbor-night.theme.json @@ -67,6 +67,10 @@ "--diff-modified": "#2f4f63", "--diff-inserted": "#294436", "--diff-deleted": "#43494f", + "--diff-modified-stripe": "#3a6380", + "--diff-inserted-stripe": "#447152", + "--diff-deleted-stripe": "#5f676f", + "--diff-conflict-stripe": "#8f5247", // Shadows and window controls "--shadow-popup": "0 6px 20px rgba(0, 0, 0, 0.45)", diff --git a/src/editor/api.ts b/src/editor/api.ts index f4581b3fc..d7bc2eec6 100644 --- a/src/editor/api.ts +++ b/src/editor/api.ts @@ -238,8 +238,12 @@ export interface ScrollbarMark { to?: number; /** CSS colour, normally a token such as `var(--danger)`. */ color: string; - /** `line` (default): 4px-wide bar on the lane; `thin`: 2px bar (search hits, occurrences). */ - kind?: "line" | "thin"; + /** + * `line` (default): 4px-wide bar on the lane; `thin`: 2px bar (search hits, occurrences); + * `full`: the lane's whole width, from the top of the first line to the top of the last (IDEA's + * diff and merge error stripe, so a one-line change is a min-height dash). + */ + kind?: "line" | "thin" | "full"; /** Shown on hover. */ tooltip?: string; /** Higher wins when marks overlap. Default 0. */ diff --git a/src/editor/core/cm/lanes.ts b/src/editor/core/cm/lanes.ts index 0a89e1f60..6c962b653 100644 --- a/src/editor/core/cm/lanes.ts +++ b/src/editor/core/cm/lanes.ts @@ -58,8 +58,14 @@ export function unfoldAround(state: EditorState, pos: number): StateEffect this.schedule()); + this.resize.observe(outer); + } else view.dom.appendChild(this.dom); this.schedule(); } @@ -104,7 +120,7 @@ class OverviewLane { key: this, read: (view) => { this.scheduled = false; - const laneHeight = view.scrollDOM.clientHeight; + const laneHeight = this.scroller.clientHeight; const len = view.state.doc.length; // The document itself, without the scroll-past-end padding that follows the editor height. const contentHeight = Math.max(view.lineBlockAt(len).bottom, 1); @@ -125,7 +141,10 @@ class OverviewLane { // Marks landing on the same lane pixel collapse into one. const px = Math.floor(a.top * scale); const top = pos(a.top); - const height = fit ? Math.round(b.bottom - a.top) : +(((b.bottom - a.top) / contentHeight) * 100).toFixed(3); + // A `full` mark runs from the top of its first line to the top of its last, as IDEA's + // error stripe (EditorMarkupModelImpl.offsetsToYPositions): one line is a min-height dash. + const span = (m.kind === "full" ? b.top : b.bottom) - a.top; + const height = fit ? Math.round(span) : +((span / contentHeight) * 100).toFixed(3); const key = `${px}:${m.color}:${m.kind ?? "line"}`; const prev = rows.get(key); if (!prev || (m.priority ?? 0) > (prev.mark.priority ?? 0) || height > prev.height) @@ -136,19 +155,25 @@ class OverviewLane { rows: [...rows.values()].sort((x, y) => (x.mark.priority ?? 0) - (y.mark.priority ?? 0)), caret, unit, + laneHeight, }; }, - write: ({ rows, caret, unit }) => { + write: ({ rows, caret, unit, laneHeight }) => { + if (this.scroller !== this.view.scrollDOM) { + this.dom.style.height = `${laneHeight}px`; + this.dom.style.marginBottom = `-${laneHeight}px`; + } const key = - rows.map((r) => `${r.top},${r.height},${r.mark.color},${r.mark.kind}`).join("|") + `#${caret}${unit}`; + rows.map((r) => `${r.top},${r.height},${r.mark.color},${r.mark.kind}`).join("|") + + `#${caret}${unit}${laneHeight}`; if (key === this.lastKey) return; this.lastKey = key; const frag = document.createDocumentFragment(); for (const r of rows) { const el = document.createElement("div"); el.className = - r.mark.kind === "thin" - ? "cm-openidea-overviewMark cm-openidea-overviewMark-thin" + r.mark.kind === "thin" || r.mark.kind === "full" + ? `cm-openidea-overviewMark cm-openidea-overviewMark-${r.mark.kind}` : "cm-openidea-overviewMark"; el.style.top = `${r.top}${unit}`; el.style.height = `${r.height}${unit}`; @@ -171,8 +196,8 @@ class OverviewLane { private scheduleThumb() { this.view.requestMeasure({ key: this.thumb, - read: (view) => { - const s = view.scrollDOM; + read: () => { + const s = this.scroller; const thumb = scrollbarThumb(s.scrollTop, s.scrollHeight, s.clientHeight); // The lane starts at the top of the editor, the scrollbar at the top of the scroller. const offset = s.getBoundingClientRect().top - this.dom.getBoundingClientRect().top; @@ -197,7 +222,7 @@ class OverviewLane { let over = false; if (p) { const lane = this.dom.getBoundingClientRect(); - const s = this.view.scrollDOM; + const s = this.scroller; const thumb = scrollbarThumb(s.scrollTop, s.scrollHeight, s.clientHeight); const y = p.y - s.getBoundingClientRect().top; over = !!thumb && p.x >= lane.left && p.x <= lane.right && y >= thumb.top && y <= thumb.bottom; @@ -226,7 +251,7 @@ class OverviewLane { if (e.button !== 0) return; e.preventDefault(); e.stopPropagation(); - const s = this.view.scrollDOM; + const s = this.scroller; const thumb = scrollbarThumb(s.scrollTop, s.scrollHeight, s.clientHeight); if (!thumb) return; const startY = e.clientY; @@ -255,7 +280,7 @@ class OverviewLane { const hit = nearestMark(marks, e.clientY); if (!hit) { // Above / below the thumb pages the editor, like the native track it replaces. - const s = this.view.scrollDOM; + const s = this.scroller; const thumb = scrollbarThumb(s.scrollTop, s.scrollHeight, s.clientHeight); e.preventDefault(); if (!thumb) return; @@ -280,12 +305,23 @@ class OverviewLane { this.thumb.removeEventListener("mousedown", this.onThumbMouseDown); this.view.dom.removeEventListener("mousemove", this.onPointerMove); this.view.dom.removeEventListener("mouseleave", this.onPointerLeave); - this.view.scrollDOM.removeEventListener("scroll", this.onScroll); + this.scroller.removeEventListener("scroll", this.onScroll); + this.resize?.disconnect(); + if (this.scroller !== this.view.scrollDOM) this.scroller.classList.remove(OUTER_SCROLLER); this.dom.remove(); } } +/** + * The overview lane over an outer scroller that holds the editor at its full height (the panes of + * an aligned side-by-side diff scroll together in their container): `scroller` finds it once the + * editor is in the DOM. The lane sticks to the right edge of this editor. + */ +export function overviewLaneIn(scroller: (view: EditorView) => HTMLElement | null): Extension { + return ViewPlugin.define((view) => new OverviewLane(view, scroller(view) ?? undefined)); +} + export const overviewLane: Extension = [ - ViewPlugin.fromClass(OverviewLane), + ViewPlugin.define((view) => new OverviewLane(view)), EditorView.editorAttributes.of({ class: "cm-openidea-withOverview" }), ]; diff --git a/src/editor/core/cm/theme.ts b/src/editor/core/cm/theme.ts index 909a80829..ab6727b9e 100644 --- a/src/editor/core/cm/theme.ts +++ b/src/editor/core/cm/theme.ts @@ -64,6 +64,60 @@ export const caretStyles = { }, }; +/** The error stripe and scrollbar lane (cm/lanes.ts), also for the VCS viewers. */ +export const overviewStyles = { + ".cm-openidea-overview": { + position: "absolute", + top: "0", + right: "0", + bottom: "0", + width: "12px", + cursor: "default", + zIndex: "3", + }, + // Over an outer scroller (an aligned diff): stays at the top of its viewport, taking no room. + ".cm-openidea-overview-sticky": { + position: "sticky", + bottom: "auto", + flex: "none", + alignSelf: "flex-end", + }, + ".cm-openidea-overviewMark": { + position: "absolute", + left: "3px", + width: "6px", + minHeight: "3px", + borderRadius: "1px", + pointerEvents: "auto", + cursor: "pointer", + }, + ".cm-openidea-overviewMark-thin": { left: "4px", width: "4px", minHeight: "2px" }, + ".cm-openidea-overviewMark-full": { left: "0", width: "100%", minHeight: "2px", borderRadius: "0" }, + // The scrollbar thumb wins over the marks under it (cm/lanes.ts, #752). + ".cm-openidea-overview-onThumb .cm-openidea-overviewMark": { pointerEvents: "none" }, + // The lane owns the scrollbar and its thumb (cm/lanes.ts): the native one is hidden so a press + // on the scrollbar can never reach the text layer and start a text selection. + "&.cm-openidea-withOverview .cm-scroller::-webkit-scrollbar:vertical": { width: "0" }, + ".cm-openidea-overviewThumb": { + position: "absolute", + right: "2px", + width: "6px", + borderRadius: "3px", + background: "var(--scrollbar-thumb)", + pointerEvents: "auto", + cursor: "default", + }, + ".cm-openidea-overview-onThumb .cm-openidea-overviewThumb": { background: "var(--scrollbar-thumb-hover)" }, + ".cm-openidea-overviewCaret": { + position: "absolute", + left: "2px", + right: "2px", + height: "2px", + pointerEvents: "none", + background: "color-mix(in srgb, var(--fg) 30%, transparent)", + }, +}; + export const openideaTheme = EditorView.theme({ "&": { height: "100%", @@ -267,49 +321,7 @@ export const openideaTheme = EditorView.theme({ ".cm-panels.cm-panels-top": { borderBottom: "1px solid var(--border)" }, ".cm-panels.cm-panels-bottom": { borderTop: "1px solid var(--border)" }, - // Error stripe and scrollbar lane (cm/lanes.ts) - ".cm-openidea-overview": { - position: "absolute", - top: "0", - right: "0", - bottom: "0", - width: "12px", - cursor: "default", - zIndex: "3", - }, - ".cm-openidea-overviewMark": { - position: "absolute", - left: "3px", - width: "6px", - minHeight: "3px", - borderRadius: "1px", - pointerEvents: "auto", - cursor: "pointer", - }, - ".cm-openidea-overviewMark-thin": { left: "4px", width: "4px", minHeight: "2px" }, - // The scrollbar thumb wins over the marks under it (cm/lanes.ts, #752). - ".cm-openidea-overview-onThumb .cm-openidea-overviewMark": { pointerEvents: "none" }, - // The lane owns the scrollbar and its thumb (cm/lanes.ts): the native one is hidden so a press - // on the scrollbar can never reach the text layer and start a text selection. - "&.cm-openidea-withOverview .cm-scroller::-webkit-scrollbar:vertical": { width: "0" }, - ".cm-openidea-overviewThumb": { - position: "absolute", - right: "2px", - width: "6px", - borderRadius: "3px", - background: "var(--scrollbar-thumb)", - pointerEvents: "auto", - cursor: "default", - }, - ".cm-openidea-overview-onThumb .cm-openidea-overviewThumb": { background: "var(--scrollbar-thumb-hover)" }, - ".cm-openidea-overviewCaret": { - position: "absolute", - left: "2px", - right: "2px", - height: "2px", - pointerEvents: "none", - background: "color-mix(in srgb, var(--fg) 30%, transparent)", - }, + ...overviewStyles, // Sticky lines (cm/stickyScroll.ts) ".cm-openidea-sticky": { diff --git a/src/features/plugins/intellijTheme.ts b/src/features/plugins/intellijTheme.ts index 33650dfb3..b24bb3f3f 100644 --- a/src/features/plugins/intellijTheme.ts +++ b/src/features/plugins/intellijTheme.ts @@ -109,7 +109,7 @@ export const ICLS_COLORS: Record = { DELETED_LINES_COLOR: ["--gutter-deleted"], }; -type Part = "FOREGROUND" | "BACKGROUND" | "EFFECT_COLOR"; +type Part = "FOREGROUND" | "BACKGROUND" | "EFFECT_COLOR" | "ERROR_STRIPE_COLOR"; /** `.icls` `` options → [part, token] pairs. */ export const ICLS_ATTRIBUTES: Record> = { @@ -117,9 +117,19 @@ export const ICLS_ATTRIBUTES: Record> = { ["FOREGROUND", "--fg-editor"], ["BACKGROUND", "--bg-editor"], ], - DIFF_MODIFIED: [["BACKGROUND", "--diff-modified"]], - DIFF_INSERTED: [["BACKGROUND", "--diff-inserted"]], - DIFF_DELETED: [["BACKGROUND", "--diff-deleted"]], + DIFF_MODIFIED: [ + ["BACKGROUND", "--diff-modified"], + ["ERROR_STRIPE_COLOR", "--diff-modified-stripe"], + ], + DIFF_INSERTED: [ + ["BACKGROUND", "--diff-inserted"], + ["ERROR_STRIPE_COLOR", "--diff-inserted-stripe"], + ], + DIFF_DELETED: [ + ["BACKGROUND", "--diff-deleted"], + ["ERROR_STRIPE_COLOR", "--diff-deleted-stripe"], + ], + DIFF_CONFLICT: [["ERROR_STRIPE_COLOR", "--diff-conflict-stripe"]], DEFAULT_KEYWORD: [["FOREGROUND", "--syn-keyword"]], DEFAULT_STRING: [["FOREGROUND", "--syn-string"]], DEFAULT_NUMBER: [["FOREGROUND", "--syn-number"]], diff --git a/src/features/vcs/DiffEditor.module.css b/src/features/vcs/DiffEditor.module.css index faa9d9de7..cb0ba6729 100644 --- a/src/features/vcs/DiffEditor.module.css +++ b/src/features/vcs/DiffEditor.module.css @@ -135,6 +135,9 @@ } .host :global(.cm-mergeViewEditor) { min-width: 0; + /* Clips like the library's `hidden` without becoming a scroll container, so the aligned view's + overview lane sticks to the MergeView's viewport (cm/lanes.ts `overviewLaneIn`). */ + overflow: clip; } .host :global(.cm-merge-revert) { width: 22px; diff --git a/src/features/vcs/DiffEditor.tsx b/src/features/vcs/DiffEditor.tsx index d9c8dc580..89089b932 100644 --- a/src/features/vcs/DiffEditor.tsx +++ b/src/features/vcs/DiffEditor.tsx @@ -34,7 +34,7 @@ import { bindingsFor, normalizeBinding, shortcutLabel, strokeOf } from "@/platfo import { tx } from "@/platform/i18n"; import { IconButton, Select, Spinner, T, showContextMenu } from "@/ui"; import { editorService, type EditorInput } from "@/workbench/contributions"; -import { changedLineGutters, diffWords, languageFor, viewerBase } from "./cm"; +import { changedLineGutters, diffOverview, diffWords, languageFor, viewerBase } from "./cm"; import { applyChunk, arrowControls, chunkAt, hoverRevert, revertUnifiedChunk } from "./diffControls"; import { HIGHLIGHT_OPTIONS, @@ -417,6 +417,8 @@ export function DiffEditor({ input, toolbarActions }: { input: EditorInput; tool extensions: [ side(attached.a, editableA, "left"), colours, + // Aligned panes scroll together: one stripe, on the right (as IDEA's shared scrollbar). + panes ? diffOverview("split") : [], panes?.extension ?? [], focusTracker("a"), findWatch("a"), @@ -428,6 +430,7 @@ export function DiffEditor({ input, toolbarActions }: { input: EditorInput; tool extensions: [ side(attached.b, editableB, "right"), colours, + diffOverview(panes ? "split" : "aligned"), panes?.extension ?? [], listener, focusTracker("b"), @@ -473,6 +476,7 @@ export function DiffEditor({ input, toolbarActions }: { input: EditorInput; tool parent: host, extensions: [ side(attached.b, editableB, "right"), + diffOverview("unified"), listener, focusTracker("b"), findWatch("b"), diff --git a/src/features/vcs/MergeEditor.tsx b/src/features/vcs/MergeEditor.tsx index 6244d82a8..385628c85 100644 --- a/src/features/vcs/MergeEditor.tsx +++ b/src/features/vcs/MergeEditor.tsx @@ -21,6 +21,7 @@ import { syncMergeScroll } from "./mergeScroll"; import { mergeTitles } from "./conflicts"; import { mergeChanged, + mergeStripe, mergeTheme, resultControls, resultMarks, @@ -176,6 +177,7 @@ export function MergeEditor({ input }: { input: EditorInput }) { }), state.base !== null ? unifiedMergeView({ original: state.base, mergeControls: false, gutter: true }) : [], sideControls(which, controls), + mergeStripe(which), mergeTheme, ]; const ours = new EditorView({ doc: state.ours, parent: oursRef.current, extensions: side("left") }); @@ -192,6 +194,7 @@ export function MergeEditor({ input }: { input: EditorInput }) { embeddedEditorExtensions(path), lang, resultMarks, + mergeStripe("result"), resultControls(controls), mergeTheme, EditorView.updateListener.of((u) => { diff --git a/src/features/vcs/__tests__/diffStripe.test.ts b/src/features/vcs/__tests__/diffStripe.test.ts new file mode 100644 index 000000000..2366d1d25 --- /dev/null +++ b/src/features/vcs/__tests__/diffStripe.test.ts @@ -0,0 +1,121 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { MergeView, unifiedMergeView } from "@codemirror/merge"; +import { EditorState } from "@codemirror/state"; +import { scrollbarMarks } from "@/editor/api"; +import { diffOverview, diffScrollbarMarks } from "../cm"; +import { mergeStripe, sideControls, syncSide, type MergeControls } from "../mergeGutters"; +import { applyTo, mergeField, startMerge, type MergeChange } from "../mergeModel"; + +// One modified line, one inserted line, one deleted line. +const A = "one\ntwo\nthree\nfour\nfive\nsix"; +const B = "one\nTWO\nthree\nnew\nfour\nsix"; + +const line = (text: string, n: number) => + text + .split("\n") + .slice(0, n - 1) + .join("\n").length + (n > 1 ? 1 : 0); +const stripe = (type: "modified" | "inserted" | "deleted" | "conflict") => `var(--diff-${type}-stripe)`; +const marksOf = (state: EditorState) => state.facet(scrollbarMarks).map((m) => ({ from: m.from, color: m.color })); + +let merge: MergeView | null = null; +afterEach(() => { + merge?.destroy(); + merge = null; +}); + +const splitView = (extA = diffOverview("split"), extB = diffOverview("split")) => + (merge = new MergeView({ a: { doc: A, extensions: extA }, b: { doc: B, extensions: extB }, parent: document.body })); + +describe("diff overview stripe (#1505)", () => { + it.each([ + // The insertion has no lines on the left: a point where they would be; likewise the deletion on the right. + { side: "a", text: A, lines: [2, 4, 5] }, + { side: "b", text: B, lines: [2, 4, 6] }, + ] as const)("marks every chunk of side-by-side pane $side in the chunk's type", ({ side, text, lines }) => { + const view = splitView()[side]; + expect(marksOf(view.state)).toEqual([ + { from: line(text, lines[0]), color: stripe("modified") }, + { from: line(text, lines[1]), color: stripe("inserted") }, + { from: line(text, lines[2]), color: stripe("deleted") }, + ]); + expect(diffScrollbarMarks(view.state, false)).toEqual(view.state.facet(scrollbarMarks)); + }); + + it("marks the unified view's old lines deleted and new lines inserted, with no modified type", () => { + const state = EditorState.create({ + doc: B, + extensions: [unifiedMergeView({ original: A }), diffOverview("unified")], + }); + expect(marksOf(state)).toEqual([ + { from: line(B, 2), color: stripe("inserted") }, + { from: line(B, 2), color: stripe("deleted") }, + { from: line(B, 4), color: stripe("inserted") }, + { from: line(B, 6), color: stripe("deleted") }, + ]); + // The old lines' point sits on top of the new lines that replaced them. + const [inserted, deleted] = state.facet(scrollbarMarks); + expect(deleted!.priority).toBeGreaterThan(inserted!.priority ?? 0); + }); + + it("follows the chunks when the other side changes without a document change of its own", () => { + const mv = splitView(); + mv.a.dispatch({ changes: { from: 0, to: mv.a.state.doc.length, insert: B } }); + expect(mv.b.state.facet(scrollbarMarks)).toEqual([]); + }); + + it("draws the aligned view's lane in the right pane over the shared scroller", () => { + const mv = splitView([], diffOverview("aligned")); + const lane = mv.b.dom.firstElementChild as HTMLElement; + expect(lane.classList.contains("cm-openidea-overview-sticky")).toBe(true); + expect(mv.dom.classList.contains("cm-openidea-overviewScroller")).toBe(true); + expect(mv.a.dom.querySelector(".cm-openidea-overview")).toBeNull(); + mv.destroy(); + expect(mv.dom.classList.contains("cm-openidea-overviewScroller")).toBe(false); + merge = null; + }); +}); + +describe("merge overview stripe (#1505)", () => { + // Line 2: both sides changed it differently (conflict); line 4: left only; line 6: right deletes it. + const base = "a\nb\nc\nd\ne\nf\ng"; + const left = "a\nB-left\nc\nD\ne\nf\ng"; + const right = "a\nB-right\nc\nd\ne\ng"; + const start = startMerge(left, base, right); + const noop: MergeControls = { accept: () => {}, ignore: () => {}, resolveAutomatically: () => {} }; + const result = () => + EditorState.create({ doc: start.text, extensions: [mergeField.init(() => start.changes), mergeStripe("result")] }); + const sidePane = (side: "left" | "right", changes: readonly MergeChange[] = start.changes) => + EditorState.create({ + doc: side === "left" ? left : right, + extensions: [sideControls(side, noop), mergeStripe(side)], + }).update({ effects: syncSide.of(changes) }).state; + + it("marks every open change of the result by its type, conflicts on top", () => { + const marks = result().facet(scrollbarMarks); + expect(marks.map((m) => m.color)).toEqual([stripe("conflict"), stripe("modified"), stripe("deleted")]); + expect(marks[0]!.priority).toBe(1); + }); + + it("marks on each side only the changes that side made", () => { + expect(marksOf(sidePane("left"))).toEqual([ + { from: line(left, 2), color: stripe("conflict") }, + { from: line(left, 4), color: stripe("modified") }, + ]); + expect(marksOf(sidePane("right"))).toEqual([ + { from: line(right, 2), color: stripe("conflict") }, + { from: line(right, 6), color: stripe("deleted") }, + ]); + }); + + it("drops a change once it is resolved, and a side's mark once that side is done", () => { + let state = result(); + state = state.update(applyTo(state, [1], { type: "accept", side: "left", resolve: false })!).state; + expect(state.facet(scrollbarMarks).map((m) => m.color)).toEqual([stripe("conflict"), stripe("deleted")]); + expect( + sidePane("left", state.field(mergeField)) + .facet(scrollbarMarks) + .map((m) => m.color), + ).toEqual([stripe("conflict")]); + }); +}); diff --git a/src/features/vcs/cm.ts b/src/features/vcs/cm.ts index 07ae5810f..67f06a428 100644 --- a/src/features/vcs/cm.ts +++ b/src/features/vcs/cm.ts @@ -30,10 +30,12 @@ import { getChunks, mergeViewSiblings, type Chunk } from "@codemirror/merge"; import { defaultKeymap } from "@codemirror/commands"; import { search, searchKeymap } from "@codemirror/search"; import { tags as t } from "@lezer/highlight"; +import { scrollbarMarks, type ScrollbarMark } from "@/editor/api"; import { findExtension } from "@/editor/core/cm/find"; +import { overviewLane, overviewLaneIn } from "@/editor/core/cm/lanes"; import { caretExtension } from "@/editor/core/cm/prefs"; import { fontExtension } from "@/editor/core/cm/setup"; -import { caretStyles, findLocals, findStyles } from "@/editor/core/cm/theme"; +import { caretStyles, findLocals, findStyles, overviewStyles } from "@/editor/core/cm/theme"; import { editorPrefs } from "@/platform/editorSettings"; import { useProject } from "@/platform/project"; import { getDiffInsights, hasInnerFragments } from "./diffOptions"; @@ -223,6 +225,62 @@ export const chunkDecorations = (build: (view: EditorView) => DecorationSet) => const changedLines = chunkDecorations(lineDeco); +/** Stripe colours by type (TextDiffType.getMarkerColor: the keys' ERROR_STRIPE_COLOR). */ +const stripeColor = byKind((kind) => `var(--diff-${kind}-stripe)`); + +/** The diff viewer's layouts, as the overview lane sees them. */ +export type DiffLayout = "split" | "aligned" | "unified"; + +/** + * The changes on the overview lane as marks across its width, after IDEA's diff error stripe + * (DiffDrawUtil.LineHighlighterBuilder): a side-by-side pane marks each chunk's lines in the + * chunk's type, an empty side a point where the lines would be. The unified view has no modified type: the old lines are marked deleted and the + * new ones inserted (createUnifiedChunkHighlighters); the old lines sit above the new ones in a + * widget, so their mark is a point on top of the inserted lines. + */ +export function diffScrollbarMarks(state: EditorState, unified: boolean): ScrollbarMark[] { + const info = getChunks(state); + if (!info) return []; + if (unified) + return info.chunks.flatMap((c) => { + const marks: ScrollbarMark[] = []; + if (c.fromB !== c.toB) marks.push({ from: c.fromB, to: c.endB, color: stripeColor.inserted, kind: "full" }); + if (c.fromA !== c.toA) marks.push({ from: c.fromB, color: stripeColor.deleted, kind: "full", priority: 1 }); + return marks; + }); + const isA = info.side === "a"; + return info.chunks.map((c) => ({ + from: isA ? c.fromA : c.fromB, + to: isA ? c.endA : c.endB, + color: stripeColor[chunkKind(c)], + kind: "full", + })); +} + +type ChunkMarks = { chunks: readonly Chunk[] | undefined; marks: readonly ScrollbarMark[] }; + +/** Recomputed when the chunks change, which can happen without a document change (MergeView, `updateOriginalDoc`). */ +const chunkMarks = (unified: boolean) => + StateField.define({ + create: (state) => ({ chunks: getChunks(state)?.chunks, marks: diffScrollbarMarks(state, unified) }), + update: (value, tr) => { + const chunks = getChunks(tr.state)?.chunks; + return chunks === value.chunks ? value : { chunks, marks: diffScrollbarMarks(tr.state, unified) }; + }, + provide: (f) => scrollbarMarks.from(f, (v) => v.marks), + }); + +/** + * The overview lane with the diff's changes, over the pane's scrollbar. The panes of an aligned + * side-by-side diff scroll together in the MergeView, whose scrollbar the right pane's lane takes. + */ +export function diffOverview(layout: DiffLayout): Extension { + return [ + chunkMarks(layout === "unified"), + layout === "aligned" ? overviewLaneIn((view) => view.dom.closest(".cm-mergeView")) : overviewLane, + ]; +} + /** * IDEA's "Highlight words" for a side-by-side pane: the inner fragments of * chunks whose words were compared, over their lighter line background. @@ -254,6 +312,7 @@ export const viewerHighlight = HighlightStyle.define([ export const viewerTheme = EditorView.theme( { + ...overviewStyles, "&": { height: "100%", backgroundColor: "var(--bg-app)", @@ -341,6 +400,8 @@ export const viewerTheme = EditorView.theme( }, ".cm-collapsedLines:before": { content: "none" }, ".cm-collapsedLines:after": { content: "none" }, + // Clear of the overview lane over the scrollbar (12px wide). + "&.cm-openidea-withOverview .cm-deletedChunk .cm-chunkButtons": { insetInlineEnd: "17px" }, ".cm-chunkButtons button": { font: "inherit", fontFamily: "var(--font-ui)", diff --git a/src/features/vcs/mergeGutters.ts b/src/features/vcs/mergeGutters.ts index 854270796..b989632bb 100644 --- a/src/features/vcs/mergeGutters.ts +++ b/src/features/vcs/mergeGutters.ts @@ -5,6 +5,8 @@ */ import { RangeSetBuilder, StateEffect, StateField, type Extension } from "@codemirror/state"; import { Decoration, EditorView, GutterMarker, WidgetType, gutter, type ViewUpdate } from "@codemirror/view"; +import { scrollbarMarks, type ScrollbarMark } from "@/editor/api"; +import { overviewLane } from "@/editor/core/cm/lanes"; import { hasPrimaryModifier } from "@/platform/keymap"; import { tx } from "@/platform/i18n"; import { iconMarkup } from "@/ui"; @@ -190,6 +192,45 @@ export const resultMarks = EditorView.decorations.compute([mergeField], (state) return out.finish(); }); +/** A change's type (MergeConflictType): a conflict, else how the changed side(s) differ from the base. */ +function changeType(c: MergeChange): "conflict" | "inserted" | "deleted" | "modified" { + if (isConflict(c)) return "conflict"; + const changed = c.kind === "right" ? c.right : c.left; + return c.base === "" ? "inserted" : changed === "" ? "deleted" : "modified"; +} + +const stripeMark = (c: MergeChange, from: number, to: number): ScrollbarMark => { + const type = changeType(c); + return { + from, + to: to > from ? to - 1 : from, + color: `var(--diff-${type}-stripe)`, + kind: "full", + priority: type === "conflict" ? 1 : 0, + }; +}; + +/** + * The open changes on a pane's overview lane, as IDEA's merge viewer: the result marks each + * change until it is resolved, a side pane the changes of that side until that side is accepted + * or ignored (TextMergeChange: a resolved side paints no background, so no stripe either). + */ +export function mergeStripe(pane: MergeSide | "result"): Extension { + const marks = + pane === "result" + ? scrollbarMarks.compute([mergeField], (state): readonly ScrollbarMark[] => + state.field(mergeField).flatMap((c) => (isResolved(c) ? [] : [stripeMark(c, c.from, c.to)])), + ) + : scrollbarMarks.compute([sideChanges], (state): readonly ScrollbarMark[] => + state.field(sideChanges).flatMap((c) => { + if (!changesSide(c, pane) || c.done[pane]) return []; + const [from, to] = pane === "left" ? c.leftRange : c.rightRange; + return [stripeMark(c, from, to)]; + }), + ); + return [marks, overviewLane]; +} + export const mergeTheme = EditorView.baseTheme({ // A fixed width, so the text does not shift as controls come and go while scrolling. ".cm-merge-gutter": { minWidth: "34px" }, diff --git a/src/styles/global.css b/src/styles/global.css index 9169b90b5..66fef15e6 100644 --- a/src/styles/global.css +++ b/src/styles/global.css @@ -237,6 +237,10 @@ a:hover { ::-webkit-scrollbar-corner { background: transparent; } +/* An outer scroller whose scrollbar the editor's overview lane draws (cm/lanes.ts `overviewLaneIn`). */ +.cm-openidea-overviewScroller::-webkit-scrollbar:vertical { + width: 0; +} /* Settings › Appearance › Antialiasing (IDE / Editor). */ :root[data-aa-ide="subpixel"] body { diff --git a/src/styles/tokens.css b/src/styles/tokens.css index 6324353c4..241ff7540 100644 --- a/src/styles/tokens.css +++ b/src/styles/tokens.css @@ -88,6 +88,11 @@ html { --diff-modified: #385570; --diff-inserted: #294436; --diff-deleted: #484a4a; + /* their error stripe marks (ERROR_STRIPE_COLOR of the same keys and DIFF_CONFLICT) */ + --diff-modified-stripe: #43698d; + --diff-inserted-stripe: #447152; + --diff-deleted-stripe: #656e76; + --diff-conflict-stripe: #8f5247; /* coverage (IDEA LINE_FULL / PARTIAL / NONE_COVERAGE), following the theme's status colours */ --coverage-covered: var(--gutter-added); @@ -480,6 +485,10 @@ html { --diff-modified: #c2d8f2; --diff-inserted: #bee6be; --diff-deleted: #d6d6d6; + --diff-modified-stripe: #b6d2f2; + --diff-inserted-stripe: #aadeaa; + --diff-deleted-stripe: #c8c8c8; + --diff-conflict-stripe: #ffc8bd; /* syntax (Light scheme + Default / Python defaults) */ --syn-keyword: #0033b3; @@ -737,6 +746,10 @@ html { --diff-modified: #000099; --diff-inserted: #00400f; --diff-deleted: #4d2e08; + --diff-modified-stripe: #1aabff; + --diff-inserted-stripe: #009924; + --diff-deleted-stripe: #a66a32; + --diff-conflict-stripe: #ff0d25; --syn-keyword: #e8a885; --syn-string: #8ccc94; @@ -1019,6 +1032,10 @@ html { --diff-modified: color-mix(in srgb, var(--palette-blue) 30%, var(--palette-bg)); --diff-inserted: color-mix(in srgb, var(--palette-green) 30%, var(--palette-bg)); --diff-deleted: color-mix(in srgb, var(--palette-fg) 18%, var(--palette-bg)); + --diff-modified-stripe: color-mix(in srgb, var(--palette-blue) 50%, var(--palette-bg)); + --diff-inserted-stripe: color-mix(in srgb, var(--palette-green) 50%, var(--palette-bg)); + --diff-deleted-stripe: color-mix(in srgb, var(--palette-fg) 35%, var(--palette-bg)); + --diff-conflict-stripe: color-mix(in srgb, var(--palette-red) 50%, var(--palette-bg)); --syn-link: var(--palette-blue); --link-hover: var(--palette-cyan); --danger: var(--palette-red); From c495c09416c0edb5e80ce5f3b727ede3c85845e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ad=C3=A1mek?= Date: Thu, 5 Nov 2026 14:50:06 +0100 Subject: [PATCH 007/185] feat(php): show PHPStan errors in the editor on save (#1518) PHPStan results were only visible in a terminal. Saving a PHP file in a trusted project with vendor/bin/phpstan now runs phpstan analyse --error-format=json on the saved files (debounced, from the project root, on the selected PHP with XDEBUG_MODE=off) and shows its errors at their lines in the editor and Problems, with the PHPStan identifier as the code; saving the fixed code clears them. Settings > Editor > Inspections lists PHPStan under PHP with each reported identifier, so rules can be turned off or re-graded, and Settings > Languages & Frameworks > PHP > PHPStan selects another configuration file. A failing run is reported once as a notification; a newer save cancels a check in progress, and Restricted Mode stops it. Closes #1212 Co-authored-by: DanielADK <18214958+DanielADK@users.noreply.github.com> --- crates/ide-core/src/settings.rs | 4 + crates/ide-host/src/lib.rs | 7 + crates/ide-host/src/phpstan.rs | 850 ++++++++++++++++++ crates/ide-host/src/project.rs | 24 +- crates/ide-host/src/trust.rs | 3 + crates/ide-sdk/src/php.rs | 105 ++- src/bindings.ts | 6 + .../settings/__tests__/inspections.test.ts | 37 + .../settings/__tests__/languages.test.ts | 5 + .../settings/languages/LanguagePages.tsx | 10 + .../settings/pages/editor/inspectionsModel.ts | 27 +- src/features/settings/pages/languages.tsx | 2 + src/platform/inspectionControls.test.ts | 7 + src/platform/inspections.test.ts | 20 + src/platform/inspections.ts | 17 +- src/platform/languageSettings.ts | 4 +- src/platform/locales/text/en.json | 9 +- src/platform/locales/text/es.json | 7 + src/platform/locales/text/fr.json | 7 + src/platform/locales/text/ko.json | 7 + src/platform/locales/text/pl.json | 7 + src/platform/locales/text/pt.json | 7 + src/platform/locales/text/zh.json | 7 + src/platform/locales/textCatalog.test.ts | 1 + 24 files changed, 1163 insertions(+), 17 deletions(-) create mode 100644 crates/ide-host/src/phpstan.rs diff --git a/crates/ide-core/src/settings.rs b/crates/ide-core/src/settings.rs index 6f65eff4c..405761d49 100644 --- a/crates/ide-core/src/settings.rs +++ b/crates/ide-core/src/settings.rs @@ -722,6 +722,10 @@ pub struct PhpSettings { /// Intelephense stub sets loaded on top of its defaults and the `ext-*` entries of /// `composer.json` (`wordpress`). pub stubs: Vec, + /// PHPStan configuration file for checking saved files; empty = the `phpstan.neon`, + /// `phpstan.neon.dist` or `phpstan.dist.neon` PHPStan finds in the project root. Absolute, + /// `~/`, or relative to the project. + pub phpstan_config: String, } #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, Type)] diff --git a/crates/ide-host/src/lib.rs b/crates/ide-host/src/lib.rs index 70fcfb9b4..1f893f47b 100644 --- a/crates/ide-host/src/lib.rs +++ b/crates/ide-host/src/lib.rs @@ -10,6 +10,7 @@ mod java_runtime; mod kotlin_bridge; pub mod notebook; pub mod ops; +mod phpstan; pub mod plugins; mod project; mod proxy; @@ -228,6 +229,9 @@ impl Host { if let Some(bridge) = &project.kotlin_bridge { bridge.stop(); } + if let Some(phpstan) = &project.phpstan { + phpstan.stop(); + } project.documents.clear_workspace_edit_history(); project.agents.shutdown(); project.notebooks.stop_admission(); @@ -349,6 +353,9 @@ impl Host { if let Some(bridge) = &project.kotlin_bridge { bridge.stop(); } + if let Some(phpstan) = &project.phpstan { + phpstan.stop(); + } project.documents.clear_workspace_edit_history(); project.notebooks.stop_admission(); project.agents.shutdown(); diff --git a/crates/ide-host/src/phpstan.rs b/crates/ide-host/src/phpstan.rs new file mode 100644 index 000000000..807b94e7b --- /dev/null +++ b/crates/ide-host/src/phpstan.rs @@ -0,0 +1,850 @@ +//! Saved PHP files -> `vendor/bin/phpstan analyse` -> `phpstan` diagnostics. + +use ide_core::event::NotifyLevel; +use ide_core::output_encoding::decode_tool_output; +use ide_core::{CancellationToken, Diagnostic, IdeEvent, Pos, Range, Severity, SharedSink}; +use ide_workspace::SettingsStore; +use ide_workspace::documents::DocumentObserver; +use parking_lot::Mutex; +use serde_json::Value; +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; +use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, +}; +use std::time::Duration; +use tokio::sync::Notify; + +const DELAY: Duration = Duration::from_millis(750); +const SOURCE: &str = "phpstan"; + +/// Saved files waiting for a check. Each save cancels the wait or the check in progress. +#[derive(Default)] +struct Queue { + generation: u64, + cancel: CancellationToken, + files: BTreeSet, +} + +impl Queue { + fn push(&mut self, files: impl IntoIterator) { + self.files.extend(files); + self.cancel.cancel(); + self.generation += 1; + self.cancel = CancellationToken::new(); + } + fn current(&self, generation: u64) -> bool { + self.generation == generation && !self.cancel.is_cancelled() + } +} + +pub(crate) struct Phpstan { + root: PathBuf, + config_dir: PathBuf, + settings: Arc, + sink: SharedSink, + trusted: Arc, + lifetime: CancellationToken, + queue: Mutex, + changed: Notify, + /// Files with PHPStan diagnostics, the only ones a clean result or a removal clears. + shown: Mutex>, +} + +impl Phpstan { + pub(crate) fn new( + root: PathBuf, + config_dir: PathBuf, + settings: Arc, + sink: SharedSink, + trusted: Arc, + ) -> Arc { + Arc::new(Self { + root, + config_dir, + settings, + sink, + trusted, + lifetime: CancellationToken::new(), + queue: Mutex::new(Queue::default()), + changed: Notify::new(), + shown: Mutex::new(HashSet::new()), + }) + } + + pub(crate) fn start(self: &Arc, runtime: &tokio::runtime::Handle) { + let checker = self.clone(); + runtime.spawn(async move { checker.run().await }); + } + + pub(crate) fn stop(&self) { + self.lifetime.cancel(); + self.queue.lock().cancel.cancel(); + } + + /// Stops the check in progress (trust changed); its files wait for the next one, which drops + /// them while the project is in Restricted Mode. + pub(crate) fn interrupt(&self) { + self.queue.lock().push([]); + self.changed.notify_one(); + } + + fn files_saved(&self, paths: impl IntoIterator) { + let mut files = paths.into_iter().filter(|p| p.extension().is_some_and(|e| e == "php")).peekable(); + if self.lifetime.is_cancelled() || files.peek().is_none() { + return; + } + self.queue.lock().push(files); + self.changed.notify_one(); + } + + /// Deleted files and folders lose their diagnostics. + pub(crate) fn files_removed<'a>(&self, removed: impl Iterator) { + let removed: Vec<&PathBuf> = removed.collect(); + let gone: Vec = + self.shown.lock().iter().filter(|p| removed.iter().any(|r| p.starts_with(r))).cloned().collect(); + for path in gone { + self.publish(path, Vec::new()); + } + } + + fn publish(&self, path: PathBuf, items: Vec) { + let mut shown = self.shown.lock(); + if items.is_empty() { + if !shown.remove(&path) { + return; + } + } else { + shown.insert(path.clone()); + } + // Under the lock, so a removal and a result for one file reach the UI in the order decided here. + self.sink.emit(IdeEvent::Diagnostics { path: path.into(), source: SOURCE.into(), items }); + } + + async fn run(self: Arc) { + // The failure last reported per root: each is reported once, until a run succeeds. + let mut failures: BTreeMap = BTreeMap::new(); + loop { + tokio::select! { + _ = self.lifetime.cancelled() => return, + _ = self.changed.notified() => {}, + } + let (generation, cancel) = { + let queue = self.queue.lock(); + (queue.generation, queue.cancel.clone()) + }; + if !debounce(&cancel, &self.lifetime).await { + continue; + } + let files = { + let mut queue = self.queue.lock(); + if !queue.current(generation) { + continue; + } + std::mem::take(&mut queue.files) + }; + if !self.trusted.load(Ordering::Acquire) { + continue; + } + let (root, config_dir, settings) = (self.root.clone(), self.config_dir.clone(), self.settings.clone()); + let Ok((checks, unchecked)) = + tokio::task::spawn_blocking(move || plan(&root, &config_dir, &settings, files)).await + else { + continue; + }; + for path in unchecked { + self.publish(path, Vec::new()); + } + let mut checks = checks.into_iter(); + while let Some(check) = checks.next() { + let started = std::time::Instant::now(); + let program = check.command.get_program().to_string_lossy().into_owned(); + let output = analyse(check.command, &cancel).await; + let Some(output) = output.filter(|_| self.queue.lock().current(generation)) else { + // A newer save: these files are checked again with it. + let mut queue = self.queue.lock(); + queue.files.extend(check.files); + queue.files.extend(checks.by_ref().flat_map(|c| c.files)); + break; + }; + tracing::debug!( + "phpstan checked {} file(s) in {} ms", + check.files.len(), + started.elapsed().as_millis() + ); + // Files deleted meanwhile make the result (or the failure) moot: check the rest again. + let (present, missing): (Vec, Vec) = + check.files.iter().cloned().partition(|f| f.is_file()); + if !missing.is_empty() { + for path in missing { + self.publish(path, Vec::new()); + } + self.files_saved(present); + continue; + } + let report = match output { + Ok(output) => { + let parsed = parse(&decode_tool_output(&output.stdout), &decode_tool_output(&output.stderr)); + parsed.map_err(|text| { + if text.is_empty() { + let status = output.status; + format!("PHPStan ended with {status} and no report") + } else { + text + } + }) + } + Err(error) => Err(format!("Cannot run {program}: {error}")), + }; + let failure = match report { + Ok(report) => { + let files = check.files.clone(); + let results = tokio::task::spawn_blocking(move || diagnostics(&files, report.files)) + .await + .unwrap_or_default(); + for (path, items) in results { + // Severe errors (a syntax error) stop the analysis of the other files. + if report.complete || !items.is_empty() { + self.publish(path, items); + } + } + report.errors.join("\n") + } + Err(text) => { + // Without a report the earlier errors of these files may be stale. + for path in check.files { + self.publish(path, Vec::new()); + } + text + } + }; + if failure.is_empty() { + failures.remove(&check.root); + } else { + self.failure(&check.root, failure, &mut failures); + } + } + } + } + + fn failure(&self, root: &Path, text: String, failures: &mut BTreeMap) { + if failures.get(root) == Some(&text) { + return; + } + self.sink.emit(IdeEvent::Notify { + level: NotifyLevel::Error, + title: "PHPStan check failed".into(), + body: Some(text.clone()), + }); + failures.insert(root.to_path_buf(), text); + } +} + +impl DocumentObserver for Phpstan { + fn saved(&self, _doc: ide_core::DocId, path: &Path, _text: &str) { + self.files_saved([path.to_path_buf()]); + } +} + +async fn debounce(cancel: &CancellationToken, lifetime: &CancellationToken) -> bool { + tokio::select! { + _ = lifetime.cancelled() => false, + _ = cancel.cancelled() => false, + _ = tokio::time::sleep(DELAY) => true, + } +} + +/// One `vendor/bin/phpstan` run: the saved files below one PHP project. +struct Check { + root: PathBuf, + files: Vec, + command: Command, +} + +/// The runs that check `files`, and the files not checked, whose earlier diagnostics no longer +/// apply. A file belongs to the innermost linked project (with its own settings) that contains it, +/// else to the project; deleted files, files in `vendor` or in excluded folders, and files of a +/// project without PHPStan are not checked. +fn plan( + project: &Path, + config_dir: &Path, + settings: &SettingsStore, + files: BTreeSet, +) -> (Vec, Vec) { + let members = + ide_workspace::workspace::Workspace::load(project).ok().flatten().map(|w| w.projects).unwrap_or_default(); + let mut groups: BTreeMap> = BTreeMap::new(); + let mut unchecked = Vec::new(); + for file in files { + let member = members.iter().filter(|m| file.starts_with(m)).max_by_key(|m| m.components().count()); + let root = member.cloned().or_else(|| file.starts_with(project).then(|| project.to_path_buf())); + match root { + Some(root) if file.is_file() => groups.entry(root).or_default().push(file), + _ => unchecked.push(file), + } + } + let mut checks = Vec::new(); + for (root, files) in groups { + let effective = if root == project { + settings.effective() + } else { + SettingsStore::load(config_dir, Some(&root)).effective() + }; + let excludes = ide_vfs::ExcludeMatcher::new(&effective.exclude_globs()); + let (files, skipped): (Vec, Vec) = files.into_iter().partition(|f| { + f.strip_prefix(&root).is_ok_and(|rel| !rel.starts_with("vendor") && !excludes.is_excluded(rel)) + }); + unchecked.extend(skipped); + match ide_sdk::php::phpstan_command(&root, &effective, &files).filter(|_| !files.is_empty()) { + Some(command) => checks.push(Check { root, files, command }), + None => unchecked.extend(files), + } + } + (checks, unchecked) +} + +/// Runs `cmd` to its end; `None` when `cancel` fires first, which kills it. +async fn analyse(mut cmd: Command, cancel: &CancellationToken) -> Option> { + if cancel.is_cancelled() { + return None; + } + ide_core::process::prepare(&mut cmd); + ide_core::process::own_group(&mut cmd); + cmd.stdin(Stdio::null()).stdout(Stdio::piped()).stderr(Stdio::piped()); + let mut cmd = tokio::process::Command::from(cmd); + cmd.kill_on_drop(true); + let child = match cmd.spawn() { + Ok(child) => child, + Err(error) => return Some(Err(error)), + }; + let pid = child.id(); + // PHPStan may start workers of its own. + let cancelled = async { + cancel.cancelled().await; + if let Some(pid) = pid { + let _ = tokio::task::spawn_blocking(move || ide_core::process::kill_tree_of(pid)).await; + } + }; + tokio::select! { + output = child.wait_with_output() => Some(output), + () = cancelled => None, + } +} + +/// A PHPStan message: 0-based line, text with its tip, identifier. +#[derive(Debug, PartialEq)] +struct Message { + line: u32, + text: String, + identifier: Option, +} + +/// A JSON report: messages per file as PHPStan names it, the errors of the run itself, and +/// whether every file was analysed (severe errors such as a syntax error stop the others). +#[derive(Debug, PartialEq)] +struct Report { + files: BTreeMap>, + errors: Vec, + complete: bool, +} + +impl Default for Report { + fn default() -> Self { + Self { files: BTreeMap::new(), errors: Vec::new(), complete: true } + } +} + +/// PHPStan's `--error-format=json` report, or the text to report when there is none (an +/// invalid configuration, a crash). PHP warnings, or a byte order mark a bootstrap file prints, +/// may precede the report on stdout. +fn parse(stdout: &str, stderr: &str) -> Result { + let json = serde_json::from_str::(stdout.trim().trim_start_matches('\u{feff}')).ok().or_else(|| { + stdout.lines().rev().find_map(|l| l.find('{').and_then(|start| serde_json::from_str(&l[start..]).ok())) + }); + let Some(Value::Object(json)) = json else { + // Every saved file is excluded by the configuration (`excludePaths`). + if stderr.contains("No files found to analyse") { + return Ok(Report::default()); + } + return Err(failure_text(stdout, stderr)); + }; + let mut report = Report::default(); + if let Some(Value::Object(files)) = json.get("files") { + for (file, result) in files { + let messages = result.get("messages").and_then(Value::as_array).into_iter().flatten(); + report.files.insert(file.clone(), messages.filter_map(message).collect()); + } + } + let errors = json.get("errors").and_then(Value::as_array).into_iter().flatten(); + report.errors = errors.map(|e| e.as_str().map_or_else(|| e.to_string(), str::to_owned)).collect(); + report.complete = !stderr.contains("Result is incomplete"); + Ok(report) +} + +fn message(m: &Value) -> Option { + let mut text = m.get("message")?.as_str()?.to_owned(); + if let Some(tip) = m.get("tip").and_then(Value::as_str).filter(|t| !t.trim().is_empty()) { + text = format!("{text}\n{}", strip_console_tags(tip)); + } + let line = m.get("line").and_then(Value::as_u64).unwrap_or(1).max(1) - 1; + Some(Message { + line: u32::try_from(line).unwrap_or(u32::MAX), + text, + identifier: m.get("identifier").and_then(Value::as_str).map(str::to_owned), + }) +} + +/// Tips may carry console styling (`…`). +fn strip_console_tags(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut rest = text; + while let Some(i) = rest.find('<') { + out.push_str(&rest[..i]); + rest = &rest[i..]; + let tag = rest.find('>').map(|end| &rest[..=end]); + match tag { + Some(tag) + if tag == "" + || tag.starts_with(">().join("\n") +} + +/// The diagnostics of each checked file (none for a clean one), matched to the report's files +/// by their real paths. Errors PHPStan reports in a trait name the class using it +/// (`Trait.php (in context of class Foo)`). +fn diagnostics(files: &[PathBuf], reported: BTreeMap>) -> Vec<(PathBuf, Vec)> { + let real = |p: &Path| dunce::canonicalize(p).unwrap_or_else(|_| p.to_path_buf()); + let mut messages: BTreeMap> = BTreeMap::new(); + for (name, list) in reported { + let name = name.split(" (in context of ").next().unwrap_or(&name); + messages.entry(real(Path::new(name))).or_default().extend(list); + } + files + .iter() + .map(|file| { + let list = messages.remove(&real(file)).unwrap_or_default(); + // Columns only: a file in another encoding than UTF-8 still gives the right lines. + let text = if list.is_empty() { + None + } else { + std::fs::read(file).ok().map(|bytes| String::from_utf8_lossy(&bytes).into_owned()) + }; + let items = list.into_iter().map(|m| diagnostic(m, text.as_deref())).collect(); + (file.clone(), items) + }) + .collect() +} + +fn diagnostic(m: Message, text: Option<&str>) -> Diagnostic { + Diagnostic { + range: line_range(text, m.line), + severity: Severity::Warning, + message: m.text, + source: Some("PHPStan".into()), + server: Some(SOURCE.into()), + code: m.identifier, + unnecessary: false, + deprecated: false, + } +} + +/// PHPStan reports lines only: the line from its first non-blank character to its end (UTF-16). +fn line_range(text: Option<&str>, line: u32) -> Range { + let text = text.map(|t| t.strip_prefix('\u{feff}').unwrap_or(t)); + let content = text.and_then(|t| t.split('\n').nth(line as usize)).unwrap_or("").trim_end(); + let indent = content.len() - content.trim_start().len(); + let col = |s: &str| u32::try_from(s.encode_utf16().count()).unwrap_or(u32::MAX); + Range { start: Pos { line, col: col(&content[..indent]) }, end: Pos { line, col: col(content) } } +} + +#[cfg(test)] +mod tests { + use super::*; + + const REPORT: &str = r#"{"totals":{"errors":0,"file_errors":3},"files":{ + "/p/src/A.php":{"errors":2,"messages":[ + {"message":"Constant UNKNOWN not found.","line":11,"ignorable":true, + "tip":"Learn more at https://phpstan.org/user-guide/discovering-symbols","identifier":"constant.notFound"}, + {"message":"Access to an undefined property.","line":3,"ignorable":true, + "tip":"Learn more: https://phpstan.org/blog/solving for array","identifier":"property.notFound"}]}, + "/p/src/T.php (in context of class App\\A)":{"errors":1,"messages":[ + {"message":"Syntax error","line":null,"ignorable":false,"identifier":"phpstan.parse"}]}}, + "errors":[]}"#; + + fn message(line: u32, text: &str, identifier: &str) -> Message { + Message { line, text: text.into(), identifier: Some(identifier.into()) } + } + + #[test] + fn messages_keep_their_line_tip_and_identifier() { + let report = parse(REPORT, "Note: Using configuration file /p/phpstan.neon.").unwrap(); + assert!(report.errors.is_empty()); + assert_eq!( + report.files["/p/src/A.php"], + [ + message( + 10, + "Constant UNKNOWN not found.\nLearn more at https://phpstan.org/user-guide/discovering-symbols", + "constant.notFound" + ), + message( + 2, + "Access to an undefined property.\nLearn more: https://phpstan.org/blog/solving for array", + "property.notFound" + ), + ] + ); + assert_eq!( + report.files["/p/src/T.php (in context of class App\\A)"], + [message(0, "Syntax error", "phpstan.parse")] + ); + } + + #[test] + fn errors_of_the_run_are_not_file_messages() { + let stdout = + r#"{"totals":{"errors":1,"file_errors":0},"files":[],"errors":["Invalid entries in excludePaths"]}"#; + let report = parse(stdout, "").unwrap(); + assert!(report.files.is_empty()); + assert_eq!(report.errors, ["Invalid entries in excludePaths"]); + // PHP warnings ahead of the report, or a byte order mark printed by a bootstrap file. + let warned = format!("Deprecated: something in /p/x.php on line 3\n{stdout}"); + assert_eq!(parse(&warned, "").unwrap().errors, ["Invalid entries in excludePaths"]); + let marked = format!("\u{feff}{stdout}"); + assert_eq!(parse(&marked, "").unwrap().errors, ["Invalid entries in excludePaths"]); + assert_eq!(parse(&format!("Notice: x{stdout}"), "").unwrap().errors, ["Invalid entries in excludePaths"]); + assert!(parse(stdout, "").unwrap().complete); + let severe = " Result is incomplete because of severe errors."; + assert!(!parse(stdout, severe).unwrap().complete); + } + + #[test] + fn output_without_a_report_is_a_failure() { + let stderr = "Note: Using configuration file /p/phpstan.neon.\n\n Invalid entry in excludePaths:\n Path \"/p/nothere\" is neither a directory\n\n"; + assert_eq!( + parse("", stderr), + Err("Invalid entry in excludePaths:\nPath \"/p/nothere\" is neither a directory".to_string()) + ); + assert_eq!( + parse("Fatal error: Allowed memory size exhausted", ""), + Err("Fatal error: Allowed memory size exhausted".into()) + ); + assert_eq!(parse("", ""), Err(String::new())); + // Excluded files are no failure: nothing to report. + assert_eq!(parse("", " [ERROR] No files found to analyse.\n"), Ok(Report::default())); + } + + #[test] + fn checked_files_get_their_messages_and_clean_files_none() { + let dir = tempfile::tempdir().unwrap(); + let root = dunce::canonicalize(dir.path()).unwrap(); + let (a, t, clean) = (root.join("A.php"), root.join("T.php"), root.join("Clean.php")); + std::fs::write(&a, ">(), + [(a, 1), (t, 1), (clean, 0)] + ); + let d = &out[0].1[0]; + // From the first non-blank character to the end of the line, in UTF-16 columns. + assert_eq!(d.range, Range { start: Pos { line: 2, col: 4 }, end: Pos { line: 2, col: 12 } }); + assert_eq!( + (d.code.as_deref(), d.source.as_deref(), d.server.as_deref(), d.severity), + (Some("variable.undefined"), Some("PHPStan"), Some("phpstan"), Severity::Warning) + ); + assert_eq!(line_range(None, 7), Range { start: Pos { line: 7, col: 0 }, end: Pos { line: 7, col: 0 } }); + } + + #[tokio::test(start_paused = true)] + async fn burst_saves_give_one_check_of_all_files() { + let mut queue = Queue::default(); + queue.push([PathBuf::from("/p/A.php")]); + let token = queue.cancel.clone(); + let lifetime = CancellationToken::new(); + let pending_lifetime = lifetime.clone(); + let waiting = tokio::spawn(async move { debounce(&token, &pending_lifetime).await }); + tokio::task::yield_now().await; + tokio::time::advance(DELAY - Duration::from_millis(1)).await; + assert!(!waiting.is_finished()); + for i in 0..20 { + queue.push([PathBuf::from(format!("/p/{}.php", i % 3))]); + } + assert!(!waiting.await.unwrap()); + let current = queue.generation; + assert!(debounce(&queue.cancel, &lifetime).await); + assert!(queue.current(current)); + assert_eq!(queue.files.len(), 4); + lifetime.cancel(); + assert!(!debounce(&queue.cancel, &lifetime).await); + } + + /// A project with PHPStan whose PHP logs each run, waits `pause` seconds, prints `stderr` and + /// reports `report`. + #[cfg(unix)] + struct Fixture { + _dir: tempfile::TempDir, + root: PathBuf, + events: Arc>>, + trusted: Arc, + checker: Arc, + } + + #[cfg(unix)] + impl Fixture { + fn new() -> Self { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let base = dunce::canonicalize(dir.path()).unwrap(); + let root = base.join("project"); + std::fs::create_dir_all(root.join("vendor/bin")).unwrap(); + std::fs::create_dir_all(root.join("src")).unwrap(); + std::fs::write(root.join("vendor/bin/phpstan"), "> runs.log\n\ + sleep \"$(cat pause 2>/dev/null || echo 0)\"\ncat stderr >&2 2>/dev/null\ncat report\n"; + std::fs::write(&php, script).unwrap(); + std::fs::set_permissions(&php, std::fs::Permissions::from_mode(0o755)).unwrap(); + std::fs::create_dir_all(root.join(".openidea")).unwrap(); + let settings = format!("[languages.php]\ninterpreter = {:?}\n", php.to_string_lossy()); + std::fs::write(root.join(".openidea/settings.toml"), settings).unwrap(); + let config = base.join("config"); + let settings = Arc::new(SettingsStore::load(&config, Some(&root))); + let events = Arc::new(Mutex::new(Vec::new())); + let sink_events = events.clone(); + let sink: SharedSink = Arc::new(move |e| sink_events.lock().push(e)); + let trusted = Arc::new(AtomicBool::new(true)); + let checker = Phpstan::new(root.clone(), config, settings, sink, trusted.clone()); + checker.start(&tokio::runtime::Handle::current()); + Self { _dir: dir, root, events, trusted, checker } + } + + fn file(&self, name: &str) -> PathBuf { + let path = self.root.join("src").join(name); + std::fs::write(&path, " = identifiers + .iter() + .map(|id| serde_json::json!({"message": "Bad", "line": 2, "identifier": id, "ignorable": true})) + .collect(); + let files = if messages.is_empty() { + serde_json::json!({}) + } else { + serde_json::json!({ path.to_string_lossy(): {"errors": messages.len(), "messages": messages} }) + }; + std::fs::write(self.root.join("report"), serde_json::json!({"files": files, "errors": []}).to_string()) + .unwrap(); + } + + fn runs(&self) -> Vec { + std::fs::read_to_string(self.root.join("runs.log")).unwrap_or_default().lines().map(str::to_owned).collect() + } + + async fn until(&self, what: &str, done: impl Fn(&Self) -> bool) { + for _ in 0..200 { + if done(self) { + return; + } + tokio::time::sleep(Duration::from_millis(25)).await; + } + panic!("timed out waiting for {what}: runs {:?}, events {:?}", self.runs(), self.events.lock()); + } + + fn diagnostics(&self) -> Vec<(PathBuf, Vec>)> { + self.events + .lock() + .iter() + .filter_map(|e| match e { + IdeEvent::Diagnostics { path, source, items } if source == SOURCE => { + Some((path.to_path_buf(), items.iter().map(|d| d.code.clone()).collect())) + } + _ => None, + }) + .collect() + } + + fn notifications(&self) -> Vec { + let events = self.events.lock(); + events.iter().filter(|e| matches!(e, IdeEvent::Notify { .. })).map(|e| format!("{e:?}")).collect() + } + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn saves_are_checked_together_and_a_newer_save_cancels_the_check() { + let f = Fixture::new(); + let a = f.file("A.php"); + f.report(&a, &["constant.notFound"]); + for _ in 0..5 { + f.checker.files_saved([a.clone(), f.root.join("src/readme.txt")]); + } + f.until("one check", |f| f.diagnostics().len() == 1).await; + assert_eq!(f.runs(), [format!("off analyse --error-format=json --no-progress {}", a.display())]); + assert_eq!(f.diagnostics(), [(a.clone(), vec![Some("constant.notFound".to_string())])]); + + // A save during a check kills it; the next check covers both files. + std::fs::write(f.root.join("pause"), "30").unwrap(); + f.checker.files_saved([a.clone()]); + f.until("the slow check", |f| f.runs().len() == 2).await; + std::fs::write(f.root.join("pause"), "0").unwrap(); + let b = f.file("B.php"); + f.report(&a, &[]); + f.checker.files_saved([b.clone()]); + f.until("the check of both", |f| f.runs().len() == 3 && f.diagnostics().len() == 2).await; + assert!(f.runs()[2].ends_with(&format!("{} {}", a.display(), b.display())), "{:?}", f.runs()); + // A clean result clears A; B never had diagnostics, so nothing is sent for it. + assert_eq!(f.diagnostics()[1], (a, vec![])); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn a_failing_run_is_reported_once_and_projects_without_phpstan_stay_quiet() { + let f = Fixture::new(); + let a = f.file("A.php"); + f.report(&a, &["constant.notFound"]); + f.checker.files_saved([a.clone()]); + f.until("the first check", |f| f.diagnostics().len() == 1).await; + + std::fs::write(f.root.join("report"), "").unwrap(); + for run in 2..=3 { + f.checker.files_saved([a.clone()]); + f.until("the failing check", |f| f.runs().len() == run).await; + } + tokio::time::sleep(Duration::from_millis(200)).await; + let notes = f.notifications(); + assert_eq!(notes.len(), 1, "{notes:?}"); + assert!(notes[0].contains("PHPStan check failed") && notes[0].contains("no report"), "{notes:?}"); + // The failed check leaves no stale errors. + assert_eq!(f.diagnostics(), [(a.clone(), vec![Some("constant.notFound".to_string())]), (a.clone(), vec![])]); + + std::fs::remove_file(f.root.join("vendor/bin/phpstan")).unwrap(); + let events = f.events.lock().len(); + f.checker.files_saved([a.clone()]); + tokio::time::sleep(DELAY * 2).await; + assert_eq!(f.events.lock().len(), events); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn files_a_severe_error_left_unchecked_keep_their_diagnostics() { + let f = Fixture::new(); + let (a, b) = (f.file("A.php"), f.file("B.php")); + f.report(&a, &["constant.notFound"]); + f.checker.files_saved([a.clone()]); + f.until("the check of A", |f| f.diagnostics().len() == 1).await; + // A syntax error in B: PHPStan reports B only and says the result is incomplete. + f.report(&b, &["phpstan.parse"]); + std::fs::write(f.root.join("stderr"), "Result is incomplete because of severe errors.").unwrap(); + f.checker.files_saved([a.clone(), b.clone()]); + f.until("the check of both", |f| f.diagnostics().len() == 2).await; + tokio::time::sleep(Duration::from_millis(200)).await; + assert_eq!(f.diagnostics()[1], (b, vec![Some("phpstan.parse".to_string())])); + assert_eq!(f.diagnostics().len(), 2, "A is not cleared"); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn a_file_deleted_during_its_check_gets_no_result_and_no_failure() { + let f = Fixture::new(); + let (a, b) = (f.file("A.php"), f.file("B.php")); + f.report(&a, &["constant.notFound"]); + std::fs::write(f.root.join("pause"), "1").unwrap(); + f.checker.files_saved([a.clone(), b.clone()]); + f.until("the check", |f| f.runs().len() == 1).await; + std::fs::remove_file(&a).unwrap(); + // B is checked again on its own. + f.until("the check of B", |f| f.runs().len() == 2).await; + assert!(f.runs()[1].ends_with(&format!("--no-progress {}", b.display())), "{:?}", f.runs()); + tokio::time::sleep(Duration::from_millis(300)).await; + assert!(f.events.lock().is_empty(), "{:?}", f.events.lock()); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn restricted_mode_stops_the_check_in_progress() { + let f = Fixture::new(); + let a = f.file("A.php"); + f.report(&a, &["constant.notFound"]); + std::fs::write(f.root.join("pause"), "30").unwrap(); + f.checker.files_saved([a.clone()]); + f.until("the check", |f| f.runs().len() == 1).await; + f.trusted.store(false, Ordering::Release); + f.checker.interrupt(); + tokio::time::sleep(DELAY * 2).await; + assert_eq!(f.runs().len(), 1); + assert!(f.events.lock().is_empty()); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn files_no_longer_checked_lose_their_diagnostics() { + let f = Fixture::new(); + let a = f.file("A.php"); + f.report(&a, &["constant.notFound"]); + f.checker.files_saved([a.clone()]); + f.until("the check", |f| f.diagnostics().len() == 1).await; + // PHPStan removed from the project: saving A no longer checks it and drops its errors. + std::fs::remove_file(f.root.join("vendor/bin/phpstan")).unwrap(); + f.checker.files_saved([a.clone()]); + f.until("the clear", |f| f.diagnostics().len() == 2).await; + assert_eq!(f.diagnostics()[1], (a, vec![])); + assert_eq!(f.runs().len(), 1); + f.checker.stop(); + } + + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn removed_files_lose_their_diagnostics() { + let f = Fixture::new(); + let a = f.file("A.php"); + f.report(&a, &["constant.notFound"]); + f.checker.files_saved([a.clone()]); + f.until("the check", |f| f.diagnostics().len() == 1).await; + f.checker.files_removed([f.root.join("src/other")].iter()); + assert_eq!(f.diagnostics().len(), 1); + f.checker.files_removed([f.root.join("src")].iter()); + assert_eq!(f.diagnostics()[1], (a, vec![])); + f.checker.stop(); + } +} diff --git a/crates/ide-host/src/project.rs b/crates/ide-host/src/project.rs index 0bdd984c9..9fd0c2139 100644 --- a/crates/ide-host/src/project.rs +++ b/crates/ide-host/src/project.rs @@ -55,6 +55,7 @@ pub struct Project { /// Native notebooks and owned kernels, scoped to this project incarnation. pub notebooks: Arc, pub(crate) kotlin_bridge: Option>, + pub(crate) phpstan: Option>, pub(crate) watcher: Watcher, /// Watchers of the Project Structure content roots outside the project folder. pub(crate) content_watchers: Mutex>, @@ -292,12 +293,21 @@ impl Host { trusted.clone(), ); documents.add_observer(kotlin_bridge.clone()); + let phpstan = crate::phpstan::Phpstan::new( + root.clone(), + self.config_dir.clone(), + settings.clone(), + sink.clone(), + trusted.clone(), + ); + documents.add_observer(phpstan.clone()); let repos = Arc::new(RwLock::new(Repos::enclosing(&root)?)); for repo in repos.read().all() { repo.set_execution_allowed(trusted.load(Ordering::Acquire)); } let watcher = { let kotlin_bridge = kotlin_bridge.clone(); + let phpstan = phpstan.clone(); let repos = repos.clone(); let settings = settings.clone(); let index = index.clone(); @@ -321,6 +331,7 @@ impl Host { let (ignored, ignored_gone) = ignored_changes(batch); index.update(&[&changed[..], &ignored].concat(), &[&batch.removed[..], &ignored_gone].concat()); kotlin_bridge.files_changed(changed.iter().chain(&batch.removed)); + phpstan.files_removed(batch.removed.iter()); lsp.project_files_changed(batch.created.iter().chain(&batch.removed)); lsp.watched_files_changed(&batch.created, &batch.modified, &batch.removed, &batch.excluded_dirs); if composer_changed(&root, changed.iter().chain(&batch.removed)) { @@ -440,6 +451,7 @@ impl Host { &history, &lsp, Some(&kotlin_bridge), + Some(&phpstan), )?; Some((r.clone(), w)) }) @@ -486,6 +498,7 @@ impl Host { agents, notebooks, kotlin_bridge: Some(kotlin_bridge), + phpstan: Some(phpstan), watcher, content_watchers: Mutex::new(content_watchers), lsp_watchers: Mutex::new(Vec::new()), @@ -516,6 +529,9 @@ impl Host { if let Some(bridge) = &project.kotlin_bridge { bridge.start(&self.runtime); } + if let Some(phpstan) = &project.phpstan { + phpstan.start(&self.runtime); + } if let Some(cmd) = post_start { self.environment.spawn_post_start(cmd, project.root.clone(), project.sink.clone()); } @@ -626,6 +642,7 @@ impl Host { agents, notebooks, kotlin_bridge: None, + phpstan: None, watcher, content_watchers: Mutex::new(Vec::new()), lsp_watchers: Mutex::new(Vec::new()), @@ -1002,6 +1019,7 @@ impl Project { &self.history, &self.lsp, self.kotlin_bridge.as_ref(), + self.phpstan.as_ref(), ) { watchers.push((r, w)); } @@ -1137,8 +1155,9 @@ fn content_root_watcher( history: &Arc, lsp: &Arc, kotlin_bridge: Option<&Arc>, + phpstan: Option<&Arc>, ) -> Option { - let kotlin_bridge = kotlin_bridge.cloned(); + let (kotlin_bridge, phpstan) = (kotlin_bridge.cloned(), phpstan.cloned()); let (index, documents, history, lsp) = (index.clone(), documents.clone(), history.clone(), lsp.clone()); let root = dir.to_path_buf(); Watcher::start( @@ -1156,6 +1175,9 @@ fn content_root_watcher( if let Some(bridge) = &kotlin_bridge { bridge.files_changed(changed.iter().chain(&batch.removed)); } + if let Some(phpstan) = &phpstan { + phpstan.files_removed(batch.removed.iter()); + } documents.on_disk_changed(&[&changed[..], &ignored].concat()); record_external_changes(&history, &changed, &batch.removed); }), diff --git a/crates/ide-host/src/trust.rs b/crates/ide-host/src/trust.rs index ca3194b50..52af748ad 100644 --- a/crates/ide-host/src/trust.rs +++ b/crates/ide-host/src/trust.rs @@ -109,6 +109,9 @@ impl crate::Host { if let Some(bridge) = &project.kotlin_bridge { bridge.schedule(); } + if let Some(phpstan) = &project.phpstan { + phpstan.interrupt(); + } if !trusted { project.lsp.set_execution_allowed(false); } diff --git a/crates/ide-sdk/src/php.rs b/crates/ide-sdk/src/php.rs index 4e1bc6c42..ff03cee85 100644 --- a/crates/ide-sdk/src/php.rs +++ b/crates/ide-sdk/src/php.rs @@ -1,12 +1,14 @@ //! The PHP language level: the one chosen in Settings › PHP, else the one the root //! `composer.json` declares (`config.platform.php`, else the lowest version `require.php` allows); -//! the Intelephense storage that goes with it; and the stub sets and include paths Intelephense indexes. +//! the Intelephense storage that goes with it; the stub sets and include paths Intelephense indexes; +//! and the PHPStan run that checks saved files. use crate::apply::{merge, merge_init_options, setting_path}; use ide_core::settings::Settings; use ide_language::LanguageServerConfig; use serde_json::{Value, json}; -use std::path::Path; +use std::path::{Path, PathBuf}; +use std::process::Command; /// A PHP version and where it comes from. #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, specta::Type)] @@ -139,6 +141,33 @@ pub fn include_paths(root: &Path, s: &Settings) -> Vec { .collect() } +/// `vendor/bin/phpstan analyse` of `files` with a JSON report and the configuration file of +/// Settings › PHP, run from `root` with Xdebug off: in debug mode it would try to reach a +/// debugger on every run. The selected PHP runs the script itself (and is first on `PATH`, as for +/// PHPUnit), so a `php8.1` is not replaced by the `php` its `#!/usr/bin/env php` finds, and +/// Windows needs no `phpstan.bat`, whose `cmd` limits the command line. `None` when the project +/// has no PHPStan. +pub fn phpstan_command(root: &Path, s: &Settings, files: &[PathBuf]) -> Option { + let bin = root.join("vendor/bin/phpstan"); + if !bin.is_file() { + return None; + } + let php = setting_path(root, &s.languages.php.interpreter) + .or_else(|| crate::exec::which("php")) + .unwrap_or_else(|| PathBuf::from("php")); + let mut cmd = Command::new(php); + cmd.current_dir(root).arg(bin).args(["analyse", "--error-format=json", "--no-progress"]); + if let Some(config) = setting_path(root, &s.languages.php.phpstan_config) { + cmd.arg("--configuration").arg(config); + } + cmd.args(files); + cmd.env("XDEBUG_MODE", "off"); + if let Some(dir) = crate::apply::php_dir(root, s) { + cmd.env("PATH", crate::exec::path_with(&[&dir])); + } + Some(cmd) +} + type Version = (u32, u32, u32); impl PhpLanguageLevel { @@ -350,6 +379,78 @@ mod tests { ); } + fn phpstan(root: &Path, s: &Settings) -> Option { + let cmd = phpstan_command(root, s, &[root.join("src").join("A.php"), root.join("src").join("B.php")])?; + assert_eq!(cmd.get_current_dir(), Some(root)); + Some(cmd) + } + + fn text(s: &std::ffi::OsStr) -> String { + s.to_string_lossy().into_owned() + } + + fn env(cmd: &Command) -> Vec<(String, Option)> { + cmd.get_envs().map(|(k, v)| (text(k), v.map(text))).collect() + } + + #[test] + fn phpstan_checks_the_files_on_the_selected_php_without_xdebug() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let mut s = Settings::default(); + assert!(phpstan(root, &s).is_none(), "no vendor/bin/phpstan, no run"); + + let bin = root.join("vendor/bin/phpstan"); + std::fs::create_dir_all(bin.parent().unwrap()).unwrap(); + std::fs::write(&bin, "").unwrap(); + let cmd = phpstan(root, &s).unwrap(); + // The PHP on PATH unless one is selected. + assert_eq!(Path::new(cmd.get_program()).file_stem(), Some("php".as_ref())); + let file = |name: &str| root.join("src").join(name).to_string_lossy().into_owned(); + assert_eq!( + cmd.get_args().map(text).collect::>(), + [ + &*bin.to_string_lossy(), + "analyse", + "--error-format=json", + "--no-progress", + &file("A.php"), + &file("B.php") + ] + ); + assert_eq!(env(&cmd), [("XDEBUG_MODE".to_string(), Some("off".to_string()))]); + + // A selected PHP runs PHPStan whatever its name, and is first on PATH for what PHPStan starts. + let php = root.join("php85/bin").join(crate::exe("php8.5")); + s.languages.php.interpreter = php.to_string_lossy().into_owned(); + let cmd = phpstan(root, &s).unwrap(); + assert_eq!(cmd.get_program(), php.as_os_str()); + let env = env(&cmd); + let path = env.iter().find(|(k, _)| k == "PATH").and_then(|(_, v)| v.clone()).expect("PATH"); + assert_eq!(std::env::split_paths(&path).next(), Some(root.join("php85/bin"))); + assert!(env.contains(&("XDEBUG_MODE".to_string(), Some("off".to_string())))); + } + + #[test] + fn phpstan_uses_the_configuration_file_of_the_settings() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let bin = root.join("vendor/bin/phpstan"); + std::fs::create_dir_all(bin.parent().unwrap()).unwrap(); + std::fs::write(&bin, "").unwrap(); + let args = |config: &str| { + let mut s = Settings::default(); + s.languages.php.phpstan_config = config.into(); + phpstan(root, &s).unwrap().get_args().map(text).collect::>() + }; + let resolved = root.join("build/phpstan.ci.neon").to_string_lossy().into_owned(); + assert_eq!(args(" build/phpstan.ci.neon ")[4..6], ["--configuration".to_string(), resolved]); + let absolute = if cfg!(windows) { "C:/etc/phpstan.neon" } else { "/etc/phpstan.neon" }; + assert_eq!(args(absolute)[4..6], ["--configuration", absolute]); + // Empty: PHPStan finds phpstan.neon, phpstan.neon.dist or phpstan.dist.neon itself. + assert!(!args(" ").iter().any(|a| a.starts_with("--configuration"))); + } + #[test] fn each_level_names_its_source() { let found = |version: &str, source| Some(PhpLanguageLevel { version: version.into(), source }); diff --git a/src/bindings.ts b/src/bindings.ts index ffe154510..c355b8051 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -5516,6 +5516,12 @@ export type PhpSettings = { * `composer.json` (`wordpress`). */ stubs?: string[], + /** + * PHPStan configuration file for checking saved files; empty = the `phpstan.neon`, + * `phpstan.neon.dist` or `phpstan.dist.neon` PHPStan finds in the project root. Absolute, + * `~/`, or relative to the project. + */ + phpstanConfig?: string, }; /** The focused editor when a plugin command is invoked. */ diff --git a/src/features/settings/__tests__/inspections.test.ts b/src/features/settings/__tests__/inspections.test.ts index 417626d15..add829dd5 100644 --- a/src/features/settings/__tests__/inspections.test.ts +++ b/src/features/settings/__tests__/inspections.test.ts @@ -62,6 +62,42 @@ describe("plugin inspections", () => { }); }); +describe("PHPStan inspections", () => { + it("list each identifier below PHPStan, from seen codes and stored overrides", () => { + const tree = buildInspectionTree( + [{ name: "PHP", servers: ["intelephense"] }], + [ + { + producer: "phpstan", + code: "return.type", + source: "PHPStan", + sample: "Should return int but returns string.", + }, + { producer: "build", code: "E0308", source: "rustc", sample: "" }, + ], + [{ "builtin:phpstan:constant.notFound": { enabled: false }, "builtin:build:E1": { enabled: false } }], + ); + const php = find(tree, "group:PHP")!; + expect(php.children.map((n) => n.id)).toEqual(["lsp:intelephense", "builtin:phpstan"]); + const phpstan = find(tree, "PHP/builtin:phpstan")!; + expect(phpstan.children.map((n) => [n.id, n.name])).toEqual([ + ["builtin:phpstan:constant.notFound", "constant.notFound"], + ["builtin:phpstan:return.type", "return.type"], + ]); + expect(phpstan.children[1]!.description).toBe( + "PHPStan problems with code return.type.\n\nExample: Should return int but returns string.", + ); + expect(pathToInspection(tree, "builtin:phpstan:return.type")).toEqual([ + "group:PHP", + "PHP/builtin:phpstan", + "PHP/builtin:phpstan:return.type", + ]); + // Other built-in checks stay single inspections. + expect(find(tree, "General/builtin:build")!.children).toEqual([]); + expect(checkState({ "builtin:phpstan:constant.notFound": { enabled: false } }, phpstan)).toBe("mixed"); + }); +}); + describe("inspections tree", () => { const tree = buildInspectionTree(languages, seen, [{ "lsp:ruff:E501": { severity: "typo" } }]); @@ -72,6 +108,7 @@ describe("inspections tree", () => { "Java", "JavaScript", "Other", + "PHP", "Proofreading", "Python", "SQL", diff --git a/src/features/settings/__tests__/languages.test.ts b/src/features/settings/__tests__/languages.test.ts index 46d86a3f7..1140093a1 100644 --- a/src/features/settings/__tests__/languages.test.ts +++ b/src/features/settings/__tests__/languages.test.ts @@ -63,7 +63,12 @@ describe("languages settings", () => { languageLevel: "", includePaths: [], stubs: [], + phpstanConfig: "", }); + expect(validateLanguages({ php: { phpstanConfig: " build/phpstan.neon " } }).php.phpstanConfig).toBe( + "build/phpstan.neon", + ); + expect(validateLanguages({ php: { phpstanConfig: 3 } }).php.phpstanConfig).toBe(""); const lists = validateLanguages({ php: { includePaths: [" ../platform ", "", 3, "../platform"], stubs: "redis" } }); expect(lists.php.includePaths).toEqual(["../platform"]); expect(lists.php.stubs).toEqual([]); diff --git a/src/features/settings/languages/LanguagePages.tsx b/src/features/settings/languages/LanguagePages.tsx index 12b9adae0..fc1cab3de 100644 --- a/src/features/settings/languages/LanguagePages.tsx +++ b/src/features/settings/languages/LanguagePages.tsx @@ -162,6 +162,16 @@ export function PhpPage() { /> +
+ +
a.code.localeCompare(b.code, undefined, { numeric: true })) - .map((c) => ({ - key: `${group}/${codeInspectionId(id, c.code)}`, - id: codeInspectionId(id, c.code), - name: c.code, - description: codeDescription(plugin ?? server, c), - children: [], - })), + children: codeNodes(group, id, plugin ?? server, codes), }; } +function codeNodes(group: string, id: string, owner: string, codes: readonly SeenCode[]): InspectionNode[] { + return [...codes] + .sort((a, b) => a.code.localeCompare(b.code, undefined, { numeric: true })) + .map((c) => ({ + key: `${group}/${codeInspectionId(id, c.code)}`, + id: codeInspectionId(id, c.code), + name: c.code, + description: codeDescription(owner, c), + children: [], + })); +} + /** * The inspection tree: one group per language that has language servers, the * built-in checks under their category, and servers known only from seen @@ -98,6 +102,9 @@ export function buildInspectionTree( const m = /^lsp:(plugin:[^:]+|[^:]+)(?::(.+))?$/.exec(id); if (m?.[2]) addCode({ producer: m[1]!, code: m[2], source: "", sample: "" }); else if (m && !codes.has(m[1]!)) codes.set(m[1]!, new Map()); + const builtin = BUILTIN_INSPECTIONS.find((b) => b.byCode && id.startsWith(`${b.id}:`)); + if (builtin) + addCode({ producer: builtin.producer, code: id.slice(builtin.id.length + 1), source: "", sample: "" }); } } const servedBy = new Map(); @@ -125,7 +132,7 @@ export function buildInspectionTree( id: b.id, name: b.name, description: b.description, - children: [], + children: b.byCode ? codeNodes(b.group, b.id, b.name, [...(codes.get(b.producer)?.values() ?? [])]) : [], }); } for (const [producer, list] of codes) { diff --git a/src/features/settings/pages/languages.tsx b/src/features/settings/pages/languages.tsx index e547ba22e..0ef497397 100644 --- a/src/features/settings/pages/languages.tsx +++ b/src/features/settings/pages/languages.tsx @@ -153,6 +153,8 @@ export const LANGUAGE_PAGES: PageSpec[] = [ "php", "composer", "phpunit", + "phpstan", + "phpstan.neon", "xampp", "herd", "homebrew", diff --git a/src/platform/inspectionControls.test.ts b/src/platform/inspectionControls.test.ts index c30a5f63d..a5ba00bb3 100644 --- a/src/platform/inspectionControls.test.ts +++ b/src/platform/inspectionControls.test.ts @@ -39,6 +39,13 @@ describe("inspection controls", () => { expect(diagnosticInspection("ruff", { code: "F401" })).toEqual({ id: "lsp:ruff:F401", name: "ruff F401" }); expect(diagnosticInspection("pyright", { code: null })).toEqual({ id: "lsp:pyright", name: "pyright" }); expect(diagnosticInspection("", { code: null })).toBeNull(); + // A built-in check reported per code: PHPStan's identifiers. + expect(diagnosticInspection("phpstan", { code: "constant.notFound" })).toEqual({ + id: "builtin:phpstan:constant.notFound", + name: "PHPStan constant.notFound", + }); + expect(diagnosticInspection("phpstan", { code: null })).toEqual({ id: "builtin:phpstan", name: "PHPStan" }); + expect(diagnosticInspection("build", { code: "E0308" })).toEqual({ id: "builtin:build", name: "Build problems" }); }); it("writes to the profile in effect, at its level", async () => { diff --git a/src/platform/inspections.test.ts b/src/platform/inspections.test.ts index f86aa4661..38d20dffd 100644 --- a/src/platform/inspections.test.ts +++ b/src/platform/inspections.test.ts @@ -112,6 +112,26 @@ describe("inspection profiles", () => { expect(isTypoStyled(applyProfile({ "lsp:ruff": { severity: "typo" } }, "ruff", [diag("x")])[0]!)).toBe(true); }); + it("shows PHPStan results as warnings, each identifier turned off or re-graded on its own", () => { + const phpstan = (code: string | null): Diagnostic => ({ ...diag(code, "warning", "phpstan"), source: "PHPStan" }); + const items = [phpstan("constant.notFound"), phpstan("return.type"), phpstan(null)]; + expect(producerInspectionId("phpstan")).toBe("builtin:phpstan"); + expect(applyProfile({}, "phpstan", items).map((d) => d.severity)).toEqual(["warning", "warning", "warning"]); + const out = applyProfile( + { + "builtin:phpstan:constant.notFound": { enabled: false }, + "builtin:phpstan:return.type": { severity: "error" }, + }, + "phpstan", + items, + ); + expect(out.map((d) => [d.code, d.severity])).toEqual([ + ["return.type", "error"], + [null, "warning"], + ]); + expect(applyProfile({ "builtin:phpstan": { enabled: false } }, "phpstan", items)).toEqual([]); + }); + it("lists, labels and resolves copied profiles", () => { const s = validateInspections({ profile: "ide:strict", diff --git a/src/platform/inspections.ts b/src/platform/inspections.ts index 5dce0afd8..88b2ec6df 100644 --- a/src/platform/inspections.ts +++ b/src/platform/inspections.ts @@ -218,6 +218,8 @@ export interface BuiltinInspection { defaultLevel?: InspectionLevel; /** `false`: off until a profile turns it on. */ defaultEnabled?: boolean; + /** Each diagnostic code is an inspection of its own (`:`), as a language server's are. */ + byCode?: boolean; options?: readonly InspectionOptionSpec[]; } @@ -298,6 +300,16 @@ export const BUILTIN_INSPECTIONS: readonly BuiltinInspection[] = [ defaultLevel: "warning", defaultEnabled: false, }, + { + id: "builtin:phpstan", + producer: "phpstan", + name: "PHPStan", + group: "PHP", + description: + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.", + defaultLevel: "warning", + byCode: true, + }, { id: "builtin:sql", producer: "sql-inspections", @@ -346,9 +358,10 @@ export interface InspectionRef { export function diagnosticInspection(producer: string, d: Pick): InspectionRef | null { if (!producer) return null; const builtin = BUILTIN_BY_PRODUCER.get(producer); - if (builtin) return { id: builtin.id, name: builtin.name }; + if (builtin && !(builtin.byCode && d.code)) return { id: builtin.id, name: builtin.name }; const base = producerInspectionId(producer); - return d.code ? { id: codeInspectionId(base, d.code), name: `${producer} ${d.code}` } : { id: base, name: producer }; + const name = builtin?.name ?? producer; + return d.code ? { id: codeInspectionId(base, d.code), name: `${name} ${d.code}` } : { id: base, name }; } /** Inspection id of everything a producer reports. */ diff --git a/src/platform/languageSettings.ts b/src/platform/languageSettings.ts index ad9fd4eb5..493527c8a 100644 --- a/src/platform/languageSettings.ts +++ b/src/platform/languageSettings.ts @@ -81,6 +81,7 @@ export interface LanguagesValid { languageLevel: (typeof PHP_LANGUAGE_LEVELS)[number]; includePaths: string[]; stubs: string[]; + phpstanConfig: string; }; rust: { toolchain: string; @@ -152,7 +153,7 @@ export const DEFAULT_LANGUAGES: LanguagesValid = { python: { interpreter: "", activateVirtualenv: true }, ruby: { interpreter: "" }, node: { interpreter: "", packageManager: "" }, - php: { interpreter: "", languageLevel: "", includePaths: [], stubs: [] }, + php: { interpreter: "", languageLevel: "", includePaths: [], stubs: [], phpstanConfig: "" }, rust: { toolchain: "", stdlib: "", checkOnSave: true, checkCommand: "check", rustfmtArgs: "" }, go: { goroot: "", gopath: "", buildTags: DEFAULT_GO_BUILD_TAGS, modules: { env: {}, autoVendor: true } }, typescript: { version: "bundled" }, @@ -271,6 +272,7 @@ export function validateLanguages(raw: unknown): LanguagesValid { languageLevel: oneOf(php.languageLevel, PHP_LANGUAGE_LEVELS, d.php.languageLevel), includePaths: strings(php.includePaths), stubs: strings(php.stubs), + phpstanConfig: text(php.phpstanConfig, d.php.phpstanConfig), }, rust: { toolchain: text(rust.toolchain, d.rust.toolchain), diff --git a/src/platform/locales/text/en.json b/src/platform/locales/text/en.json index d7f27a388..675492bcd 100644 --- a/src/platform/locales/text/en.json +++ b/src/platform/locales/text/en.json @@ -5,5 +5,12 @@ "WSL distribution:": "WSL distribution:", "WSL distribution": "WSL distribution", "Project runs on {label}": "Project runs on {label}", - "Forwarding ports is not supported in a WSL project yet": "Forwarding ports is not supported in a WSL project yet" + "Forwarding ports is not supported in a WSL project yet": "Forwarding ports is not supported in a WSL project yet", + "PHPStan check failed": "PHPStan check failed", + "PHPStan ended with {status} and no report": "PHPStan ended with {status} and no report", + "Cannot run {program}: {error}": "Cannot run {program}: {error}", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.", + "Select PHPStan Configuration File": "Select PHPStan Configuration File" } diff --git a/src/platform/locales/text/es.json b/src/platform/locales/text/es.json index 52e968ef0..81abd1976 100644 --- a/src/platform/locales/text/es.json +++ b/src/platform/locales/text/es.json @@ -58,6 +58,9 @@ "Kotlin bridge bundle is unavailable": "El paquete del puente de Kotlin no está disponible", "Kotlin bridge command did not acknowledge its library snapshot": "El comando del puente de Kotlin no confirmó su instantánea de biblioteca", "Kotlin editor compilation failed": "Error en la compilación del editor de Kotlin", + "PHPStan check failed": "Error en la verificación de PHPStan", + "PHPStan ended with {status} and no report": "PHPStan terminó ({status}) sin generar un reporte", + "Cannot run {program}: {error}": "No se puede ejecutar {program}: {error}", "Updating results…": "Actualizando resultados…", "Uploading files": "Subiendo archivos", "No files on the clipboard": "No hay archivos en el portapapeles", @@ -3898,6 +3901,7 @@ "Change Severity": "Cambiar severidad", "Edit Inspection Profile Setting…": "Editar configuración del perfil de inspección…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "Errores y advertencias del compilador de la última compilación del proyecto, mostrados en sus posiciones en el código fuente.", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "Errores que reporta el vendor/bin/phpstan del proyecto en un archivo PHP al guardarlo, mostrados en sus líneas. PHPStan lee phpstan.neon, phpstan.neon.dist o phpstan.dist.neon de la raíz del proyecto, o el archivo de configuración elegido en Configuración › Lenguajes y frameworks › PHP. Cada regla que PHPStan ha reportado aparece abajo con su identificador y puede desactivarse o cambiar de severidad por separado.", "Process code": "Procesar código", "Process literals": "Procesar literales", "Process comments": "Procesar comentarios", @@ -5677,6 +5681,9 @@ "No additional stubs": "Sin stubs adicionales", "Stub": "Stub", "Stub name, e.g. wordpress": "Nombre del stub, p. ej., wordpress", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist o phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "La configuración con la que vendor/bin/phpstan verifica los archivos guardados. Vacío usa phpstan.neon, phpstan.neon.dist o phpstan.dist.neon en la raíz del proyecto. Ruta absoluta o relativa al proyecto.", + "Select PHPStan Configuration File": "Seleccionar archivo de configuración de PHPStan", "select files in a git repository to shelve": "seleccione archivos en un repositorio git para guardar en el shelf", "select files from a single git repository to shelve": "seleccione archivos de un único repositorio git para guardar en el shelf", "External file changes sync may be slow": "La sincronización de cambios externos de archivos puede ser lenta", diff --git a/src/platform/locales/text/fr.json b/src/platform/locales/text/fr.json index c72eb66c0..9d73910ab 100644 --- a/src/platform/locales/text/fr.json +++ b/src/platform/locales/text/fr.json @@ -7,6 +7,9 @@ "Kotlin bridge bundle is unavailable": "Le bundle du pont Kotlin est indisponible", "Kotlin bridge command did not acknowledge its library snapshot": "La commande du pont Kotlin n'a pas accusé réception de son instantané de bibliothèques", "Kotlin editor compilation failed": "Échec de la compilation de l'éditeur Kotlin", + "PHPStan check failed": "Échec de la vérification PHPStan", + "PHPStan ended with {status} and no report": "PHPStan s'est terminé ({status}) sans produire de rapport", + "Cannot run {program}: {error}": "Impossible d'exécuter {program} : {error}", "Updating results…": "Mise à jour des résultats…", "Uploading files": "Envoi des fichiers", "No files on the clipboard": "Aucun fichier dans le presse-papiers", @@ -3764,6 +3767,7 @@ "Change Severity": "Modifier la gravité", "Edit Inspection Profile Setting…": "Modifier le paramètre du profil d'inspection…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "Erreurs et avertissements du compilateur du dernier build du projet, affichés à leur emplacement dans la source.", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "Erreurs que le vendor/bin/phpstan du projet signale dans un fichier PHP lors de son enregistrement, affichées sur leurs lignes. PHPStan lit phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon à la racine du projet, ou le fichier de configuration choisi dans Paramètres › Langages et frameworks › PHP. Chaque règle signalée par PHPStan est listée ci-dessous par son identifiant ; on peut la désactiver ou modifier sa gravité individuellement.", "Process code": "Traiter le code", "Process literals": "Traiter les littéraux", "Process comments": "Traiter les commentaires", @@ -5770,6 +5774,9 @@ "PHP {version}, from config.platform.php in composer.json": "PHP {version}, depuis config.platform.php de composer.json", "PHP {version}, the lowest version require.php in composer.json allows": "PHP {version}, la version la plus basse autorisée par require.php de composer.json", "Set in the settings of the linked project {name}": "Défini dans les paramètres du projet lié {name}", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "La configuration avec laquelle vendor/bin/phpstan vérifie les fichiers enregistrés. Vide : utilise phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon à la racine du projet. Chemin absolu ou relatif au projet.", + "Select PHPStan Configuration File": "Sélectionner le fichier de configuration PHPStan", "select files in a git repository to shelve": "sélectionnez les fichiers d'un dépôt git à mettre de côté", "select files from a single git repository to shelve": "sélectionnez les fichiers d'un seul dépôt git à mettre de côté", "External file changes sync may be slow": "La synchronisation des modifications externes de fichiers peut être lente", diff --git a/src/platform/locales/text/ko.json b/src/platform/locales/text/ko.json index 893c520c9..6dfb27960 100644 --- a/src/platform/locales/text/ko.json +++ b/src/platform/locales/text/ko.json @@ -50,6 +50,9 @@ "Kotlin bridge bundle is unavailable": "Kotlin 브리지 번들을 사용할 수 없습니다", "Kotlin bridge command did not acknowledge its library snapshot": "Kotlin 브리지 명령이 라이브러리 스냅샷을 확인하지 않았습니다", "Kotlin editor compilation failed": "Kotlin 편집기 컴파일 실패", + "PHPStan check failed": "PHPStan 검사 실패", + "PHPStan ended with {status} and no report": "PHPStan이 보고서 없이 종료되었습니다({status})", + "Cannot run {program}: {error}": "{program}을(를) 실행할 수 없습니다: {error}", "Updating results…": "결과 업데이트 중…", "Uploading files": "파일 업로드 중", "No files on the clipboard": "클립보드에 파일이 없습니다", @@ -3877,6 +3880,7 @@ "Change Severity": "심각도 변경", "Edit Inspection Profile Setting…": "검사 프로필 설정 편집…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "마지막 프로젝트 빌드의 컴파일러 오류 및 경고를 해당 소스 위치에 표시합니다.", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "PHP 파일을 저장할 때 프로젝트의 vendor/bin/phpstan이 보고하는 오류를 해당 줄에 표시합니다. PHPStan은 프로젝트 루트의 phpstan.neon, phpstan.neon.dist 또는 phpstan.dist.neon이나 설정 › 언어 및 프레임워크 › PHP에서 선택한 구성 파일을 읽습니다. PHPStan이 보고한 각 규칙은 아래에 식별자로 나열되며 개별적으로 끄거나 심각도를 변경할 수 있습니다.", "Process code": "코드 처리", "Process literals": "리터럴 처리", "Process comments": "주석 처리", @@ -5624,6 +5628,9 @@ "PHP {version}, from config.platform.php in composer.json": "PHP {version}, composer.json의 config.platform.php 기준", "PHP {version}, the lowest version require.php in composer.json allows": "PHP {version}, composer.json의 require.php가 허용하는 가장 낮은 버전", "Set in the settings of the linked project {name}": "연결된 프로젝트 {name}의 설정에서 지정", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist 또는 phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "vendor/bin/phpstan이 저장된 파일을 검사할 때 사용하는 구성입니다. 비워 두면 프로젝트 루트의 phpstan.neon, phpstan.neon.dist 또는 phpstan.dist.neon을 사용합니다. 절대 경로 또는 프로젝트 기준 상대 경로입니다.", + "Select PHPStan Configuration File": "PHPStan 구성 파일 선택", "select files in a git repository to shelve": "셸브할 git 저장소의 파일을 선택하세요", "select files from a single git repository to shelve": "셸브할 단일 git 저장소의 파일을 선택하세요", "External file changes sync may be slow": "외부 파일 변경 사항 동기화가 느릴 수 있습니다", diff --git a/src/platform/locales/text/pl.json b/src/platform/locales/text/pl.json index 16d33e9e0..6162c9597 100644 --- a/src/platform/locales/text/pl.json +++ b/src/platform/locales/text/pl.json @@ -50,6 +50,9 @@ "Kotlin bridge bundle is unavailable": "Pakiet mostka Kotlin jest niedostępny", "Kotlin bridge command did not acknowledge its library snapshot": "Polecenie mostka Kotlin nie potwierdziło migawki swojej biblioteki", "Kotlin editor compilation failed": "Kompilacja edytora Kotlin nie powiodła się", + "PHPStan check failed": "Sprawdzanie PHPStan nie powiodło się", + "PHPStan ended with {status} and no report": "PHPStan zakończył działanie ({status}) i nie wygenerował raportu", + "Cannot run {program}: {error}": "Nie można uruchomić {program}: {error}", "Updating results…": "Aktualizowanie wyników…", "Uploading files": "Przesyłanie plików", "No files on the clipboard": "Brak plików w schowku", @@ -3864,6 +3867,7 @@ "Change Severity": "Zmień poziom ważności", "Edit Inspection Profile Setting…": "Edytuj ustawienia profilu inspekcji…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "Błędy i ostrzeżenia kompilatora z ostatniego budowania projektu, wyświetlane w miejscach w kodzie.", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "Błędy, które vendor/bin/phpstan projektu zgłasza w pliku PHP po jego zapisaniu, wyświetlane w odpowiednich wierszach. PHPStan odczytuje phpstan.neon, phpstan.neon.dist lub phpstan.dist.neon z katalogu głównego projektu albo plik konfiguracyjny wybrany w Ustawienia › Języki i technologie › PHP. Każda reguła zgłoszona przez PHPStan jest wymieniona poniżej według identyfikatora i można ją osobno wyłączyć lub zmienić jej poziom ważności.", "Process code": "Przetwarzaj kod", "Process literals": "Przetwarzaj literały", "Process comments": "Przetwarzaj komentarze", @@ -5607,6 +5611,9 @@ "PHP interpreter": "Interpretator PHP", "Runs PHP scripts, PHPUnit, Composer and artisan, and the PHP-based language servers (phpactor). Its folder is put first on PATH, so vendor/bin/phpunit and other #!/usr/bin/env php scripts use it too.": "Uruchamia skrypty PHP, PHPUnit, Composer, artisan oraz serwery językowe oparte na PHP (phpactor). Jego folder umieszczany jest na początku zmiennej PATH, dzięki czemu skrypty vendor/bin/phpunit i inne #!/usr/bin/env php również z niego korzystają.", "Select PHP Interpreter": "Wybierz interpretator PHP", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist lub phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "Konfiguracja, której vendor/bin/phpstan używa do sprawdzania zapisanych plików. Puste pole spowoduje użycie phpstan.neon, phpstan.neon.dist lub phpstan.dist.neon z katalogu głównego projektu. Ścieżka bezwzględna lub względna wobec projektu.", + "Select PHPStan Configuration File": "Wybierz plik konfiguracyjny PHPStan", "select files in a git repository to shelve": "wybierz pliki w repozytorium Git do odłożenia na półkę (shelve)", "select files from a single git repository to shelve": "wybierz pliki z pojedynczego repozytorium Git do odłożenia na półkę (shelve)", "External file changes sync may be slow": "Synchronizacja zewnętrznych zmian w plikach może działać wolno", diff --git a/src/platform/locales/text/pt.json b/src/platform/locales/text/pt.json index ff82111dd..848774f0d 100644 --- a/src/platform/locales/text/pt.json +++ b/src/platform/locales/text/pt.json @@ -50,6 +50,9 @@ "Kotlin bridge bundle is unavailable": "O pacote da ponte do Kotlin está indisponível", "Kotlin bridge command did not acknowledge its library snapshot": "O comando da ponte do Kotlin não confirmou o instantâneo da sua biblioteca", "Kotlin editor compilation failed": "Falha na compilação do editor Kotlin", + "PHPStan check failed": "Falha na verificação do PHPStan", + "PHPStan ended with {status} and no report": "O PHPStan terminou ({status}) sem gerar um relatório", + "Cannot run {program}: {error}": "Não é possível executar {program}: {error}", "Updating results…": "Atualizando resultados…", "Uploading files": "Enviando arquivos", "No files on the clipboard": "Nenhum arquivo na área de transferência", @@ -3870,6 +3873,7 @@ "Change Severity": "Alterar gravidade", "Edit Inspection Profile Setting…": "Editar configuração do perfil de inspeção…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "Erros e avisos do compilador do último build do projeto, exibidos em suas posições no código-fonte.", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "Erros que o vendor/bin/phpstan do projeto relata em um arquivo PHP quando ele é salvo, exibidos em suas linhas. O PHPStan lê phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon da raiz do projeto, ou o arquivo de configuração escolhido em Configurações › Linguagens e frameworks › PHP. Cada regra que o PHPStan relatou é listada abaixo pelo seu identificador e pode ser desativada ou ter sua gravidade alterada individualmente.", "Process code": "Processar código", "Process literals": "Processar literais", "Process comments": "Processar comentários", @@ -5621,6 +5625,9 @@ "PHP interpreter": "Interpretador PHP", "Runs PHP scripts, PHPUnit, Composer and artisan, and the PHP-based language servers (phpactor). Its folder is put first on PATH, so vendor/bin/phpunit and other #!/usr/bin/env php scripts use it too.": "Executa scripts PHP, PHPUnit, Composer e artisan, e os servidores de linguagem baseados em PHP (phpactor). Sua pasta é colocada primeiro no PATH, de modo que vendor/bin/phpunit e outros scripts #!/usr/bin/env php também a utilizem.", "Select PHP Interpreter": "Selecionar interpretador PHP", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "A configuração que o vendor/bin/phpstan usa para verificar os arquivos salvos. Vazio usa phpstan.neon, phpstan.neon.dist ou phpstan.dist.neon na raiz do projeto. Caminho absoluto ou relativo ao projeto.", + "Select PHPStan Configuration File": "Selecionar arquivo de configuração do PHPStan", "select files in a git repository to shelve": "selecione arquivos em um repositório git para guardar no Shelf", "select files from a single git repository to shelve": "selecione arquivos de um único repositório git para guardar no Shelf", "External file changes sync may be slow": "A sincronização de alterações externas de arquivos pode ser lenta", diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index e26e17b4c..e7c13cc60 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -79,6 +79,9 @@ "Kotlin bridge bundle is unavailable": "Kotlin 桥接扩展不可用", "Kotlin bridge command did not acknowledge its library snapshot": "Kotlin 桥接命令未确认类库更新", "Kotlin editor compilation failed": "Kotlin 编辑器编译失败", + "PHPStan check failed": "PHPStan 检查失败", + "PHPStan ended with {status} and no report": "PHPStan 已结束（{status}），但没有生成报告", + "Cannot run {program}: {error}": "无法运行 {program}：{error}", "Updating results…": "正在更新结果…", "Uploading files": "正在上传文件", "No files on the clipboard": "剪贴板中没有文件", @@ -3926,6 +3929,7 @@ "Change Severity": "更改严重性", "Edit Inspection Profile Setting…": "编辑检查配置文件设置…", "Compiler errors and warnings of the last Build Project, shown at their source positions.": "上次“构建项目”产生的编译器错误和警告，显示在对应的源代码位置。", + "Errors the project's vendor/bin/phpstan reports in a PHP file when it is saved, shown at their lines. PHPStan reads phpstan.neon, phpstan.neon.dist or phpstan.dist.neon from the project root, or the configuration file chosen in Settings › Languages & Frameworks › PHP. Each rule PHPStan has reported is listed below by its identifier and can be turned off or re-graded on its own.": "保存 PHP 文件时，项目的 vendor/bin/phpstan 在其中报告的错误，显示在对应的行。PHPStan 从项目根目录读取 phpstan.neon、phpstan.neon.dist 或 phpstan.dist.neon，或者读取在“设置 › 语言和框架 › PHP”中选择的配置文件。PHPStan 报告过的每条规则都按标识符列在下方，可以单独关闭或调整级别。", "Process code": "处理代码", "Process literals": "处理字面量", "Process comments": "处理注释", @@ -5714,6 +5718,9 @@ "No additional stubs": "没有附加存根", "Stub": "存根", "Stub name, e.g. wordpress": "存根名称，例如 wordpress", + "phpstan.neon, phpstan.neon.dist or phpstan.dist.neon": "phpstan.neon、phpstan.neon.dist 或 phpstan.dist.neon", + "The configuration vendor/bin/phpstan checks saved files with. Empty uses phpstan.neon, phpstan.neon.dist or phpstan.dist.neon in the project root. Absolute, or relative to the project.": "vendor/bin/phpstan 检查已保存文件时使用的配置。留空则使用项目根目录中的 phpstan.neon、phpstan.neon.dist 或 phpstan.dist.neon。可以是绝对路径，或相对于项目的路径。", + "Select PHPStan Configuration File": "选择 PHPStan 配置文件", "select files in a git repository to shelve": "请选择 Git 仓库中的文件以搁置更改", "select files from a single git repository to shelve": "请一次只选择同一个 Git 仓库中的文件以搁置更改", "External file changes sync may be slow": "外部文件更改同步可能较慢", diff --git a/src/platform/locales/textCatalog.test.ts b/src/platform/locales/textCatalog.test.ts index a98fdceed..2392c344e 100644 --- a/src/platform/locales/textCatalog.test.ts +++ b/src/platform/locales/textCatalog.test.ts @@ -92,6 +92,7 @@ const UNTRANSLATED = new Set([ "Prettier", "oxfmt", "IntelliJ IDEA", + "PHPStan", // Go environment variables and placeholder values "CGO", "GOPROXY", From d4a5b547206d611ad37e628fe04f163ea22510c5 Mon Sep 17 00:00:00 2001 From: Muu Date: Thu, 5 Nov 2026 21:53:22 +0800 Subject: [PATCH 008/185] feat(editor): add PyCharm-style Python semantic highlighting (#1523) Python files got semantic colours only once the language server answered, and locals, globals, instance and class attributes, constants, properties and type parameters all looked the same. A highlighter on the Lezer syntax tree now colours them right away in PyCharm's finer categories (also self, builtins, decorators, definitions, method calls, brackets); LSP semantic tokens stay authoritative when they arrive. The categories are new --syn-python-* theme tokens wired through the built-in themes and Harbor Night, and PyCharm .icls schemes import their PY.* attributes. Co-authored-by: muutot <92625353+muutot@users.noreply.github.com> --- docs/plugins/themes.md | 9 +- e2e/pythonHighlight.fixture.ts | 84 +++ e2e/pythonHighlight.html | 9 + e2e/pythonHighlight.spec.ts | 51 ++ knip.ts | 1 + .../themes/harbor-night.scheme.json | 7 + src/editor/lsp/features.test.ts | 8 + src/editor/lsp/pythonHighlight.test.ts | 179 ++++++ src/editor/lsp/pythonHighlight.ts | 513 ++++++++++++++++++ src/editor/lsp/semantic.ts | 163 +++++- src/editor/lsp/semanticTyping.test.ts | 90 +++ src/editor/lsp/theme.ts | 34 ++ .../__fixtures__/colorSchemes/forest.icls | 2 +- src/platform/colorScheme.ts | 17 + src/platform/icls.test.ts | 45 ++ src/platform/icls.ts | 39 +- src/platform/locales/text/zh.json | 15 + src/styles/tokens.css | 28 + 18 files changed, 1276 insertions(+), 18 deletions(-) create mode 100644 e2e/pythonHighlight.fixture.ts create mode 100644 e2e/pythonHighlight.html create mode 100644 e2e/pythonHighlight.spec.ts create mode 100644 src/editor/lsp/pythonHighlight.test.ts create mode 100644 src/editor/lsp/pythonHighlight.ts create mode 100644 src/editor/lsp/semanticTyping.test.ts diff --git a/docs/plugins/themes.md b/docs/plugins/themes.md index 96b639c2a..760a7c3d1 100644 --- a/docs/plugins/themes.md +++ b/docs/plugins/themes.md @@ -324,7 +324,7 @@ a token. -228 themeable tokens: 121 in the UI theme file, 107 in the editor colour scheme file. +235 themeable tokens: 121 in the UI theme file, 114 in the editor colour scheme file. ### Surfaces (UI theme) @@ -434,6 +434,13 @@ a token. | `--syn-param` | `#bcbec4` | `#080808` | | `--syn-property` | `#c77dbb` | `#871094` | | `--syn-constant` | `#c77dbb` | `#871094` | +| `--syn-python-local-variable` | `var(--syn-variable)` | `var(--syn-variable)` | +| `--syn-python-global-variable` | `var(--syn-global)` | `var(--syn-global)` | +| `--syn-python-instance-attribute` | `var(--syn-property)` | `var(--syn-property)` | +| `--syn-python-class-attribute` | `var(--syn-property)` | `var(--syn-property)` | +| `--syn-python-constant` | `var(--syn-constant)` | `var(--syn-constant)` | +| `--syn-python-property` | `var(--syn-property)` | `var(--syn-property)` | +| `--syn-python-type-parameter` | `var(--syn-type)` | `var(--syn-type)` | | `--syn-builtin` | `#8888c6` | `#000080` | | `--syn-self` | `#94558d` | `#94558d` | | `--syn-keyword-arg` | `#aa4926` | `#660099` | diff --git a/e2e/pythonHighlight.fixture.ts b/e2e/pythonHighlight.fixture.ts new file mode 100644 index 000000000..e42a08e4c --- /dev/null +++ b/e2e/pythonHighlight.fixture.ts @@ -0,0 +1,84 @@ +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { syntaxHighlighting } from "@codemirror/language"; +import { openideaHighlightStyle } from "../src/editor/core/cm/highlight"; +import { findLanguageDescription, loadSupport } from "../src/editor/core/cm/languages"; +import { semanticExtension } from "../src/editor/lsp/semantic"; +import { lspDoc } from "../src/editor/lsp/lsp"; +import { lspTheme } from "../src/editor/lsp/theme"; +import { parseIcls } from "../src/platform/icls"; +import { schemeCss } from "../src/platform/colorScheme"; +import type { EditorDocContext } from "../src/editor/api"; + +const attributes = { + TEXT: { FOREGROUND: "ddeeff", BACKGROUND: "101820" }, + "PY.KEYWORD": { FOREGROUND: "112233" }, + "PY.FUNC_DEFINITION": { FOREGROUND: "224466" }, + "PY.NESTED_FUNC_DEFINITION": { FOREGROUND: "335577" }, + DEFAULT_FUNCTION_CALL: { FOREGROUND: "446688" }, + DEFAULT_LINE_COMMENT: { FOREGROUND: "667788" }, + DEFAULT_BLOCK_COMMENT: { FOREGROUND: "aabbcc" }, + "PY.METHOD_CALL": { FOREGROUND: "557799" }, + "PY.DOC_COMMENT": { FOREGROUND: "6688aa", EFFECT_TYPE: "1" }, + "PY.STRING.B": { FOREGROUND: "7799bb", FONT_TYPE: "1" }, + "PY.VALID_STRING_ESCAPE": { FOREGROUND: "88aacc" }, + "PY.ANNOTATION": { FOREGROUND: "99bbdd", BACKGROUND: "182838" }, + "PY.SELF_PARAMETER": { FOREGROUND: "aa7788" }, + "PY.BUILTIN_NAME": { FOREGROUND: "bb8899" }, + "PY.KEYWORD_ARGUMENT": { FOREGROUND: "cc99aa" }, + OPENIDEA_pyLocalVariable: { FOREGROUND: "aa1101" }, + OPENIDEA_pyGlobalVariable: { FOREGROUND: "aa2202" }, + OPENIDEA_pyInstanceAttribute: { FOREGROUND: "aa3303" }, + OPENIDEA_pyClassAttribute: { FOREGROUND: "aa4404" }, + OPENIDEA_pyConstant: { FOREGROUND: "aa5505" }, + OPENIDEA_pyProperty: { FOREGROUND: "aa6606" }, + OPENIDEA_pyTypeParameter: { FOREGROUND: "aa7707" }, +}; +const xml = `${Object.entries(attributes) + .map( + ([key, values]) => + ``, + ) + .join("")}`; +const style = document.createElement("style"); +style.textContent = + schemeCss("custom-python", {}, null, { "custom-python": parseIcls(xml).scheme }) + + "body { margin: 24px; background: #101820; color: #ddeeff; } .cm-editor { font: 14px/1.6 monospace; }"; +document.head.append(style); +const support = await loadSupport(findLanguageDescription("Python", "demo.py")!); +new EditorView({ + parent: document.body, + state: EditorState.create({ + doc: `MAX_SIZE: Final = 10 +name = "global" +T = TypeVar("T") + +def outer(x: T) -> T: + """Syntax Highlighting Demo""" + def nested(y): + print(y + 1) + nested(x) + +class Foo[T]: + count = 0 + def __init__(self: Foo, value: T): + value = b"newline:\\n" + local = 1 + self.instance_attr = value + print(local, MAX_SIZE, name, T) + self.run(whatever=1) # Comment + @property + def size(self) -> int: + return self.instance_attr +`, + extensions: [ + support!, + syntaxHighlighting(openideaHighlightStyle), + lspTheme, + lspDoc.of({ language: "python" } as EditorDocContext), + ...(new URLSearchParams(location.search).has("baseline") ? [] : [semanticExtension()]), + ], + }), +}); diff --git a/e2e/pythonHighlight.html b/e2e/pythonHighlight.html new file mode 100644 index 000000000..f3414d19b --- /dev/null +++ b/e2e/pythonHighlight.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/e2e/pythonHighlight.spec.ts b/e2e/pythonHighlight.spec.ts new file mode 100644 index 000000000..f3e5b0847 --- /dev/null +++ b/e2e/pythonHighlight.spec.ts @@ -0,0 +1,51 @@ +import { expect, test } from "@playwright/test"; + +test("Python syntax supplements retain PyCharm colours without a language server", async ({ page }, info) => { + const errors: string[] = []; + page.on("pageerror", (e) => errors.push(e.message)); + await page.goto("/e2e/pythonHighlight.html?baseline"); + await expect(page.locator(".cm-content")).toBeVisible(); + await page.screenshot({ path: info.outputPath("before.png") }); + await page.goto("/e2e/pythonHighlight.html"); + await expect(page.locator(".cm-sem-python-definition")).toHaveText("outer"); + await expect(page.locator(".cm-sem-python-definition")).toHaveCSS("color", "rgb(34, 68, 102)"); + await expect(page.locator(".cm-sem-python-nested")).toHaveCSS("color", "rgb(51, 85, 119)"); + await expect(page.locator(".cm-sem-python-docstring")).toHaveCSS("color", "rgb(102, 136, 170)"); + await expect(page.locator(".cm-sem-python-docstring")).toHaveCSS("text-decoration-line", "none"); + await expect(page.locator(".cm-sem-python-bytes").first()).toHaveCSS("color", "rgb(119, 153, 187)"); + await expect(page.locator(".cm-sem-python-bytes").first()).toHaveCSS("font-weight", "700"); + await expect(page.locator(".cm-sem-python-escape")).toHaveCSS("color", "rgb(136, 170, 204)"); + await expect(page.locator(".cm-sem-python-annotation").first()).toHaveCSS("color", "rgb(153, 187, 221)"); + await expect(page.locator(".cm-sem-self").first()).toHaveCSS("color", "rgb(170, 119, 136)"); + await expect(page.locator(".cm-sem-python-method-call")).toHaveCSS("color", "rgb(85, 119, 153)"); + await expect( + page + .locator(".cm-line") + .filter({ hasText: "nested(x)" }) + .locator(".cm-sem-function") + .filter({ hasText: /^nested$/ }), + ).toHaveCSS("color", "rgb(68, 102, 136)"); + await expect( + page + .locator(".cm-line") + .filter({ hasText: "# Comment" }) + .locator("span") + .filter({ hasText: /^# Comment$/ }), + ).toHaveCSS("color", "rgb(102, 119, 136)"); + const print = page + .locator(".cm-sem-python-builtin") + .filter({ hasText: /^print$/ }) + .first(); + await expect(print).toHaveText("print"); + await expect(print).toHaveCSS("color", "rgb(187, 136, 153)"); + await expect(page.locator(".cm-sem-python-keyword-arg")).toHaveCSS("color", "rgb(204, 153, 170)"); + await expect(page.locator(".cm-sem-python-local-variable").first()).toHaveCSS("color", "rgb(170, 17, 1)"); + await expect(page.locator(".cm-sem-python-global-variable").first()).toHaveCSS("color", "rgb(170, 34, 2)"); + await expect(page.locator(".cm-sem-python-instance-attribute").first()).toHaveCSS("color", "rgb(170, 51, 3)"); + await expect(page.locator(".cm-sem-python-class-attribute").first()).toHaveCSS("color", "rgb(170, 68, 4)"); + await expect(page.locator(".cm-sem-python-constant").first()).toHaveCSS("color", "rgb(170, 85, 5)"); + await expect(page.locator(".cm-sem-python-property").first()).toHaveCSS("color", "rgb(170, 102, 6)"); + await expect(page.locator(".cm-sem-python-type-parameter").first()).toHaveCSS("color", "rgb(170, 119, 7)"); + expect(errors).toEqual([]); + await page.screenshot({ path: info.outputPath("python-highlighting.png") }); +}); diff --git a/knip.ts b/knip.ts index 572c234ad..9fab76732 100644 --- a/knip.ts +++ b/knip.ts @@ -2,6 +2,7 @@ import type { KnipConfig } from "knip"; const config: KnipConfig = { // Entries come from the Vite (index.html), Vitest, Playwright and package.json script plugins. + entry: ["e2e/pythonHighlight.fixture.ts"], project: [ "src/**/*.{ts,tsx,css}", "!src/editor/core/cm/angular/parser.terms.ts", diff --git a/plugins/official/harbor-night/themes/harbor-night.scheme.json b/plugins/official/harbor-night/themes/harbor-night.scheme.json index 41bd29668..a6f7dc8e3 100644 --- a/plugins/official/harbor-night/themes/harbor-night.scheme.json +++ b/plugins/official/harbor-night/themes/harbor-night.scheme.json @@ -22,6 +22,13 @@ "--syn-param": "#d9dfe8", "--syn-property": "#b9a3f0", "--syn-constant": "#e58fa3", + "--syn-python-local-variable": "#d9dfe8", + "--syn-python-global-variable": "#d9dfe8", + "--syn-python-instance-attribute": "#b9a3f0", + "--syn-python-class-attribute": "#b9a3f0", + "--syn-python-constant": "#e58fa3", + "--syn-python-property": "#b9a3f0", + "--syn-python-type-parameter": "#8fd4c9", "--syn-builtin": "#6fc3df", "--syn-self": "#d6849c", "--syn-keyword-arg": "#c4a0f2", diff --git a/src/editor/lsp/features.test.ts b/src/editor/lsp/features.test.ts index 0d23cf9bc..4f221b34a 100644 --- a/src/editor/lsp/features.test.ts +++ b/src/editor/lsp/features.test.ts @@ -35,6 +35,14 @@ describe("semanticRanges", () => { expect(semanticRanges(doc, input, false, "java")[0]?.className).toBe("cm-sem-property cm-sem-static"); }); + it("routes Python library symbols through its built-in name colour", () => { + const input = { tokenTypes: ["function"], tokenModifiers: ["defaultLibrary"], data: [0, 0, 3, 0, 1] }; + expect(semanticRanges(doc, input, false, "python")[0]?.className).toBe( + "cm-sem-function cm-sem-builtin cm-sem-python-builtin", + ); + expect(semanticRanges(doc, input, false, "java")[0]?.className).toBe("cm-sem-function cm-sem-builtin"); + }); + it("ignores type and modifier names that only exist on Object.prototype", () => { const input = { tokenTypes: ["method", "toString"], diff --git a/src/editor/lsp/pythonHighlight.test.ts b/src/editor/lsp/pythonHighlight.test.ts new file mode 100644 index 000000000..81ba2d625 --- /dev/null +++ b/src/editor/lsp/pythonHighlight.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { ensureSyntaxTree, syntaxTree } from "@codemirror/language"; +import { findLanguageDescription, loadSupport } from "../core/cm/languages"; +import { pythonSyntaxRanges, pythonFallbackRanges, pythonSymbolKinds } from "./pythonHighlight"; +import { semanticRanges } from "./semantic"; + +async function parse(source: string) { + const support = await loadSupport(findLanguageDescription("Python", "demo.py")!); + const state = EditorState.create({ doc: source, extensions: [support!] }); + ensureSyntaxTree(state, state.doc.length, 1000); + return state; +} + +describe("Python PyCharm categories", () => { + it("distinguishes definitions, nested definitions, docstrings, self, bytes and keyword arguments", async () => { + const state = await parse(`"""module doc""" +def outer(x): + """function doc""" + def nested(y): + return y + nested(x) +class Foo: + """class doc""" + def __init__(self): + value = b"bytes\\n" + self.run(whatever=1) + print(self.__doc__) +`); + const ranges = pythonSyntaxRanges(syntaxTree(state), state.doc); + const styles = ranges.map((r) => [state.doc.sliceString(r.from, r.to), r.className]); + expect(styles).toContainEqual(["outer", "cm-sem-python-definition"]); + expect(styles).toContainEqual(["nested", "cm-sem-python-nested"]); + expect(styles).toContainEqual(["__init__", "cm-sem-python-predefined-definition"]); + expect(styles).toContainEqual(["whatever", "cm-sem-python-keyword-arg"]); + expect(styles).toContainEqual(["__doc__", "cm-sem-python-predefined-usage"]); + expect(styles.filter(([, c]) => c === "cm-sem-python-docstring")).toHaveLength(3); + expect(styles.filter(([, c]) => c === "cm-sem-self")).toHaveLength(3); + expect(styles).toContainEqual(['b"bytes\\n"', "cm-sem-python-bytes"]); + expect(styles).toContainEqual(["\\n", "cm-sem-python-escape"]); + }); + + it("does not treat a locally shadowed builtin as a library symbol", async () => { + const state = await parse("def f(print):\n print(1)\nlen([1])"); + const ranges = pythonFallbackRanges(syntaxTree(state), state.doc); + const print = ranges.filter((r) => state.doc.sliceString(r.from, r.to) === "print"); + expect(print.map((r) => r.className)).toEqual(["cm-sem-parameter", "cm-sem-parameter"]); + expect(ranges.find((r) => state.doc.sliceString(r.from, r.to) === "len")?.className).toContain( + "cm-sem-python-builtin", + ); + }); + + it("lets server classification override the local builtin guess", async () => { + const state = await parse("print(1)"); + const ranges = semanticRanges( + state.doc, + { tokenTypes: ["variable"], tokenModifiers: [], data: [0, 0, 5, 0, 0] }, + false, + "python", + syntaxTree(state), + ); + expect(ranges.some((r) => r.className.includes("python-builtin"))).toBe(false); + }); + + it("keeps calls yellow when Pyrefly omits declaration modifiers", async () => { + const state = await parse("def outer(x):\n outer(x)"); + const result = semanticRanges( + state.doc, + { + tokenTypes: ["function"], + tokenModifiers: [], + data: [0, 4, 5, 0, 0, 1, 4, 5, 0, 0], + }, + false, + "python", + syntaxTree(state), + ); + expect(result.filter((r) => state.doc.sliceString(r.from, r.to) === "outer").map((r) => r.className)).toEqual([ + "cm-sem-python-definition", + "cm-sem-function", + ]); + }); + + it("does not misclassify ordinary strings, later expressions, bytes or f strings as docstrings", async () => { + const state = await parse( + 'x = "ordinary"\n"later"\ndef f():\n b"bytes"\n "later"\ndef g():\n f"format {x}"', + ); + expect( + pythonSyntaxRanges(syntaxTree(state), state.doc).some((r) => r.className === "cm-sem-python-docstring"), + ).toBe(false); + }); + + it("honours Pyrefly selfParameter modifiers without applying rainbow colours", async () => { + const state = await parse("self arg"); + const result = semanticRanges( + state.doc, + { + tokenTypes: ["parameter"], + tokenModifiers: ["selfParameter"], + data: [0, 0, 4, 0, 1, 0, 5, 3, 0, 0], + }, + true, + "python", + ); + expect(result[0]?.className).toBe("cm-sem-parameter cm-sem-self"); + expect(result[1]?.className).toContain("cm-sem-rainbow-"); + }); + + it("classifies Python variables, attributes, properties, constants and type parameters", async () => { + const state = await parse(`MAX_SIZE: Final = 10 +name = "x" +T = TypeVar("T") +type Alias[U] = list[U] + +class Box[T]: + count = 0 + ICON: Final = "box" + + def __init__(self, value: T): + self.value = value + local = 1 + self.count + self.value + self.size + + @property + def size(self) -> T: + return self.value + + def read(self) -> T: + return Box.count + Box.ICON +`); + const tree = syntaxTree(state); + const kinds = pythonSymbolKinds(tree, state.doc); + const kindOf = (name: string) => + [...kinds] + .filter(([range]) => { + const [from, to] = range.split(":").map(Number); + return state.doc.sliceString(from!, to!) === name; + }) + .map(([, kind]) => kind); + expect(kindOf("local")).toEqual(["localVariable"]); + expect(kindOf("name")).toEqual(["globalVariable"]); + expect(kindOf("MAX_SIZE")).toEqual(["constant"]); + expect(kindOf("value")).toContain("instanceAttribute"); + expect(kindOf("count")).toContain("classAttribute"); + expect(kindOf("ICON")).toContain("constant"); + expect(kindOf("size")).toContain("property"); + expect(kindOf("T")).toEqual(expect.arrayContaining(["typeParameter"])); + expect(kindOf("U")).toEqual(["typeParameter", "typeParameter"]); + + const syntax = pythonSyntaxRanges(tree, state.doc, kinds); + const syntaxClass = (name: string) => + syntax.filter((range) => state.doc.sliceString(range.from, range.to) === name).map((range) => range.className); + expect(syntaxClass("size")).toContain("cm-sem-property cm-sem-python-property"); + expect(syntaxClass("T")).toContain("cm-sem-typeParameter cm-sem-python-type-parameter"); + }); + + it("keeps Pyrefly tokens but applies the finer Python category", async () => { + const state = await parse("MAX_SIZE: Final = 10\ndef f(x):\n local = x\n return local"); + const result = semanticRanges( + state.doc, + { + tokenTypes: ["variable"], + tokenModifiers: [], + data: [0, 0, 8, 0, 0, 2, 4, 5, 0, 1, 3, 11, 5, 0, 2], + }, + true, + "python", + syntaxTree(state), + ); + const classOf = (name: string) => + result.filter((range) => state.doc.sliceString(range.from, range.to) === name).map((range) => range.className); + expect(classOf("MAX_SIZE")).toEqual(["cm-sem-constant cm-sem-python-constant"]); + expect(classOf("local")[0]).toContain("cm-sem-variable cm-sem-python-local-variable cm-sem-rainbow-"); + expect(classOf("local")[1]).toContain("cm-sem-python-local-variable"); + expect(classOf("MAX_SIZE")[0]).not.toContain("cm-sem-rainbow-"); + }); +}); diff --git a/src/editor/lsp/pythonHighlight.ts b/src/editor/lsp/pythonHighlight.ts new file mode 100644 index 000000000..a57c0e584 --- /dev/null +++ b/src/editor/lsp/pythonHighlight.ts @@ -0,0 +1,513 @@ +import type { SyntaxNode, Tree } from "@lezer/common"; +import type { Text } from "@codemirror/state"; + +interface PythonRange { + from: number; + to: number; + className: string; + source: "pythonSyntax" | "pythonFallback"; +} + +export type PythonSymbolKind = + | "function" + | "type" + | "parameter" + | "localVariable" + | "globalVariable" + | "instanceAttribute" + | "classAttribute" + | "constant" + | "property" + | "typeParameter" + | "builtin"; + +const PYTHON_CLASSES: Record = { + function: "cm-sem-function", + type: "cm-sem-type", + parameter: "cm-sem-parameter", + localVariable: "cm-sem-variable cm-sem-python-local-variable", + globalVariable: "cm-sem-global cm-sem-python-global-variable", + instanceAttribute: "cm-sem-property cm-sem-python-instance-attribute", + classAttribute: "cm-sem-property cm-sem-python-class-attribute", + constant: "cm-sem-constant cm-sem-python-constant", + property: "cm-sem-property cm-sem-python-property", + typeParameter: "cm-sem-typeParameter cm-sem-python-type-parameter", + builtin: "cm-sem-builtin cm-sem-python-builtin", +}; + +export const pythonSymbolClasses = (kind: PythonSymbolKind) => PYTHON_CLASSES[kind]; + +const BUILTIN_TYPES = new Set([ + "int", + "str", + "bool", + "bytes", + "float", + "complex", + "list", + "dict", + "tuple", + "set", + "frozenset", + "object", + "type", + "None", +]); + +function hasAncestor(node: SyntaxNode, name: string): boolean { + for (let parent = node.parent; parent; parent = parent.parent) if (parent.name === name) return true; + return false; +} + +function enclosingFunction(node: SyntaxNode | null): SyntaxNode | null { + for (; node; node = node.parent) if (node.name === "FunctionDefinition") return node; + return null; +} + +const FUNCTION_SCOPES = new Set(["FunctionDefinition", "LambdaExpression"]); +const COMPREHENSION_SCOPES = new Set([ + "ArrayComprehensionExpression", + "DictionaryComprehensionExpression", + "SetComprehensionExpression", +]); +const SCOPE_NODES = new Set([ + "FunctionDefinition", + "LambdaExpression", + "ClassDefinition", + "TypeDefinition", + "Script", + ...COMPREHENSION_SCOPES, +]); +const PATTERN_CONTAINERS = new Set([ + "TupleExpression", + "ArrayExpression", + "ParenthesizedExpression", + "SequencePattern", + "PatternArgList", + "StarPattern", +]); +const TYPE_VAR_CALLS = new Set(["TypeVar", "ParamSpec", "TypeVarTuple"]); +const KIND_PRIORITY: Record = { + globalVariable: 70, + typeParameter: 90, + parameter: 80, + property: 80, + constant: 70, + classAttribute: 60, + localVariable: 50, + function: 40, + type: 40, + instanceAttribute: 40, + builtin: 30, +}; + +function scope(node: SyntaxNode | null): SyntaxNode | null { + for (; node; node = node.parent) if (SCOPE_NODES.has(node.name)) return node; + return null; +} + +function enclosingClass(node: SyntaxNode | null): SyntaxNode | null { + for (; node; node = node.parent) if (node.name === "ClassDefinition") return node; + return null; +} + +function directName(node: SyntaxNode | null): SyntaxNode | null { + for (let child = node?.firstChild; child; child = child.nextSibling) if (child.name === "VariableName") return child; + return null; +} + +function isConstantName(name: string): boolean { + return /^_*[A-Z][A-Z0-9_]*$/.test(name); +} + +function hasFinalAnnotation(node: SyntaxNode, doc: Text): boolean { + for (let child = node.firstChild; child; child = child.nextSibling) { + if ( + (child.name === "VariableName" || child.name === "PropertyName") && + doc.sliceString(child.from, child.to) === "Final" + ) + return true; + if (hasFinalAnnotation(child, doc)) return true; + } + return false; +} + +function callName(node: SyntaxNode | null, doc: Text): string | undefined { + if (node?.name !== "CallExpression") return undefined; + const callee = node.firstChild; + if (callee?.name === "VariableName") return doc.sliceString(callee.from, callee.to); + if (callee?.name === "MemberExpression") { + const property = callee.getChild("PropertyName"); + if (property) return doc.sliceString(property.from, property.to); + } + return undefined; +} + +function assignmentRight(node: SyntaxNode): SyntaxNode | null { + const assign = node.getChild("AssignOp"); + return assign?.nextSibling ?? null; +} + +function assignmentAnnotation(node: SyntaxNode): SyntaxNode | null { + return node.getChild("TypeDef"); +} + +function collectPattern(node: SyntaxNode, out: SyntaxNode[]) { + if (node.name === "VariableName" || node.name === "MemberExpression") { + out.push(node); + return; + } + if (!PATTERN_CONTAINERS.has(node.name)) return; + for (let child = node.firstChild; child; child = child.nextSibling) collectPattern(child, out); +} + +function assignmentTargets(node: SyntaxNode, stop: string): SyntaxNode[] { + const out: SyntaxNode[] = []; + for (let child = node.firstChild; child; child = child.nextSibling) { + if (child.name === stop) break; + if (child.name === "TypeDef") continue; + collectPattern(child, out); + } + return out; +} + +function forTargets(node: SyntaxNode): SyntaxNode[] { + const out: SyntaxNode[] = []; + let collecting = false; + for (let child = node.firstChild; child; child = child.nextSibling) { + if (child.name === "for") { + collecting = true; + continue; + } + if (!collecting) continue; + if (child.name === "in") break; + collectPattern(child, out); + } + return out; +} + +function aliasTargets(node: SyntaxNode): SyntaxNode[] { + const out: SyntaxNode[] = []; + for (let child = node.firstChild; child; child = child.nextSibling) { + if (child.name === "as") { + const target = child.nextSibling; + if (target) collectPattern(target, out); + } + } + return out; +} + +function isPropertyFunction(node: SyntaxNode, name: string, doc: Text): boolean { + const decorated = node.parent; + if (decorated?.name !== "DecoratedStatement") return false; + for (let child = decorated.firstChild; child; child = child.nextSibling) { + if (child.name !== "Decorator") continue; + const decorator = doc.sliceString(child.from + 1, child.to).trim(); + if (decorator === "property" || decorator === "cached_property" || decorator.endsWith(".cached_property")) + return true; + const dot = decorator.lastIndexOf("."); + if ( + dot > 0 && + decorator.slice(0, dot) === name && + ["setter", "getter", "deleter"].includes(decorator.slice(dot + 1)) + ) + return true; + } + return false; +} + +/** Classifies every Python identifier from scope bindings; the server only distinguishes some of them. */ +export function pythonSymbolKinds(tree: Tree, doc: Text): Map { + const bindings = new Map>(); + const classByName = new Map(); + const classData = new Map>(); + const propertyNames = new Set(); + const key = (node: SyntaxNode) => `${node.name}:${node.from}`; + const text = (node: SyntaxNode) => doc.sliceString(node.from, node.to); + const dataFor = (node: SyntaxNode) => { + const scopeKey = key(node); + let data = classData.get(scopeKey); + if (!data) classData.set(scopeKey, (data = new Set())); + return data; + }; + const bind = (owner: SyntaxNode | null, name: string, kind: PythonSymbolKind) => { + if (!owner) return; + let names = bindings.get(key(owner)); + if (!names) bindings.set(key(owner), (names = new Map())); + const previous = names.get(name); + if (previous && KIND_PRIORITY[previous] >= KIND_PRIORITY[kind]) return; + names.set(name, kind); + }; + + tree.iterate({ + enter(cursor) { + const node = cursor.node; + if (node.name === "ClassDefinition") { + const classNode = directName(node); + if (classNode) { + bind(scope(node.parent), text(classNode), "type"); + classByName.set(text(classNode), node); + dataFor(node); + } + } else if (node.name === "TypeDefinition") { + const typeNode = directName(node); + if (typeNode) bind(scope(node.parent), text(typeNode), "type"); + } else if (node.name === "FunctionDefinition") { + const functionNode = directName(node); + if (!functionNode) return; + const functionName = text(functionNode); + const property = isPropertyFunction(node, functionName, doc); + bind(scope(node.parent), functionName, property ? "property" : "function"); + if (property) { + propertyNames.add(functionName); + } + } else if (node.name === "ParamList") { + const owner = scope(node); + for (let child = node.firstChild; child; child = child.nextSibling) + if (child.name === "VariableName") bind(owner, text(child), "parameter"); + } else if (node.name === "TypeParam") { + const param = directName(node); + if (param) bind(scope(node), text(param), "typeParameter"); + } else if (node.name === "ScopeStatement" && node.firstChild?.name === "global") { + const owner = scope(node); + for (let child: SyntaxNode | null = node.firstChild; child; child = child.nextSibling) + if (child.name === "VariableName") bind(owner, text(child), "globalVariable"); + } + if (node.name === "CapturePattern") { + const capture = directName(node); + if (capture) { + const owner = scope(node); + const kind = + owner?.name === "ClassDefinition" + ? "classAttribute" + : owner?.name === "Script" + ? "globalVariable" + : "localVariable"; + bind(owner, text(capture), kind); + } + } + }, + }); + + const assignmentKind = (owner: SyntaxNode | null, name: string, annotation: SyntaxNode | null): PythonSymbolKind => { + if (annotation && hasFinalAnnotation(annotation, doc)) return "constant"; + if (isConstantName(name)) return "constant"; + if (owner?.name === "Script") return "globalVariable"; + if (owner?.name === "ClassDefinition") return "classAttribute"; + return "localVariable"; + }; + + tree.iterate({ + enter(cursor) { + const node = cursor.node; + if (node.name === "AssignStatement" || node.name === "UpdateStatement") { + const stop = node.name === "AssignStatement" ? "AssignOp" : "UpdateOp"; + const owner = scope(node); + const targets = assignmentTargets(node, stop); + const annotation = node.name === "AssignStatement" ? assignmentAnnotation(node) : null; + const called = node.name === "AssignStatement" ? callName(assignmentRight(node), doc) : undefined; + const typeParameter = called !== undefined && TYPE_VAR_CALLS.has(called); + for (const target of targets) { + if (target.name === "MemberExpression") { + const object = target.firstChild; + const property = target.getChild("PropertyName"); + if (object?.name === "VariableName" && property && text(object) === "self") { + const ownerClass = enclosingClass(node); + if (ownerClass) dataFor(ownerClass).add(text(property)); + } + continue; + } + const name = text(target); + bind(owner, name, typeParameter ? "typeParameter" : assignmentKind(owner, name, annotation)); + } + } else if (node.name === "NamedExpression") { + const target = node.firstChild; + if (target?.name === "VariableName") + bind(scope(node), text(target), assignmentKind(scope(node), text(target), null)); + } else if (node.name === "ForStatement") { + const owner = scope(node); + for (const target of forTargets(node)) + if (target.name === "VariableName") bind(owner, text(target), assignmentKind(owner, text(target), null)); + } else if (COMPREHENSION_SCOPES.has(node.name)) { + const owner = scope(node); + for (const target of forTargets(node)) + if (target.name === "VariableName") bind(owner, text(target), "localVariable"); + } else if (node.name === "WithStatement" || node.name === "TryStatement") { + const owner = scope(node); + for (const target of aliasTargets(node)) + if (target.name === "VariableName") bind(owner, text(target), assignmentKind(owner, text(target), null)); + } + }, + }); + + const resolve = ( + node: SyntaxNode, + name: string, + start: SyntaxNode | null = scope(node), + ): PythonSymbolKind | undefined => { + let crossedFunction = false; + for (let current = start; current; current = scope(current.parent)) { + if (FUNCTION_SCOPES.has(current.name)) crossedFunction = true; + const kind = bindings.get(key(current))?.get(name); + if (!kind) continue; + if (crossedFunction && current.name === "ClassDefinition" && kind !== "typeParameter") continue; + return kind; + } + return undefined; + }; + const classMember = (node: SyntaxNode | null, name: string) => + node ? bindings.get(key(node))?.get(name) : undefined; + const memberKind = (node: SyntaxNode, name: string): PythonSymbolKind | undefined => { + const member = node.parent; + if (member?.name !== "MemberExpression") return undefined; + const object = member.firstChild; + if (!object) return undefined; + const objectName = object.name === "VariableName" ? text(object) : undefined; + const currentClass = enclosingClass(node); + const namedClass = objectName ? classByName.get(objectName) : undefined; + const owner = objectName === "self" || objectName === "cls" ? currentClass : namedClass; + const bound = classMember(owner ?? null, name); + const call = member.parent?.name === "CallExpression" && member.parent.firstChild?.from === member.from; + if (call) return bound === "property" || bound === "constant" ? bound : undefined; + if (objectName === "self" && currentClass) { + if (bound === "property") return "property"; + if (owner && dataFor(owner).has(name)) return "instanceAttribute"; + if (bound) return bound; + if (propertyNames.has(name)) return "property"; + if (isConstantName(name)) return "constant"; + return "instanceAttribute"; + } + if (objectName === "cls" && currentClass) { + if (bound) return bound; + if (isConstantName(name)) return "constant"; + return "classAttribute"; + } + if (namedClass) { + if (bound) return bound; + if (isConstantName(name)) return "constant"; + return "classAttribute"; + } + if (propertyNames.has(name)) return "property"; + if (isConstantName(name)) return "constant"; + return "instanceAttribute"; + }; + const kindFor = (node: SyntaxNode): PythonSymbolKind | undefined => { + const name = text(node); + if (node.name === "PropertyName") return memberKind(node, name); + const start = node.parent?.name === "FunctionDefinition" ? scope(node.parent.parent) : scope(node); + const resolved = resolve(node, name, start); + if (resolved) return resolved; + if (isConstantName(name) && !BUILTIN_NAMES.has(name)) return "constant"; + return undefined; + }; + + const out = new Map(); + tree.iterate({ + enter(cursor) { + const node = cursor.node; + if (node.name !== "VariableName" && node.name !== "PropertyName") return; + const kind = kindFor(node); + if (kind) out.set(`${node.from}:${node.to}`, kind); + }, + }); + return out; +} + +/** Python categories absent from LSP tokens, derived from the editor's existing parse tree. */ +export function pythonSyntaxRanges( + tree: Tree, + doc: Text, + symbols: Map = pythonSymbolKinds(tree, doc), +): PythonRange[] { + const ranges: PythonRange[] = []; + tree.iterate({ + enter(cursor) { + const node = cursor.node; + if (!["VariableName", "PropertyName", "String", "Escape", "(", ")", "[", "]", "{", "}"].includes(node.name)) + return; + const text = doc.sliceString(node.from, node.to); + const symbol = symbols.get(`${node.from}:${node.to}`); + let style: string | undefined; + let className: string | undefined; + if (["(", ")"].includes(node.name)) style = "python-parentheses"; + else if (["[", "]"].includes(node.name)) style = "python-brackets"; + else if (["{", "}"].includes(node.name)) style = "python-braces"; + else if (node.name === "VariableName") { + if (symbol === "typeParameter") className = pythonSymbolClasses(symbol); + else if (node.parent?.name === "FunctionDefinition") { + if (symbol === "property") className = pythonSymbolClasses(symbol); + else + style = /^__.*__$/.test(text) + ? "python-predefined-definition" + : enclosingFunction(node.parent.parent) + ? "python-nested" + : "python-definition"; + } else if (hasAncestor(node, "TypeDef") && !BUILTIN_TYPES.has(text)) style = "python-annotation"; + else if ((text === "self" || text === "cls") && enclosingFunction(node.parent)) style = "self"; + else if (node.parent?.name === "ArgList" && node.nextSibling?.name === "AssignOp") style = "python-keyword-arg"; + } else if (node.name === "PropertyName" && /^__.*__$/.test(text)) style = "python-predefined-usage"; + else if ( + node.name === "PropertyName" && + node.parent?.name === "MemberExpression" && + node.parent.parent?.name === "CallExpression" + ) + style = "python-method-call"; + else if (node.name === "String") { + const statement = node.parent; + const body = statement?.parent; + let first = body?.firstChild; + while (first && (first.name === ":" || first.name === "Comment")) first = first.nextSibling; + const docstring = + statement?.name === "ExpressionStatement" && + statement.firstChild?.from === node.from && + statement.firstChild?.to === node.to && + first?.from === statement.from && + (body?.name === "Script" || + (body?.name === "Body" && ["FunctionDefinition", "ClassDefinition"].includes(body.parent?.name ?? ""))); + if (docstring && !/^[bf]/i.test(text)) style = "python-docstring"; + else if (/^[rub]*b[rub]*['"]/i.test(text)) style = "python-bytes"; + } else if (node.name === "Escape") style = "python-escape"; + if (style) className = `cm-sem-${style}`; + if (className) ranges.push({ from: node.from, to: node.to, className, source: "pythonSyntax" }); + if (style === "python-docstring") return false; + }, + }); + return ranges; +} + +const BUILTIN_NAMES = new Set( + "abs aiter all anext any ascii bin bool breakpoint bytearray bytes callable chr classmethod compile complex delattr dict dir divmod enumerate eval exec filter float format frozenset getattr globals hasattr hash help hex id input int isinstance issubclass iter len list locals map max memoryview min next object oct open ord pow print property range repr reversed round set setattr slice sorted staticmethod str sum super tuple type vars zip __import__ BaseException Exception ArithmeticError AssertionError AttributeError EOFError ImportError IndexError KeyError LookupError NameError NotImplementedError OSError OverflowError RuntimeError StopIteration SyntaxError TypeError UnboundLocalError ValueError ZeroDivisionError".split( + " ", + ), +); + +/** Fast local guesses while LSP is pending. Server tokens take precedence over these. */ +export function pythonFallbackRanges( + tree: Tree, + doc: Text, + rainbow?: (name: string) => string, + symbols: Map = pythonSymbolKinds(tree, doc), +): PythonRange[] { + const ranges: PythonRange[] = []; + tree.iterate({ + enter(cursor) { + const node = cursor.node; + if (node.name !== "VariableName" && node.name !== "PropertyName") return; + const name = doc.sliceString(node.from, node.to); + const kind = + symbols.get(`${node.from}:${node.to}`) ?? + (node.name === "VariableName" && BUILTIN_NAMES.has(name) ? "builtin" : undefined); + if (!kind) return; + const local = kind === "parameter" || kind === "localVariable"; + ranges.push({ + from: node.from, + to: node.to, + className: + pythonSymbolClasses(kind) + + (local && rainbow && name !== "self" && name !== "cls" ? ` ${rainbow(name)}` : ""), + source: "pythonFallback", + }); + }, + }); + return ranges; +} diff --git a/src/editor/lsp/semantic.ts b/src/editor/lsp/semantic.ts index 9b3d3e956..3d8473814 100644 --- a/src/editor/lsp/semantic.ts +++ b/src/editor/lsp/semantic.ts @@ -9,6 +9,15 @@ * `cm-sem-rainbow-`, `n` derived from the name, so one name keeps one * colour (`--syn-rainbow-`) throughout the file. */ +import { syntaxTree } from "@codemirror/language"; +import type { Tree } from "@lezer/common"; +import { + pythonSyntaxRanges, + pythonFallbackRanges, + pythonSymbolClasses, + pythonSymbolKinds, + type PythonSymbolKind, +} from "./pythonHighlight"; import { Prec, StateEffect, StateField, type Extension, type Text } from "@codemirror/state"; import { Decoration, EditorView, ViewPlugin, type DecorationSet, type ViewUpdate } from "@codemirror/view"; import { commands, type SemanticTokens } from "@/platform/ipc"; @@ -57,6 +66,11 @@ const MODIFIER_CLASS: Record = { definition: "declaration", defaultLibrary: "builtin", global: "global", + selfParameter: "self", + byteString: "python-bytes", + escapeSequence: "python-escape", + formatSpecifier: "python-format", + formatPlaceholder: "python-format", }; /** Number of `--syn-rainbow-` colours (IDEA's five semantic highlighting colours). */ @@ -75,15 +89,60 @@ export function rainbowIndex(name: string): number { /** Locals and parameters only: no fields, statics, constants, library or `self` names. */ const RAINBOW_TYPES = new Set(["variable", "parameter"]); const NOT_LOCAL = new Set(["static", "constant", "defaultLibrary", "global", "classScope", "fileScope"]); +const PYTHON_VARIABLE_KINDS = new Set([ + "localVariable", + "globalVariable", + "instanceAttribute", + "classAttribute", + "constant", + "property", + "typeParameter", +]); +const PYTHON_SYMBOL_BASES = new Set([ + "cm-sem-variable", + "cm-sem-global", + "cm-sem-property", + "cm-sem-constant", + "cm-sem-type", + "cm-sem-typeParameter", + "cm-sem-builtin", +]); export interface SemanticRange { from: number; to: number; className: string; + source?: "pythonSyntax" | "pythonFallback"; +} + +function rangeCoverage(ranges: Array<{ from: number; to: number }>) { + ranges.sort((a, b) => a.from - b.from || a.to - b.to); + const ends: number[] = []; + for (const r of ranges) ends.push(Math.max(ends.at(-1) ?? 0, r.to)); + return (from: number, to: number) => { + let low = 0, + high = ranges.length; + while (low < high) { + const mid = (low + high) >>> 1; + if (ranges[mid]!.from <= from) low = mid + 1; + else high = mid; + } + return low > 0 && ends[low - 1]! >= to; + }; } /** Decodes absolute `[line, col, len, type, modifiers]` quintuples into styled ranges. */ -export function semanticRanges(doc: Text, tokens: SemanticTokens, rainbow = false, language?: string): SemanticRange[] { +export function semanticRanges( + doc: Text, + tokens: SemanticTokens, + rainbow = false, + language?: string, + tree?: Tree, +): SemanticRange[] { + const symbols = language === "python" && tree ? pythonSymbolKinds(tree, doc) : undefined; + const syntax = language === "python" && tree ? pythonSyntaxRanges(tree, doc, symbols) : []; + const covered = rangeCoverage(syntax); + const serverRanges: Array<{ from: number; to: number }> = []; const out: SemanticRange[] = []; const { data, tokenTypes, tokenModifiers } = tokens; // Servers with a `local` modifier (TypeScript) tell module-level variables apart. @@ -95,7 +154,12 @@ export function semanticRanges(doc: Text, tokens: SemanticTokens, rainbow = fals typeIdx = data[i + 3]!, mods = data[i + 4]!; if (line >= doc.lines || len <= 0) continue; + const l = doc.line(line + 1); + const from = l.from + Math.min(col, l.length); + const to = Math.min(from + len, l.to); + if (from >= to) continue; const type = tokenTypes[typeIdx] ?? ""; + const pythonKind = symbols?.get(`${from}:${to}`); const names: string[] = []; for (let b = 0; b < tokenModifiers.length; b++) if (mods & (1 << b)) names.push(tokenModifiers[b]!); const classes: string[] = []; @@ -109,27 +173,49 @@ export function semanticRanges(doc: Text, tokens: SemanticTokens, rainbow = fals // `hasOwn`: servers send names like `constructor` (jdtls) that `Object.prototype` also has. for (const n of names) if (Object.hasOwn(MODIFIER_CLASS, n)) classes.push(`cm-sem-${MODIFIER_CLASS[n]}`); if (language === "python" && names.includes("static")) classes.push("cm-sem-python-static"); + if (language === "python" && type === "method") classes.push("cm-sem-python-method-call"); + if (language === "python" && names.includes("defaultLibrary")) classes.push("cm-sem-python-builtin"); + if (pythonKind && PYTHON_VARIABLE_KINDS.has(pythonKind)) { + for (let j = classes.length - 1; j >= 0; j--) if (PYTHON_SYMBOL_BASES.has(classes[j]!)) classes.splice(j, 1); + classes.push(...pythonSymbolClasses(pythonKind).split(" ")); + } const local = rainbow && !constant && - RAINBOW_TYPES.has(type) && + !names.includes("selfParameter") && !names.some((n) => NOT_LOCAL.has(n)) && - (type === "parameter" || !marksLocals || names.includes("local")); + (language === "python" + ? pythonKind + ? pythonKind === "localVariable" || pythonKind === "parameter" + : type === "parameter" + : RAINBOW_TYPES.has(type) && (type === "parameter" || !marksLocals || names.includes("local"))); + serverRanges.push({ from, to }); if (!classes.length && !local) continue; - const l = doc.line(line + 1); - const from = l.from + Math.min(col, l.length); - const to = Math.min(from + len, l.to); - if (from >= to) continue; + if (covered(from, to)) continue; if (local) classes.push(`cm-sem-rainbow-${rainbowIndex(doc.sliceString(from, to))}`); out.push({ from, to, className: classes.join(" ") }); } + if (language === "python" && tree) { + const serverCovered = rangeCoverage(serverRanges); + out.push( + ...pythonFallbackRanges( + tree, + doc, + rainbow ? (name) => `cm-sem-rainbow-${rainbowIndex(name)}` : undefined, + symbols, + ).filter((r) => !covered(r.from, r.to) && !serverCovered(r.from, r.to)), + ); + } + out.push(...syntax); return out.sort((a, b) => a.from - b.from || a.to - b.to); } const markCache = new Map(); -const markFor = (cls: string) => { - let m = markCache.get(cls); - if (!m) markCache.set(cls, (m = Decoration.mark({ class: cls }))); +const markFor = (cls: string, source?: "pythonSyntax" | "pythonFallback", symbol?: string) => { + if (symbol !== undefined) return Decoration.mark({ class: cls, symbol }); + const key = `${source ?? "semantic"}:${cls}`; + let m = markCache.get(key); + if (!m) markCache.set(key, (m = Decoration.mark({ class: cls, pythonSyntax: source !== undefined }))); return m; }; @@ -151,12 +237,24 @@ const semanticPlugin = ViewPlugin.fromClass( /** Last answer, re-decorated without a request when Semantic highlighting is toggled. */ private last: { doc: Text; tokens: SemanticTokens | null } | null = null; private destroyed = false; + private tree: Tree | null = null; constructor(private view: EditorView) { + queueMicrotask(() => { + if (!this.destroyed) this.decorate(this.view.state.doc, null); + }); this.refresh(); this.offReady = onLspRefresh("semanticTokens", () => this.refresh()); } update(u: ViewUpdate) { - if (u.docChanged) this.refresh(); + const tree = syntaxTree(u.state); + if (u.docChanged || tree !== this.tree) { + this.tree = tree; + this.refresh(); + const doc = u.state.doc; + queueMicrotask(() => { + if (!this.destroyed && this.view.state.doc === doc) this.refreshSyntax(doc); + }); + } const rainbow = editorPrefs().semanticHighlighting; if (rainbow === this.rainbow) return; this.rainbow = rainbow; @@ -168,13 +266,52 @@ const semanticPlugin = ViewPlugin.fromClass( }); else this.refresh(); } + refreshSyntax(doc: Text) { + const tree = syntaxTree(this.view.state); + const python = docOf(this.view)?.language === "python"; + const symbols = python ? pythonSymbolKinds(tree, doc) : undefined; + const syntax = python ? pythonSyntaxRanges(tree, doc, symbols) : []; + const covered = rangeCoverage(syntax); + const retained: Array<{ from: number; to: number }> = []; + const marks = syntax.map((r) => markFor(r.className, r.source).range(r.from, r.to)); + this.view.state.field(semanticField).between(0, doc.length, (from, to, decoration) => { + if ( + !decoration.spec.pythonSyntax && + !covered(from, to) && + decoration.spec.symbol === doc.sliceString(from, to) + ) { + marks.push(decoration.range(from, to)); + retained.push({ from, to }); + } + }); + if (docOf(this.view)?.language === "python") { + const retainedCovered = rangeCoverage(retained); + for (const r of pythonFallbackRanges( + tree, + doc, + this.rainbow ? (name) => `cm-sem-rainbow-${rainbowIndex(name)}` : undefined, + symbols, + )) + if (!covered(r.from, r.to) && !retainedCovered(r.from, r.to)) + marks.push(markFor(r.className, r.source).range(r.from, r.to)); + } + this.view.dispatch({ effects: setSemantic.of(Decoration.set(marks, true)) }); + } decorate(doc: Text, tokens: SemanticTokens | null) { this.last = { doc, tokens }; - const ranges = tokens ? semanticRanges(doc, tokens, this.rainbow, docOf(this.view)?.language) : []; + const ranges = semanticRanges( + doc, + tokens ?? { data: [], tokenTypes: [], tokenModifiers: [] }, + this.rainbow, + docOf(this.view)?.language, + syntaxTree(this.view.state), + ); this.view.dispatch({ effects: setSemantic.of( Decoration.set( - ranges.map((r) => markFor(r.className).range(r.from, r.to)), + ranges.map((r) => + markFor(r.className, r.source, r.source ? undefined : doc.sliceString(r.from, r.to)).range(r.from, r.to), + ), true, ), ), diff --git a/src/editor/lsp/semanticTyping.test.ts b/src/editor/lsp/semanticTyping.test.ts new file mode 100644 index 000000000..9d4fc85db --- /dev/null +++ b/src/editor/lsp/semanticTyping.test.ts @@ -0,0 +1,90 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { findLanguageDescription, loadSupport } from "../core/cm/languages"; +import type { SemanticTokens } from "@/platform/ipc"; +import type { EditorDocContext } from "@/editor/api"; +import { lspDoc } from "./lsp"; +import { semanticExtension } from "./semantic"; + +const requests = vi.hoisted(() => [] as Array<(tokens: SemanticTokens) => void>); +vi.mock("./lsp", async (importOriginal) => ({ + ...(await importOriginal()), + lspRequest: () => new Promise((resolve) => requests.push(resolve)), +})); +vi.mock("./availability", () => ({ + featureBackoff: () => ({ active: () => false, mark: () => {} }), + onLspRefresh: () => () => {}, +})); + +afterEach(() => { + vi.useRealTimers(); + requests.length = 0; +}); + +describe("semantic colours while typing", () => { + it("colours a newly completed builtin and parameters before any LSP response", async () => { + const support = await loadSupport(findLanguageDescription("Python", "demo.py")!); + vi.useFakeTimers(); + const view = new EditorView({ + parent: document.body, + state: EditorState.create({ + doc: "def f(x):\n pri", + extensions: [support!, lspDoc.of({ doc: 92, language: "python" } as EditorDocContext), semanticExtension()], + }), + }); + try { + await vi.advanceTimersByTimeAsync(0); + expect(view.dom.querySelector(".cm-sem-python-builtin")).toBeNull(); + view.dispatch({ changes: { from: view.state.doc.length, insert: "nt(x)" } }); + await vi.advanceTimersByTimeAsync(0); + expect(requests).toHaveLength(0); + expect(view.dom.querySelector(".cm-sem-python-builtin")?.textContent).toBe("print"); + expect(view.dom.querySelectorAll(".cm-sem-parameter")).toHaveLength(2); + } finally { + view.destroy(); + } + }); + + it("retains mapped server colours until the delayed response arrives, while updating syntax colours", async () => { + const support = await loadSupport(findLanguageDescription("Python", "demo.py")!); + vi.useFakeTimers(); + const view = new EditorView({ + parent: document.body, + state: EditorState.create({ + doc: "def f(x):\n print(x)", + extensions: [support!, lspDoc.of({ doc: 91, language: "python" } as EditorDocContext), semanticExtension()], + }), + }); + try { + await vi.advanceTimersByTimeAsync(600); + requests.shift()!({ + tokenTypes: ["function", "parameter"], + tokenModifiers: ["defaultLibrary"], + data: [1, 4, 5, 0, 1, 1, 10, 1, 1, 0], + }); + await vi.advanceTimersByTimeAsync(0); + expect(view.dom.querySelector(".cm-sem-python-builtin")?.textContent).toBe("print"); + expect(view.dom.querySelector(".cm-sem-parameter")?.textContent).toBe("x"); + + view.dispatch({ changes: { from: 0, insert: "# comment\n" } }); + await vi.advanceTimersByTimeAsync(0); + expect(view.dom.querySelector(".cm-sem-python-builtin")?.textContent).toBe("print"); + expect(view.dom.querySelector(".cm-sem-parameter")?.textContent).toBe("x"); + expect(view.dom.querySelector(".cm-sem-python-definition")?.textContent).toBe("f"); + + await vi.advanceTimersByTimeAsync(600); + expect(requests).toHaveLength(1); + expect(view.dom.querySelector(".cm-sem-python-builtin")?.textContent).toBe("print"); + requests.shift()!({ + tokenTypes: ["function", "parameter"], + tokenModifiers: ["defaultLibrary"], + data: [2, 4, 5, 0, 1, 2, 10, 1, 1, 0], + }); + await vi.advanceTimersByTimeAsync(0); + expect(view.dom.querySelector(".cm-sem-python-builtin")?.textContent).toBe("print"); + } finally { + view.destroy(); + } + }); +}); diff --git a/src/editor/lsp/theme.ts b/src/editor/lsp/theme.ts index 8ea732ac7..79350190f 100644 --- a/src/editor/lsp/theme.ts +++ b/src/editor/lsp/theme.ts @@ -69,6 +69,16 @@ const markdown = { ".tok-invalid": { color: "var(--syn-invalid)" }, }; +function pythonStyle(token: string, fallback: string) { + return { + color: `var(--syn-${token}, var(--syn-${fallback}))`, + fontWeight: `var(--syn-${token}-bold, inherit)`, + fontStyle: `var(--syn-${token}-italic, inherit)`, + backgroundColor: `var(--syn-${token}-bg, transparent)`, + textDecoration: `var(--syn-${token}-effect, none)`, + }; +} + export const lspTheme = EditorView.theme({ // --- completion (IDEA lookup: icon column, bold blue match, grey tail/type, advertiser) --- ".cm-tooltip.cm-tooltip-autocomplete.cm-lsp-completion": { @@ -348,15 +358,31 @@ export const lspTheme = EditorView.theme({ ".cm-lsp-inlay-pl": { marginLeft: "4px" }, ".cm-lsp-inlay-pr": { marginRight: "4px" }, + ".cm-sem-python-method-call": pythonStyle("python-method-call", "function-call"), + ".cm-sem-python-annotation": pythonStyle("python-annotation", "type"), + ".cm-sem-python-parentheses": pythonStyle("python-parentheses", "punct"), + ".cm-sem-python-brackets": pythonStyle("python-brackets", "punct"), + ".cm-sem-python-braces": pythonStyle("python-braces", "punct"), + ".cm-sem-python-definition": pythonStyle("function", "function"), + ".cm-sem-python-nested": pythonStyle("python-nested-function", "function"), + ".cm-sem-python-predefined-definition": pythonStyle("python-predefined-definition", "function"), + ".cm-sem-python-predefined-usage": pythonStyle("python-predefined-usage", "self"), + ".cm-sem-python-docstring, .cm-sem-python-docstring *": pythonStyle("docstring", "string"), + ".cm-sem-python-bytes": pythonStyle("python-bytes", "string"), + ".cm-sem-python-escape": pythonStyle("escape", "escape"), + ".cm-sem-python-format": pythonStyle("python-format", "escape"), + ".cm-sem-python-keyword-arg": pythonStyle("keyword-arg", "param"), // --- semantic tokens ----------------------------------------------------- ".cm-sem-type, .cm-sem-typeParameter": { color: "var(--syn-type)" }, // IDEA colours declarations, calls stay (nearly) plain text. ".cm-sem-function": { color: "var(--syn-function-call, var(--syn-function))" }, ".cm-sem-function.cm-sem-declaration": { color: "var(--syn-function)" }, ".cm-sem-function.cm-sem-builtin": { color: "var(--syn-builtin, var(--syn-function))" }, + ".cm-sem-function.cm-sem-python-method-call": pythonStyle("python-method-call", "function-call"), ".cm-sem-macro": { color: "var(--syn-annotation)" }, ".cm-sem-annotation": { color: "var(--syn-decorator, var(--syn-annotation))" }, ".cm-sem-constant": { color: "var(--syn-constant)" }, + ".cm-sem-variable": { color: "var(--syn-variable)" }, ".cm-sem-property": { color: "var(--syn-property)" }, ".cm-sem-parameter": { color: "var(--syn-param, var(--syn-variable))" }, ".cm-sem-self": { color: "var(--syn-self, var(--syn-keyword))" }, @@ -364,6 +390,7 @@ export const lspTheme = EditorView.theme({ // Before the builtin rule: library globals (`print`, `string`) keep the predefined colour. ".cm-sem-global:not(.cm-sem-function)": { color: "var(--syn-global, inherit)" }, ".cm-sem-builtin:not(.cm-sem-function)": { color: "var(--syn-builtin, inherit)" }, + ".cm-sem-builtin.cm-sem-python-builtin": pythonStyle("predefined", "builtin"), ".cm-sem-static": { fontStyle: "var(--syn-static-italic, italic)" }, ".cm-sem-property.cm-sem-static, .cm-sem-property.cm-sem-static *": { color: "var(--syn-static-property, var(--syn-property))", @@ -380,6 +407,13 @@ export const lspTheme = EditorView.theme({ textUnderlineOffset: "3px", }, ".cm-sem-deprecated": { textDecoration: "line-through" }, + ".cm-sem-python-local-variable": pythonStyle("python-local-variable", "variable"), + ".cm-sem-global.cm-sem-python-global-variable": pythonStyle("python-global-variable", "global"), + ".cm-sem-python-instance-attribute": pythonStyle("python-instance-attribute", "property"), + ".cm-sem-python-class-attribute": pythonStyle("python-class-attribute", "property"), + ".cm-sem-python-constant": pythonStyle("python-constant", "constant"), + ".cm-sem-python-property": pythonStyle("python-property", "property"), + ".cm-sem-python-type-parameter": pythonStyle("python-type-parameter", "type"), // Semantic highlighting: after the base classes so a parameter's colour gives way, and over the // syntax highlighter's span inside. ...Object.fromEntries( diff --git a/src/platform/__fixtures__/colorSchemes/forest.icls b/src/platform/__fixtures__/colorSchemes/forest.icls index b38a1aaee..b1a61ccff 100644 --- a/src/platform/__fixtures__/colorSchemes/forest.icls +++ b/src/platform/__fixtures__/colorSchemes/forest.icls @@ -15,7 +15,7 @@ - + diff --git a/src/platform/colorScheme.ts b/src/platform/colorScheme.ts index 4e0dbc5ec..ef2e9534d 100644 --- a/src/platform/colorScheme.ts +++ b/src/platform/colorScheme.ts @@ -135,6 +135,16 @@ const BASE_ATTRIBUTES: readonly AttributeDef[] = [ ...[1, 2, 3, 4, 5].map((n) => syn(`semantic${n}`, `Semantic highlighting › Color ${n}`, [LD], `rainbow-${n}`)), ...[1, 2, 3].map((n) => syn(`bracket${n}`, `Rainbow brackets › Level ${n}`, [LD], `bracket-${n}`)), + syn("pyMethodCall", "Method call", ["Python"], "python-method-call"), + syn("pyAnnotation", "Type annotation", ["Python"], "python-annotation"), + syn("pyParentheses", "Parentheses", ["Python"], "python-parentheses"), + syn("pyBrackets", "Brackets", ["Python"], "python-brackets"), + syn("pyBraces", "Braces", ["Python"], "python-braces"), + syn("pyNestedFunction", "Nested function definition", ["Python"], "python-nested-function"), + syn("pyBytes", "Byte string", ["Python"], "python-bytes"), + syn("pyPredefinedDefinition", "Predefined function definition", ["Python"], "python-predefined-definition"), + syn("pyPredefinedUsage", "Predefined name usage", ["Python"], "python-predefined-usage"), + syn("pyFormat", "Format specifier", ["Python"], "python-format"), syn("pySelf", "Self parameter", ["Python"], "self"), { id: "pyStaticSymbol", @@ -148,6 +158,13 @@ const BASE_ATTRIBUTES: readonly AttributeDef[] = [ syn("pyDocstring", "Docstring", ["Python"], "docstring"), syn("pyBuiltin", "Built-in name", ["Python"], "predefined"), syn("pyDecorator", "Decorator", ["Python"], "annotation"), + syn("pyLocalVariable", "Local variable", ["Python"], "python-local-variable"), + syn("pyGlobalVariable", "Global variable", ["Python"], "python-global-variable"), + syn("pyInstanceAttribute", "Instance attribute", ["Python"], "python-instance-attribute"), + syn("pyClassAttribute", "Class attribute", ["Python"], "python-class-attribute"), + syn("pyConstant", "Constant", ["Python"], "python-constant"), + syn("pyProperty", "Property", ["Python"], "python-property"), + syn("pyTypeParameter", "Type parameter", ["Python"], "python-type-parameter"), syn("mdHeading", "Heading", ["Markdown"], "markup-heading"), syn("mdCode", "Code span", ["Markdown"], "markup-code"), syn("mdLink", "Link", ["Markdown"], "link"), diff --git a/src/platform/icls.test.ts b/src/platform/icls.test.ts index 4f72ec1d7..018a30031 100644 --- a/src/platform/icls.test.ts +++ b/src/platform/icls.test.ts @@ -77,6 +77,51 @@ describe(".icls schemes", () => { expect(customs).toEqual(before); }); + it("imports PyCharm dot keys after shared defaults without inherited font effects", () => { + const xml = ` + + + + + + + + `; + const attrs = parseIcls(xml).scheme.attributes; + expect(attrs.keyword).toEqual({ foreground: "#112233", bold: false, italic: false, effect: "none" }); + expect(attrs.pyNestedFunction?.foreground).toBe("#223344"); + expect(attrs.pyDocstring?.foreground).toBe("#334455"); + expect(attrs.pyDocstring?.effect).toBe("none"); + expect(attrs.pyBytes?.foreground).toBe("#445566"); + expect(attrs.pyBytes?.bold).toBe(true); + expect(attrs.pyBuiltin?.foreground).toBe("#00abcd"); + const imported = importScheme({}, parseIcls(xml).scheme); + const roundTrip = parseIcls(exportIcls(imported.id, imported.customs)).scheme.attributes; + expect(roundTrip.pyNestedFunction?.foreground).toBe("#223344"); + expect(roundTrip.pyBytes?.foreground).toBe("#445566"); + }); + + it("round trips the Python identifier attributes independently", () => { + const names = [ + ["pyLocalVariable", "101010"], + ["pyGlobalVariable", "202020"], + ["pyInstanceAttribute", "303030"], + ["pyClassAttribute", "404040"], + ["pyConstant", "505050"], + ["pyProperty", "606060"], + ["pyTypeParameter", "707070"], + ] as const; + const xml = `${names + .map( + ([id, value]) => + ``, + ) + .join("")}`; + const { id, customs } = importScheme({}, parseIcls(xml).scheme); + const attrs = parseIcls(exportIcls(id, customs)).scheme.attributes; + for (const [name, value] of names) expect(attrs[name]?.foreground).toBe(`#${value}`); + }); + it("imports the global variable colour", () => { const xml = '