fix(fsharp): correct selectionRange for module declarations (#925)

FsAutoComplete reports selectionRange for a `module <Name>` declaration
pointing at the `module` keyword itself, not at `<Name>`. Since
LanguageServerSymbol.line/.column (src/serena/symbol.py) build hover and
reference lookups from selectionRange.start, hovering a module returns
the LSP server's docs for the generic keyword instead of the module's
own, as reported in #925 by the F# team and confirmed by dsyme
(F# tooling) and MischaPanch in the issue thread.

This mirrors the fix already shipped for fortls in
fortran_language_server.py: a request_document_symbols override that
recovers the real identifier column by matching the declaration line
with a regex, scoped to Module symbols only since other kinds (functions,
types, open) already select correctly.

Verified with a synthetic-payload unit test (module declaration,
nested-with-equals, recursive module, an already-correct range left
untouched, and a non-Module symbol left untouched), since F# language
server tests are unconditionally disabled in this repo
(test/conftest.py: "F# language server is currently unreliable"), so no
live-FsAutoComplete regression test can run here or on CI. Confirmed the
new test fails on main (AttributeError / unfixed selectionRange) and
passes on this branch.

Fixes #925

CHANGELOG.md

@@ -31,6 +31,9 @@ Status of the `main` branch. Changes prior to the next official version change w
     and the request used to map diagnostics onto owning symbols just threw. `AnsibleLanguageServer` now
     overrides that request to return `None` directly, so diagnostics fall back to being grouped under
     the file-level path as already documented #1758
+  - Fix: F#'s `module <Name>` declarations reported a `selectionRange` pointing at the `module`
+    keyword instead of at `<Name>`, so looking up hover/references from a module symbol's position
+    returned the keyword's own docs instead of the module's #925
 
 * JetBrains:
   - `jet_brains_find_symbol`: Disallow wildcard-only search, delegating to overview tool if request is for file

src/solidlsp/language_servers/fsharp_language_server.py

@@ -4,6 +4,7 @@ Provides F# specific instantiation of the LanguageServer class.
 
 import logging
 import os
+import re
 import shutil
 import subprocess
 import threading
@@ -12,8 +13,9 @@ from pathlib import Path
 from overrides import override
 
 from serena.util.dotnet import DotNETUtil
+from solidlsp import ls_types
 from solidlsp.language_servers.common import RuntimeDependency, RuntimeDependencyCollection
-from solidlsp.ls import SolidLanguageServer
+from solidlsp.ls import DocumentSymbols, LSPFileBuffer, SolidLanguageServer
 from solidlsp.ls_config import LanguageServerConfig, LanguageServerId
 from solidlsp.ls_exceptions import SolidLSPException
 from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
@@ -29,6 +31,10 @@ INITIAL_FSAUTOCOMPLETE_VERSION = "0.83.0"
 DEFAULT_FSAUTOCOMPLETE_VERSION = "0.83.0"
 FSAUTOCOMPLETE_VERSION = DEFAULT_FSAUTOCOMPLETE_VERSION
 
+# Matches an F# module declaration line ("module Foo", "module Foo =", "module rec Foo =",
+# dotted names), used to recover the module name's real column. See _fix_module_selection_range.
+_MODULE_DECLARATION_PATTERN = re.compile(r"^\s*module\s+(?:rec\s+)?([A-Za-z_]\w*(?:\.[A-Za-z_]\w*)*)")
+
 
 class FSharpLanguageServer(SolidLanguageServer):
     """
@@ -68,6 +74,60 @@ class FSharpLanguageServer(SolidLanguageServer):
             ".ionide",
         ]
 
+    def _fix_module_selection_range(
+        self, symbol: ls_types.UnifiedSymbolInformation, file_content: str
+    ) -> ls_types.UnifiedSymbolInformation:
+        """
+        FsAutoComplete's selectionRange for a `module <Name>` declaration points at the `module`
+        keyword itself instead of at `<Name>` (confirmed by dsyme and MischaPanch in #925), so any
+        caller that hovers or searches references from selectionRange.start (e.g.
+        serena.symbol.LanguageServerSymbol.line/.column) gets the generic keyword's info instead of
+        the module's own. Other symbol kinds (functions, types, `open`) already select the
+        identifier correctly, so only Module symbols are touched here.
+        """
+        if symbol.get("kind") != ls_types.SymbolKind.Module or "selectionRange" not in symbol:
+            return symbol
+
+        sel_range = symbol["selectionRange"]
+        start_line = sel_range["start"]["line"]
+        lines = file_content.split("\n")
+        if start_line >= len(lines):
+            return symbol
+
+        match = _MODULE_DECLARATION_PATTERN.match(lines[start_line])
+        if not match:
+            return symbol
+
+        name_start, name_end = match.span(1)
+        if sel_range["start"]["character"] == name_start:
+            return symbol  # already correct (e.g. a future FsAutoComplete release)
+
+        corrected_symbol = symbol.copy()
+        corrected_symbol["selectionRange"] = {
+            "start": {"line": start_line, "character": name_start},
+            "end": {"line": start_line, "character": name_end},
+        }
+        return corrected_symbol
+
+    @override
+    def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols:
+        # Override to fix FsAutoComplete's incorrect selectionRange for module declarations (#925):
+        # it points at the `module` keyword instead of the module name, so hover-by-selectionRange
+        # returns the keyword's docs instead of the module's own.
+        document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer)
+
+        with self.open_file(relative_file_path) as file_data:
+            file_content = file_data.contents
+
+        def fix_symbol_and_children(symbol: ls_types.UnifiedSymbolInformation) -> ls_types.UnifiedSymbolInformation:
+            fixed = self._fix_module_selection_range(symbol, file_content)
+            if fixed.get("children"):
+                fixed["children"] = [fix_symbol_and_children(child) for child in fixed["children"]]
+            return fixed
+
+        fixed_root_symbols = [fix_symbol_and_children(sym) for sym in document_symbols.root_symbols]
+        return DocumentSymbols(fixed_root_symbols)
+
     @classmethod
     def _setup_runtime_dependencies(cls, config: LanguageServerConfig, solidlsp_settings: SolidLSPSettings) -> str:
         """

test/solidlsp/fsharp/test_fsharp_module_selection_range.py

@@ -0,0 +1,132 @@
+"""
+Regression test for #925: FsAutoComplete's selectionRange for a `module <Name>` declaration
+points at the `module` keyword rather than at `<Name>`, so hovering a module reports the
+generic keyword's docs instead of the module's own (confirmed by dsyme and MischaPanch in the
+issue thread). This pins FSharpLanguageServer._fix_module_selection_range and the
+request_document_symbols override directly, without spinning up FsAutoComplete: F# language
+server tests are unconditionally disabled (test/conftest.py, category 1, "F# language server is
+currently unreliable"), so a live-LS test can never run here or on CI.
+"""
+
+from contextlib import contextmanager
+
+from solidlsp import ls_types
+from solidlsp.language_servers.fsharp_language_server import FSharpLanguageServer
+from solidlsp.ls import DocumentSymbols, SolidLanguageServer
+
+
+def _bare_fsharp_server() -> FSharpLanguageServer:
+    """Instance without running __init__ (no dotnet tool install, no process); same technique
+    as test_typescript_timeout_policy.py's _bare_ts_server / test_rename_didopen.py.
+    """
+    return object.__new__(FSharpLanguageServer)
+
+
+def _module_symbol(name: str, line: int, start_char: int, end_char: int) -> ls_types.UnifiedSymbolInformation:
+    return {
+        "name": name,
+        "kind": ls_types.SymbolKind.Module,
+        "selectionRange": {
+            "start": {"line": line, "character": start_char},
+            "end": {"line": line, "character": end_char},
+        },
+        "children": [],
+    }
+
+
+class TestFixModuleSelectionRange:
+    def test_top_level_module_declaration(self) -> None:
+        # "module Calculator" -- FsAutoComplete reports selectionRange over "module" (0-6)
+        server = _bare_fsharp_server()
+        symbol = _module_symbol("Calculator", line=0, start_char=0, end_char=6)
+        file_content = "module Calculator\n\nlet add a b = a + b\n"
+
+        fixed = server._fix_module_selection_range(symbol, file_content)
+
+        assert fixed["selectionRange"]["start"] == {"line": 0, "character": 7}
+        assert fixed["selectionRange"]["end"] == {"line": 0, "character": 17}
+        assert file_content.splitlines()[0][7:17] == "Calculator"
+
+    def test_nested_module_declaration_with_equals(self) -> None:
+        # "module PersonModule =" -- same keyword-position bug, trailing '='
+        server = _bare_fsharp_server()
+        symbol = _module_symbol("PersonModule", line=2, start_char=0, end_char=6)
+        file_content = "namespace Models\n\nmodule PersonModule =\n    let x = 1\n"
+
+        fixed = server._fix_module_selection_range(symbol, file_content)
+
+        assert fixed["selectionRange"]["start"] == {"line": 2, "character": 7}
+        assert fixed["selectionRange"]["end"] == {"line": 2, "character": 19}
+
+    def test_recursive_module_declaration(self) -> None:
+        # "module rec Foo" -- the "rec" keyword must not be mistaken for the module name
+        server = _bare_fsharp_server()
+        symbol = _module_symbol("Foo", line=0, start_char=0, end_char=6)
+        file_content = "module rec Foo\n"
+
+        fixed = server._fix_module_selection_range(symbol, file_content)
+
+        assert fixed["selectionRange"]["start"] == {"line": 0, "character": 11}
+        assert fixed["selectionRange"]["end"] == {"line": 0, "character": 14}
+
+    def test_already_correct_selection_range_is_left_alone(self) -> None:
+        # A future FsAutoComplete release that already points at the identifier must not be
+        # rewritten (guards against masking an upstream fix instead of a no-op).
+        server = _bare_fsharp_server()
+        symbol = _module_symbol("Calculator", line=0, start_char=7, end_char=17)
+        file_content = "module Calculator\n"
+
+        fixed = server._fix_module_selection_range(symbol, file_content)
+
+        assert fixed is symbol
+
+    def test_non_module_symbol_is_untouched(self) -> None:
+        # Functions/types already select the identifier correctly per MischaPanch (#925); only
+        # Module symbols should ever be corrected.
+        server = _bare_fsharp_server()
+        symbol: ls_types.UnifiedSymbolInformation = {
+            "name": "add",
+            "kind": ls_types.SymbolKind.Function,
+            "selectionRange": {
+                "start": {"line": 2, "character": 4},
+                "end": {"line": 2, "character": 7},
+            },
+            "children": [],
+        }
+        file_content = "module Calculator\n\nlet add a b = a + b\n"
+
+        fixed = server._fix_module_selection_range(symbol, file_content)
+
+        assert fixed is symbol
+
+
+class TestRequestDocumentSymbolsWiring:
+    def test_fixes_every_symbol_in_the_tree_not_just_roots(self, monkeypatch) -> None:
+        """End-to-end wiring test: the override must fix nested symbols too, mirroring the
+        already-merged fortran_language_server.py recursive-fix pattern this follows.
+        """
+        outer = _module_symbol("Outer", line=0, start_char=0, end_char=6)
+        inner = _module_symbol("Inner", line=1, start_char=4, end_char=10)
+        outer["children"] = [inner]
+
+        file_content = "module Outer\n    module Inner =\n        let x = 1\n"
+
+        def fake_super_request_document_symbols(self, relative_file_path, file_buffer=None):
+            return DocumentSymbols([outer])
+
+        monkeypatch.setattr(SolidLanguageServer, "request_document_symbols", fake_super_request_document_symbols)
+
+        server = _bare_fsharp_server()
+
+        @contextmanager
+        def fake_open_file(relative_file_path):
+            yield type("FakeFileData", (), {"contents": file_content})()
+
+        monkeypatch.setattr(server, "open_file", fake_open_file)
+
+        result = server.request_document_symbols("Test.fs")
+
+        root = result.root_symbols[0]
+        assert root["selectionRange"]["start"] == {"line": 0, "character": 7}
+        child = root["children"][0]
+        assert child["selectionRange"]["start"] == {"line": 1, "character": 11}