Make chplcheck available from chpl-language-server

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

@@ -62,6 +62,8 @@ VSCode
 Install the ``chapel`` extension from the `Visual Studio Code marketplace
 <https://marketplace.visualstudio.com/items?itemName=chpl-hpe.chapel-vscode>`_.
 
+.. _chpl-language-server-emacs:
+
 Emacs
 ^^^^^
 
@@ -77,7 +79,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 +93,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 +190,25 @@ The following features are extra visual aids:
 | Instantiations | instantiations of a generic function.      |                                       |
 +----------------+--------------------------------------------+---------------------------------------+
 
+Using ``chplcheck`` from ``CLS``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Although :ref:`chplcheck <readme-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

@@ -171,16 +171,14 @@ 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-chpl-language-server` such that both can be used in Emacs at the
-   same time, stay tuned!
-
-
+   be registered per language. To use ``chplcheck`` and ``chapel-language-server``
+   at the same time, see the
+   :ref:`Emacs documentation for chapel-language-server <chpl-language-server-emacs>`.
 
 Writing New Rules
 -----------------
 
-Rules are written using the :ref:`Python bindings for Chapel's compiler frontend<readme-chapel-py>`. In
+Rules are written using the :ref:`Python bindings for Chapel's compiler frontend <readme-chapel-py>`. In
 essence, a rule is a Python function that is used to detect issues with the
 AST. When registered with ``chplcheck``, the name of the function becomes the name
 of the rule (which can be used to enable and disable the rule, as per the

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

@@ -32,6 +32,7 @@
     TypeVar,
     Union,
 )
+from types import ModuleType
 from collections import defaultdict
 from dataclasses import dataclass, field
 from bisect_compat import bisect_left, bisect_right
@@ -40,7 +41,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 +144,64 @@
 import argparse
 import configargparse
 
+
+class ChplcheckProxy:
+
+    def __init__(
+        self,
+        main: ModuleType,
+        config: ModuleType,
+        lsp: ModuleType,
+        driver: ModuleType,
+        rules: ModuleType,
+    ):
+        self.main = main
+        self.config = config
+        self.lsp = lsp
+        self.driver = driver
+        self.rules = rules
+
+    @classmethod
+    def get(cls) -> Optional["ChplcheckProxy"]:
+
+        def error(msg: str):
+            if os.environ.get("CHPL_DEVELOPER", None):
+                print("Error loading chplcheck: ", str(e), file=sys.stderr)
+
+        chpl_home = os.environ.get("CHPL_HOME")
+        if chpl_home is None:
+            error("CHPL_HOME not set")
+            return None
+
+        def load_module(module_name: str) -> Optional[ModuleType]:
+            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:
+                error(f"Could not load module from {file_path}")
+                return None
+            module = importlib.util.module_from_spec(spec)
+            sys.modules[module_name] = module
+            if spec.loader is None:
+                error(f"Could not load module from {file_path}")
+                return None
+            spec.loader.exec_module(module)
+            return module
+
+        mods = []
+        for mod in ["main", "config", "lsp", "driver", "rules"]:
+            m = load_module(mod)
+            if m is None:
+                return None
+            mods.append(m)
+        proxy = ChplcheckProxy(*mods)
+
+        return proxy
+
+
+chplcheck = ChplcheckProxy.get()
+
 REAL_NUMBERIC = (chapel.RealLiteral, chapel.IntLiteral, chapel.UintLiteral)
 NUMERIC = REAL_NUMBERIC + (chapel.ImagLiteral,)
 
@@ -952,6 +1012,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:
+            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 +1069,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 +1088,24 @@ 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):
+            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.main.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 +1203,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 +1216,13 @@ def build_diagnostics(self, uri: str) -> List[Diagnostic]:
             ]
             diagnostics.append(diag)
 
+        # get lint diagnostics if applicable
+        if self.lint_driver and chplcheck:
+            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 +1778,13 @@ async def code_action(ls: ChapelLanguageServer, params: CodeActionParams):
                 )
             )
 
+        # get lint fixits if applicable
+        if ls.lint_driver and chplcheck:
+            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:")