Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make chplcheck available from chpl-language-server #25214

Merged
merged 11 commits into from
Jun 11, 2024
30 changes: 24 additions & 6 deletions doc/rst/tools/chpl-language-server/chpl-language-server.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^

Expand All @@ -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"))))
jabraham17 marked this conversation as resolved.
Show resolved Hide resolved

This will enable using the language server with a particular ``.chpl`` file by
calling ``M-x eglot``.
Expand All @@ -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
------------------
Expand Down Expand Up @@ -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
---------------------------

Expand Down
10 changes: 4 additions & 6 deletions doc/rst/tools/chplcheck/chplcheck.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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!
jabraham17 marked this conversation as resolved.
Show resolved Hide resolved


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
Expand Down
104 changes: 102 additions & 2 deletions tools/chpl-language-server/src/chpl-language-server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,)

Expand Down Expand Up @@ -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(",")
Expand Down Expand Up @@ -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()

Expand All @@ -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]]:
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
75 changes: 75 additions & 0 deletions tools/chpl-language-server/test/linting.py
Original file line number Diff line number Diff line change
@@ -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
34 changes: 8 additions & 26 deletions tools/chplcheck/src/chplcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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:")
Expand Down
Loading
Loading