Merge pull request #1028 from oraios/fix-ls-open-file-stale-buffers

LSPFileBuffer: Ensure that file contents are never stale

src/serena/code_editor.py

@@ -81,8 +81,9 @@ class CodeEditor(Generic[TSymbol], ABC):
 
     def _save_edited_file(self, edited_file: "CodeEditor.EditedFile") -> None:
         abs_path = os.path.join(self.project_root, edited_file.relative_path)
+        new_contents = edited_file.get_contents()
         with open(abs_path, "w", encoding=self.encoding) as f:
-            f.write(edited_file.get_contents())
+            f.write(new_contents)
 
     @abstractmethod
     def _find_unique_symbol(self, name_path: str, relative_file_path: str) -> TSymbol:

src/solidlsp/ls.py

@@ -67,8 +67,8 @@ class LSPFileBuffer:
 
     def __init__(
         self,
+        abs_path: Path,
         uri: str,
-        contents: str,
         encoding: str,
         version: int,
         language_id: str,
@@ -76,9 +76,11 @@ class LSPFileBuffer:
         language_server: "SolidLanguageServer",
         open_in_ls: bool = True,
     ) -> None:
+        self.abs_path = abs_path
         self.language_server = language_server
         self.uri = uri
-        self.contents = contents
+        self._read_file_modified_date: float | None = None
+        self._contents: str | None = None
         self.version = version
         self.language_id = language_id
         self.ref_count = ref_count
@@ -120,6 +122,34 @@ class LSPFileBuffer:
         """Ensure that the file is opened in the language server."""
         self._open_in_ls()
 
+    @property
+    def contents(self) -> str:
+        file_modified_date = self.abs_path.stat().st_mtime
+
+        # if contents are cached, check if they are stale (file modification since last read) and invalidate if so
+        if self._contents is not None:
+            assert self._read_file_modified_date is not None
+            if file_modified_date > self._read_file_modified_date:
+                self._contents = None
+
+        if self._contents is None:
+            self._read_file_modified_date = file_modified_date
+            self._contents = FileUtils.read_file(str(self.abs_path), self.encoding)
+            self._content_hash = None
+
+        return self._contents
+
+    @contents.setter
+    def contents(self, new_contents: str) -> None:
+        """
+        Sets new contents for the file buffer (in-memory change only).
+        Persistence of the change to disk must be handled separately.
+
+        :param new_contents: the new contents to set
+        """
+        self._contents = new_contents
+        self._content_hash = None
+
     @property
     def content_hash(self) -> str:
         if self._content_hash is None:
@@ -675,8 +705,8 @@ class SolidLanguageServer(ABC):
             log.error("open_file called before Language Server started")
             raise SolidLSPException("Language Server not started")
 
-        absolute_file_path = str(PurePath(self.repository_root_path, relative_file_path))
-        uri = pathlib.Path(absolute_file_path).as_uri()
+        absolute_file_path = Path(self.repository_root_path, relative_file_path)
+        uri = absolute_file_path.as_uri()
 
         if uri in self.open_file_buffers:
             fb = self.open_file_buffers[uri]
@@ -689,13 +719,11 @@ class SolidLanguageServer(ABC):
             yield fb
             fb.ref_count -= 1
         else:
-            contents = FileUtils.read_file(absolute_file_path, self._encoding)
-
             version = 0
             language_id = self._get_language_id_for_file(relative_file_path)
             fb = LSPFileBuffer(
+                abs_path=absolute_file_path,
                 uri=uri,
-                contents=contents,
                 encoding=self._encoding,
                 version=version,
                 language_id=language_id,

test/solidlsp/python/test_python_basic.py

@@ -16,7 +16,7 @@ from solidlsp.ls_config import Language
 
 
 @pytest.mark.python
-class TestLanguageServerBasics:
+class TestPythonLanguageServerBasics:
     """Test basic functionality of the language server."""
 
     @pytest.mark.parametrize("language_server", [Language.PYTHON], indirect=True)

test/solidlsp/test_ls_common.py

@@ -0,0 +1,44 @@
+import os
+import platform
+import time
+
+import pytest
+
+from solidlsp import SolidLanguageServer
+from solidlsp.ls_config import Language
+from test.conftest import is_ci
+
+
+class TestLanguageServerCommonFunctionality:
+    """Test common functionality of SolidLanguageServer base implementation (not language-specific behaviour)."""
+
+    @pytest.mark.parametrize("language_server", [Language.PYTHON], indirect=True)
+    def test_open_file_cache_invalidate(self, language_server: SolidLanguageServer) -> None:
+        """
+        Tests that the file buffer cache is invalidated when the file is changed on disk.
+        """
+        file_path = os.path.join(language_server.repository_root_path, "test_open_file.py")
+        test_string1 = "# foo"
+        test_string2 = "# bar"
+
+        with open(file_path, "w") as f:
+            f.write(test_string1)
+
+        try:
+            with language_server.open_file(file_path) as fb:
+                assert fb.contents == test_string1
+
+                # apply external change to file
+                with open(file_path, "w") as f:
+                    f.write(test_string2)
+
+                # ensure that mtime is updated for Windows CI (odd FS behaviour, updates lazily)
+                if is_ci and platform.system() == "Windows":
+                    now = time.time()
+                    os.utime(file_path, (now, now))
+
+                # check that the file buffer has been invalidated and reloaded
+                assert fb.contents == test_string2
+
+        finally:
+            os.remove(file_path)