Skip to content

Commit

Permalink
Auto-generate documentation for chplcheck lint rules (#26208)
Browse files Browse the repository at this point in the history
Adds documentation for `chplcheck` rules, auto-generated from the
existing `chplcheck` rule defintions.

This PR makes the following changes:
1. `make docs` will now build a `rules.rst` file which is included in
the `chplcheck` documentation which lists all lint rules
- This is enabled by the `doc/util/chplcheck-docs.py`, which is also
available to users for their own custom rule files
- Rules can also have examples, specified in `tools/chplcheck/examples`.
These are included in the online docs and provide reasoning to users
about why a lint rule exists
- ![Screenshot 2024-12-16 at 3 00 10
PM](https://github.com/user-attachments/assets/7896d06d-0e45-4ae3-845d-51ac7b022876)
- ![Screenshot 2024-12-16 at 2 57 24
PM](https://github.com/user-attachments/assets/254241fd-d58f-4645-9acb-ec5ed1290009)
2. The `chplcheck` langauge server will provides a link straight to rule
documentation
![Screenshot 2024-11-04 at 4 08 44
PM](https://github.com/user-attachments/assets/e96ee882-324c-4561-8731-24ab254b51cc)

Future work:
- Add more examples to `tools/chplcheck/examples`

[Reviewed by @DanilaFe]
  • Loading branch information
jabraham17 authored Jan 8, 2025
2 parents 8e43ed6 + 362b92f commit 9d1bb98
Show file tree
Hide file tree
Showing 26 changed files with 359 additions and 20 deletions.
1 change: 1 addition & 0 deletions doc/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ doctrees
/rst/examples
/rst/language/spec/syntax.rst
/rst/developer/compiler-internals/
/rst/tools/chplcheck/generated/

man.rst
/rst/tools/chplvis/examples
Expand Down
12 changes: 10 additions & 2 deletions doc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ man-chapel: FORCE
$(MAKE) man


source: collect-syntax module-docs primers examples symlinks compiler-internals
source: collect-syntax module-docs primers examples symlinks compiler-internals chplcheck-docs


collect-syntax:
Expand Down Expand Up @@ -135,6 +135,11 @@ compiler-internals: FORCE
ln -s ../meta/compiler-internals-no-doxygen "$(COMPILER_INTERNALS)" ; \
fi

chplcheck-docs: FORCE
@echo
@echo "Generating docs for chplcheck"
@(cd ../tools/chplcheck && $(MAKE) chplcheck-docs)

checkdocs: FORCE
$(MAKE) check

Expand All @@ -154,7 +159,7 @@ cleanall: clean-source clean-build clean-build-dir clean-doxygen clean-pycache F

clobber: clean-source clean-build clean-build-dir clean-doxygen clean-pycache FORCE

clean-source: clean-module-docs clean-primers clean-examples clean-symlinks clean-collect-syntax clean-compiler-internals FORCE
clean-source: clean-module-docs clean-primers clean-examples clean-symlinks clean-collect-syntax clean-compiler-internals clean-chplcheck-docs FORCE

clean-build-dir: FORCE
rm -rf ../build/doc
Expand Down Expand Up @@ -197,6 +202,9 @@ clean-pycache: FORCE
rm -rf util/__pycache__
rm -rf $(SOURCEDIR)/__pycache__

clean-chplcheck-docs: FORCE
cd ../tools/chplcheck && $(MAKE) clean-chplcheck-docs

FORCE:

# Disable parallel builds to prevent race conditions
Expand Down
21 changes: 21 additions & 0 deletions doc/rst/tools/chplcheck/chplcheck.rst
Original file line number Diff line number Diff line change
Expand Up @@ -486,3 +486,24 @@ The linter is run as follows:
path/to/myfile/myfile.chpl:1: node violates rule NoFunctionFoo
path/to/myfile/myfile.chpl:2: node violates rule NoVariableBar
Developers may also find it helpful to maintain documentation for their custom
rules. Adding a Python docstring to the rule function will include the
documentation in the ``--list-rules`` output. This docstring can also be used to
generate Sphinx documentation for the rule. This can be done by running the
``chplcheck-docs.py`` script. For example:

.. code-block:: bash
> $CHPL_HOME/doc/util/chplcheck-docs.py path/to/my/myrules.py -o my/out/directory
This will generate a ``rules.rst`` file in ``my/out/directory`` that contains
the documentation for the rules in ``myrules.py``. Note that this script is
currently only available in the Chapel source tree.

Current Rules
-------------

The following is a list of all the rules currently implemented in ``chplcheck``:

.. include:: generated/rules.rst
210 changes: 210 additions & 0 deletions doc/util/chplcheck-docs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
#!/usr/bin/env python3

"""
Auto-generates a *.rst for the chplcheck rules defined
`$CHPL_HOME/tools/chplcheck/src/rules.py`
"""

import os
import typing
from dataclasses import dataclass
import argparse as ap
import ast
import shutil
import chpl2rst


@dataclass
class Rule:
name: str
description: str
patterns: typing.List[str]
default: bool
settings: typing.List[str]
example_text: str = ""

def rst(self):
lines = []
lines.append(self.name)
lines.append("~" * len(self.name))
lines.append("")
lines.append(
"Is enabled by default? " + ("Yes" if self.default else "No")
)
lines.append("")

if self.description:
lines.append(self.description)
lines.append("")

if self.settings:
lines.append("Settings:")
for setting in self.settings:
lines.append(f" - ``{setting}``")
lines.append("")

if self.example_text:
lines.append(self.example_text)
lines.append("")

return "\n".join(lines)

def add_example(self, example_directory: str):
# find the example file
example_file = os.path.join(example_directory, self.name + ".chpl")
if not os.path.exists(example_file):
return

with open(example_file) as handle:
pieces = chpl2rst.to_pieces(handle, False)
rstoutput = chpl2rst.gen_rst(pieces, example_file)

self.example_text = rstoutput


def find_rules(file: str):

def get_rule(func) -> typing.Optional[Rule]:
"""
Given a function definition, return a Rule object if it is a rule, otherwise None
"""
if not isinstance(func, ast.FunctionDef):
return None

# find all decorators by walking decorator_list and filtering for Attribute
decorators = [
i
for d in func.decorator_list
for i in ast.walk(d)
if isinstance(i, ast.Attribute)
]

# keep only the decorators that are rules
decorators = [
d
for d in decorators
if d.attr in ["basic_rule", "advanced_rule", "location_rule"]
]
# if there are no decorators, this is not a rule
if not decorators:
return None

# get the name of the rule
name = func.name
# get the docstring of the rule
description = ast.get_docstring(func) or ""

# filter out decorators that are not calls
decorators_filtered = [
d
for d in decorators
if isinstance(d, ast.Call) and isinstance(d.func, ast.Attribute)
]

# get the patterns, if they exist
# the patterns are the first argument of the decorator
# we get the string representation of the pattern
patterns = [
ast.unparse(d.args[0])
for d in decorators_filtered
if len(d.args) > 0
]

# determine if the rule is enabled by default
# default = any(d.attr == 'default' for d in decorators)
default_settings = [
ast.unparse(next(k.value for k in d.keywords))
for d in decorators_filtered
if len(d.keywords) > 0
and any(k.arg == "default" for k in d.keywords)
]
is_default = not any(d == "False" for d in default_settings)

# grab the settings for the rule
settings = []
settings_node = [
next(k.value for k in d.keywords)
for d in func.decorator_list
if isinstance(d, ast.Call)
and isinstance(d.func, ast.Attribute)
and len(d.keywords) > 0
and any(k.arg == "settings" for k in d.keywords)
]
if settings_node:
# settings node should be a list with 1 element, an ast.List object
# get each the value of each element from the list
settings = [s.value for s in settings_node[0].elts]

return Rule(name, description, patterns, is_default, settings)

with open(file) as f:
tree = ast.parse(f.read())

# find the rules function using a matcher
rules_def_func = None
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef) and node.name == "rules":
rules_def_func = node
break

if rules_def_func is None:
return []

rules = [get_rule(r) for r in rules_def_func.body]
return list([r for r in rules if r is not None])


def rst_rules(rules):
return "\n".join([r.rst() for r in rules])


def output_rules(rules: typing.List[Rule], output_dir: str):
# remove the existing output directory
if os.path.exists(output_dir):
shutil.rmtree(output_dir)
os.makedirs(output_dir)

# output the rules file
with open(os.path.join(output_dir, "rules.rst"), "w") as f:
f.write(rst_rules(rules))


def main():
a = ap.ArgumentParser()
a.add_argument(
"-r",
"--rules",
default=[],
action="append",
help="Rules to generate documentation for",
)
a.add_argument(
"-o",
"--output",
default="chplcheck-rules-out",
help="Directory where all the relevant docs files will be written",
)
a.add_argument(
"--examples-directory",
default=None,
help="Directory where all the relevant examples are located",
)
args = a.parse_args()

rules: typing.List[Rule] = []

# collect the rules
for rule_file in args.rules:
rules.extend(find_rules(rule_file))

# collect the examples
if args.examples_directory:
for rule in rules:
rule.add_example(args.examples_directory)

# output the rules
output_rules(rules, args.output)


if __name__ == "__main__":
main()
1 change: 1 addition & 0 deletions test/chplcheck/CHPLCHECKOPTS
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--enable-rule ConsecutiveDecls --enable-rule BoolLitInCondStatement --enable-rule UseExplicitModules --enable-rule CamelOrPascalCaseVariables --enable-rule NestedCoforalls --internal-prefix myprefix_ --internal-prefix _ --skip-unstable
14 changes: 6 additions & 8 deletions test/chplcheck/PREDIFF
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
#!/usr/bin/env bash

FLAGS="--enable-rule ConsecutiveDecls"\
" --enable-rule BoolLitInCondStatement"\
" --enable-rule UseExplicitModules"\
" --enable-rule CamelOrPascalCaseVariables"\
" --enable-rule NestedCoforalls"\
" --internal-prefix myprefix_ --internal-prefix _"\
" --skip-unstable"
FLAGS=""

# read extra arguments from a $1.chplcheckopts file
# currently only supports 1 line with nothing but flags
if [ -e CHPLCHECKOPTS ]; then
FLAGS="$FLAGS $(cat CHPLCHECKOPTS)"
fi
if [ -e $1.chplcheckopts ]; then
FLAGS="$FLAGS $(cat $1.chplcheckopts)"
fi


$CHPL_HOME/tools/chplcheck/chplcheck $FLAGS $1.chpl >>$2 2>&1

if sed "s#$(pwd)/##" $2 >$2.tmp; then
dirpath=$(dirname $(realpath $1.chpl))
if sed "s#$dirpath/##" $2 >$2.tmp; then
mv $2.tmp $2
fi

Expand Down
1 change: 1 addition & 0 deletions test/chplcheck/examples
1 change: 1 addition & 0 deletions tools/chapel-py/src/core-types-gen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "chpl/parsing/parsing-queries.h"
#include "chpl/resolution/resolution-queries.h"
#include "chpl/resolution/scope-queries.h"
#include "chpl/util/version-info.h"

using namespace chpl;
using namespace uast;
Expand Down
2 changes: 2 additions & 0 deletions tools/chapel-py/src/method-tables/core-methods.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ CLASS_BEGIN(Context)
node->advanceToNextRevision(prepareToGc))
METHOD(Context, get_file_text, "Get the text of the file at the given path",
std::string(chpl::UniqueString), return parsing::fileText(node, std::get<0>(args)).text())
METHOD(Context, get_compiler_version, "Get the version of the Chapel compiler",
std::string(), std::ignore = node; return chpl::getVersion())
CLASS_END(Context)

CLASS_BEGIN(Location)
Expand Down
2 changes: 1 addition & 1 deletion tools/chpl-language-server/src/chpl-language-server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ def _setup_linter(self, clsConfig: CLSConfig):
config = chplcheck.config.Config.from_args(clsConfig.args)
self.lint_driver = chplcheck.driver.LintDriver(config)

chplcheck.rules.register_rules(self.lint_driver)
chplcheck.rules.rules(self.lint_driver)

for p in config.add_rules:
chplcheck.main.load_module(self.lint_driver, os.path.abspath(p))
Expand Down
14 changes: 13 additions & 1 deletion tools/chplcheck/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
ifndef CHPL_MAKE_HOME
export CHPL_MAKE_HOME=$(realpath $(shell pwd)/../..)
endif
ifndef CHPL_MAKE_PYTHON
export CHPL_MAKE_PYTHON := $(shell $(CHPL_MAKE_HOME)/util/config/find-python.sh)
endif

include $(CHPL_MAKE_HOME)/make/Makefile.base
include $(CHPL_MAKE_HOME)/third-party/chpl-venv/Makefile.include
Expand All @@ -37,6 +40,15 @@ chplcheck-venv:

chplcheck: chplcheck-venv

chplcheck-docs:
$(CHPL_MAKE_PYTHON) $(CHPL_MAKE_HOME)/doc/util/chplcheck-docs.py \
-r $(CHPL_MAKE_HOME)/tools/chplcheck/src/rules.py \
-o $(CHPL_MAKE_HOME)/doc/rst/tools/chplcheck/generated \
--examples-directory $(CHPL_MAKE_HOME)/tools/chplcheck/examples

clean-chplcheck-docs:
rm -rf $(CHPL_MAKE_HOME)/doc/rst/tools/chplcheck/generated

clean: clean-pycache
ifneq ($(wildcard $(link)),)
@echo "Removing old symbolic link..."
Expand All @@ -46,7 +58,7 @@ endif

cleanall: clean

clobber: clean
clobber: clean clean-chplcheck-docs

clean-pycache:
find $(CHPL_MAKE_HOME)/tools/chplcheck -type d -name __pycache__ -exec rm -rf {} +
Expand Down
1 change: 1 addition & 0 deletions tools/chplcheck/examples/CHPLCHECKOPTS
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--disable-rule UseExplicitModules
1 change: 1 addition & 0 deletions tools/chplcheck/examples/CLEANFILES
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
*.chpl.fixed
1 change: 1 addition & 0 deletions tools/chplcheck/examples/COMPOPTS
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--stop-after-pass=parseAndConvertUast
10 changes: 10 additions & 0 deletions tools/chplcheck/examples/ControlFlowParentheses.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
Conditional statements in Chapel do not require parentheses around the
condition. The following demonstrate this, the two if statements are
equivalent.
*/
config const value = 5;
if (value > 0) then
writeln("Value is positive");
if value > 0 then
writeln("Value is positive");
1 change: 1 addition & 0 deletions tools/chplcheck/examples/ControlFlowParentheses.good
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ControlFlowParentheses.chpl:7: node violates rule ControlFlowParentheses
Loading

0 comments on commit 9d1bb98

Please sign in to comment.