mirror of
https://github.com/servo/servo.git
synced 2025-08-04 21:20:23 +01:00
Mach clippy & test-tidy github inline annotation (#37294)
When the --report-ci flag is passed to ./mach clippy or ./mach test-tidy, the commands emit CI-friendly output to files in the tempy directory. These files can later be used by [GitHub Workflow Commands](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions#setting-an-error-message) for annotations. If the flag is not provided, the default behavior remains unchanged. Both clippy and test-tidy will limit have 10 limit annotation ⚠️ Note: For ./mach clippy --report-ci to work correctly, the Clippy command must use --message-format=json. If it's not specified, CI output will not be generated, and no warning or error will be shown. Example PR: https://github.com/jerensl/servo/pull/1/files Fixes: #37231 --------- Signed-off-by: Jerens Lensun <jerensslensun@gmail.com>
This commit is contained in:
parent
18d55f4884
commit
877010c5f6
6 changed files with 175 additions and 29 deletions
5
.github/workflows/lint.yml
vendored
5
.github/workflows/lint.yml
vendored
|
@ -43,9 +43,8 @@ jobs:
|
||||||
run: |
|
run: |
|
||||||
sudo apt update
|
sudo apt update
|
||||||
./mach bootstrap
|
./mach bootstrap
|
||||||
# TODO: Do GitHub anotaions
|
|
||||||
- name: Clippy
|
- name: Clippy
|
||||||
run: |
|
run: |
|
||||||
./mach clippy --use-crown --locked -- -- --deny warnings
|
./mach clippy --use-crown --locked --github-annotations -- -- --deny warnings
|
||||||
- name: Tidy
|
- name: Tidy
|
||||||
run: ./mach test-tidy --no-progress --all
|
run: ./mach test-tidy --no-progress --all --github-annotations
|
||||||
|
|
|
@ -10,8 +10,6 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import contextlib
|
import contextlib
|
||||||
from enum import Enum
|
|
||||||
from typing import Any, Dict, List, Optional
|
|
||||||
import functools
|
import functools
|
||||||
import gzip
|
import gzip
|
||||||
import itertools
|
import itertools
|
||||||
|
@ -24,24 +22,23 @@ import sys
|
||||||
import tarfile
|
import tarfile
|
||||||
import urllib
|
import urllib
|
||||||
import zipfile
|
import zipfile
|
||||||
|
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
|
from enum import Enum
|
||||||
from errno import ENOENT as NO_SUCH_FILE_OR_DIRECTORY
|
from errno import ENOENT as NO_SUCH_FILE_OR_DIRECTORY
|
||||||
from glob import glob
|
from glob import glob
|
||||||
from os import path
|
from os import path
|
||||||
from subprocess import PIPE
|
from subprocess import PIPE
|
||||||
|
from typing import Any, Dict, List, Optional
|
||||||
from xml.etree.ElementTree import XML
|
from xml.etree.ElementTree import XML
|
||||||
|
|
||||||
import toml
|
import toml
|
||||||
|
|
||||||
from mach.decorators import CommandArgument, CommandArgumentGroup
|
from mach.decorators import CommandArgument, CommandArgumentGroup
|
||||||
from mach.registrar import Registrar
|
from mach.registrar import Registrar
|
||||||
|
|
||||||
from servo.platform.build_target import BuildTarget, AndroidTarget, OpenHarmonyTarget
|
|
||||||
from servo.util import download_file, get_default_cache_dir
|
|
||||||
|
|
||||||
import servo.platform
|
import servo.platform
|
||||||
import servo.util as util
|
import servo.util as util
|
||||||
|
from servo.platform.build_target import AndroidTarget, BuildTarget, OpenHarmonyTarget
|
||||||
|
from servo.util import download_file, get_default_cache_dir
|
||||||
|
|
||||||
from python.servo.platform.build_target import SanitizerKind
|
from python.servo.platform.build_target import SanitizerKind
|
||||||
|
|
||||||
|
@ -804,6 +801,7 @@ class CommandBase(object):
|
||||||
with_debug_assertions=False,
|
with_debug_assertions=False,
|
||||||
with_frame_pointer=False,
|
with_frame_pointer=False,
|
||||||
use_crown=False,
|
use_crown=False,
|
||||||
|
capture_output=False,
|
||||||
target_override: Optional[str] = None,
|
target_override: Optional[str] = None,
|
||||||
**_kwargs,
|
**_kwargs,
|
||||||
):
|
):
|
||||||
|
@ -876,6 +874,9 @@ class CommandBase(object):
|
||||||
# but uv venv on Windows only provides a `python`, not `python3`.
|
# but uv venv on Windows only provides a `python`, not `python3`.
|
||||||
env["PYTHON3"] = "python"
|
env["PYTHON3"] = "python"
|
||||||
|
|
||||||
|
if capture_output:
|
||||||
|
return subprocess.run(["cargo", command] + args + cargo_args, env=env, capture_output=capture_output)
|
||||||
|
|
||||||
return call(["cargo", command] + args + cargo_args, env=env, verbose=verbose)
|
return call(["cargo", command] + args + cargo_args, env=env, verbose=verbose)
|
||||||
|
|
||||||
def android_adb_path(self, env):
|
def android_adb_path(self, env):
|
||||||
|
|
|
@ -7,20 +7,21 @@
|
||||||
# option. This file may not be copied, modified, or distributed
|
# option. This file may not be copied, modified, or distributed
|
||||||
# except according to those terms.
|
# except according to those terms.
|
||||||
|
|
||||||
from os import path, listdir, getcwd
|
import json
|
||||||
|
|
||||||
import signal
|
import signal
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
import tempfile
|
import tempfile
|
||||||
|
from os import getcwd, listdir, path
|
||||||
|
|
||||||
from mach.decorators import (
|
from mach.decorators import (
|
||||||
|
Command,
|
||||||
CommandArgument,
|
CommandArgument,
|
||||||
CommandProvider,
|
CommandProvider,
|
||||||
Command,
|
|
||||||
)
|
)
|
||||||
|
from tidy.linting_report import GitHubAnnotationManager
|
||||||
|
|
||||||
from servo.command_base import CommandBase, cd, call
|
from servo.command_base import CommandBase, call, cd
|
||||||
|
|
||||||
|
|
||||||
@CommandProvider
|
@CommandProvider
|
||||||
|
@ -108,8 +109,14 @@ class MachCommands(CommandBase):
|
||||||
|
|
||||||
@Command("clippy", description='Run "cargo clippy"', category="devenv")
|
@Command("clippy", description='Run "cargo clippy"', category="devenv")
|
||||||
@CommandArgument("params", default=None, nargs="...", help="Command-line arguments to be passed through to clippy")
|
@CommandArgument("params", default=None, nargs="...", help="Command-line arguments to be passed through to clippy")
|
||||||
|
@CommandArgument(
|
||||||
|
"--github-annotations",
|
||||||
|
default=False,
|
||||||
|
action="store_true",
|
||||||
|
help="Emit the clippy warnings in the Github Actions annotations format",
|
||||||
|
)
|
||||||
@CommandBase.common_command_arguments(build_configuration=True, build_type=False)
|
@CommandBase.common_command_arguments(build_configuration=True, build_type=False)
|
||||||
def cargo_clippy(self, params, **kwargs):
|
def cargo_clippy(self, params, github_annotations=False, **kwargs):
|
||||||
if not params:
|
if not params:
|
||||||
params = []
|
params = []
|
||||||
|
|
||||||
|
@ -117,6 +124,23 @@ class MachCommands(CommandBase):
|
||||||
self.ensure_clobbered()
|
self.ensure_clobbered()
|
||||||
env = self.build_env()
|
env = self.build_env()
|
||||||
env["RUSTC"] = "rustc"
|
env["RUSTC"] = "rustc"
|
||||||
|
|
||||||
|
if github_annotations:
|
||||||
|
if "--message-format=json" not in params:
|
||||||
|
params.insert(0, "--message-format=json")
|
||||||
|
|
||||||
|
github_annotation_manager = GitHubAnnotationManager("clippy")
|
||||||
|
|
||||||
|
results = self.run_cargo_build_like_command("clippy", params, env=env, capture_output=True, **kwargs)
|
||||||
|
if results.returncode == 0:
|
||||||
|
return 0
|
||||||
|
try:
|
||||||
|
github_annotation_manager.emit_annotations_for_clippy(
|
||||||
|
[json.loads(line) for line in results.stdout.splitlines() if line.strip()]
|
||||||
|
)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
pass
|
||||||
|
return results.returncode
|
||||||
return self.run_cargo_build_like_command("clippy", params, env=env, **kwargs)
|
return self.run_cargo_build_like_command("clippy", params, env=env, **kwargs)
|
||||||
|
|
||||||
@Command("grep", description="`git grep` for selected directories.", category="devenv")
|
@Command("grep", description="`git grep` for selected directories.", category="devenv")
|
||||||
|
|
|
@ -8,34 +8,32 @@
|
||||||
# except according to those terms.
|
# except according to those terms.
|
||||||
|
|
||||||
import argparse
|
import argparse
|
||||||
|
import json
|
||||||
import logging
|
import logging
|
||||||
import re
|
|
||||||
import sys
|
|
||||||
import os
|
import os
|
||||||
import os.path as path
|
import os.path as path
|
||||||
import platform
|
import platform
|
||||||
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
|
import sys
|
||||||
import textwrap
|
import textwrap
|
||||||
import json
|
|
||||||
|
|
||||||
import servo.devtools_tests
|
import tidy
|
||||||
from servo.post_build_commands import PostBuildCommands
|
|
||||||
import wpt
|
import wpt
|
||||||
import wpt.manifestupdate
|
import wpt.manifestupdate
|
||||||
import wpt.run
|
import wpt.run
|
||||||
import wpt.update
|
import wpt.update
|
||||||
|
|
||||||
from mach.decorators import (
|
from mach.decorators import (
|
||||||
|
Command,
|
||||||
CommandArgument,
|
CommandArgument,
|
||||||
CommandProvider,
|
CommandProvider,
|
||||||
Command,
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
import servo.devtools_tests
|
||||||
import servo.try_parser
|
import servo.try_parser
|
||||||
import tidy
|
|
||||||
|
|
||||||
from servo.command_base import BuildType, CommandBase, call, check_call
|
from servo.command_base import BuildType, CommandBase, call, check_call
|
||||||
|
from servo.post_build_commands import PostBuildCommands
|
||||||
from servo.util import delete
|
from servo.util import delete
|
||||||
|
|
||||||
SCRIPT_PATH = os.path.split(__file__)[0]
|
SCRIPT_PATH = os.path.split(__file__)[0]
|
||||||
|
@ -254,8 +252,14 @@ class MachCommands(CommandBase):
|
||||||
help="Check all files, and run the WPT lint in tidy, even if unchanged",
|
help="Check all files, and run the WPT lint in tidy, even if unchanged",
|
||||||
)
|
)
|
||||||
@CommandArgument("--no-progress", default=False, action="store_true", help="Don't show progress for tidy")
|
@CommandArgument("--no-progress", default=False, action="store_true", help="Don't show progress for tidy")
|
||||||
def test_tidy(self, all_files, no_progress):
|
@CommandArgument(
|
||||||
tidy_failed = tidy.scan(not all_files, not no_progress)
|
"--github-annotations",
|
||||||
|
default=False,
|
||||||
|
action="store_true",
|
||||||
|
help="Emit tidy warnings in the Github Actions annotations format",
|
||||||
|
)
|
||||||
|
def test_tidy(self, all_files, no_progress, github_annotations):
|
||||||
|
tidy_failed = tidy.scan(not all_files, not no_progress, github_annotations)
|
||||||
|
|
||||||
print("\r ➤ Checking formatting of Rust files...")
|
print("\r ➤ Checking formatting of Rust files...")
|
||||||
rustfmt_failed = format_with_rustfmt(check_only=True)
|
rustfmt_failed = format_with_rustfmt(check_only=True)
|
||||||
|
|
114
python/tidy/linting_report.py
Normal file
114
python/tidy/linting_report.py
Normal file
|
@ -0,0 +1,114 @@
|
||||||
|
# Copyright 2025 The Servo Project Developers. See the COPYRIGHT
|
||||||
|
# file at the top-level directory of this distribution.
|
||||||
|
#
|
||||||
|
# Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
|
||||||
|
# http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
|
||||||
|
# <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
|
||||||
|
# option. This file may not be copied, modified, or distributed
|
||||||
|
# except according to those terms.
|
||||||
|
|
||||||
|
from dataclasses import dataclass
|
||||||
|
from typing import Any, Literal, NotRequired
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass
|
||||||
|
class GithubAnnotation:
|
||||||
|
file_name: str
|
||||||
|
line_start: int
|
||||||
|
line_end: int
|
||||||
|
level: Literal["notice", "warning", "error"]
|
||||||
|
title: str
|
||||||
|
message: str
|
||||||
|
column_start: NotRequired[int]
|
||||||
|
column_end: NotRequired[int]
|
||||||
|
|
||||||
|
|
||||||
|
class GitHubAnnotationManager:
|
||||||
|
def __init__(self, annotation_prefix: str, limit: int = 10):
|
||||||
|
self.annotation_prefix: str = annotation_prefix
|
||||||
|
self.limit: int = limit
|
||||||
|
self.total_count: int = 0
|
||||||
|
|
||||||
|
def clean_path(self, path: str):
|
||||||
|
return path.removeprefix("./")
|
||||||
|
|
||||||
|
def escape(self, s: str):
|
||||||
|
return s.replace("\r", "%0D").replace("\n", "%0A")
|
||||||
|
|
||||||
|
def emit_annotation(
|
||||||
|
self,
|
||||||
|
title: str,
|
||||||
|
message: str,
|
||||||
|
file_name: str,
|
||||||
|
line_start: int,
|
||||||
|
line_end: int | None = None,
|
||||||
|
annotation_level: str = "error",
|
||||||
|
column_start: int | None = None,
|
||||||
|
column_end: int | None = None,
|
||||||
|
):
|
||||||
|
if self.total_count >= self.limit:
|
||||||
|
return
|
||||||
|
|
||||||
|
if line_end is None:
|
||||||
|
line_end = line_start
|
||||||
|
|
||||||
|
annotation: GithubAnnotation = {
|
||||||
|
"title": f"{self.annotation_prefix}: {self.escape(title)}",
|
||||||
|
"message": self.escape(message),
|
||||||
|
"file_name": self.clean_path(file_name),
|
||||||
|
"line_start": line_start,
|
||||||
|
"line_end": line_end,
|
||||||
|
"level": annotation_level,
|
||||||
|
}
|
||||||
|
|
||||||
|
if line_start == line_end and column_start is not None and column_end is not None:
|
||||||
|
annotation["column_start"] = column_start
|
||||||
|
annotation["column_end"] = column_end
|
||||||
|
|
||||||
|
line_info = f"line={annotation['line_start']},endLine={annotation['line_end']},title={annotation['title']}"
|
||||||
|
|
||||||
|
column_info = ""
|
||||||
|
if "column_end" in annotation and "column_start" in annotation:
|
||||||
|
column_info = f"col={annotation['column_start']},endColumn={annotation['column_end']},"
|
||||||
|
|
||||||
|
print(
|
||||||
|
f"::{annotation['level']} file={annotation['file_name']},{column_info}{line_info}::{annotation['message']}"
|
||||||
|
)
|
||||||
|
|
||||||
|
self.total_count += 1
|
||||||
|
|
||||||
|
def emit_annotations_for_clippy(self, data: list[dict[str, Any]]):
|
||||||
|
severenty_map: dict[str, Literal["notice", "warning", "error"]] = {
|
||||||
|
"help": "notice",
|
||||||
|
"note": "notice",
|
||||||
|
"warning": "warning",
|
||||||
|
"error": "error",
|
||||||
|
}
|
||||||
|
|
||||||
|
for item in data:
|
||||||
|
if self.total_count >= self.limit:
|
||||||
|
break
|
||||||
|
|
||||||
|
message = item.get("message")
|
||||||
|
if not message:
|
||||||
|
continue
|
||||||
|
|
||||||
|
spans = message.get("spans") or []
|
||||||
|
primary_span = next((span for span in spans if span.get("is_primary")), None)
|
||||||
|
if not primary_span:
|
||||||
|
continue
|
||||||
|
|
||||||
|
annotation_level = severenty_map.get(message.get("level"), "error")
|
||||||
|
title = message.get("message", "")
|
||||||
|
rendered_message = message.get("rendered", "")
|
||||||
|
|
||||||
|
self.emit_annotation(
|
||||||
|
title,
|
||||||
|
rendered_message,
|
||||||
|
primary_span["file_name"],
|
||||||
|
primary_span["line_start"],
|
||||||
|
primary_span["line_end"],
|
||||||
|
annotation_level,
|
||||||
|
primary_span["column_start"],
|
||||||
|
primary_span["column_end"],
|
||||||
|
)
|
|
@ -21,10 +21,10 @@ from typing import Any, Dict, List
|
||||||
|
|
||||||
import colorama
|
import colorama
|
||||||
import toml
|
import toml
|
||||||
|
|
||||||
import wpt.manifestupdate
|
import wpt.manifestupdate
|
||||||
|
|
||||||
from .licenseck import OLD_MPL, MPL, APACHE, COPYRIGHT, licenses_toml
|
from .licenseck import APACHE, COPYRIGHT, MPL, OLD_MPL, licenses_toml
|
||||||
|
from .linting_report import GitHubAnnotationManager
|
||||||
|
|
||||||
TOPDIR = os.path.abspath(os.path.dirname(sys.argv[0]))
|
TOPDIR = os.path.abspath(os.path.dirname(sys.argv[0]))
|
||||||
WPT_PATH = os.path.join(".", "tests", "wpt")
|
WPT_PATH = os.path.join(".", "tests", "wpt")
|
||||||
|
@ -982,7 +982,8 @@ def collect_errors_for_files(files_to_check, checking_functions, line_checking_f
|
||||||
yield (filename,) + error
|
yield (filename,) + error
|
||||||
|
|
||||||
|
|
||||||
def scan(only_changed_files=False, progress=False):
|
def scan(only_changed_files=False, progress=False, github_annotations=False):
|
||||||
|
github_annotation_manager = GitHubAnnotationManager("test-tidy")
|
||||||
# check config file for errors
|
# check config file for errors
|
||||||
config_errors = check_config_file(CONFIG_FILE_PATH)
|
config_errors = check_config_file(CONFIG_FILE_PATH)
|
||||||
# check directories contain expected files
|
# check directories contain expected files
|
||||||
|
@ -1018,6 +1019,9 @@ def scan(only_changed_files=False, progress=False):
|
||||||
+ f"{colorama.Fore.RED}{error[2]}{colorama.Style.RESET_ALL}"
|
+ f"{colorama.Fore.RED}{error[2]}{colorama.Style.RESET_ALL}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
if github_annotations:
|
||||||
|
github_annotation_manager.emit_annotation(error[2], error[2], error[0], error[1])
|
||||||
|
|
||||||
return int(error is not None)
|
return int(error is not None)
|
||||||
|
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue