fix(test): merge PHP test classes into TestPhpLanguageServers

Consolidate TestPhpLanguageServer and TestPhpPhpactorLanguageServer
into a single TestPhpLanguageServers class. Both Language.PHP and
Language.PHP_PHPACTOR are now parametrized together, with conditional
branching where the two language servers differ in behavior.

Addresses review comment on PR #1002.

test/solidlsp/php/test_php_basic.py

@@ -7,16 +7,15 @@ from solidlsp.ls_config import Language
 
 
 @pytest.mark.php
-class TestPhpLanguageServer:
+class TestPhpLanguageServers:
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_ls_is_running(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
         """Test that the language server starts and stops successfully."""
-        # The fixture already handles start and stop
         assert language_server.is_running()
         assert Path(language_server.language_server.repository_root_path).resolve() == repo_path.resolve()
 
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_find_definition_within_file(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
 
@@ -26,32 +25,47 @@ class TestPhpLanguageServer:
         # We want to find the definition of $greeting (defined on line 9)
         # from its usage in echo $greeting; on line 11.
         # LSP is 0-indexed: definition on line 8, usage on line 10.
-        # $greeting in echo $greeting; is at char 5 on line 11 (0-indexed: line 10, char 5)
+        # $greeting in echo $greeting;  (e c h o   $ g r e e t i n g)
-        # e c h o   $ g r e e t i n g
+        #                           ^ char 5
-        #           ^ char 5
+        # Intelephense uses line 10 (0-indexed), Phpactor uses line 11 (0-indexed)
-        definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 10, 6)  # cursor on 'g' in $greeting
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
+            definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 11, 6)
+        else:
+            definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 10, 6)
 
         assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) == 1
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
+            assert len(definition_location_list) >= 1
+        else:
+            assert len(definition_location_list) == 1
         definition_location = definition_location_list[0]
         assert definition_location["uri"].endswith("index.php")
         # Definition of $greeting is on line 10 (1-indexed) / line 9 (0-indexed), char 0
         assert definition_location["range"]["start"]["line"] == 9
-        assert definition_location["range"]["start"]["character"] == 0
+        if language_server.language_server.language != Language.PHP_PHPACTOR:
+            assert definition_location["range"]["start"]["character"] == 0
 
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_find_definition_across_files(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 12, 5)  # helperFunction
+        # Intelephense uses line 12 (0-indexed), Phpactor uses line 13 (0-indexed)
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
+            definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 13, 5)
+        else:
+            definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 12, 5)
 
         assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) == 1
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
+            assert len(definition_location_list) >= 1
+        else:
+            assert len(definition_location_list) == 1
         definition_location = definition_location_list[0]
         assert definition_location["uri"].endswith("helper.php")
         assert definition_location["range"]["start"]["line"] == 2
-        assert definition_location["range"]["start"]["character"] == 0
+        if language_server.language_server.language != Language.PHP_PHPACTOR:
+            assert definition_location["range"]["start"]["character"] == 0
 
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_find_definition_simple_variable(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
         file_path = str(repo_path / "simple_var.php")
@@ -66,13 +80,17 @@ class TestPhpLanguageServer:
         definition_location_list = language_server.request_definition(file_path, 2, 6)  # cursor on 'l' in $localVar
 
         assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) == 1
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
+            assert len(definition_location_list) >= 1
+        else:
+            assert len(definition_location_list) == 1
         definition_location = definition_location_list[0]
         assert definition_location["uri"].endswith("simple_var.php")
         assert definition_location["range"]["start"]["line"] == 1  # Definition of $localVar (0-indexed)
-        assert definition_location["range"]["start"]["character"] == 0  # $localVar (0-indexed)
+        if language_server.language_server.language != Language.PHP_PHPACTOR:
+            assert definition_location["range"]["start"]["character"] == 0  # $localVar (0-indexed)
 
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_find_references_within_file(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
         index_php_path = str(repo_path / "index.php")
@@ -83,30 +101,44 @@ class TestPhpLanguageServer:
         # Find references for $greeting from its usage in "echo $greeting;" (line 11, char 6 for 'g')
         references = language_server.request_references(index_php_path, 11, 6)
 
-        assert references
+        assert references, f"Expected non-empty references for $greeting but got {references=}"
-        # Intelephense, when asked for references from usage, seems to only return the usage itself.
-        assert len(references) == 1, "Expected to find 1 reference for $greeting (the usage itself)"
 
-        expected_locations = [{"uri_suffix": "index.php", "line": 11, "character": 5}]  # Usage: echo $greeting (points to $)
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
-
+            actual_locations = [
-        # Convert actual references to a comparable format and sort
-        actual_locations = sorted(
-            [
                 {
                     "uri_suffix": loc["uri"].split("/")[-1],
                     "line": loc["range"]["start"]["line"],
-                    "character": loc["range"]["start"]["character"],
                 }
                 for loc in references
-            ],
+            ]
-            key=lambda x: (x["uri_suffix"], x["line"], x["character"]),
-        )
 
-        expected_locations = sorted(expected_locations, key=lambda x: (x["uri_suffix"], x["line"], x["character"]))
+            # Check that at least one reference points to $greeting usage in index.php
+            matching = [loc for loc in actual_locations if loc["uri_suffix"] == "index.php" and loc["line"] == 11]
+            assert matching, f"Expected reference to $greeting on line 11 of index.php, got {actual_locations}"
+        else:
+            # Intelephense, when asked for references from usage, seems to only return the usage itself.
+            assert len(references) == 1, "Expected to find 1 reference for $greeting (the usage itself)"
 
-        assert actual_locations == expected_locations
+            expected_locations = [{"uri_suffix": "index.php", "line": 11, "character": 5}]  # Usage: echo $greeting (points to $)
 
-    @pytest.mark.parametrize("language_server", [Language.PHP], indirect=True)
+            # Convert actual references to a comparable format and sort
+            actual_locations = sorted(
+                [
+                    {
+                        "uri_suffix": loc["uri"].split("/")[-1],
+                        "line": loc["range"]["start"]["line"],
+                        "character": loc["range"]["start"]["character"],
+                    }
+                    for loc in references
+                ],
+                key=lambda x: (x["uri_suffix"], x["line"], x["character"]),
+            )
+
+            expected_locations = sorted(expected_locations, key=lambda x: (x["uri_suffix"], x["line"], x["character"]))
+
+            assert actual_locations == expected_locations
+
+    @pytest.mark.parametrize("language_server", [Language.PHP, Language.PHP_PHPACTOR], indirect=True)
     @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
     def test_find_references_across_files(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
         helper_php_path = str(repo_path / "helper.php")
@@ -116,20 +148,34 @@ class TestPhpLanguageServer:
         references = language_server.request_references(helper_php_path, 2, len("function "))
 
         assert references, f"Expected non-empty references for helperFunction but got {references=}"
-        # Intelephense might return 1 (usage) or 2 (usage + definition) references.
-        # Let's check for at least the usage in index.php
-        # Definition is in helper.php, line 2, char 0 (based on previous findings)
-        # Usage is in index.php, line 13, char 0
 
-        actual_locations_comparable = []
+        if language_server.language_server.language == Language.PHP_PHPACTOR:
-        for loc in references:
+            actual_locations_comparable = []
-            actual_locations_comparable.append(
+            for loc in references:
-                {
+                actual_locations_comparable.append(
-                    "uri_suffix": loc["uri"].split("/")[-1],
+                    {
-                    "line": loc["range"]["start"]["line"],
+                        "uri_suffix": loc["uri"].split("/")[-1],
-                    "character": loc["range"]["start"]["character"],
+                        "line": loc["range"]["start"]["line"],
-                }
+                    }
-            )
+                )
 
-        usage_in_index_php = {"uri_suffix": "index.php", "line": 13, "character": 0}
+            # Check that helperFunction usage in index.php line 13 is found
-        assert usage_in_index_php in actual_locations_comparable, "Usage of helperFunction in index.php not found"
+            matching = [loc for loc in actual_locations_comparable if loc["uri_suffix"] == "index.php" and loc["line"] == 13]
+            assert matching, f"Usage of helperFunction in index.php (line 13) not found in {actual_locations_comparable}"
+        else:
+            # Intelephense might return 1 (usage) or 2 (usage + definition) references.
+            # Let's check for at least the usage in index.php
+            # Definition is in helper.php, line 2, char 0 (based on previous findings)
+            # Usage is in index.php, line 13, char 0
+            actual_locations_comparable = []
+            for loc in references:
+                actual_locations_comparable.append(
+                    {
+                        "uri_suffix": loc["uri"].split("/")[-1],
+                        "line": loc["range"]["start"]["line"],
+                        "character": loc["range"]["start"]["character"],
+                    }
+                )
+
+            usage_in_index_php = {"uri_suffix": "index.php", "line": 13, "character": 0}
+            assert usage_in_index_php in actual_locations_comparable, "Usage of helperFunction in index.php not found"

test/solidlsp/php/test_php_phpactor.py

@@ -1,111 +0,0 @@
-from pathlib import Path
-
-import pytest
-
-from solidlsp import SolidLanguageServer
-from solidlsp.ls_config import Language
-
-
-@pytest.mark.php
-class TestPhpPhpactorLanguageServer:
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_ls_is_running(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        """Test that the phpactor language server starts and stops successfully."""
-        assert language_server.is_running()
-        assert Path(language_server.language_server.repository_root_path).resolve() == repo_path.resolve()
-
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_find_definition_within_file(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        # In index.php (0-indexed lines):
-        # Line 9: $greeting = greet($userName);
-        # Line 11: echo $greeting;
-        # Find the definition of $greeting from its usage on line 11 (0-indexed), char 6
-        definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 11, 6)
-
-        assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) >= 1
-        definition_location = definition_location_list[0]
-        assert definition_location["uri"].endswith("index.php")
-        assert definition_location["range"]["start"]["line"] == 9
-
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_find_definition_across_files(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        # In index.php (0-indexed lines):
-        # Line 13: helperFunction();
-        # Find the definition of helperFunction which is in helper.php, line 2 (0-indexed)
-        definition_location_list = language_server.request_definition(str(repo_path / "index.php"), 13, 5)
-
-        assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) >= 1
-        definition_location = definition_location_list[0]
-        assert definition_location["uri"].endswith("helper.php")
-        assert definition_location["range"]["start"]["line"] == 2
-
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_find_definition_simple_variable(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        file_path = str(repo_path / "simple_var.php")
-
-        # In simple_var.php (0-indexed lines):
-        # Line 1: $localVar = "test";
-        # Line 2: echo $localVar;
-        # Find definition of $localVar from its usage on line 2, char 6
-        definition_location_list = language_server.request_definition(file_path, 2, 6)
-
-        assert definition_location_list, f"Expected non-empty definition_location_list but got {definition_location_list=}"
-        assert len(definition_location_list) >= 1
-        definition_location = definition_location_list[0]
-        assert definition_location["uri"].endswith("simple_var.php")
-        assert definition_location["range"]["start"]["line"] == 1
-
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_find_references_within_file(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        index_php_path = str(repo_path / "index.php")
-
-        # In index.php (0-indexed lines):
-        # Line 9: $greeting = greet($userName);  // Definition of $greeting
-        # Line 11: echo $greeting;               // Usage of $greeting
-        # Find references for $greeting from its usage on line 11, char 6
-        references = language_server.request_references(index_php_path, 11, 6)
-
-        assert references, f"Expected non-empty references for $greeting but got {references=}"
-
-        actual_locations = [
-            {
-                "uri_suffix": loc["uri"].split("/")[-1],
-                "line": loc["range"]["start"]["line"],
-            }
-            for loc in references
-        ]
-
-        # Check that at least one reference points to $greeting usage in index.php
-        matching = [loc for loc in actual_locations if loc["uri_suffix"] == "index.php" and loc["line"] == 11]
-        assert matching, f"Expected reference to $greeting on line 11 of index.php, got {actual_locations}"
-
-    @pytest.mark.parametrize("language_server", [Language.PHP_PHPACTOR], indirect=True)
-    @pytest.mark.parametrize("repo_path", [Language.PHP], indirect=True)
-    def test_find_references_across_files(self, language_server: SolidLanguageServer, repo_path: Path) -> None:
-        helper_php_path = str(repo_path / "helper.php")
-
-        # Find references for helperFunction from its definition in helper.php
-        # Line 2 (0-indexed): function helperFunction(): void {
-        references = language_server.request_references(helper_php_path, 2, len("function "))
-
-        assert references, f"Expected non-empty references for helperFunction but got {references=}"
-
-        actual_locations_comparable = []
-        for loc in references:
-            actual_locations_comparable.append(
-                {
-                    "uri_suffix": loc["uri"].split("/")[-1],
-                    "line": loc["range"]["start"]["line"],
-                }
-            )
-
-        # Check that helperFunction usage in index.php line 13 is found
-        matching = [loc for loc in actual_locations_comparable if loc["uri_suffix"] == "index.php" and loc["line"] == 13]
-        assert matching, f"Usage of helperFunction in index.php (line 13) not found in {actual_locations_comparable}"