diff --git a/.github/workflows/code_formatting.yml b/.github/workflows/code_formatting.yml index 7e74776af..71c50aa1b 100644 --- a/.github/workflows/code_formatting.yml +++ b/.github/workflows/code_formatting.yml @@ -7,8 +7,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - run: pip install --user ruff - - run: ruff --format=github . - uses: actions/setup-python@v4 - name: Install packages run: source tools/ci.sh && ci_code_formatting_setup diff --git a/.github/workflows/ruff.yml b/.github/workflows/ruff.yml new file mode 100644 index 000000000..b8e43dc78 --- /dev/null +++ b/.github/workflows/ruff.yml @@ -0,0 +1,10 @@ +# https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python +name: Python code lint with ruff +on: [push, pull_request] +jobs: + ruff: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - run: pip install --user ruff + - run: ruff --format=github . diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..e017b86e8 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,11 @@ +repos: + - repo: local + hooks: + - id: codeformat + name: MicroPython codeformat.py for changed files + entry: tools/codeformat.py -v -f + language: python + - repo: https://github.com/charliermarsh/ruff-pre-commit + rev: v0.0.280 + hooks: + - id: ruff diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 804a26bef..d590754e9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -25,11 +25,21 @@ or packages from micropython-lib, please post at the ### Pull requests The same rules for commit messages, signing-off commits, and commit structure -apply as for the main MicroPython repository. All Python code is formatted -using `black`. See [`tools/codeformat.py`](tools/codeformat.py) to apply -`black` automatically before submitting a PR. +apply [as for the main MicroPython repository](https://github.com/micropython/micropython/blob/master/CODECONVENTIONS.md). -There are some specific conventions and guidelines for micropython-lib: +All Python code is formatted using the [black](https://github.com/psf/black) +tool. You can run [`tools/codeformat.py`](tools/codeformat.py) to apply +`black` automatically before submitting a PR. The GitHub CI will also run the +[ruff](https://github.com/astral-sh/ruff) tool to apply further "linting" +checks. + +Similar to the main repository, a configuration is provided for the +[pre-commit](https://pre-commit.com/) tool to apply `black` code formatting +rules and run `ruff` automatically. See the documentation for using pre-commit +in [the code conventions document](https://github.com/micropython/micropython/blob/master/CODECONVENTIONS.md#automatic-pre-commit-hooks) + +In addition to the conventions from the main repository, there are some +specific conventions and guidelines for micropython-lib: * The first line of the commit message should start with the name of the package, followed by a short description of the commit. Package names are diff --git a/tools/codeformat.py b/tools/codeformat.py index 63c9c5988..2bc0c7f44 100755 --- a/tools/codeformat.py +++ b/tools/codeformat.py @@ -5,7 +5,7 @@ # The MIT License (MIT) # # Copyright (c) 2020 Damien P. George -# Copyright (c) 2020 Jim Mussared +# Copyright (c) 2023 Jim Mussared # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -25,6 +25,9 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. +# This is based on tools/codeformat.py from the main micropython/micropython +# repository but without support for .c/.h files. + import argparse import glob import itertools @@ -34,9 +37,6 @@ # Relative to top-level repo dir. PATHS = [ - # C - "**/*.[ch]", - # Python "**/*.py", ] @@ -45,19 +45,9 @@ # Path to repo top-level dir. TOP = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -UNCRUSTIFY_CFG = os.path.join(TOP, "tools/uncrustify.cfg") - -C_EXTS = ( - ".c", - ".h", -) PY_EXTS = (".py",) -MAIN_BRANCH = "master" -BASE_BRANCH = os.environ.get("GITHUB_BASE_REF", MAIN_BRANCH) - - def list_files(paths, exclusions=None, prefix=""): files = set() for pattern in paths: @@ -67,128 +57,33 @@ def list_files(paths, exclusions=None, prefix=""): return sorted(files) -def fixup_c(filename): - # Read file. - with open(filename) as f: - lines = f.readlines() - - # Write out file with fixups. - with open(filename, "w", newline="") as f: - dedent_stack = [] - while lines: - # Get next line. - l = lines.pop(0) - - # Dedent #'s to match indent of following line (not previous line). - m = re.match(r"( +)#(if |ifdef |ifndef |elif |else|endif)", l) - if m: - indent = len(m.group(1)) - directive = m.group(2) - if directive in ("if ", "ifdef ", "ifndef "): - l_next = lines[0] - indent_next = len(re.match(r"( *)", l_next).group(1)) - if indent - 4 == indent_next and re.match(r" +(} else |case )", l_next): - # This #-line (and all associated ones) needs dedenting by 4 spaces. - l = l[4:] - dedent_stack.append(indent - 4) - else: - # This #-line does not need dedenting. - dedent_stack.append(-1) - else: - if dedent_stack[-1] >= 0: - # This associated #-line needs dedenting to match the #if. - indent_diff = indent - dedent_stack[-1] - assert indent_diff >= 0 - l = l[indent_diff:] - if directive == "endif": - dedent_stack.pop() - - # Write out line. - f.write(l) - - assert not dedent_stack, filename - - -def query_git_files(verbose): - def cmd_result_set(cmd): - ret = subprocess.run(cmd, capture_output=True).stdout.strip().decode() - if not ret: - return set() - return {f.strip() for f in ret.split("\n")} - - def rel_paths(files, root): - return {os.path.relpath(os.path.join(root, f.strip()), ".") for f in files} - - try: - ret = set() - - # get path to root of repository - root_dir = ( - subprocess.run(["git", "rev-parse", "--show-toplevel"], capture_output=True) - .stdout.strip() - .decode() - ) - - # Check locally modified files - status = cmd_result_set(["git", "status", "--porcelain"]) - dirty_files = rel_paths({line.split(" ", 1)[-1] for line in status}, root_dir) - ret |= dirty_files - - # Current commit and branch - current_commit = ( - subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True) - .stdout.strip() - .decode() - ) - current_branches = cmd_result_set(["git", "branch", "--contains", current_commit]) - if MAIN_BRANCH in current_branches: - if ret: - if verbose: - print("Local changes detected, only scanning them.") - return ret - - # We're on clean master, run on entire repo - if verbose: - print("Scanning whole repository") - return None - - # List the files modified on current branch - if verbose: - print("Scanning changes from current branch and any local changes") - files_on_branch = rel_paths( - cmd_result_set(["git", "diff", "--name-only", BASE_BRANCH]), root_dir - ) - ret |= files_on_branch - return ret - except: - # Git not available, run on entire repo - return None - - def main(): - cmd_parser = argparse.ArgumentParser(description="Auto-format C and Python files.") - cmd_parser.add_argument("-c", action="store_true", help="Format C code only") - cmd_parser.add_argument("-p", action="store_true", help="Format Python code only") + cmd_parser = argparse.ArgumentParser(description="Auto-format Python files.") cmd_parser.add_argument("-v", action="store_true", help="Enable verbose output") cmd_parser.add_argument( - "files", - nargs="*", - help="Run on specific globs. If not specied current branch changes will be used", + "-f", + action="store_true", + help="Filter files provided on the command line against the default list of files to check.", ) + cmd_parser.add_argument("files", nargs="*", help="Run on specific globs") args = cmd_parser.parse_args() - # Setting only one of -c or -p disables the other. If both or neither are set, then do both. - format_c = args.c or not args.p - format_py = args.p or not args.c - # Expand the globs passed on the command line, or use the default globs above. files = [] if args.files: files = list_files(args.files) + if args.f: + # Filter against the default list of files. This is a little fiddly + # because we need to apply both the inclusion globs given in PATHS + # as well as the EXCLUSIONS, and use absolute paths + files = {os.path.abspath(f) for f in files} + all_files = set(list_files(PATHS, EXCLUSIONS, TOP)) + if args.v: # In verbose mode, log any files we're skipping + for f in files - all_files: + print("Not checking: {}".format(f)) + files = list(files & all_files) else: - files = query_git_files(verbose=args.v) - if not files: - files = list_files(PATHS, EXCLUSIONS, TOP) + files = list_files(PATHS, EXCLUSIONS, TOP) # Extract files matching a specific language. def lang_files(exts): @@ -204,23 +99,13 @@ def batch(cmd, files, N=200): break subprocess.check_call(cmd + file_args) - # Format C files with uncrustify. - if format_c: - command = ["uncrustify", "-c", UNCRUSTIFY_CFG, "-lC", "--no-backup"] - if not args.v: - command.append("-q") - batch(command, lang_files(C_EXTS)) - for file in lang_files(C_EXTS): - fixup_c(file) - # Format Python files with black. - if format_py: - command = ["black", "--fast", "--line-length=99"] - if args.v: - command.append("-v") - else: - command.append("-q") - batch(command, lang_files(PY_EXTS)) + command = ["black", "--fast", "--line-length=99"] + if args.v: + command.append("-v") + else: + command.append("-q") + batch(command, lang_files(PY_EXTS)) if __name__ == "__main__":