From 714c260e1fe621372d0cb753b92e545a42417b9d Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Wed, 23 Sep 2026 07:15:49 -0600 Subject: [PATCH] fix(csharp): fold newly created files into the loaded Roslyn project (#1981) * fix(csharp): fold newly created files into the loaded Roslyn project Roslyn only learns which files belong to a project from the solution/open and project/open notifications _open_solution_and_projects sends once at startup. The generic didChangeWatchedFiles/open-close cycle that poll_and_notify sends for every backend on file creation does not make it re-evaluate the project, so a .cs file created after the project was already indexed stayed a standalone Miscellaneous Files document, producing phantom diagnostics (e.g. an incorrect "using directive is unnecessary") instead of the real ones. Add an overridable SolidLanguageServer.notify_files_created hook, called by LanguageServerFileChangeNotifier.poll_and_notify before it opens newly created files. CSharpLanguageServer overrides it to resend the same solution/project notifications and wait for Roslyn's own reload-complete log line before the file is opened; every other backend keeps its current behavior via the no-op default. Fixes #1961 * fix(test): use get_language_server_manager_or_raise in csharp fold test project.language_server_manager is typed LanguageServerManager | None, so calling .iter_language_servers() on it directly failed ty's unresolved-attribute check. Agent already exposes a non-Optional accessor for the same value. --- CHANGELOG.md | 3 ++ src/serena/ls_manager.py | 12 +++-- .../csharp_language_server.py | 27 +++++++++++- src/solidlsp/ls.py | 12 ++++- test/serena/test_ls_file_sync.py | 44 +++++++++++++++++++ 5 files changed, 93 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fea2756c..b323a369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -130,6 +130,9 @@ Status of the `main` branch. Changes prior to the next official version change w VTS initialization options now override defaults per top-level key rather than replacing the entire configuration; a user-provided `typescript` block replaces the ATA default too. `initializationOptions` takes precedence over the legacy `initialization_options` alias. + - Fix: a C# file created after the project was already indexed was analyzed by Roslyn as a + standalone Miscellaneous Files document instead of being folded into the loaded project, + causing phantom diagnostics on the new file and on files referencing its symbols (#1961) - Add FreeBSD mapping to platform detection - Remove unnecessary platform checks from the following language servers, expanding the set of supported platforms accordingly: Elixir Tools, Intelephense, Perl, TypeScript, VTS diff --git a/src/serena/ls_manager.py b/src/serena/ls_manager.py index cb8a736c..d0fb60c1 100644 --- a/src/serena/ls_manager.py +++ b/src/serena/ls_manager.py @@ -366,9 +366,15 @@ class LanguageServerFileChangeNotifier: # (observed with pyright) to fold a brand-new file into its cross-file reference graph; # an open/close cycle forces the parse+bind that Serena's own file tools trigger via # SolidLanguageServer.open_file(). - for rel_path in created_paths: - if ls.is_ignored_path(rel_path, ignore_unsupported_files=True): - continue + relevant_created_paths = [ + rel_path for rel_path in created_paths if not ls.is_ignored_path(rel_path, ignore_unsupported_files=True) + ] + if relevant_created_paths: + try: + ls.notify_files_created(relevant_created_paths) + except Exception as e: + log.error("Failed to notify language server of newly created files", exc_info=e) + for rel_path in relevant_created_paths: try: with ls.open_file(rel_path): pass diff --git a/src/solidlsp/language_servers/csharp_language_server.py b/src/solidlsp/language_servers/csharp_language_server.py index a411cc78..7a4a52b9 100644 --- a/src/solidlsp/language_servers/csharp_language_server.py +++ b/src/solidlsp/language_servers/csharp_language_server.py @@ -9,7 +9,7 @@ import platform import shutil import tempfile import threading -from collections.abc import Hashable, Iterable +from collections.abc import Hashable, Iterable, Sequence from dataclasses import replace from pathlib import Path from typing import Any, cast @@ -223,6 +223,9 @@ class CSharpLanguageServer(SolidLanguageServer): # Cache for original Roslyn symbol names with type annotations # Key: (relative_file_path, line, character) -> Value: original name self._original_symbol_names: dict[tuple[str, int, int], str] = {} + # Set once Roslyn confirms it finished (re)loading the solution/projects; reused by + # notify_files_created to wait out a reload triggered after startup. + self._project_reload_complete = threading.Event() def _create_dependency_provider(self) -> LanguageServerDependencyProvider: return self.DependencyProvider(self._custom_settings, self._ls_resources_dir, self._solidlsp_settings, self.repository_root_path) @@ -553,6 +556,12 @@ class CSharpLanguageServer(SolidLanguageServer): log.log(level_map.get(level, logging.DEBUG), f"LSP: {message_text}") + # workspace/projectInitializationComplete (below) fires too early on a project + # *re*-open to signal reload completion; this log line is the reliable one for + # notify_files_created's wait. + if "Completed (re)load of all projects" in message_text: + self._project_reload_complete.set() + def handle_progress(params: dict) -> None: """Handle progress notifications from the language server.""" token = params.get("token", "") @@ -776,3 +785,19 @@ class CSharpLanguageServer(SolidLanguageServer): @override def _get_wait_time_for_cross_file_referencing(self) -> float: return 2 + + @override + def notify_files_created(self, relative_file_paths: Sequence[str]) -> None: + """ + Roslyn's project system only learns which files belong to a project from the + `solution/open`/`project/open` notifications sent once at startup (see + `_open_solution_and_projects`); a plain `didChangeWatchedFiles`/open-close cycle + does not make it re-evaluate the project, so a file created after startup is + analyzed as a standalone Miscellaneous Files document instead of being folded into + the already-loaded compilation. Resend the same notifications and wait for the + server to confirm the reload before the file is opened. + """ + self._project_reload_complete.clear() + self._open_solution_and_projects() + if not self._project_reload_complete.wait(30): + log.warning("Timeout waiting for project reload after new file(s) were created: %s", list(relative_file_paths)) diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 26dda831..ba7d4303 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -10,7 +10,7 @@ import shutil import threading from abc import ABC, abstractmethod from collections import defaultdict -from collections.abc import Callable, Hashable, Iterator +from collections.abc import Callable, Hashable, Iterator, Sequence from contextlib import contextmanager from copy import copy from dataclasses import dataclass @@ -1045,6 +1045,16 @@ class SolidLanguageServer(ABC): """ return 2 + def notify_files_created(self, relative_file_paths: Sequence[str]) -> None: + """ + Called when one or more files were newly created on disk (detected outside of Serena's own file + tools, e.g. by :class:`serena.ls_manager.LanguageServerFileChangeNotifier`), before those files are + opened via :meth:`open_file`. The default implementation does nothing: a `didChangeWatchedFiles` + notification followed by an open/close cycle is enough for most backends to fold a new file into + their index. Override this for a language server whose project system needs an explicit reload to + become aware of a file that did not exist when the project was first loaded. + """ + # --- Cross-workspace / additional workspace folder support --- @staticmethod diff --git a/test/serena/test_ls_file_sync.py b/test/serena/test_ls_file_sync.py index 381e31de..c428dd9c 100644 --- a/test/serena/test_ls_file_sync.py +++ b/test/serena/test_ls_file_sync.py @@ -3,6 +3,7 @@ End-to-end test for language server file synchronisation as a result of external Exercises all three ``FileChangeType`` branches against a real pyright backend: Created (new caller file), Changed (append a second caller), Deleted (remove the caller file). +Also covers a Roslyn-specific Created regression (test_new_csharp_file_is_folded_into_compilation). """ import json @@ -192,3 +193,46 @@ def test_find_symbol_tool_reflects_external_change_to_open_buffer(tmp_path): of Serena's own edit tools. """ SymbolPositionStaleAfterExternalEditTestCase().run(tmp_path) + + +@pytest.mark.csharp +def test_new_csharp_file_is_folded_into_compilation(tmp_path): + """ + Regression test for oraios/serena#1961: a .cs file created after the project has already + been indexed used to stay a standalone Miscellaneous Files document to Roslyn, since the + generic didChangeWatchedFiles/open-close cycle that poll_and_notify sends for every backend + is not enough to make Roslyn's project system re-evaluate which files belong to the already- + loaded .csproj. The tell is a phantom 'Using directive is unnecessary' (IDE0005) on a using + the new file genuinely needs, since Roslyn cannot resolve the type it imports. + """ + repo_root = tmp_path / "repo" + shutil.copytree(get_repo_path(LanguageServerId.CSHARP), repo_root) + new_file_rel = "UsesPerson.cs" + new_file_abs = repo_root / new_file_rel + + with agent_for_project_context(LanguageServerId.CSHARP, str(repo_root)) as agent: + project = agent.get_active_project_or_raise() + ls = next(iter(agent.get_language_server_manager_or_raise().iter_language_servers())) + + new_file_abs.write_text( + "using TestProject.Models;\n\n" + "namespace TestProject;\n\n" + "public class UsesPerson\n" + "{\n" + " public Person Get() => new Person();\n" + "}\n", + encoding="utf-8", + ) + assert project.ls_sync_file_system_changes() == 1, "expected exactly one Created event" + + diagnostics = ls.request_text_document_diagnostics(new_file_rel, min_severity=4) + messages = [d["message"] for d in diagnostics] + + assert not any("unnecessary" in m.lower() for m in messages), ( + f"new file was analyzed as a standalone document instead of being folded into the " + f"already-loaded project (phantom 'using directive is unnecessary'): {messages}" + ) + assert any("Person" in m and "name" in m for m in messages), ( + f"expected the real compiler diagnostic for the missing Person(name, age, email) " + f"argument, which only fires once the file is actually part of the compilation: {messages}" + )