editor: Fix double $ sign on completion accept in PHP (#34726)

Smit Barmase created 4 months ago

Closes #33510 https://github.com/zed-extensions/php/issues/29

If certain language servers do not provide an insert/replace range, we
use `surrounding_word` as a fallback for that range, which internally
uses `word_characters`. It makes sense to use
`completion_query_characters` instead of `word_characters` to get that
range, because we use `completion_query_characters` to query completions
in the first place.

That means, for some hypothetical reason (e.g., if the Tailwind server
stops providing insert/replace ranges), we would correctly fall back to
the range "bg-blue-200^" instead of "200^", because
`completion_query_characters` includes "-" in this case.

For this particular fix, right now the default PHP language server
`phpactor` does not provide an insert/replace range, and hence
completion query character is used, which is `$` in this case.

Note that `$` isn't in word characters for reasons mentioned here:
https://github.com/zed-extensions/php/issues/14

Release Notes:

- Fixed an issue where accepting variable completion in PHP would result
in a double $ sign in the prefix.

Change summary

crates/editor/src/editor.rs       |  8 ++++----
crates/language/src/buffer.rs     | 10 ++++++++--
crates/project/src/lsp_command.rs |  4 ++--
3 files changed, 14 insertions(+), 8 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -5451,7 +5451,7 @@ impl Editor {
         };
 
         let (word_replace_range, word_to_exclude) = if let (word_range, Some(CharKind::Word)) =
-            buffer_snapshot.surrounding_word(buffer_position)
+            buffer_snapshot.surrounding_word(buffer_position, false)
         {
             let word_to_exclude = buffer_snapshot
                 .text_for_range(word_range.clone())
@@ -6605,8 +6605,8 @@ impl Editor {
         }
 
         let snapshot = cursor_buffer.read(cx).snapshot();
-        let (start_word_range, _) = snapshot.surrounding_word(cursor_buffer_position);
-        let (end_word_range, _) = snapshot.surrounding_word(tail_buffer_position);
+        let (start_word_range, _) = snapshot.surrounding_word(cursor_buffer_position, false);
+        let (end_word_range, _) = snapshot.surrounding_word(tail_buffer_position, false);
         if start_word_range != end_word_range {
             self.document_highlights_task.take();
             self.clear_background_highlights::<DocumentHighlightRead>(cx);
@@ -22137,7 +22137,7 @@ impl SemanticsProvider for Entity<Project> {
                         // Fallback on using TreeSitter info to determine identifier range
                         buffer.read_with(cx, |buffer, _| {
                             let snapshot = buffer.snapshot();
-                            let (range, kind) = snapshot.surrounding_word(position);
+                            let (range, kind) = snapshot.surrounding_word(position, false);
                             if kind != Some(CharKind::Word) {
                                 return None;
                             }

crates/language/src/buffer.rs

@@ -3364,13 +3364,19 @@ impl BufferSnapshot {
 
     /// Returns a tuple of the range and character kind of the word
     /// surrounding the given position.
-    pub fn surrounding_word<T: ToOffset>(&self, start: T) -> (Range<usize>, Option<CharKind>) {
+    pub fn surrounding_word<T: ToOffset>(
+        &self,
+        start: T,
+        for_completion: bool,
+    ) -> (Range<usize>, Option<CharKind>) {
         let mut start = start.to_offset(self);
         let mut end = start;
         let mut next_chars = self.chars_at(start).take(128).peekable();
         let mut prev_chars = self.reversed_chars_at(start).take(128).peekable();
 
-        let classifier = self.char_classifier_at(start);
+        let classifier = self
+            .char_classifier_at(start)
+            .for_completion(for_completion);
         let word_kind = cmp::max(
             prev_chars.peek().copied().map(|c| classifier.kind(c)),
             next_chars.peek().copied().map(|c| classifier.kind(c)),

crates/project/src/lsp_command.rs

@@ -350,7 +350,7 @@ impl LspCommand for PrepareRename {
             }
             Some(lsp::PrepareRenameResponse::DefaultBehavior { .. }) => {
                 let snapshot = buffer.snapshot();
-                let (range, _) = snapshot.surrounding_word(self.position);
+                let (range, _) = snapshot.surrounding_word(self.position, false);
                 let range = snapshot.anchor_after(range.start)..snapshot.anchor_before(range.end);
                 Ok(PrepareRenameResponse::Success(range))
             }
@@ -2297,7 +2297,7 @@ impl LspCommand for GetCompletions {
                             range_for_token
                                 .get_or_insert_with(|| {
                                     let offset = self.position.to_offset(&snapshot);
-                                    let (range, kind) = snapshot.surrounding_word(offset);
+                                    let (range, kind) = snapshot.surrounding_word(offset, true);
                                     let range = if kind == Some(CharKind::Word) {
                                         range
                                     } else {