Make chplcheck available from chpl-language-server

doc/rst/tools/chpl-language-server/chpl-language-server.rst

@@ -77,7 +77,7 @@ syntax highlighting in Emacs):
 
   (with-eval-after-load 'eglot
     (add-to-list 'eglot-server-programs
-                 '(chpl-mode . ("chpl-language-server"))))
+                 '(chpl-mode . ("chpl-language-server", "--chplcheck"))))
 
 This will enable using the language server with a particular ``.chpl`` file by
 calling ``M-x eglot``.
@@ -91,11 +91,8 @@ additionally add the following to your ``.emacs`` file:
 
 .. note::
 
-   There is currently a limitation with Eglot that only one language server can
-   be registered per language.  We are investigating merging the support for
-   :ref:`readme-chplcheck` such that both can be used in Emacs at the same time,
-   stay tuned!
-
+   The above uses the ``--chplcheck`` flag to enable additional diagnostics from
+   ``chplcheck``. If you do not want to use ``chplcheck``, you can remove this.
 
 Supported Features
 ------------------
@@ -191,6 +188,25 @@ The following features are extra visual aids:
 | Instantiations | instantiations of a generic function.      |                                       |
 +----------------+--------------------------------------------+---------------------------------------+
 
+Using ``chplcheck`` from ``CLS``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Although ``chplcheck`` is a separate tool from ``CLS``, it can be used from
+``CLS`` to provide additional diagnostics. This is done by enabling the
+``--chplcheck`` flag. This will incorporate the diagnostics and fixits from
+``chplcheck``.
+
+You can also still pass many of the same ``chplcheck`` flags to ``CLS``, just
+prefixed with ``--chplcheck-``. For example, the following command runs the
+language server with linting enabled various ``chplcheck`` flags:
+
+.. code-block:: bash
+
+   chpl-language-server --chplcheck \
+     --chplcheck-enable-rule UseExplicitModules \
+     --chplcheck-disable-rule UnusedLoopIndex \
+     --chplcheck-add-rules path/to/my/myrules.py
+
 Configuring Chapel Projects
 ---------------------------
 

doc/rst/tools/chplcheck/chplcheck.rst

@@ -168,13 +168,6 @@ additionally add the following to your ``.emacs`` file:
 
    (add-hook 'chpl-mode-hook 'eglot-ensure)
 
-.. note::
-
-   There is currently a limitation with Eglot that only one language server can
-   be registered per language.  We are investigating merging the support for
-   :ref:`readme-chpl-language-server` such that both can be used in Emacs at the
-   same time, stay tuned!
-
 
 
 Writing New Rules

tools/chpl-language-server/src/chpl-language-server.py

@@ -40,7 +40,8 @@
 import os
 import json
 import re
-
+import sys
+import importlib.util
 
 import chapel
 from chapel.lsp import location_to_range, error_to_diagnostic
@@ -142,6 +143,43 @@
 import argparse
 import configargparse
 
+
+# attempt to import chplcheck
+try:
+    chpl_home = os.environ.get("CHPL_HOME")
+    if chpl_home is None:
+        raise ValueError("CHPL_HOME not set")
+    chplcheck_path = os.path.join(chpl_home, "tools", "chplcheck", "src")
+    # Add chplcheck to the path, but load via importlib
+    sys.path.append(chplcheck_path)
+
+    def load_module(module_name: str):
+        file_path = os.path.join(chplcheck_path, module_name + ".py")
+        spec = importlib.util.spec_from_file_location(module_name, file_path)
+        if spec is None:
+            raise ValueError(f"Could not load module from {file_path}")
+        module = importlib.util.module_from_spec(spec)
+        sys.modules[module_name] = module
+        if spec.loader is None:
+            raise ValueError(f"Could not load module from {file_path}")
+        spec.loader.exec_module(module)
+        return module
+
+    chplcheck = load_module("chplcheck")
+    chplcheck_lsp = load_module("lsp")
+    chplcheck_config = load_module("config")
+    chplcheck_driver = load_module("driver")
+    chplcheck_rules = load_module("rules")
+
+except ValueError as e:
+    if os.environ.get("CHPL_DEVELOPER", None):
+        print("Error loading chplcheck: ", str(e), file=sys.stderr)
+    chplcheck = None
+    chplcheck_lsp = None
+    chplcheck_config = None
+    chplcheck_driver = None
+    chplcheck_rules = None
+
 REAL_NUMBERIC = (chapel.RealLiteral, chapel.IntLiteral, chapel.UintLiteral)
 NUMERIC = REAL_NUMBERIC + (chapel.ImagLiteral,)
 
@@ -952,6 +990,10 @@ def add_bool_flag(name: str, dest: str, default: bool):
         self.parser.add_argument("--end-markers", default="none")
         self.parser.add_argument("--end-marker-threshold", type=int, default=10)
 
+        add_bool_flag("chplcheck", "do_linting", False)
+        if chplcheck_config:
+            chplcheck_config.Config.add_arguments(self.parser, "chplcheck-")
+
     def _parse_end_markers(self):
         self.args["end_markers"] = [
             a.strip() for a in self.args["end_markers"].split(",")
@@ -1005,6 +1047,10 @@ def __init__(self, config: CLSConfig):
         self.end_markers: List[str] = config.get("end_markers")
         self.end_marker_threshold: int = config.get("end_marker_threshold")
         self.end_marker_patterns = self._get_end_marker_patterns()
+        self.do_linting: bool = config.get("do_linting")
+
+        self.lint_driver = None
+        self._setup_linter(config)
 
         self._setup_regexes()
 
@@ -1020,6 +1066,31 @@ def _setup_regexes(self):
         # use pat2 first since it is more specific
         self._find_rename_deprecation_regex = re.compile(f"({pat2})|({pat1})")
 
+    def _setup_linter(self, clsConfig: CLSConfig):
+        """
+        Setup the linter, if it is enabled
+        """
+        if not (
+            self.do_linting
+            and chplcheck
+            and chplcheck_lsp
+            and chplcheck_config
+            and chplcheck_driver
+            and chplcheck_rules
+        ):
+            return
+
+        config = chplcheck_config.Config.from_args(clsConfig.args)
+        self.lint_driver = chplcheck_driver.LintDriver(config)
+
+        chplcheck_rules.register_rules(self.lint_driver)
+
+        for p in config.add_rules:
+            chplcheck.load_module(self.lint_driver, os.path.abspath(p))
+
+        self.lint_driver.disable_rules(*config.disabled_rules)
+        self.lint_driver.enable_rules(*config.enabled_rules)
+
     def get_deprecation_replacement(
         self, text: str
     ) -> Tuple[Optional[str], Optional[str]]:
@@ -1117,7 +1188,7 @@ def build_diagnostics(self, uri: str) -> List[Diagnostic]:
         as a list of LSP Diagnostics.
         """
 
-        _, errors = self.get_file_info(uri, do_update=True)
+        fi, errors = self.get_file_info(uri, do_update=True)
 
         diagnostics = []
         for e in errors:
@@ -1130,6 +1201,14 @@ def build_diagnostics(self, uri: str) -> List[Diagnostic]:
             ]
             diagnostics.append(diag)
 
+        # get lint diagnostics if applicable
+        if self.lint_driver:
+            assert chplcheck_lsp
+            lint_diagnostics = chplcheck_lsp.get_lint_diagnostics(
+                fi.context.context, self.lint_driver, fi.get_asts()
+            )
+            diagnostics.extend(lint_diagnostics)
+
         return diagnostics
 
     def get_text(self, text_doc: TextDocument, rng: Range) -> str:
@@ -1685,6 +1764,14 @@ async def code_action(ls: ChapelLanguageServer, params: CodeActionParams):
                 )
             )
 
+        # get lint fixits if applicable
+        if ls.lint_driver:
+            assert chplcheck_lsp
+            lint_actions = chplcheck_lsp.get_lint_actions(
+                params.context.diagnostics
+            )
+            actions.extend(lint_actions)
+
         return actions
 
     @server.feature(TEXT_DOCUMENT_RENAME)

tools/chpl-language-server/test/linting.py

@@ -0,0 +1,75 @@
+"""
+Test that the linter is working properly from CLS. These tests are not meant to be exhaustive of the linter's capabilities, rather they are meant to ensure that the integration between CLS and chplcheck is working properly.
+"""
+
+import sys
+
+from lsprotocol.types import ClientCapabilities
+from lsprotocol.types import InitializeParams
+from lsprotocol.types import CodeActionParams, CodeActionContext
+import pytest
+import pytest_lsp
+from pytest_lsp import ClientServerConfig, LanguageClient
+
+from util.utils import *
+from util.config import CLS_PATH
+
+
+@pytest_lsp.fixture(
+    config=ClientServerConfig(
+        server_command=[sys.executable, CLS_PATH(), "--chplcheck"],
+        client_factory=get_base_client,
+    )
+)
+async def client(lsp_client: LanguageClient):
+    # Setup
+    params = InitializeParams(capabilities=ClientCapabilities())
+    await lsp_client.initialize_session(params)
+
+    yield
+
+    # Teardown
+    await lsp_client.shutdown_session()
+
+
+@pytest.mark.asyncio
+async def test_lint_warnings(client: LanguageClient):
+    """
+    Test that lint warnings are reported properly
+    """
+    file = """
+            for i in {1..10} do { }
+           """
+    async with source_file(client, file, 3) as doc:
+        pass
+
+
+@pytest.mark.asyncio
+async def test_lint_fixits(client: LanguageClient):
+    """
+    Test that fixits work properly
+    """
+    file = """
+            for i in {1..10} { i; }
+           """
+    async with source_file(client, file, None) as doc:
+        await save_file(client, doc)
+        assert len(client.diagnostics[doc.uri]) == 1
+
+        diagnostics = client.diagnostics[doc.uri]
+        actions = await client.text_document_code_action_async(
+            CodeActionParams(
+                doc, rng((0, 0), endpos(file)), CodeActionContext(diagnostics)
+            )
+        )
+        assert actions is not None
+        # there should 2 actions, one to fixit and one to ignore. make sure that both exists
+        # TODO: check that the fixits are valid and what we expect for a given input string
+        # TODO: this should also be made into a helper function
+        assert len(actions) == 2
+        assert (
+            "Apply Fix for" in actions[0].title
+            and "Ignore" not in actions[0].title
+        )
+        assert "Ignore" in actions[1].title
+        # TODO: check that applying the fixits actually resolves the warning

tools/chplcheck/src/chplcheck.py

@@ -32,6 +32,7 @@
 from lsp import run_lsp
 from rules import register_rules
 from fixits import Fixit, Edit
+from config import Config
 
 
 def print_violation(node: chapel.AstNode, name: str):
@@ -201,26 +202,8 @@ def main():
         prog="chplcheck", description="A linter for the Chapel language"
     )
     parser.add_argument("filenames", nargs="*")
-    parser.add_argument(
-        "--disable-rule", action="append", dest="disabled_rules", default=[]
-    )
-    parser.add_argument(
-        "--enable-rule", action="append", dest="enabled_rules", default=[]
-    )
+    Config.add_arguments(parser)
     parser.add_argument("--lsp", action="store_true", default=False)
-    parser.add_argument("--skip-unstable", action="store_true", default=False)
-    parser.add_argument(
-        "--internal-prefix",
-        action="append",
-        dest="internal_prefixes",
-        default=[],
-    )
-    parser.add_argument(
-        "--add-rules",
-        action="append",
-        default=[],
-        help="Add a custom rule file",
-    )
     parser.add_argument(
         "--list-rules",
         action="store_true",
@@ -253,22 +236,21 @@ def main():
     )
     args = parser.parse_args()
 
-    driver = LintDriver(
-        skip_unstable=args.skip_unstable,
-        internal_prefixes=args.internal_prefixes,
-    )
+    config = Config.from_args(args)
+    driver = LintDriver(config)
+
     # register rules before enabling/disabling
     register_rules(driver)
-    for p in args.add_rules:
+    for p in config.add_rules:
         load_module(driver, os.path.abspath(p))
 
     if args.list_rules:
         print("Available rules (default rules marked with *):")
         print_rules(driver)
         return
 
-    driver.disable_rules(*args.disabled_rules)
-    driver.enable_rules(*args.enabled_rules)
+    driver.disable_rules(*config.disabled_rules)
+    driver.enable_rules(*config.enabled_rules)
 
     if args.list_active_rules:
         print("Active rules:")