Skip to content

Fix gopls replace_symbol_body corrupting type/var/const declarations#1530

Open
tphakala wants to merge 1 commit into
oraios:mainfrom
tphakala:fix/gopls-type-var-const-keyword-duplication
Open

Fix gopls replace_symbol_body corrupting type/var/const declarations#1530
tphakala wants to merge 1 commit into
oraios:mainfrom
tphakala:fix/gopls-type-var-const-keyword-duplication

Conversation

@tphakala
Copy link
Copy Markdown

Fixes #1529

Problem

gopls reports the document-symbol range of single type/var/const declarations starting at the identifier (after the keyword), unlike func declarations whose range includes the func keyword. Serena derives both the displayed symbol body and the replace_symbol_body replacement range from that range, so the keyword is dropped from both for these declarations. A keyword-inclusive edit then corrupts the file (type Foo becomes type type Foo), the tool returns OK, and the resulting syntax error breaks subsequent symbolic edits on the file.

Fix

Override request_document_symbols in gopls.py to extend the range start of a single type/var/const declaration back to the keyword when only that keyword (plus optional indentation) precedes the identifier on the start line, mirroring the existing nixd/fortran post-processing pattern. The body is recomputed so the displayed body stays consistent with the replacement range. func declarations (range already at the keyword) and grouped declarations (var ( ... ), keyword on a separate line) are left untouched, and selectionRange is preserved so references and rename continue to point at the identifier.

The override operates on copies so the cached symbols are not mutated.

Tests

  • test/solidlsp/go/test_go_basic.py::TestGoLanguageServer::test_type_var_const_body_includes_leading_keyword: asserts the body and range of type/var/const symbols include the keyword, that selectionRange still points at the identifier, and that grouped vars are unaffected.
  • test/serena/test_symbol_editing.py::test_go_symbol_replacement_no_double_keyword: an end-to-end replace_symbol_body round-trip that produces type type on the old code and passes with the fix.

Both tests fail against the pre-fix code and pass with the fix. poe format, poe type-check, and the Go test suite are green (gopls v0.22.0, go 1.26.1).

@tphakala
Copy link
Copy Markdown
Author

The failing CI check is unrelated to this change.

The only failing test is test/solidlsp/svelte/test_svelte_basic.py::TestSvelteLanguageServer::test_diagnostics_in_typescript_file, which asserts on the Svelte language server's diagnostic output:

AssertionError: ['Unexpected token\nhttps://svelte.dev/e/js_parse_error', 'Expected token }\nhttps://svelte.dev/e/expected_token']

This PR only touches the Go language server (gopls.py) and Go tests, so it cannot affect Svelte diagnostics. All Go tests pass on every runner, including the two added here (test_type_var_const_body_includes_leading_keyword and test_go_symbol_replacement_no_double_keyword). The per-OS summaries are "1 failed, 1472 passed" on ubuntu and "1 failed, 1464 passed" on macos, and in both cases the single failure is the Svelte test.

This looks like a CI environment issue rather than a regression: the same test fails the same way on other unrelated open PRs (for example #1528), and it was still passing on main on 2026-05-27. CI installs svelte-language-server at latest, so a newer version changing its diagnostic messages is the likely cause.

gopls reports the document-symbol range of single type/var/const
declarations starting at the identifier (after the keyword), unlike func
declarations whose range includes the func keyword. Serena derives both
the displayed body and the replacement range from that range, so the
keyword was dropped from both for these declarations and a
keyword-inclusive edit corrupted the file (type Foo became type type Foo).

Override request_document_symbols in gopls to extend the range start back
to the keyword when only the keyword and indentation precede the
identifier, recomputing the body so it stays consistent with the
replacement range. Funcs, grouped declarations, and selectionRange are
left untouched.

Adds a unit test for the symbol body and range plus an end-to-end
replace_symbol_body round-trip regression test.
@tphakala tphakala force-pushed the fix/gopls-type-var-const-keyword-duplication branch from fef00bb to ec8cfe1 Compare June 2, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[LS] replace_symbol_body corrupts Go type/var/const declarations by duplicating the leading keyword

1 participant