zed-industries / zed

Code at the speed of thought – Zed is a high-performance, multiplayer code editor from the creators of Atom and Tree-sitter.
https://zed.dev
Other
50.68k stars 3.14k forks source link

Svelte autocompletes insert the import multiple times :( #19214

Open ArturGalstyan1 opened 1 month ago

ArturGalstyan1 commented 1 month ago

Check for existing issues

Describe the bug / provide steps to reproduce it

Check this video out:

https://github.com/user-attachments/assets/f3bb6ea6-50d2-43a0-ab34-2a3b7ab8a0ee

This is really bugging me and I don't know why this happens. I'm using the svelte plugin.

Environment

Zed: v0.158.0 (Zed Dev 71a878aa399877268c6aa50ff2e8527f16cbf9c8) OS: macOS 15.0.1 Memory: 18 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your Zed.log file to this issue.

Zed.log


arggh commented 1 month ago

Happening also on Zed v0.156.2 (Mac OS 14.5).

vitallium commented 1 month ago

At first glance, the problem is in the Svelte LSP that sends duplicate additionalTextEdits, an optional array of additional text edits that are applied when selecting this completion. Here is the LSP response when you expand the onMount:

{
  "jsonrpc": "2.0",
  "id": 689,
  "result": {
    "label": "onMount",
    "labelDetails": { "description": "svelte" },
    "kind": 3,
    "detail": "Add import from \"svelte\"\n\nfunction onMount<T>(fn: () => NotFunction<T> | Promise<NotFunction<T>> | (() => any)): void",
    "documentation": {
      "value": "The `onMount` function schedules a callback to run as soon as the component has been mounted to the DOM.\nIt must be called during the component's initialisation (but doesn't need to live *inside* the component;\nit can be called from an external module).\n\nIf a function is returned _synchronously_ from `onMount`, it will be called when the component is unmounted.\n\n`onMount` does not run inside a [server-side component](https://svelte.dev/docs#run-time-server-side-component-api).\n\nhttps://svelte.dev/docs/svelte#onmount",
      "kind": "markdown"
    },
    "sortText": "16",
    "additionalTextEdits": [
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      },
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      },
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      }
    ],
    "commitCharacters": [".", ",", ";", "("],
    "data": {
      "name": "onMount",
      "source": "svelte",
      "data": {
        "exportName": "onMount",
        "exportMapKey": "7 2867 onMount svelte",
        "moduleSpecifier": "svelte",
        "ambientModuleName": "svelte"
      },
      "uri": "file:///Users/vslobodin/Development/my-svelte-app/src/routes/%2Bpage.svelte",
      "position": { "line": 1, "character": 3 }
    }
  }
}
vitallium commented 1 month ago

At first glance, the problem is in the Svelte LSP that sends duplicate additionalTextEdits, an optional array of additional text edits that are applied when selecting this completion. Here is the LSP response when you expand the onMount:

{
  "jsonrpc": "2.0",
  "id": 689,
  "result": {
    "label": "onMount",
    "labelDetails": { "description": "svelte" },
    "kind": 3,
    "detail": "Add import from \"svelte\"\n\nfunction onMount<T>(fn: () => NotFunction<T> | Promise<NotFunction<T>> | (() => any)): void",
    "documentation": {
      "value": "The `onMount` function schedules a callback to run as soon as the component has been mounted to the DOM.\nIt must be called during the component's initialisation (but doesn't need to live *inside* the component;\nit can be called from an external module).\n\nIf a function is returned _synchronously_ from `onMount`, it will be called when the component is unmounted.\n\n`onMount` does not run inside a [server-side component](https://svelte.dev/docs#run-time-server-side-component-api).\n\nhttps://svelte.dev/docs/svelte#onmount",
      "kind": "markdown"
    },
    "sortText": "16",
    "additionalTextEdits": [
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      },
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      },
      {
        "range": {
          "start": { "line": 0, "character": 18 },
          "end": { "line": 0, "character": 18 }
        },
        "newText": "\n  import { onMount } from \"svelte\";\n"
      }
    ],
    "commitCharacters": [".", ",", ";", "("],
    "data": {
      "name": "onMount",
      "source": "svelte",
      "data": {
        "exportName": "onMount",
        "exportMapKey": "7 2867 onMount svelte",
        "moduleSpecifier": "svelte",
        "ambientModuleName": "svelte"
      },
      "uri": "file:///Users/vslobodin/Development/my-svelte-app/src/routes/%2Bpage.svelte",
      "position": { "line": 1, "character": 3 }
    }
  }
}

The duplication happens because of an issue in the Svelte LSP, I think. Zed as a LSP server receives completions, asks to resolve the selected, for example, receives the first response from the Svelte LSP with a single entry in the additionalTextEdits, then Zed asks to resolve the same completion again, here I am not sure if this is a correct behaviour, but I assume yes. Svelte LSP receives that command and just concatenates the additionalTextEdits field with some sort of codeActions here https://github.com/sveltejs/language-tools/blob/156bd7d7f445899dd7cf861495a570059d9e47ef/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts#L892 I'd say the current behaviour is incorrect and the server should filter out duplicate entries or do not generate them at all.

vitallium commented 1 month ago

I briefly checked the linked code and I think something like this:

const additionalTextEdits = completionItem.additionalTextEdits ?? [];
completionItem.additionalTextEdits = [
    ...new Set([...additionalTextEdits, ...edit].map((i) => JSON.stringify(i)))
].map((i) => JSON.parse(i));

will remove duplicate entries by utilising Set's features and the JSON presentation of objects. Alternatively, instead of using JSON, we could compare all keys in objects.