fix(editor): let plugin completions and definitions own the strings they answer (#1364)

Inside PHP strings, plugin completions were buried under project file paths and go to definition mixed the plugin's answer with the server's guesses and with IDE helper stubs. Now path items are left out when a plugin answered at that position, and in PHP and Blade paths only show once the string reads like a path. Inside a string literal (checked on the document's syntax tree) plugin definitions replace the server's; elsewhere both are kept, the plugin's first, so e.g. MyBatis mapper navigation still offers jdtls' answer. Generated helpers (_ide_helper*.php, .phpstorm.meta.php, vendor/_laravel_ide*) give way to real declarations, an empty plugin answer is marked incomplete so typing asks again, and a callable accepted before a closing quote no longer gets ().

Closes #1336

Co-authored-by: yzPeedro <[email protected]>

crates/ide-host/src/ops/lsp.rs

@@ -47,7 +47,7 @@ pub async fn lsp_completion(
     let Some((plugins, d)) = plugin_providers(host, window, doc, ProviderKind::Completion) else { return result };
     let items = plugin_call(move || plugins.provide_completion(&d, pos, trigger.as_deref())).await.unwrap_or_default();
     if items.is_empty() {
-        return result;
+        return Ok(result.unwrap_or(CompletionList { is_incomplete: true, items: Vec::new() }));
     }
     let mut list = result.unwrap_or(CompletionList { is_incomplete: false, items: Vec::new() });
     list.items.extend(items.into_iter().map(|(_, item)| plugin_completion(item)));
@@ -222,14 +222,65 @@ pub async fn lsp_goto(host: &Host, window: &str, doc: DocId, pos: Pos, kind: Got
     }
     let Some((plugins, d)) = plugin_providers(host, window, doc, ProviderKind::Definition) else { return result };
     let found = plugin_call(move || plugins.provide_definition(&d, pos)).await.unwrap_or_default();
-    if found.is_empty() {
-        return result;
+    let found: Vec<Location> =
+        found.into_iter().map(|l| Location { path: l.path.into(), range: l.range, uri: None }).collect();
+    let in_string = !found.is_empty() && in_string_literal(host, window, doc, pos).await;
+    definition_targets(result, found, in_string)
+}
+
+async fn in_string_literal(host: &Host, window: &str, doc: DocId, pos: Pos) -> bool {
+    let Ok(project) = host.project(window) else { return false };
+    blocking(move || {
+        let snap = project.documents.text_snapshot(doc)?;
+        if snap.large {
+            return Ok(false);
+        }
+        let s = ide_syntax::Snapshot::doc(doc, snap.version, &snap.language, &snap.text);
+        Ok(string_at(&project.structure, &s, pos))
+    })
+    .await
+    .unwrap_or(false)
+}
+
+fn string_at(structure: &ide_syntax::StructureService, snap: &ide_syntax::Snapshot, pos: Pos) -> bool {
+    let cancel = ide_core::CancellationToken::new();
+    let Ok(Some(at)) = structure.node_at(snap, Range::new(pos, pos), &cancel) else { return false };
+    let stringy = |kind: &str| kind.contains("string") || kind.contains("heredoc") || kind.contains("nowdoc");
+    let part = |kind: &str| kind.contains("content") || kind.contains("fragment") || kind.contains("escape");
+    stringy(&at.node.kind) || (part(&at.node.kind) && at.ancestors.first().is_some_and(|parent| stringy(&parent.kind)))
+}
+
+fn definition_targets(
+    server: IdeResult<Vec<Location>>,
+    plugin: Vec<Location>,
+    in_string: bool,
+) -> IdeResult<Vec<Location>> {
+    if plugin.is_empty() {
+        return server.map(without_generated_helpers);
     }
-    let mut out = result.unwrap_or_default();
-    out.extend(found.into_iter().map(|l| Location { path: l.path.into(), range: l.range, uri: None }));
+    if in_string {
+        return Ok(plugin);
+    }
+    let same_place = |l: &Location| plugin.iter().any(|p| p.path == l.path && p.range.start.line == l.range.start.line);
+    let server: Vec<Location> =
+        without_generated_helpers(server.unwrap_or_default()).into_iter().filter(|l| !same_place(l)).collect();
+    let mut out = plugin;
+    out.extend(server);
     Ok(out)
 }
 
+fn without_generated_helpers(locations: Vec<Location>) -> Vec<Location> {
+    let generated = |l: &Location| {
+        let path = l.path.to_string_lossy().replace('\\', "/");
+        let file = path.rsplit('/').next().unwrap_or(&path);
+        file.starts_with("_ide_helper") || file == ".phpstorm.meta.php" || path.contains("/vendor/_laravel_ide")
+    };
+    if locations.iter().all(generated) {
+        return locations;
+    }
+    locations.into_iter().filter(|l| !generated(l)).collect()
+}
+
 /// Gutter markers from plugins (`line-markers` providers) for `doc`.
 pub async fn plugin_line_markers(
     host: &Host,
@@ -956,6 +1007,79 @@ mod tests {
         assert!(eager.edit.is_some() && eager.data.is_none());
     }
 
+    #[test]
+    fn plugin_definitions_own_strings_and_join_the_server_elsewhere() {
+        let at = |path: &str, line: u32| Location {
+            path: path.into(),
+            range: Range::new(Pos::new(line, 0), Pos::new(line, 4)),
+            uri: None,
+        };
+        let paths = |found: IdeResult<Vec<Location>>| {
+            found.unwrap().into_iter().map(|l| l.path.to_string_lossy().into_owned()).collect::<Vec<_>>()
+        };
+        let xml = at("/p/src/main/resources/UserMapper.xml", 3);
+        let java = at("/p/src/main/java/UserMapper.java", 7);
+        let helper = at("/p/_ide_helper.php", 1);
+        let route = at("/p/routes/web.php", 2);
+        let guess = at("/p/app/helpers.php", 9);
+
+        assert_eq!(
+            paths(definition_targets(Ok(vec![java.clone(), helper.clone()]), vec![xml.clone()], false)),
+            ["/p/src/main/resources/UserMapper.xml", "/p/src/main/java/UserMapper.java"]
+        );
+        assert_eq!(
+            paths(definition_targets(Ok(vec![xml.clone(), java.clone()]), vec![xml.clone()], false)),
+            ["/p/src/main/resources/UserMapper.xml", "/p/src/main/java/UserMapper.java"]
+        );
+        assert_eq!(
+            paths(definition_targets(Ok(vec![guess.clone(), helper.clone()]), vec![route.clone()], true)),
+            ["/p/routes/web.php"]
+        );
+        assert_eq!(
+            paths(definition_targets(Err(ide_core::IdeError::unavailable("no server")), vec![route.clone()], false)),
+            ["/p/routes/web.php"]
+        );
+        assert_eq!(paths(definition_targets(Ok(vec![helper, guess]), Vec::new(), false)), ["/p/app/helpers.php"]);
+    }
+
+    #[test]
+    fn positions_inside_string_literals_are_told_apart() {
+        let syntax = std::sync::Arc::new(ide_language::Syntax::new());
+        let structure = ide_syntax::StructureService::new(std::sync::Arc::new(ide_language::SyntaxCache::new(syntax)));
+        let php = ide_core::LanguageId::new("php");
+        let text = "<?php\nreturn to_route('users.show', $user);\necho \"Hi {$name}\";\n";
+        let snap = ide_syntax::Snapshot::text(&php, text);
+        assert!(string_at(&structure, &snap, Pos::new(1, 20)), "inside 'users.show'");
+        assert!(!string_at(&structure, &snap, Pos::new(1, 9)), "on to_route");
+        assert!(!string_at(&structure, &snap, Pos::new(1, 35)), "on $user");
+        assert!(!string_at(&structure, &snap, Pos::new(2, 12)), "on an interpolated variable");
+        let java = ide_core::LanguageId::new("java");
+        let text = "class A {\n    User find(long id) { return mapper.selectById(\"users\"); }\n}\n";
+        let snap = ide_syntax::Snapshot::text(&java, text);
+        assert!(!string_at(&structure, &snap, Pos::new(1, 9)), "on a method name");
+        assert!(string_at(&structure, &snap, Pos::new(1, 53)), "inside \"users\"");
+    }
+
+    #[test]
+    fn generated_ide_helpers_give_way_to_real_declarations() {
+        let at = |path: &str| Location { path: path.into(), range: Range::default(), uri: None };
+        let paths =
+            |found: Vec<Location>| found.into_iter().map(|l| l.path.to_string_lossy().into_owned()).collect::<Vec<_>>();
+        let real = at("/app/app/Models/User.php");
+        let helper = at("/app/_ide_helper_models.php");
+        let meta = at("/app/.phpstorm.meta.php");
+        let vendor = at("/app/vendor/_laravel_idea/_ide_helper.php");
+        assert_eq!(
+            paths(without_generated_helpers(vec![helper.clone(), real.clone(), meta.clone(), vendor])),
+            ["/app/app/Models/User.php"]
+        );
+        assert_eq!(
+            paths(without_generated_helpers(vec![helper.clone(), meta.clone()])),
+            ["/app/_ide_helper_models.php", "/app/.phpstorm.meta.php"]
+        );
+        assert!(without_generated_helpers(Vec::new()).is_empty());
+    }
+
     #[test]
     fn every_completion_kind_the_plugin_api_documents_is_understood() {
         let wit = include_str!("../../../../wit/providers.wit");

src/editor/lsp/completion.test.ts

@@ -15,6 +15,7 @@ import {
 import {
   applyLspCompletion,
   callParens,
+  offersPaths,
   commitsLookup,
   completionExtension,
   explicitCompletion,
@@ -373,11 +374,27 @@ describe("insert parentheses", () => {
 
   it("skips calls that already have parentheses and imports", () => {
     expect(callParens(item(), line("a.cr(x)"), 2, 4)).toBeNull();
+    expect(callParens(item(), line("cr'"), 0, 2)).toBeNull();
+    expect(callParens(item(), line('cr"'), 0, 2)).toBeNull();
     expect(callParens(item(), line("import { cr"), 9, 11)).toBeNull();
     expect(callParens(item(), line("use crate::cr"), 11, 13)).toBeNull();
   });
 });
 
+describe("file paths in strings", () => {
+  it("are offered in PHP and Blade only once the string reads like a path", () => {
+    expect(offersPaths("php", "config('app.")).toBe(false);
+    expect(offersPaths("php", "User::create(['na")).toBe(false);
+    expect(offersPaths("php", "require './")).toBe(true);
+    expect(offersPaths("blade", "@include('partials/")).toBe(true);
+  });
+
+  it("are offered as before in other languages", () => {
+    expect(offersPaths("typescript", "import x from 'mod")).toBe(true);
+    expect(offersPaths("rust", 'include_str!("da')).toBe(true);
+  });
+});
+
 describe("sort suggestions alphabetically", () => {
   const result = (labels: string[]) => ({ from: 0, options: labels.map((label) => ({ label })), validFor: /^\w*$/ });
 

src/editor/lsp/completion.ts

@@ -51,7 +51,7 @@ import { selectableText } from "@/ui/selectableText";
 import { iconNode } from "./iconMarkup";
 import { parameterInfoOrLeave, triggerSignatureHelp } from "./signature";
 import { featureBackoff } from "./availability";
-import { pathCompletionSource } from "./fileReferences";
+import { pathCompletionSource, pathContext, pathLike } from "./fileReferences";
 import { editorPrefs } from "@/platform/editorSettings";
 import { tx } from "@/platform/i18n";
 import { showHint } from "./hint";
@@ -298,7 +298,7 @@ export function callParens(
 ): "inside" | "after" | null {
   if (item.isSnippet || !item.kind || !CALLABLE.has(item.kind) || /[(<[{]/.test(item.insertText)) return null;
   const next = line.text.charAt(end - line.from);
-  if (next && /[(\p{L}\p{N}_$]/u.test(next)) return null;
+  if (next && /[(\p{L}\p{N}_$'"`]/u.test(next)) return null;
   if (IMPORT_LINE.test(line.text.slice(0, start - line.from))) return null;
   const sig = item.labelDetail ?? item.detail ?? "";
   return /^\s*\(\s*\)/.test(sig) || /^\s*fn\s*\(\s*\)|^\s*\(\)\s*(=>|->|:)/.test(sig) ? "after" : "inside";
@@ -764,12 +764,39 @@ const completionFooter = ViewPlugin.fromClass(
   },
 );
 
+let pluginClaim: { pos: number; claimed: Promise<boolean> } | null = null;
+
+const claimingLspSource: CompletionSource = (ctx) => {
+  const result = Promise.resolve(lspCompletionSource(ctx));
+  pluginClaim = {
+    pos: ctx.pos,
+    claimed: result.then((r) => !!r?.options.some((o) => isLsp(o) && o.lsp.server === 0)).catch(() => false),
+  };
+  return result;
+};
+
+const PATH_ONLY_WHEN_PATH_LIKE = new Set(["php", "blade"]);
+
+export function offersPaths(language: string, before: string): boolean {
+  if (!PATH_ONLY_WHEN_PATH_LIKE.has(language)) return true;
+  const found = pathContext(before, language);
+  return !!found && pathLike(found.prefix);
+}
+
+const pathFallbackSource: CompletionSource = async (ctx) => {
+  const line = ctx.state.doc.lineAt(ctx.pos);
+  if (!offersPaths(ctx.state.facet(lspDoc)?.language ?? "", line.text.slice(0, ctx.pos - line.from))) return null;
+  const claim = pluginClaim;
+  if (claim && claim.pos === ctx.pos && (await claim.claimed)) return null;
+  return pathCompletionSource(ctx);
+};
+
 export function completionExtension() {
   return [
     lookupKeys,
     prefetchResolve,
     autocompletion({
-      override: [gated(lspCompletionSource), gated(contributedSource), gated(pathCompletionSource)],
+      override: [gated(claimingLspSource), gated(contributedSource), gated(pathFallbackSource)],
       icons: false,
       addToOptions: [
         { render: kindBadge, position: 20 },