diff --git a/doc/.gitignore b/doc/.gitignore index d8e589325247..4688ab02bcdf 100644 --- a/doc/.gitignore +++ b/doc/.gitignore @@ -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 diff --git a/doc/Makefile b/doc/Makefile index 0901996a0bc1..3a87e5ca62ac 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -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: @@ -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 @@ -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 @@ -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 diff --git a/doc/rst/tools/chplcheck/chplcheck.rst b/doc/rst/tools/chplcheck/chplcheck.rst index 5f04f4909db6..5ee208d7408d 100644 --- a/doc/rst/tools/chplcheck/chplcheck.rst +++ b/doc/rst/tools/chplcheck/chplcheck.rst @@ -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 diff --git a/doc/util/chplcheck-docs.py b/doc/util/chplcheck-docs.py new file mode 100755 index 000000000000..eaf54dc008be --- /dev/null +++ b/doc/util/chplcheck-docs.py @@ -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() diff --git a/test/chplcheck/CHPLCHECKOPTS b/test/chplcheck/CHPLCHECKOPTS new file mode 100644 index 000000000000..10cd4042ffa9 --- /dev/null +++ b/test/chplcheck/CHPLCHECKOPTS @@ -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 diff --git a/test/chplcheck/PREDIFF b/test/chplcheck/PREDIFF index 6e973fa4056f..526cadd5200c 100755 --- a/test/chplcheck/PREDIFF +++ b/test/chplcheck/PREDIFF @@ -1,15 +1,12 @@ #!/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 @@ -17,7 +14,8 @@ 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 diff --git a/test/chplcheck/examples b/test/chplcheck/examples new file mode 120000 index 000000000000..9e09f9aac1e2 --- /dev/null +++ b/test/chplcheck/examples @@ -0,0 +1 @@ +../../tools/chplcheck/examples/ \ No newline at end of file diff --git a/tools/chapel-py/src/core-types-gen.cpp b/tools/chapel-py/src/core-types-gen.cpp index c478f4544c92..69cc2df721ca 100644 --- a/tools/chapel-py/src/core-types-gen.cpp +++ b/tools/chapel-py/src/core-types-gen.cpp @@ -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; diff --git a/tools/chapel-py/src/method-tables/core-methods.h b/tools/chapel-py/src/method-tables/core-methods.h index c38cd8c17e64..da8430429337 100644 --- a/tools/chapel-py/src/method-tables/core-methods.h +++ b/tools/chapel-py/src/method-tables/core-methods.h @@ -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) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index 5c03b99ce721..43052ca2dab6 100755 --- a/tools/chpl-language-server/src/chpl-language-server.py +++ b/tools/chpl-language-server/src/chpl-language-server.py @@ -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)) diff --git a/tools/chplcheck/Makefile b/tools/chplcheck/Makefile index 007aa2f50887..62c188bdcd89 100644 --- a/tools/chplcheck/Makefile +++ b/tools/chplcheck/Makefile @@ -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 @@ -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..." @@ -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 {} + diff --git a/tools/chplcheck/examples/CHPLCHECKOPTS b/tools/chplcheck/examples/CHPLCHECKOPTS new file mode 100644 index 000000000000..c15d77a1014b --- /dev/null +++ b/tools/chplcheck/examples/CHPLCHECKOPTS @@ -0,0 +1 @@ +--disable-rule UseExplicitModules diff --git a/tools/chplcheck/examples/CLEANFILES b/tools/chplcheck/examples/CLEANFILES new file mode 100644 index 000000000000..870f11438be9 --- /dev/null +++ b/tools/chplcheck/examples/CLEANFILES @@ -0,0 +1 @@ +*.chpl.fixed diff --git a/tools/chplcheck/examples/COMPOPTS b/tools/chplcheck/examples/COMPOPTS new file mode 100644 index 000000000000..16b1d028e72c --- /dev/null +++ b/tools/chplcheck/examples/COMPOPTS @@ -0,0 +1 @@ + --stop-after-pass=parseAndConvertUast diff --git a/tools/chplcheck/examples/ControlFlowParentheses.chpl b/tools/chplcheck/examples/ControlFlowParentheses.chpl new file mode 100644 index 000000000000..f6eccf923e14 --- /dev/null +++ b/tools/chplcheck/examples/ControlFlowParentheses.chpl @@ -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"); diff --git a/tools/chplcheck/examples/ControlFlowParentheses.good b/tools/chplcheck/examples/ControlFlowParentheses.good new file mode 100644 index 000000000000..737abbbfd49e --- /dev/null +++ b/tools/chplcheck/examples/ControlFlowParentheses.good @@ -0,0 +1 @@ +ControlFlowParentheses.chpl:7: node violates rule ControlFlowParentheses diff --git a/tools/chplcheck/examples/DoKeywordAndBlock.chpl b/tools/chplcheck/examples/DoKeywordAndBlock.chpl new file mode 100644 index 000000000000..e17c14dd2280 --- /dev/null +++ b/tools/chplcheck/examples/DoKeywordAndBlock.chpl @@ -0,0 +1,7 @@ +/* + Using both the 'do' keyword and curly braces is redundant. +*/ +for i in 1..10 do { + writeln(i); +} + diff --git a/tools/chplcheck/examples/DoKeywordAndBlock.good b/tools/chplcheck/examples/DoKeywordAndBlock.good new file mode 100644 index 000000000000..41d2502420a3 --- /dev/null +++ b/tools/chplcheck/examples/DoKeywordAndBlock.good @@ -0,0 +1 @@ +DoKeywordAndBlock.chpl:4: node violates rule DoKeywordAndBlock diff --git a/tools/chplcheck/examples/NOEXEC b/tools/chplcheck/examples/NOEXEC new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tools/chplcheck/examples/PREDIFF b/tools/chplcheck/examples/PREDIFF new file mode 100755 index 000000000000..526cadd5200c --- /dev/null +++ b/tools/chplcheck/examples/PREDIFF @@ -0,0 +1,34 @@ +#!/usr/bin/env bash + +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 + +dirpath=$(dirname $(realpath $1.chpl)) +if sed "s#$dirpath/##" $2 >$2.tmp; then + mv $2.tmp $2 +fi + +# if there is a good-fixit file, try and run chplcheck with fixit +if [ -e $1.good-fixit ]; then + + $CHPL_HOME/tools/chplcheck/chplcheck \ + $FLAGS --fixit --fixit-suffix .fixed $1.chpl >/dev/null + + if diff $1.good-fixit $1.chpl.fixed; then + echo "[Success matching fixit for $1]" >> $2 + rm $1.chpl.fixed + else + echo "[Error matching fixit for $1]" >> $2 + fi +fi diff --git a/tools/chplcheck/examples/SKIPIF b/tools/chplcheck/examples/SKIPIF new file mode 100755 index 000000000000..25d28cbcdcea --- /dev/null +++ b/tools/chplcheck/examples/SKIPIF @@ -0,0 +1,8 @@ +#!/bin/bash + +if $CHPL_HOME/util/config/run-in-venv-with-python-bindings.bash python3 -c "import chapel" 2> /dev/null; then + echo "False" +else + echo "True" +fi + diff --git a/tools/chplcheck/src/chplcheck.py b/tools/chplcheck/src/chplcheck.py index 82c680077129..4f19f837e24e 100755 --- a/tools/chplcheck/src/chplcheck.py +++ b/tools/chplcheck/src/chplcheck.py @@ -30,7 +30,7 @@ import chapel.replace from driver import LintDriver from lsp import run_lsp -from rules import register_rules +from rules import rules from fixits import Fixit, Edit from rule_types import CheckResult, RuleLocation from config import Config @@ -239,7 +239,7 @@ def main(): driver = LintDriver(config) # register rules before enabling/disabling - register_rules(driver) + rules(driver) for p in config.add_rules: load_module(driver, os.path.abspath(p)) diff --git a/tools/chplcheck/src/driver.py b/tools/chplcheck/src/driver.py index a67f8444f87c..4934c04a5f8e 100644 --- a/tools/chplcheck/src/driver.py +++ b/tools/chplcheck/src/driver.py @@ -75,7 +75,12 @@ def rules_and_descriptions(self): for rule in itertools.chain( self.BasicRules, self.AdvancedRules, self.LocationRules ): - to_return[rule.name] = rule.check_func.__doc__ + doc = rule.check_func.__doc__ or "" + + # if there is an escaped underscore, remove the escape + doc = doc.replace("\\_", "_") + + to_return[rule.name] = doc to_return = list(to_return.items()) to_return.sort() diff --git a/tools/chplcheck/src/lsp.py b/tools/chplcheck/src/lsp.py index 6f8186a5c59e..204112073bdb 100644 --- a/tools/chplcheck/src/lsp.py +++ b/tools/chplcheck/src/lsp.py @@ -31,7 +31,13 @@ WorkspaceEdit, TextEdit, ) -from lsprotocol.types import Diagnostic, Range, Position, DiagnosticSeverity +from lsprotocol.types import ( + Diagnostic, + Range, + Position, + DiagnosticSeverity, + CodeDescription, +) from fixits import Fixit, Edit from driver import LintDriver @@ -45,12 +51,22 @@ def get_lint_diagnostics( diagnostics = [] # Silence errors from scope resolution etc., especially since they # may be emitted from other files (dependencies). + + # get the version, keep only the major and minor version + version = ".".join(context.get_compiler_version().split(".")[:2]) + base_url = ( + "https://chapel-lang.org/docs/{}/tools/chplcheck/chplcheck.html".format( + version + ) + ) with context.track_errors() as _: for loc, node, rule, fixits in driver.run_checks(context, asts): diagnostic = Diagnostic( range=chapel.lsp.location_to_range(loc), message="Lint: rule [{}] violated".format(rule), severity=DiagnosticSeverity.Warning, + code=rule, + code_description=CodeDescription(base_url + "#" + rule.lower()), ) if fixits: fixits = [Fixit.to_dict(f) for f in fixits] diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index a4d5f17dc827..5830d1b1bf8f 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -153,7 +153,7 @@ def check_pascal_case( ) -def register_rules(driver: LintDriver): +def rules(driver: LintDriver): @driver.basic_rule(VarLikeDecl, default=False) def CamelOrPascalCaseVariables(context: Context, node: VarLikeDecl): """ @@ -391,7 +391,7 @@ def FixBoolLitInCondStmt_KeepBraces( @driver.basic_rule(NamedDecl) def ChplPrefixReserved(context: Context, node: NamedDecl): """ - Warn for user-defined names that start with the 'chpl_' reserved prefix. + Warn for user-defined names that start with the 'chpl\\_' reserved prefix. """ if node.name().startswith("chpl_"): @@ -1054,7 +1054,6 @@ def FixMissingInIntent(context: Context, result: AdvancedRuleResult): def LineLength(_: chapel.Context, path: str, lines: List[str], Max=None): """ Warn for lines that exceed a maximum length. - By default, the maximum line length is 80 characters. """ diff --git a/util/test/checkCopyrights.bash b/util/test/checkCopyrights.bash index dc1768861458..137c4ef6567a 100755 --- a/util/test/checkCopyrights.bash +++ b/util/test/checkCopyrights.bash @@ -94,7 +94,7 @@ root_files_wo_copy=$(find . -maxdepth 1 -name Make\* -o -name CMakeLists.txt | x # tools/chplvis: cxx fl h H # tools/mason: (excluding files named test*): chpl -tools_wo_copy=$(find tools \( -type d \( -name test -o -name utils \) -prune \) -o \( -type f \( \ +tools_wo_copy=$(find tools \( -type d \( -name test -o -name utils -o -name examples \) -prune \) -o \( -type f \( \ -name Make\* -o \ -name CMakeLists.txt -o \ -name \*.c -o \