Skip to content

Commit

Permalink
Merge pull request FreeCAD#20012 from tritao/ci-lint-refactor-python
Browse files Browse the repository at this point in the history
CI: Refactor Python checks linting setup.
  • Loading branch information
chennes authored Mar 6, 2025
2 parents 6552797 + 7fdd000 commit dc084c0
Show file tree
Hide file tree
Showing 3 changed files with 279 additions and 93 deletions.
110 changes: 17 additions & 93 deletions .github/workflows/sub_lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -264,14 +264,17 @@ jobs:
uses: actions/checkout@v4
with:
submodules: true

- name: Make needed directories, files and initializations
id: Init
run: |
mkdir -p ${{ env.logdir }}
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 }}
Expand Down Expand Up @@ -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 "<details><summary>:fire: Pylint found :fire: $pylintErrors errors, :warning: $pylintWarnings warnings, :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions</summary>" >> ${{env.reportdir}}${{ env.reportfilename }}
elif [ $pylintWarnings -gt 0 ]
then
echo "<details><summary>:warning: Pylint found :warning: $pylintWarnings warnings, :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions</summary>" >> ${{env.reportdir}}${{ env.reportfilename }}
elif [ $pylintRefactorings -gt 0 ]
then
echo "<details><summary>:construction: Pylint found :construction: $pylintRefactorings refactorings and :pencil2: $pylintConventions conventions</summary>" >> ${{env.reportdir}}${{ env.reportfilename }}
elif [ $pylintConventions -gt 0 ]
then
echo "<details><summary>:pencil2: Pylint found :pencil2: $pylintConventions conventions</summary>" >> ${{env.reportdir}}${{ env.reportfilename }}
else
echo "<details><summary>:heavy_check_mark: No pylint errors found </summary> " >> ${{env.reportdir}}${{ env.reportfilename }}
fi
echo "" >> ${{env.reportdir}}${{ env.reportfilename }}
# List enabled checks in the report
echo "<details><summary>:information_source: Enabled checks</summary>" >> ${{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 "</details>" >> ${{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 "</details>" >> ${{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 "<details><summary>:pencil2: Black would reformat $blackReformats files</summary>" >> ${{env.reportdir}}${{ env.reportfilename }}
else
echo "<details><summary>:heavy_check_mark: Black would reformat no file</summary>" >> ${{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 "</details>" >> ${{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: |
Expand Down
102 changes: 102 additions & 0 deletions tools/lint/black.py
Original file line number Diff line number Diff line change
@@ -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"<details><summary>:pencil2: Black would reformat {black_reformats} file(s)"
f" and {black_fails} file(s) would fail to reformat</summary>"
)
else:
report_lines.append(
"<details><summary>:heavy_check_mark: Black would reformat no file</summary>"
)
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("</details>")
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()
Loading

0 comments on commit dc084c0

Please sign in to comment.