From 27229bba6b8705369ca362a0de7824cb2af44cf7 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Fri, 24 May 2024 21:00:23 +0200 Subject: [PATCH] tasks: Provide task variables from matching runnable ranges in task modal (#12237) In #12003 we found ourselves in need for precise region tracking in which a given runnable has an effect in order to grab variables from it. This PR makes it so that in task modal all task variables from queries overlapping current cursor position. However, in the process of working on that I've found that we cannot always use a top-level capture to represent the full match range of runnable (which has been my assumption up to this point). Tree-sitter captures cannot capture sibling groups; we did just that in Rust queries. Thankfully, none of the extensions are affected as in them, a capture is always attached to single node. This PR adds annotations to them nonetheless; we'll be able to get rid of top-level captures in extension runnables.scm once this PR is in stable version of Zed. Release Notes: - N/A --- crates/editor/src/editor.rs | 40 ++--- crates/editor/src/element.rs | 4 +- crates/editor/src/tasks.rs | 28 +++- crates/language/src/buffer.rs | 75 +++++++--- crates/language/src/language.rs | 40 ++--- crates/language/src/task_context.rs | 1 + crates/languages/src/go/runnables.scm | 18 ++- crates/languages/src/rust/runnables.scm | 40 +++-- .../gleam/languages/gleam/runnables.scm | 11 +- extensions/php/languages/php/runnables.scm | 140 ++++++++++-------- extensions/ruby/languages/ruby/runnables.scm | 81 ++++++---- 11 files changed, 295 insertions(+), 183 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9009795a92..48c13be23e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -406,10 +406,13 @@ impl Default for ScrollbarMarkerState { #[derive(Clone, Debug)] struct RunnableTasks { templates: Vec<(TaskSourceKind, TaskTemplate)>, - // We need the column at which the task context evaluation should take place. + offset: MultiBufferOffset, + // We need the column at which the task context evaluation should take place (when we're spawning it via gutter). column: u32, // Values of all named captures, including those starting with '_' extra_variables: HashMap, + // Full range of the tagged region. We use it to determine which `extra_variables` to grab for context resolution in e.g. a modal. + context_range: Range, } #[derive(Clone)] @@ -417,7 +420,10 @@ struct ResolvedTasks { templates: SmallVec<[(TaskSourceKind, ResolvedTask); 1]>, position: Anchor, } - +#[derive(Copy, Clone, Debug)] +struct MultiBufferOffset(usize); +#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] +struct BufferOffset(usize); /// Zed's primary text input `View`, allowing users to edit a [`MultiBuffer`] /// /// See the [module level documentation](self) for more information. @@ -516,7 +522,7 @@ pub struct Editor { >, last_bounds: Option>, expect_bounds_change: Option>, - tasks: HashMap<(BufferId, BufferRow), (usize, RunnableTasks)>, + tasks: BTreeMap<(BufferId, BufferRow), RunnableTasks>, tasks_update_task: Option>, } @@ -4053,7 +4059,7 @@ impl Editor { this.discard_inline_completion(false, cx); let tasks = tasks.as_ref().zip(this.workspace.clone()).and_then( |(tasks, (workspace, _))| { - let position = Point::new(buffer_row, tasks.1.column); + let position = Point::new(buffer_row, tasks.column); let range_start = buffer.read(cx).anchor_at(position, Bias::Right); let location = Location { buffer: buffer.clone(), @@ -4061,7 +4067,7 @@ impl Editor { }; // Fill in the environmental variables from the tree-sitter captures let mut captured_task_variables = TaskVariables::default(); - for (capture_name, value) in tasks.1.extra_variables.clone() { + for (capture_name, value) in tasks.extra_variables.clone() { captured_task_variables.insert( task::VariableName::Custom(capture_name.into()), value.clone(), @@ -4082,7 +4088,6 @@ impl Editor { .map(|task_context| { Arc::new(ResolvedTasks { templates: tasks - .1 .templates .iter() .filter_map(|(kind, template)| { @@ -4092,7 +4097,7 @@ impl Editor { }) .collect(), position: snapshot.buffer_snapshot.anchor_before( - Point::new(multibuffer_point.row, tasks.1.column), + Point::new(multibuffer_point.row, tasks.column), ), }) }) @@ -4693,7 +4698,7 @@ impl Editor { self.tasks.clear() } - fn insert_tasks(&mut self, key: (BufferId, BufferRow), value: (usize, RunnableTasks)) { + fn insert_tasks(&mut self, key: (BufferId, BufferRow), value: RunnableTasks) { if let Some(_) = self.tasks.insert(key, value) { // This case should hopefully be rare, but just in case... log::error!("multiple different run targets found on a single line, only the last target will be rendered") @@ -7931,7 +7936,7 @@ impl Editor { snapshot: DisplaySnapshot, runnable_ranges: Vec, mut cx: AsyncWindowContext, - ) -> Vec<((BufferId, u32), (usize, RunnableTasks))> { + ) -> Vec<((BufferId, u32), RunnableTasks)> { runnable_ranges .into_iter() .filter_map(|mut runnable| { @@ -7953,16 +7958,17 @@ impl Editor { .start .row; + let context_range = + BufferOffset(runnable.full_range.start)..BufferOffset(runnable.full_range.end); Some(( (runnable.buffer_id, row), - ( - runnable.run_range.start, - RunnableTasks { - templates: tasks, - column: point.column, - extra_variables: runnable.extra_captures, - }, - ), + RunnableTasks { + templates: tasks, + offset: MultiBufferOffset(runnable.run_range.start), + context_range, + column: point.column, + extra_variables: runnable.extra_captures, + }, )) }) .collect() diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 71eb7f7e25..19a7703b0a 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1535,8 +1535,8 @@ impl EditorElement { editor .tasks .iter() - .filter_map(|(_, (multibuffer_offset, _))| { - let multibuffer_point = multibuffer_offset.to_point(&snapshot.buffer_snapshot); + .filter_map(|(_, tasks)| { + let multibuffer_point = tasks.offset.0.to_point(&snapshot.buffer_snapshot); let multibuffer_row = MultiBufferRow(multibuffer_point.row); if snapshot.is_line_folded(multibuffer_row) { return None; diff --git a/crates/editor/src/tasks.rs b/crates/editor/src/tasks.rs index 73ab73f26a..adcb2fc90f 100644 --- a/crates/editor/src/tasks.rs +++ b/crates/editor/src/tasks.rs @@ -5,7 +5,7 @@ use gpui::{Model, WindowContext}; use language::ContextProvider; use project::{BasicContextProvider, Location, Project}; use task::{TaskContext, TaskVariables, VariableName}; -use text::Point; +use text::{Point, ToOffset, ToPoint}; use util::ResultExt; use workspace::Workspace; @@ -70,14 +70,26 @@ fn task_context_with_editor( }; let captured_variables = { let mut variables = TaskVariables::default(); - for range in location - .buffer - .read(cx) - .snapshot() - .runnable_ranges(location.range.clone()) + let buffer = location.buffer.read(cx); + let buffer_id = buffer.remote_id(); + let snapshot = buffer.snapshot(); + let starting_point = location.range.start.to_point(&snapshot); + let starting_offset = starting_point.to_offset(&snapshot); + for (_, tasks) in editor + .tasks + .range((buffer_id, 0)..(buffer_id, starting_point.row + 1)) { - for (capture_name, value) in range.extra_captures { - variables.insert(VariableName::Custom(capture_name.into()), value); + if !tasks + .context_range + .contains(&crate::BufferOffset(starting_offset)) + { + continue; + } + for (capture_name, value) in tasks.extra_variables.iter() { + variables.insert( + VariableName::Custom(capture_name.to_owned().into()), + value.clone(), + ); } } variables diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 9703302e76..5482ab2f1b 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -14,7 +14,7 @@ use crate::{ SyntaxSnapshot, ToTreeSitterPoint, }, task_context::RunnableRange, - LanguageScope, Outline, RunnableTag, + LanguageScope, Outline, RunnableCapture, RunnableTag, }; use anyhow::{anyhow, Context, Result}; pub use clock::ReplicaId; @@ -3061,41 +3061,76 @@ impl BufferSnapshot { iter::from_fn(move || loop { let mat = syntax_matches.peek()?; + let test_range = test_configs[mat.grammar_index].and_then(|test_configs| { - let mut tags: SmallVec<[(Range, RunnableTag); 1]> = + let mut run_range = None; + let full_range = mat.captures.iter().fold( + Range { + start: usize::MAX, + end: 0, + }, + |mut acc, next| { + let byte_range = next.node.byte_range(); + if acc.start > byte_range.start { + acc.start = byte_range.start; + } + if acc.end < byte_range.end { + acc.end = byte_range.end; + } + acc + }, + ); + if full_range.start > full_range.end { + // We did not find a full spanning range of this match. + return None; + } + let extra_captures: SmallVec<[_; 1]> = SmallVec::from_iter(mat.captures.iter().filter_map(|capture| { test_configs - .runnable_tags - .get(&capture.index) + .extra_captures + .get(capture.index as usize) .cloned() - .map(|tag_name| (capture.node.byte_range(), tag_name)) + .and_then(|tag_name| match tag_name { + RunnableCapture::Named(name) => { + Some((capture.node.byte_range(), name)) + } + RunnableCapture::Run => { + let _ = run_range.insert(capture.node.byte_range()); + None + } + }) })); - let maximum_range = tags + let run_range = run_range?; + let tags = test_configs + .query + .property_settings(mat.pattern_index) .iter() - .max_by_key(|(byte_range, _)| byte_range.len()) - .map(|(range, _)| range)? - .clone(); - tags.sort_by_key(|(range, _)| range == &maximum_range); - let split_point = tags.partition_point(|(range, _)| range != &maximum_range); - let (extra_captures, tags) = tags.split_at(split_point); - + .filter_map(|property| { + if *property.key == *"tag" { + property + .value + .as_ref() + .map(|value| RunnableTag(value.to_string().into())) + } else { + None + } + }) + .collect(); let extra_captures = extra_captures .into_iter() .map(|(range, name)| { ( - name.0.to_string(), + name.to_string(), self.text_for_range(range.clone()).collect::(), ) }) .collect(); + // All tags should have the same range. Some(RunnableRange { - run_range: mat - .captures - .iter() - .find(|capture| capture.index == test_configs.run_capture_ix) - .map(|mat| mat.node.byte_range())?, + run_range, + full_range, runnable: Runnable { - tags: tags.into_iter().cloned().map(|(_, tag)| tag).collect(), + tags, language: mat.language, buffer: self.remote_id(), }, diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 99f01456c9..e2c1cc7089 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -25,7 +25,7 @@ use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; use collections::{HashMap, HashSet}; use futures::Future; -use gpui::{AppContext, AsyncAppContext, Model, Task}; +use gpui::{AppContext, AsyncAppContext, Model, SharedString, Task}; pub use highlight_map::HighlightMap; use http::HttpClient; use lazy_static::lazy_static; @@ -882,12 +882,16 @@ struct RedactionConfig { pub redaction_capture_ix: u32, } +#[derive(Clone, Debug, PartialEq)] +enum RunnableCapture { + Named(SharedString), + Run, +} + struct RunnableConfig { pub query: Query, - /// A mapping from captures indices to known test tags - pub runnable_tags: HashMap, - /// index of the capture that corresponds to @run - pub run_capture_ix: u32, + /// A mapping from capture indice to capture kind + pub extra_captures: Vec, } struct OverrideConfig { @@ -1009,23 +1013,21 @@ impl Language { .ok_or_else(|| anyhow!("cannot mutate grammar"))?; let query = Query::new(&grammar.ts_language, source)?; - let mut run_capture_index = None; - let mut runnable_tags = HashMap::default(); - for (ix, name) in query.capture_names().iter().enumerate() { - if *name == "run" { - run_capture_index = Some(ix as u32); + let mut extra_captures = Vec::with_capacity(query.capture_names().len()); + + for name in query.capture_names().iter() { + let kind = if *name == "run" { + RunnableCapture::Run } else { - runnable_tags.insert(ix as u32, RunnableTag(name.to_string().into())); - } + RunnableCapture::Named(name.to_string().into()) + }; + extra_captures.push(kind); } - if let Some(run_capture_ix) = run_capture_index { - grammar.runnable_config = Some(RunnableConfig { - query, - run_capture_ix, - runnable_tags, - }); - } + grammar.runnable_config = Some(RunnableConfig { + extra_captures, + query, + }); Ok(self) } diff --git a/crates/language/src/task_context.rs b/crates/language/src/task_context.rs index a290c64c89..743085f6ec 100644 --- a/crates/language/src/task_context.rs +++ b/crates/language/src/task_context.rs @@ -11,6 +11,7 @@ use text::BufferId; pub struct RunnableRange { pub buffer_id: BufferId, pub run_range: Range, + pub full_range: Range, pub runnable: Runnable, pub extra_captures: HashMap, } diff --git a/crates/languages/src/go/runnables.scm b/crates/languages/src/go/runnables.scm index a412ecd87c..d5d01879e2 100644 --- a/crates/languages/src/go/runnables.scm +++ b/crates/languages/src/go/runnables.scm @@ -1,9 +1,15 @@ ( - (function_declaration name: (_) @run - (#match? @run "^Test.*")) -) @go-test + ( + (function_declaration name: (_) @run + (#match? @run "^Test.*")) + ) @_ + (#set! tag go-test) +) ( - (function_declaration name: (_) @run - (#eq? @run "main")) -) @go-main + ( + (function_declaration name: (_) @run + (#eq? @run "main")) + ) @_ + (#set! tag go-main) +) diff --git a/crates/languages/src/rust/runnables.scm b/crates/languages/src/rust/runnables.scm index f722fb00d7..90e4900188 100644 --- a/crates/languages/src/rust/runnables.scm +++ b/crates/languages/src/rust/runnables.scm @@ -1,17 +1,27 @@ -(mod_item - name: (_) @run - (#eq? @run "tests") -) @rust-mod-test ( - (attribute_item (attribute - [((identifier) @_attribute) - (scoped_identifier (identifier) @_attribute) - ]) - (#eq? @_attribute "test")) - . - (attribute_item) * - . - (function_item - name: (_) @run) -) @rust-test + (mod_item + name: (_) @run + (#eq? @run "tests") + ) @rust-mod-test + (#set! tag rust-mod-test) +) + +( + ( + (attribute_item (attribute + [((identifier) @_attribute) + (scoped_identifier (identifier) @_attribute) + ]) + (#eq? @_attribute "test") + ) @start + . + (attribute_item) * + . + (function_item + name: (_) @run + body: _ + ) @end + ) + (#set! tag rust-test) +) diff --git a/extensions/gleam/languages/gleam/runnables.scm b/extensions/gleam/languages/gleam/runnables.scm index ca5ad00e59..b0b37b11a4 100644 --- a/extensions/gleam/languages/gleam/runnables.scm +++ b/extensions/gleam/languages/gleam/runnables.scm @@ -1,9 +1,13 @@ ; Functions with names ending in `_test`. ; This matches the standalone test style used by Startest and Gleeunit. ( - (function name: (_) @run - (#match? @run ".*_test$")) -) @gleam-test + ( + (function name: (_) @run + (#match? @run ".*_test$")) + ) @gleam-test + (#set! tag gleam-test) +) + ; `describe` API for Startest. ( @@ -17,4 +21,5 @@ ) ) ) + (#set! tag gleam-test) ) @gleam-test diff --git a/extensions/php/languages/php/runnables.scm b/extensions/php/languages/php/runnables.scm index 8e36d901ea..cde3d95636 100644 --- a/extensions/php/languages/php/runnables.scm +++ b/extensions/php/languages/php/runnables.scm @@ -2,87 +2,101 @@ ; and that doesn't have the abstract modifier ; and have a method that follow the naming convention of PHPUnit test methods ; and the method is public -(class_declaration - modifier: (_)? @_modifier - (#not-eq? @_modifier "abstract") - name: (_) @_name - (#match? @_name ".*Test$") - body: (declaration_list - (method_declaration - (visibility_modifier)? @_visibility - (#eq? @_visibility "public") - name: (_) @run - (#match? @run "^test.*") +( + (class_declaration + modifier: (_)? @_modifier + (#not-eq? @_modifier "abstract") + name: (_) @_name + (#match? @_name ".*Test$") + body: (declaration_list + (method_declaration + (visibility_modifier)? @_visibility + (#eq? @_visibility "public") + name: (_) @run + (#match? @run "^test.*") + ) ) - ) -) @phpunit-test + ) @phpunit-test + (#set! tag phpunit-test) +) ; Class that follow the naming convention of PHPUnit test classes ; and that doesn't have the abstract modifier ; and have a method that has the @test annotation ; and the method is public -(class_declaration - modifier: (_)? @_modifier - (#not-eq? @_modifier "abstract") - name: (_) @_name - (#match? @_name ".*Test$") - body: (declaration_list - ((comment) @_comment - (#match? @_comment ".*@test\\b.*") - . - (method_declaration - (visibility_modifier)? @_visibility - (#eq? @_visibility "public") - name: (_) @run - (#not-match? @run "^test.*") - )) - ) -) @phpunit-test - +( + (class_declaration + modifier: (_)? @_modifier + (#not-eq? @_modifier "abstract") + name: (_) @_name + (#match? @_name ".*Test$") + body: (declaration_list + ((comment) @_comment + (#match? @_comment ".*@test\\b.*") + . + (method_declaration + (visibility_modifier)? @_visibility + (#eq? @_visibility "public") + name: (_) @run + (#not-match? @run "^test.*") + )) + ) + ) @phpunit-test + (#set! tag phpunit-test) +) ; Class that follow the naming convention of PHPUnit test classes ; and that doesn't have the abstract modifier ; and have a method that has the #[Test] attribute ; and the method is public -(class_declaration - modifier: (_)? @_modifier - (#not-eq? @_modifier "abstract") - name: (_) @_name - (#match? @_name ".*Test$") - body: (declaration_list - (method_declaration - (attribute_list - (attribute_group - (attribute (name) @_attribute) +( + (class_declaration + modifier: (_)? @_modifier + (#not-eq? @_modifier "abstract") + name: (_) @_name + (#match? @_name ".*Test$") + body: (declaration_list + (method_declaration + (attribute_list + (attribute_group + (attribute (name) @_attribute) + ) ) + (#eq? @_attribute "Test") + (visibility_modifier)? @_visibility + (#eq? @_visibility "public") + name: (_) @run + (#not-match? @run "^test.*") ) - (#eq? @_attribute "Test") - (visibility_modifier)? @_visibility - (#eq? @_visibility "public") - name: (_) @run - (#not-match? @run "^test.*") ) - ) -) @phpunit-test + ) @phpunit-test + (#set! tag phpunit-test) +) ; Class that follow the naming convention of PHPUnit test classes ; and that doesn't have the abstract modifier -(class_declaration - modifier: (_)? @_modifier - (#not-eq? @_modifier "abstract") - name: (_) @run - (#match? @run ".*Test$") -) @phpunit-test +( + (class_declaration + modifier: (_)? @_modifier + (#not-eq? @_modifier "abstract") + name: (_) @run + (#match? @run ".*Test$") + ) @phpunit-test + (#set! tag phpunit-test) +) ; Add support for Pest runnable ; Function expression that has `it`, `test` or `describe` as the function name -(function_call_expression - function: (_) @_name - (#any-of? @_name "it" "test" "describe") - arguments: (arguments - . - (argument - (encapsed_string (string_value) @run) +( + (function_call_expression + function: (_) @_name + (#any-of? @_name "it" "test" "describe") + arguments: (arguments + . + (argument + (encapsed_string (string_value) @run) + ) ) - ) -) @pest-test + ) @pest-test + (#set! tag pest-test) +) diff --git a/extensions/ruby/languages/ruby/runnables.scm b/extensions/ruby/languages/ruby/runnables.scm index 4170a655ea..4be83dbd22 100644 --- a/extensions/ruby/languages/ruby/runnables.scm +++ b/extensions/ruby/languages/ruby/runnables.scm @@ -4,46 +4,67 @@ ; Minitest ;; Rails unit tests -(class - name: [ - (constant) @run - (scope_resolution scope: (constant) name: (constant) @run) - ] - (superclass (scope_resolution) @superclass (#match? @superclass "(::IntegrationTest|::TestCase|::SystemTestCase)$")) -) @minitest-test +( + (class + name: [ + (constant) @run + (scope_resolution scope: (constant) name: (constant) @run) + ] + (superclass (scope_resolution) @superclass (#match? @superclass "(::IntegrationTest|::TestCase|::SystemTestCase)$")) + ) @minitest-test + (#set! tag minitest-test) +) -(call - method: (identifier) @run (#eq? @run "test") - arguments: (argument_list (string (string_content) @name)) -) @minitest-test +( + (call + method: (identifier) @run (#eq? @run "test") + arguments: (argument_list (string (string_content) @name)) + ) @minitest-test + (#set! tag minitest-test) +) ; Methods that begin with test_ -(method - name: (identifier) @run (#match? @run "^test_") -) @minitest-test +( + (method + name: (identifier) @run (#match? @run "^test_") + ) @minitest-test + (#set! tag minitest-test) +) ; System tests that inherit from ApplicationSystemTestCase -(class - name: (constant) @run (superclass) @superclass (#match? @superclass "(ApplicationSystemTestCase)$") -) @minitest-test +( + (class + name: (constant) @run (superclass) @superclass (#match? @superclass "(ApplicationSystemTestCase)$") + ) @minitest-test + (#set! tag minitest-test) +) ; RSpec ; Example groups with literals -(call - method: (identifier) @run (#any-of? @run "describe" "context") - arguments: (argument_list . (_) @name) -) @rspec-test +( + (call + method: (identifier) @run (#any-of? @run "describe" "context") + arguments: (argument_list . (_) @name) + ) @rspec-test + (#set! tag rspec-test) +) ; Examples -(call - method: (identifier) @run (#any-of? @run "it" "its" "specify") - arguments: (argument_list (string (string_content) @name)) -) @rspec-test +( + (call + method: (identifier) @run (#any-of? @run "it" "its" "specify") + arguments: (argument_list (string (string_content) @name)) + ) @rspec-test + (#set! tag rspec-test) +) ; Examples (one-liner syntax) -(call - method: (identifier) @run (#any-of? @run "it" "its" "specify") - block: (_) @name - !arguments -) @rspec-test +( + (call + method: (identifier) @run (#any-of? @run "it" "its" "specify") + block: (_) @name + !arguments + ) @rspec-test + (#set! tag rspec-test) +)