From ebae6d35783b8c51dff8ab90da271938fdf3e642 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:02:59 -0800 Subject: [PATCH 01/19] add initial support to document chplcheck rules Signed-off-by: Jade Abraham --- doc/.gitignore | 1 + doc/Makefile | 12 +- doc/rst/tools/chplcheck/chplcheck.rst | 7 ++ doc/util/chplcheck-docs.py | 166 ++++++++++++++++++++++++++ tools/chplcheck/Makefile | 13 +- 5 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 doc/util/chplcheck-docs.py 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..a2fba902bf53 100644 --- a/doc/rst/tools/chplcheck/chplcheck.rst +++ b/doc/rst/tools/chplcheck/chplcheck.rst @@ -486,3 +486,10 @@ 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 + +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 100644 index 000000000000..4391767fb3a4 --- /dev/null +++ b/doc/util/chplcheck-docs.py @@ -0,0 +1,166 @@ +""" +Auto-generates a *.rst for the chplcheck rules defined +`$CHPL_HOME/tools/chplcheck/src/rules.py` +""" + +import sys +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("") + lines.append(self.description) + 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) + print(rstoutput) + + 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) + ] + + # if there is at least 1 decorator thats a `basic_rule`, an `advanced_rule`, + # or a `location_rule`, keep it + if not any( + d.attr in ["basic_rule", "advanced_rule", "location_rule"] + for d in decorators + ): + return None + + # get the name of the rule + name = func.name + # get the docstring of the rule + description = ast.get_docstring(func) or "" + + # 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 func.decorator_list + if isinstance(d, ast.Call) + and isinstance(d.func, ast.Attribute) + and 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 func.decorator_list + if isinstance(d, ast.Call) + and isinstance(d.func, ast.Attribute) + and 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) + + # TODO: settings + settings = [] + + 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/tools/chplcheck/Makefile b/tools/chplcheck/Makefile index 96c029185e89..1e1204184c2d 100644 --- a/tools/chplcheck/Makefile +++ b/tools/chplcheck/Makefile @@ -37,6 +37,17 @@ chplcheck-venv: chplcheck: chplcheck-venv +chplcheck-docs: chplcheck + CHPL_HOME=$(CHPL_MAKE_HOME) bash \ + $(CHPL_MAKE_HOME)/util/config/run-in-venv-with-python-bindings.bash \ + python3 $(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 +57,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 {} + From f08a1e9e4b2188fb184c88aa80a32b92d36c5436 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:03:25 -0800 Subject: [PATCH 02/19] fix sphinx issues Signed-off-by: Jade Abraham --- tools/chplcheck/src/driver.py | 9 ++++++++- tools/chplcheck/src/rules.py | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/chplcheck/src/driver.py b/tools/chplcheck/src/driver.py index 270b4cf26227..a986127bd426 100644 --- a/tools/chplcheck/src/driver.py +++ b/tools/chplcheck/src/driver.py @@ -75,7 +75,14 @@ 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__ + if doc is None: + doc = "" + + # 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/rules.py b/tools/chplcheck/src/rules.py index d5541e1cc717..c46f5abce020 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -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_"): From 69e527034210deafca8590cfc66d316835d952ab Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:04:08 -0800 Subject: [PATCH 03/19] remove debug print Signed-off-by: Jade Abraham --- doc/util/chplcheck-docs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/util/chplcheck-docs.py b/doc/util/chplcheck-docs.py index 4391767fb3a4..81364e4861be 100644 --- a/doc/util/chplcheck-docs.py +++ b/doc/util/chplcheck-docs.py @@ -46,7 +46,6 @@ def add_example(self, example_directory: str): with open(example_file) as handle: pieces = chpl2rst.to_pieces(handle, False) rstoutput = chpl2rst.gen_rst(pieces, example_file) - print(rstoutput) self.example_text = rstoutput From 563fec451b2f6299a30516a1ff5a9d79c7a8a402 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:04:41 -0800 Subject: [PATCH 04/19] use consistent name of 'rules' for chplcheck rules Signed-off-by: Jade Abraham --- tools/chpl-language-server/src/chpl-language-server.py | 2 +- tools/chplcheck/src/chplcheck.py | 4 ++-- tools/chplcheck/src/rules.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/chpl-language-server/src/chpl-language-server.py b/tools/chpl-language-server/src/chpl-language-server.py index dc12ba45c3bb..543f47263476 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/src/chplcheck.py b/tools/chplcheck/src/chplcheck.py index 6c6b333aac12..9c5a4dcbd455 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/rules.py b/tools/chplcheck/src/rules.py index c46f5abce020..742c65d9d163 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): """ From f67d77aff7a1d41055c845bd3e431902d9565b82 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:05:45 -0800 Subject: [PATCH 05/19] add lsp support Signed-off-by: Jade Abraham --- tools/chplcheck/src/lsp.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tools/chplcheck/src/lsp.py b/tools/chplcheck/src/lsp.py index b64e4fc18b39..588ee06e4080 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 @@ -53,12 +59,18 @@ def get_lint_diagnostics( diagnostics = [] # Silence errors from scope resolution etc., especially since they # may be emitted from other files (dependencies). + # TODO: use a chapel version specific error link + base_url = ( + "https://chapel-lang.org/docs/main/tools/chplcheck/chplcheck.html" + ) 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] From ef383a49e200f398ceb3ec9675df252d95209598 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:05:55 -0800 Subject: [PATCH 06/19] add example for DoKeywordAndBlock Signed-off-by: Jade Abraham --- tools/chplcheck/examples/DoKeywordAndBlock.chpl | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 tools/chplcheck/examples/DoKeywordAndBlock.chpl 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); +} + From 92856aa5411f0e098e5ceeeb7defcb93adb46443 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:07:00 -0800 Subject: [PATCH 07/19] remove requirement to build chplcheck to build docs Signed-off-by: Jade Abraham --- tools/chplcheck/Makefile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/chplcheck/Makefile b/tools/chplcheck/Makefile index 1e1204184c2d..005fed4a6e29 100644 --- a/tools/chplcheck/Makefile +++ b/tools/chplcheck/Makefile @@ -37,10 +37,8 @@ chplcheck-venv: chplcheck: chplcheck-venv -chplcheck-docs: chplcheck - CHPL_HOME=$(CHPL_MAKE_HOME) bash \ - $(CHPL_MAKE_HOME)/util/config/run-in-venv-with-python-bindings.bash \ - python3 $(CHPL_MAKE_HOME)/doc/util/chplcheck-docs.py \ +chplcheck-docs: + python3 $(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 From 93026becfa46404a1232fb7315566be98c737674 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:08:08 -0800 Subject: [PATCH 08/19] use CHPL_MAKE_PYTHON Signed-off-by: Jade Abraham --- tools/chplcheck/Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/chplcheck/Makefile b/tools/chplcheck/Makefile index 005fed4a6e29..0fd8c447d018 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 @@ -38,7 +41,7 @@ chplcheck-venv: chplcheck: chplcheck-venv chplcheck-docs: - python3 $(CHPL_MAKE_HOME)/doc/util/chplcheck-docs.py \ + $(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 From 9b3ce23b471926ef7511cde6f8d6566e958a05bf Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 4 Nov 2024 16:19:27 -0800 Subject: [PATCH 09/19] cleanup strip code Signed-off-by: Jade Abraham --- tools/chplcheck/src/driver.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tools/chplcheck/src/driver.py b/tools/chplcheck/src/driver.py index a986127bd426..0514ff02be6a 100644 --- a/tools/chplcheck/src/driver.py +++ b/tools/chplcheck/src/driver.py @@ -75,9 +75,7 @@ def rules_and_descriptions(self): for rule in itertools.chain( self.BasicRules, self.AdvancedRules, self.LocationRules ): - doc = rule.check_func.__doc__ - if doc is None: - doc = "" + doc = rule.check_func.__doc__ or "" # if there is an escaped underscore, remove the escape doc = doc.replace("\\_", "_") From 1b288c95c72a087ce14a0286e465997ea6eab9da Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 13:12:24 -0800 Subject: [PATCH 10/19] expose compiler version to chapel-py Signed-off-by: Jade Abraham --- tools/chapel-py/src/core-types-gen.cpp | 1 + tools/chapel-py/src/method-tables/core-methods.h | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tools/chapel-py/src/core-types-gen.cpp b/tools/chapel-py/src/core-types-gen.cpp index d8be4723f767..a6df0382a40d 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 ac312c9ac7c4..6bf17d56d1ba 100644 --- a/tools/chapel-py/src/method-tables/core-methods.h +++ b/tools/chapel-py/src/method-tables/core-methods.h @@ -64,6 +64,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) From 76387feb6a16e76d1faf63891d1505f77f1eedff Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 13:12:38 -0800 Subject: [PATCH 11/19] use version specific link Signed-off-by: Jade Abraham --- tools/chplcheck/src/lsp.py | 6 ++++-- tools/chplcheck/src/rules.py | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/chplcheck/src/lsp.py b/tools/chplcheck/src/lsp.py index 588ee06e4080..74dd06ab0e8f 100644 --- a/tools/chplcheck/src/lsp.py +++ b/tools/chplcheck/src/lsp.py @@ -59,9 +59,11 @@ def get_lint_diagnostics( diagnostics = [] # Silence errors from scope resolution etc., especially since they # may be emitted from other files (dependencies). - # TODO: use a chapel version specific error link + + # 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/main/tools/chplcheck/chplcheck.html" + "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): diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index 742c65d9d163..68c5de2c191f 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -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. """ From 26529e8163bba38509afe362f16f62e15b79a966 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:03:41 -0800 Subject: [PATCH 12/19] improve chplcheck-docs to include settings Signed-off-by: Jade Abraham --- doc/util/chplcheck-docs.py | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) mode change 100644 => 100755 doc/util/chplcheck-docs.py diff --git a/doc/util/chplcheck-docs.py b/doc/util/chplcheck-docs.py old mode 100644 new mode 100755 index 81364e4861be..819a5342b257 --- a/doc/util/chplcheck-docs.py +++ b/doc/util/chplcheck-docs.py @@ -1,9 +1,10 @@ +#!/usr/bin/env python3 + """ Auto-generates a *.rst for the chplcheck rules defined `$CHPL_HOME/tools/chplcheck/src/rules.py` """ -import sys import os import typing from dataclasses import dataclass @@ -29,8 +30,17 @@ def rst(self): lines.append("") lines.append("Is enabled by default? " + ("Yes" if self.default else "No")) lines.append("") - lines.append(self.description) - 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("") @@ -104,8 +114,20 @@ def get_rule(func) -> typing.Optional[Rule]: ] is_default = not any(d == "False" for d in default_settings) - # TODO: 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) From 01560680b65aae85cbe8c89963b02b5627db9586 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:04:03 -0800 Subject: [PATCH 13/19] enable testing of chplcheck examples Signed-off-by: Jade Abraham --- test/chplcheck/CHPLCHECKOPTS | 1 + test/chplcheck/PREDIFF | 11 +++---- test/chplcheck/examples | 1 + tools/chplcheck/examples/CHPLCHECKOPTS | 1 + tools/chplcheck/examples/CLEANFILES | 1 + tools/chplcheck/examples/COMPOPTS | 1 + .../chplcheck/examples/DoKeywordAndBlock.good | 1 + tools/chplcheck/examples/NOEXEC | 0 tools/chplcheck/examples/PREDIFF | 33 +++++++++++++++++++ tools/chplcheck/examples/SKIPIF | 8 +++++ 10 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 test/chplcheck/CHPLCHECKOPTS create mode 120000 test/chplcheck/examples create mode 100644 tools/chplcheck/examples/CHPLCHECKOPTS create mode 100644 tools/chplcheck/examples/CLEANFILES create mode 100644 tools/chplcheck/examples/COMPOPTS create mode 100644 tools/chplcheck/examples/DoKeywordAndBlock.good create mode 100644 tools/chplcheck/examples/NOEXEC create mode 100755 tools/chplcheck/examples/PREDIFF create mode 100755 tools/chplcheck/examples/SKIPIF 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..b5de6da3eca6 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 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/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/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..b5de6da3eca6 --- /dev/null +++ b/tools/chplcheck/examples/PREDIFF @@ -0,0 +1,33 @@ +#!/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 + +if sed "s#$(pwd)/##" $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 + From 9bacbbde21eb83d6a9c8433b1519a611eb84ffac Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:04:13 -0800 Subject: [PATCH 14/19] add examples for control flow parentheses Signed-off-by: Jade Abraham --- tools/chplcheck/examples/ControlFlowParentheses.chpl | 10 ++++++++++ tools/chplcheck/examples/ControlFlowParentheses.good | 1 + 2 files changed, 11 insertions(+) create mode 100644 tools/chplcheck/examples/ControlFlowParentheses.chpl create mode 100644 tools/chplcheck/examples/ControlFlowParentheses.good 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..91bda15a9172 --- /dev/null +++ b/tools/chplcheck/examples/ControlFlowParentheses.good @@ -0,0 +1 @@ +ControlFlowParentheses.chpl:6: node violates rule ControlFlowParentheses From d9a845ab9d8ebbcb0fed6706ebfbff422c059386 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:04:50 -0800 Subject: [PATCH 15/19] formatting Signed-off-by: Jade Abraham --- doc/util/chplcheck-docs.py | 28 +++++++++++++++++++++++----- tools/chplcheck/src/lsp.py | 4 +++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/doc/util/chplcheck-docs.py b/doc/util/chplcheck-docs.py index 819a5342b257..864b04d71bc9 100755 --- a/doc/util/chplcheck-docs.py +++ b/doc/util/chplcheck-docs.py @@ -28,7 +28,9 @@ def rst(self): 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( + "Is enabled by default? " + ("Yes" if self.default else "No") + ) lines.append("") if self.description: @@ -60,7 +62,6 @@ def add_example(self, example_directory: str): self.example_text = rstoutput - def find_rules(file: str): def get_rule(func) -> typing.Optional[Rule]: @@ -147,9 +148,11 @@ def get_rule(func) -> typing.Optional[Rule]: 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): @@ -163,9 +166,24 @@ def output_rules(rules: typing.List[Rule], output_dir: str): 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") + 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] = [] diff --git a/tools/chplcheck/src/lsp.py b/tools/chplcheck/src/lsp.py index 74dd06ab0e8f..781589a38b5c 100644 --- a/tools/chplcheck/src/lsp.py +++ b/tools/chplcheck/src/lsp.py @@ -63,7 +63,9 @@ def get_lint_diagnostics( # 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) + "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): From 4ebce6cab320ff66d54c45edf04acc19c4e2b59a Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:12:16 -0800 Subject: [PATCH 16/19] add docs Signed-off-by: Jade Abraham --- doc/rst/tools/chplcheck/chplcheck.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/doc/rst/tools/chplcheck/chplcheck.rst b/doc/rst/tools/chplcheck/chplcheck.rst index a2fba902bf53..a7af92c7b618 100644 --- a/doc/rst/tools/chplcheck/chplcheck.rst +++ b/doc/rst/tools/chplcheck/chplcheck.rst @@ -487,6 +487,21 @@ The linter is run as follows: 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 use 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 ------------- From df8c8f034a783028ade98325220be7a4626c358b Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:13:47 -0800 Subject: [PATCH 17/19] fix line wrapping Signed-off-by: Jade Abraham --- doc/rst/tools/chplcheck/chplcheck.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/rst/tools/chplcheck/chplcheck.rst b/doc/rst/tools/chplcheck/chplcheck.rst index a7af92c7b618..4f9c3822261d 100644 --- a/doc/rst/tools/chplcheck/chplcheck.rst +++ b/doc/rst/tools/chplcheck/chplcheck.rst @@ -495,8 +495,7 @@ generate Sphinx documentation for the rule. This can be done by running the .. code-block:: bash - > $CHPL_HOME/doc/util/chplcheck-docs.py path/to/my/myrules.py -o - my/out/directory + > $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 From 9e67a5f66b1888313474009ce1307f3506a420c3 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:14:18 -0800 Subject: [PATCH 18/19] wordsmithing Signed-off-by: Jade Abraham --- doc/rst/tools/chplcheck/chplcheck.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/rst/tools/chplcheck/chplcheck.rst b/doc/rst/tools/chplcheck/chplcheck.rst index 4f9c3822261d..5ee208d7408d 100644 --- a/doc/rst/tools/chplcheck/chplcheck.rst +++ b/doc/rst/tools/chplcheck/chplcheck.rst @@ -489,7 +489,7 @@ The linter is run as follows: 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 use to +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: From 9afef46ff31be20adc50e83712a33b6356dc83f4 Mon Sep 17 00:00:00 2001 From: Jade Abraham Date: Mon, 16 Dec 2024 15:25:28 -0800 Subject: [PATCH 19/19] exclude examples from copyright check Signed-off-by: Jade Abraham --- util/test/checkCopyrights.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 \