CI: Refactor Python checks linting setup.
This commit is contained in:
110
.github/workflows/sub_lint.yml
vendored
110
.github/workflows/sub_lint.yml
vendored
@@ -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 "<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: |
|
||||
|
||||
102
tools/lint/black.py
Executable file
102
tools/lint/black.py
Executable file
@@ -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()
|
||||
160
tools/lint/pylint.py
Executable file
160
tools/lint/pylint.py
Executable file
@@ -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"<details><summary>{summary}</summary>")
|
||||
report_lines.append("")
|
||||
report_lines.append(
|
||||
"<details><summary>:information_source: Enabled checks</summary>"
|
||||
)
|
||||
report_lines.append("")
|
||||
report_lines.append("````")
|
||||
report_lines.append(read_file_contents(enabled_checks_log))
|
||||
report_lines.append("````")
|
||||
report_lines.append("</details>")
|
||||
report_lines.append("")
|
||||
report_lines.append("<details>")
|
||||
report_lines.append("````")
|
||||
report_lines.append(read_file_contents(pylint_log))
|
||||
report_lines.append("````")
|
||||
report_lines.append("</details>")
|
||||
report_lines.append("</details>")
|
||||
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()
|
||||
Reference in New Issue
Block a user