From 7fdd0004777c19e8eb6a53a0cafda1b13b90db58 Mon Sep 17 00:00:00 2001 From: Joao Matos Date: Mon, 3 Mar 2025 23:00:24 +0000 Subject: [PATCH] CI: Refactor Python checks linting setup. --- .github/workflows/sub_lint.yml | 110 ++++------------------- tools/lint/black.py | 102 +++++++++++++++++++++ tools/lint/pylint.py | 160 +++++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+), 93 deletions(-) create mode 100755 tools/lint/black.py create mode 100755 tools/lint/pylint.py diff --git a/.github/workflows/sub_lint.yml b/.github/workflows/sub_lint.yml index 92b38b2199..2fffb9b76c 100644 --- a/.github/workflows/sub_lint.yml +++ b/.github/workflows/sub_lint.yml @@ -264,6 +264,7 @@ jobs: uses: actions/checkout@v4 with: submodules: true + - name: Make needed directories, files and initializations id: Init run: | @@ -271,7 +272,9 @@ jobs: mkdir -p ${{ env.fixesdir }} mkdir -p ${{ env.reportdir }} echo "reportFile=${{ env.reportfilename }}" >> $GITHUB_OUTPUT - # Run generic lints + + # Generic lints steps + - name: Check for non Unix line ending if: inputs.checkLineendings && always() continue-on-error: ${{ inputs.lineendingsFailSilent }} @@ -302,105 +305,26 @@ jobs: --report-file "${{ env.reportdir }}${{ env.reportfilename }}" \ --log-dir "${{ env.logdir }}" + # Python linting steps + - name: Pylint if: inputs.checkPylint && inputs.changedPythonFiles != '' && always() continue-on-error: ${{ inputs.pylintFailSilent }} run: | - pylintErrors=0 - pylintWarnings=0 - pylintRefactorings=0 - pylintConventions=0 - pip install --break-system-packages pylint - # List enabled pylint checks - pylint --list-msgs-enabled > ${{ env.logdir }}pylint-enabled-checks.log - # Run pylint on all python files - set +e - pylint --disable=${{ inputs.pylintDisable }} ${{ inputs.changedPythonFiles }} > ${{ env.logdir }}pylint.log - exitCode=$? - set -e - # If pylint has run successfully, write the Log to the console with the Problem Matchers - if [ -f ${{ env.logdir }}pylint.log ] - then - echo "::add-matcher::${{ runner.workspace }}/FreeCAD/.github/problemMatcher/pylintError.json" - echo "::add-matcher::${{ runner.workspace }}/FreeCAD/.github/problemMatcher/pylintWarning.json" - cat ${{ env.logdir }}pylint.log - echo "::remove-matcher owner=pylint-error::" - echo "::remove-matcher owner=pylint-warning::" - pylintErrors=$( grep -oP '(?<=error \|)\d+' ${{ env.logdir }}pylint.log) || true # grep returns 0 if no match is found - pylintWarnings=$( grep -oP '(?<=warning \|)\d+' ${{ env.logdir }}pylint.log) || true - pylintRefactorings=$( grep -oP '(?<=refactor \|)\d+' ${{ env.logdir }}pylint.log) || true - pylintConventions=$( grep -oP '(?<=convention \|)\d+' ${{ env.logdir }}pylint.log) || true - fi - echo "Found $pylintErrors errors, $pylintWarnings warnings, $pylintRefactorings refactorings, $pylintConventions conventions" - # Write the report - if [ $pylintErrors -gt 0 ] - then - echo "
:fire: Pylint found :fire: $pylintErrors errors, :warning: $pylintWarnings warnings, :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions" >> ${{env.reportdir}}${{ env.reportfilename }} - elif [ $pylintWarnings -gt 0 ] - then - echo "
:warning: Pylint found :warning: $pylintWarnings warnings, :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions" >> ${{env.reportdir}}${{ env.reportfilename }} - elif [ $pylintRefactorings -gt 0 ] - then - echo "
:construction: Pylint found :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions" >> ${{env.reportdir}}${{ env.reportfilename }} - elif [ $pylintConventions -gt 0 ] - then - echo "
:pencil2: Pylint found :pencil2: $pylintConventions conventions" >> ${{env.reportdir}}${{ env.reportfilename }} - else - echo "
:heavy_check_mark: No pylint errors found " >> ${{env.reportdir}}${{ env.reportfilename }} - fi - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - # List enabled checks in the report - echo "
:information_source: Enabled checks" >> ${{env.reportdir}}${{ env.reportfilename }} - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - cat ${{ env.logdir }}pylint-enabled-checks.log >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - echo "
" >> ${{env.reportdir}}${{ env.reportfilename }} - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - cat ${{ env.logdir }}pylint.log >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - echo "
" >> ${{env.reportdir}}${{ env.reportfilename }} - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - # Exit the step with appropriate code - [ $pylintErrors -eq 0 ] - - name: Black (Python) + python3 tools/lint/pylint.py \ + --files "${{ inputs.changedPythonFiles }}" \ + --disable "${{ inputs.pylintDisable }}" \ + --log-dir "${{ env.logdir }}" \ + --report-file "${{ env.reportdir }}${{ env.reportfilename }}" + + - name: Black if: inputs.checkBlack && inputs.changedPythonFiles != '' && always() continue-on-error: ${{ inputs.blackFailSilent }} run: | - blackReformats=0 - blackFails=0 - pip install --break-system-packages black - set +e - black --line-length 100 --check ${{ inputs.changedPythonFiles }} &> ${{ env.logdir }}black.log - exitCode=$? - set -e - # If black has run successfully, write the Log to the console with the Problem Matchers - if [ -f ${{ env.logdir }}black.log ] - then - echo "::add-matcher::${{ runner.workspace }}/FreeCAD/.github/problemMatcher/blackWarning.json" - cat ${{ env.logdir }}black.log - echo "::remove-matcher owner=black-warning::" - blackReformats=$( grep -oP '\d+(?= fil.+ would be reformatted)' ${{ env.logdir }}black.log) || true # grep returns 0 if no match is found - blackFails=$( grep -oP '\d+(?= fil.+ would fail to reformat)' ${{ env.logdir }}black.log) || true - fi - echo "Found $blackReformats files would be reformatted and $blackFails files would fail to reformat" - # Write the report - if [ $blackReformats -gt 0 ] || [ $blackFails -gt 0 ] #FIXME purpose of testing $blackFails as we don't use it then - then - echo "
:pencil2: Black would reformat $blackReformats files" >> ${{env.reportdir}}${{ env.reportfilename }} - else - echo "
:heavy_check_mark: Black would reformat no file" >> ${{env.reportdir}}${{ env.reportfilename }} - fi - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - cat ${{ env.logdir }}black.log >> ${{env.reportdir}}${{ env.reportfilename }} - echo '```' >> ${{env.reportdir}}${{ env.reportfilename }} - echo "
" >> ${{env.reportdir}}${{ env.reportfilename }} - echo "" >> ${{env.reportdir}}${{ env.reportfilename }} - # Exit the step with appropriate code - [ $exitCode -eq 0 ] - # Run C++ lints + python3 tools/lint/black.py \ + --files "${{ inputs.changedPythonFiles }}" \ + --log-dir "${{ env.logdir }}" \ + --report-file "${{ env.reportdir }}${{ env.reportfilename }}" - name: Install FreeCAD dependencies if: inputs.changedCppFiles != '' && always() run: | diff --git a/tools/lint/black.py b/tools/lint/black.py new file mode 100755 index 0000000000..a916244075 --- /dev/null +++ b/tools/lint/black.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +import argparse +import sys +import os +import re +import logging +from typing import Tuple +from utils import ( + run_command, + init_environment, + append_file, + emit_problem_matchers, + write_file, + add_common_arguments, +) + +DEFAULT_LINE_LENGTH_LIMIT = 100 + + +def parse_black_output(output: str) -> Tuple[int, int]: + """ + Parse Black output to determine how many files would be reformatted and how many would fail. + + Returns: + A tuple (black_reformats, black_fails). + """ + black_reformats = 0 + black_fails = 0 + + # Look for a pattern like: "X files would be reformatted" + reformats_matches = re.findall(r"(\d+)(?=\s+fil.+ would be reformatted)", output) + if reformats_matches: + black_reformats = int(reformats_matches[0]) + + # Look for a pattern like: "Y files would fail to reformat" + fails_matches = re.findall(r"(\d+)(?=\s+fil.+ would fail to reformat)", output) + if fails_matches: + black_fails = int(fails_matches[0]) + + return black_reformats, black_fails + + +def generate_markdown_report( + black_reformats: int, black_fails: int, log_file: str +) -> str: + """Generate a Markdown report section based on Black results and log file.""" + report_lines = [] + if black_reformats > 0 or black_fails > 0: + report_lines.append( + f"
:pencil2: Black would reformat {black_reformats} file(s)" + f" and {black_fails} file(s) would fail to reformat" + ) + else: + report_lines.append( + "
:heavy_check_mark: Black would reformat no file" + ) + report_lines.append("") + report_lines.append("````") + with open(log_file, "r", encoding="utf-8") as f: + report_lines.append(f.read()) + report_lines.append("````") + report_lines.append("
") + report_lines.append("") + return "\n".join(report_lines) + + +def main(): + parser = argparse.ArgumentParser( + description="Run Black in check mode on Python files and append a Markdown report." + ) + add_common_arguments(parser) + args = parser.parse_args() + init_environment(args) + + cmd = [ + "black", + "--line-length", + str(DEFAULT_LINE_LENGTH_LIMIT), + "--check", + ] + args.files.split() + stdout, stderr, exit_code = run_command(cmd, check=False) + output = stdout + "\n" + stderr + + log_file = os.path.join(args.log_dir, "black.log") + write_file(log_file, output) + emit_problem_matchers(log_file, "blackWarning.json", "black-warning") + + black_reformats, black_fails = parse_black_output(output) + logging.info( + "Found %s files would be reformatted and %s files would fail to reformat", + black_reformats, + black_fails, + ) + + report = generate_markdown_report(black_reformats, black_fails, log_file) + append_file(args.report_file, report) + + sys.exit(0 if exit_code == 0 else 1) + + +if __name__ == "__main__": + main() diff --git a/tools/lint/pylint.py b/tools/lint/pylint.py new file mode 100755 index 0000000000..38e00a42bd --- /dev/null +++ b/tools/lint/pylint.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 +import argparse +import sys +import re +import os +import logging +from utils import ( + run_command, + add_common_arguments, + init_environment, + write_file, + append_file, + emit_problem_matchers, +) + + +def get_enabled_checks(log_dir: str) -> str: + """Run pylint to list enabled messages and write to a log file.""" + enabled_log_path = os.path.join(log_dir, "pylint-enabled-checks.log") + stdout, _, _ = run_command(["pylint", "--list-msgs-enabled"]) + write_file(enabled_log_path, stdout) + return enabled_log_path + + +def run_pylint(files: str, disable: str, log_dir: str) -> str: + """Run pylint on the provided files and log the output.""" + pylint_log_path = os.path.join(log_dir, "pylint.log") + cmd = ["pylint", f"--disable={disable}"] + files.split() + stdout, stderr, _ = run_command(cmd, check=False) + combined_output = stdout + "\n" + stderr + write_file(pylint_log_path, combined_output) + return pylint_log_path + + +def count_pattern(pattern: str, text: str) -> int: + """Count numeric values found by a regex pattern in the text.""" + matches = re.findall(pattern, text) + logging.debug("Pattern '%s' found matches: %s", pattern, matches) + return sum(int(match) for match in matches) if matches else 0 + + +def parse_pylint_output(log_path: str) -> dict: + """Parse pylint log file for error, warning, refactor, and convention counts.""" + with open(log_path, "r", encoding="utf-8") as f: + content = f.read() + errors = count_pattern(r"error\s+\|(\d+)", content) + warnings = count_pattern(r"warning\s+\|(\d+)", content) + refactorings = count_pattern(r"refactor\s+\|(\d+)", content) + conventions = count_pattern(r"convention\s+\|(\d+)", content) + logging.info( + "Pylint issues - Errors: %s, Warnings: %s, Refactorings: %s, Conventions: %s", + errors, + warnings, + refactorings, + conventions, + ) + return { + "errors": errors, + "warnings": warnings, + "refactorings": refactorings, + "conventions": conventions, + "full_output": content, + } + + +def generate_markdown_report( + pylint_counts: dict, enabled_checks_log: str, pylint_log: str +) -> str: + """Generate a Markdown-formatted report based on pylint results and logs.""" + + def generate_summary(counts: dict) -> str: + errors = counts.get("errors", 0) + warnings = counts.get("warnings", 0) + refactorings = counts.get("refactorings", 0) + conventions = counts.get("conventions", 0) + if errors: + return ( + f":fire: Pylint found :fire: {errors} errors, " + f":warning: {warnings} warnings, :construction: {refactorings} refactorings and " + f":pencil2: {conventions} conventions" + ) + if warnings: + return ( + f":warning: Pylint found :warning: {warnings} warnings, " + f":construction: {refactorings} refactorings and " + f":pencil2: {conventions} conventions" + ) + if refactorings: + return ( + f":construction: Pylint found :construction: {refactorings} refactorings and " + f":pencil2: {conventions} conventions" + ) + if conventions: + return f":pencil2: Pylint found :pencil2: {conventions} conventions" + return ":heavy_check_mark: No pylint errors found" + + def read_file_contents(file_path: str) -> str: + with open(file_path, "r", encoding="utf-8") as f: + return f.read() + + report_lines = [] + summary = generate_summary(pylint_counts) + report_lines.append(f"
{summary}") + report_lines.append("") + report_lines.append( + "
:information_source: Enabled checks" + ) + report_lines.append("") + report_lines.append("````") + report_lines.append(read_file_contents(enabled_checks_log)) + report_lines.append("````") + report_lines.append("
") + report_lines.append("") + report_lines.append("
") + report_lines.append("````") + report_lines.append(read_file_contents(pylint_log)) + report_lines.append("````") + report_lines.append("
") + report_lines.append("
") + return "\n".join(report_lines) + + +def main(): + parser = argparse.ArgumentParser( + description="Run Pylint on provided Python files and append a Markdown report." + ) + add_common_arguments(parser) + parser.add_argument( + "--disable", + default="C0302,C0303", + help="Comma-separated list of pylint checks to disable.", + ) + args = parser.parse_args() + init_environment(args) + + logging.info("Installing pylint (if needed)...") + run_command(["pip", "install", "-q", "pylint"], check=True) + + enabled_checks_log = get_enabled_checks(args.log_dir) + pylint_log = run_pylint(args.files, args.disable, args.log_dir) + + emit_problem_matchers(pylint_log, "pylintError.json", "pylint-error") + emit_problem_matchers(pylint_log, "pylintWarning.json", "pylint-warning") + + pylint_counts = parse_pylint_output(pylint_log) + + summary_msg = ( + f"Found {pylint_counts['errors']} errors, {pylint_counts['warnings']} warnings, " + f"{pylint_counts['refactorings']} refactorings, {pylint_counts['conventions']} conventions" + ) + logging.info(summary_msg) + + report = generate_markdown_report(pylint_counts, enabled_checks_log, pylint_log) + append_file(args.report_file, report) + + sys.exit(0 if pylint_counts["errors"] == 0 else 1) + + +if __name__ == "__main__": + main()