From b525b885bf59ce674653948025ba5d5942584d97 Mon Sep 17 00:00:00 2001 From: Tim Trippel Date: Mon, 9 Sep 2024 11:36:28 -0700 Subject: [PATCH] [quality] consolidate lint rule implementations This consolidates the lint rule implementations for clang_format, gofmt, and protolint, so a common rule implementation and template script is used for all lint rules, which operate in a similar fashion. Additionally, this updates the clang_format lint rule to use the clang_format binary included in the lowRISC RV32 toolchain, for improved build reproducibility. Signed-off-by: Tim Trippel --- quality/BUILD.bazel | 1 + rules/quality.bzl | 130 +++++++----------- rules/scripts/clang_format.sh | 21 +++ rules/scripts/clang_format.template.sh | 42 ------ ...t.template.sh => general_lint.template.sh} | 27 ++-- rules/scripts/gofmt.sh | 17 +++ rules/scripts/gofmt.template.sh | 37 ----- rules/scripts/protolint.sh | 17 +++ rules/util.bzl | 18 +++ src/ate/ate_api.h | 2 +- third_party/lint/BUILD.clang-format.bazel | 7 + third_party/lint/repos.bzl | 7 + 12 files changed, 147 insertions(+), 179 deletions(-) create mode 100644 rules/scripts/clang_format.sh delete mode 100644 rules/scripts/clang_format.template.sh rename rules/scripts/{protolint.template.sh => general_lint.template.sh} (73%) create mode 100644 rules/scripts/gofmt.sh delete mode 100644 rules/scripts/gofmt.template.sh create mode 100644 rules/scripts/protolint.sh create mode 100644 rules/util.bzl create mode 100644 third_party/lint/BUILD.clang-format.bazel diff --git a/quality/BUILD.bazel b/quality/BUILD.bazel index 28078a3..97b3502 100644 --- a/quality/BUILD.bazel +++ b/quality/BUILD.bazel @@ -60,6 +60,7 @@ gofmt_check( name = "gofmt_check", mode = "diff", tags = ["lint"], + workspace = "//:WORKSPACE", ) gofmt_fix( diff --git a/rules/quality.bzl b/rules/quality.bzl index dc6e868..62e2f00 100644 --- a/rules/quality.bzl +++ b/rules/quality.bzl @@ -12,33 +12,39 @@ def _ensure_tag(tags, *tag): tags.append(t) return tags -################################################################################ -# gofmt -################################################################################ -def _gofmt_impl(ctx): +def _general_lint_impl(ctx): out_file = ctx.actions.declare_file(ctx.label.name + ".bash") exclude_patterns = ["\\! -path {}".format(shell.quote(p)) for p in ctx.attr.exclude_patterns] include_patterns = ["-name {}".format(shell.quote(p)) for p in ctx.attr.patterns] + workspace = ctx.file.workspace.path if ctx.file.workspace else "" substitutions = { "@@EXCLUDE_PATTERNS@@": " ".join(exclude_patterns), "@@INCLUDE_PATTERNS@@": " -o ".join(include_patterns), - "@@GOFMT@@": shell.quote(ctx.executable.gofmt.short_path), - "@@DIFF_COMMAND@@": shell.quote(ctx.attr.diff_command), + "@@LINT_TOOL@@": shell.quote(ctx.executable.lint_tool.short_path), "@@MODE@@": shell.quote(ctx.attr.mode), + "@@WORKSPACE@@": workspace, + "@@RUNNER_SH@@": ctx.file._runner.path, } ctx.actions.expand_template( - template = ctx.file._runner, + template = ctx.file._runner_general, output = out_file, substitutions = substitutions, is_executable = True, ) + runfiles = [ctx.executable.lint_tool, ctx.file._runner] + if ctx.file.workspace: + runfiles.append(ctx.file.workspace) + return DefaultInfo( files = depset([out_file]), - runfiles = ctx.runfiles(files = [ctx.executable.gofmt]), + runfiles = ctx.runfiles(files = runfiles), executable = out_file, ) +################################################################################ +# gofmt +################################################################################ gofmt_attrs = { "patterns": attr.string_list( default = ["*.go"], @@ -52,31 +58,35 @@ gofmt_attrs = { values = ["diff", "fix"], doc = "Execution mode: display diffs or fix formatting", ), - "diff_command": attr.string( - default = "diff -u", - doc = "Command to execute to display diffs", - ), - "gofmt": attr.label( + "lint_tool": attr.label( default = "@go_sdk//:bin/gofmt", allow_single_file = True, cfg = "host", executable = True, doc = "The gofmt executable", ), + "workspace": attr.label( + allow_single_file = True, + doc = "Label of the WORKSPACE file", + ), "_runner": attr.label( - default = "//rules/scripts:gofmt.template.sh", + default = "//rules/scripts:gofmt.sh", + allow_single_file = True, + ), + "_runner_general": attr.label( + default = "//rules/scripts:general_lint.template.sh", allow_single_file = True, ), } gofmt_fix = rule( - implementation = _gofmt_impl, + implementation = _general_lint_impl, attrs = gofmt_attrs, executable = True, ) _gofmt_test = rule( - implementation = _gofmt_impl, + implementation = _general_lint_impl, attrs = gofmt_attrs, test = True, ) @@ -91,31 +101,6 @@ def gofmt_check(**kwargs): ################################################################################ # clang-format ################################################################################ -def _clang_format_impl(ctx): - out_file = ctx.actions.declare_file(ctx.label.name + ".bash") - exclude_patterns = ["\\! -path {}".format(shell.quote(p)) for p in ctx.attr.exclude_patterns] - include_patterns = ["-name {}".format(shell.quote(p)) for p in ctx.attr.patterns] - workspace = ctx.file.workspace.path if ctx.file.workspace else "" - substitutions = { - "@@EXCLUDE_PATTERNS@@": " ".join(exclude_patterns), - "@@INCLUDE_PATTERNS@@": " -o ".join(include_patterns), - "@@CLANG_FORMAT@@": shell.quote(ctx.attr.clang_format_command), - "@@DIFF_COMMAND@@": shell.quote(ctx.attr.diff_command), - "@@MODE@@": shell.quote(ctx.attr.mode), - "@@WORKSPACE@@": workspace, - } - ctx.actions.expand_template( - template = ctx.file._runner, - output = out_file, - substitutions = substitutions, - is_executable = True, - ) - - return DefaultInfo( - files = depset([out_file]), - executable = out_file, - ) - clang_format_attrs = { "patterns": attr.string_list( default = ["*.c", "*.h", "*.cc", "*.cpp"], @@ -129,12 +114,11 @@ clang_format_attrs = { values = ["diff", "fix"], doc = "Execution mode: display diffs or fix formatting", ), - "diff_command": attr.string( - default = "diff -u", - doc = "Command to execute to display diffs", - ), - "clang_format_command": attr.string( - default = "clang-format", + "lint_tool": attr.label( + default = "@clang-format//:clang-format", + allow_single_file = True, + cfg = "host", + executable = True, doc = "The clang-format executable", ), "workspace": attr.label( @@ -142,19 +126,23 @@ clang_format_attrs = { doc = "Label of the WORKSPACE file", ), "_runner": attr.label( - default = "//rules/scripts:clang_format.template.sh", + default = "//rules/scripts:clang_format.sh", + allow_single_file = True, + ), + "_runner_general": attr.label( + default = "//rules/scripts:general_lint.template.sh", allow_single_file = True, ), } clang_format_fix = rule( - implementation = _clang_format_impl, + implementation = _general_lint_impl, attrs = clang_format_attrs, executable = True, ) _clang_format_test = rule( - implementation = _clang_format_impl, + implementation = _general_lint_impl, attrs = clang_format_attrs, test = True, ) @@ -169,36 +157,6 @@ def clang_format_check(**kwargs): ################################################################################ # protolint ################################################################################ -def _protolint_impl(ctx): - out_file = ctx.actions.declare_file(ctx.label.name + ".bash") - exclude_patterns = ["\\! -path {}".format(shell.quote(p)) for p in ctx.attr.exclude_patterns] - include_patterns = ["-name {}".format(shell.quote(p)) for p in ctx.attr.patterns] - workspace = ctx.file.workspace.path if ctx.file.workspace else "" - substitutions = { - "@@EXCLUDE_PATTERNS@@": " ".join(exclude_patterns), - "@@INCLUDE_PATTERNS@@": " -o ".join(include_patterns), - "@@PROTOLINT@@": shell.quote(ctx.executable.protolint.short_path), - "@@MODE@@": shell.quote(ctx.attr.mode), - "@@WORKSPACE@@": workspace, - } - ctx.actions.expand_template( - template = ctx.file._runner, - output = out_file, - substitutions = substitutions, - is_executable = True, - ) - - runfiles = [ctx.executable.protolint] - if ctx.file.workspace: - runfiles.append(ctx.file.workspace) - - return DefaultInfo( - files = depset([out_file]), - runfiles = ctx.runfiles(files = runfiles), - executable = out_file, - ) - return - protolint_attrs = { "patterns": attr.string_list( default = ["*.proto"], @@ -212,7 +170,7 @@ protolint_attrs = { values = ["diff", "fix"], doc = "Execution mode: display diffs or fix formatting.", ), - "protolint": attr.label( + "lint_tool": attr.label( default = "@protolint//:protolint", allow_single_file = True, cfg = "host", @@ -224,19 +182,23 @@ protolint_attrs = { doc = "Label of the WORKSPACE file", ), "_runner": attr.label( - default = "//rules/scripts:protolint.template.sh", + default = "//rules/scripts:protolint.sh", + allow_single_file = True, + ), + "_runner_general": attr.label( + default = "//rules/scripts:general_lint.template.sh", allow_single_file = True, ), } protolint_fix = rule( - implementation = _protolint_impl, + implementation = _general_lint_impl, attrs = protolint_attrs, executable = True, ) _protolint_test = rule( - implementation = _protolint_impl, + implementation = _general_lint_impl, attrs = protolint_attrs, test = True, ) diff --git a/rules/scripts/clang_format.sh b/rules/scripts/clang_format.sh new file mode 100644 index 0000000..ec70188 --- /dev/null +++ b/rules/scripts/clang_format.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +case "$MODE" in + diff) + RESULT=0 + for f in $FILES; do + diff -Naur "$f" <(${lint_tool} ${f}) + RESULT=$(($RESULT | $?)) + done + exit $RESULT + ;; + fix) + echo "$FILES" | xargs ${lint_tool} -i + ;; + *) + echo "Unknown mode: $MODE" + exit 2 +esac diff --git a/rules/scripts/clang_format.template.sh b/rules/scripts/clang_format.template.sh deleted file mode 100644 index e25a36c..0000000 --- a/rules/scripts/clang_format.template.sh +++ /dev/null @@ -1,42 +0,0 @@ -#!/usr/bin/env bash -# -# Copyright lowRISC contributors (OpenTitan project). -# Licensed under the Apache License, Version 2.0, see LICENSE for details. -# SPDX-License-Identifier: Apache-2.0 - -CLANG_FORMAT=@@CLANG_FORMAT@@ -MODE=@@MODE@@ - -clang_format=${CLANG_FORMAT} - -if ! cd "$BUILD_WORKSPACE_DIRECTORY"; then - echo "Unable to change to workspace (BUILD_WORKSPACE_DIRECTORY: ${BUILD_WORKSPACE_DIRECTORY})" - exit 1 -fi - -if [[ $# != 0 ]]; then - FILES="$@" -else - FILES=$(find . \ - -type f \ - @@EXCLUDE_PATTERNS@@ \ - \( @@INCLUDE_PATTERNS@@ \) \ - -print) -fi - -case "$MODE" in - diff) - RESULT=0 - for f in $FILES; do - diff -Naur "$f" <(${clang_format} ${f}) - RESULT=$(($RESULT | $?)) - done - exit $RESULT - ;; - fix) - echo "$FILES" | xargs ${clang_format} -i - ;; - *) - echo "Unknown mode: $MODE" - exit 2 -esac diff --git a/rules/scripts/protolint.template.sh b/rules/scripts/general_lint.template.sh similarity index 73% rename from rules/scripts/protolint.template.sh rename to rules/scripts/general_lint.template.sh index df0245b..aa7cd3d 100644 --- a/rules/scripts/protolint.template.sh +++ b/rules/scripts/general_lint.template.sh @@ -3,11 +3,14 @@ # Licensed under the Apache License, Version 2.0, see LICENSE for details. # SPDX-License-Identifier: Apache-2.0 -PROTOLINT=@@PROTOLINT@@ +# Set mode and lint tool. +LINT_TOOL=@@LINT_TOOL@@ MODE=@@MODE@@ WORKSPACE="@@WORKSPACE@@" +RUNNER_SH=@@RUNNER_SH@@ -protolint=$(readlink "$PROTOLINT") +lint_tool=$(readlink "$LINT_TOOL") +runner_sh=$(readlink "$RUNNER_SH") # Change directories based on whether the mode is to "fix" or to "diff". if [[ -n "${WORKSPACE}" ]]; then @@ -32,16 +35,10 @@ else -print) fi -# Perfom the "diff" or "fix" operation. -case "$MODE" in - diff) - echo "$FILES" | xargs "${protolint}" - exit $? - ;; - fix) - echo "$FILES" | xargs "${protolint}" -fix - ;; - *) - echo "Unknown mode: $MODE" - exit 2 -esac +if [[ -z "$FILES" ]]; then + echo "Error no files found to lint for pattern: \"$INCLUDE_PATTERNS\"." + exit 1 +fi + +# Execute the runner script. +source "$runner_sh" diff --git a/rules/scripts/gofmt.sh b/rules/scripts/gofmt.sh new file mode 100644 index 0000000..a1edb68 --- /dev/null +++ b/rules/scripts/gofmt.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +case "$MODE" in + diff) + echo "$FILES" | xargs ${lint_tool} -s -d + exit $? + ;; + fix) + echo "$FILES" | xargs ${lint_tool} -s -w + ;; + *) + echo "Unknown mode: $MODE" + exit 2 +esac diff --git a/rules/scripts/gofmt.template.sh b/rules/scripts/gofmt.template.sh deleted file mode 100644 index 755566b..0000000 --- a/rules/scripts/gofmt.template.sh +++ /dev/null @@ -1,37 +0,0 @@ -#!/usr/bin/env bash -# Copyright lowRISC contributors (OpenTitan project). -# Licensed under the Apache License, Version 2.0, see LICENSE for details. -# SPDX-License-Identifier: Apache-2.0 - -GOFMT=@@GOFMT@@ -MODE=@@MODE@@ - -gofmt=$(readlink "$GOFMT") - -if ! cd "$BUILD_WORKSPACE_DIRECTORY"; then - echo "Unable to change to workspace (BUILD_WORKSPACE_DIRECTORY: ${BUILD_WORKSPACE_DIRECTORY})" - exit 1 -fi - -if [[ $# != 0 ]]; then - FILES="$@" -else - FILES=$(find . \ - -type f \ - @@EXCLUDE_PATTERNS@@ \ - \( @@INCLUDE_PATTERNS@@ \) \ - -print) -fi - -case "$MODE" in - diff) - echo "$FILES" | xargs ${gofmt} -s -d - exit $? - ;; - fix) - echo "$FILES" | xargs ${gofmt} -s -w - ;; - *) - echo "Unknown mode: $MODE" - exit 2 -esac diff --git a/rules/scripts/protolint.sh b/rules/scripts/protolint.sh new file mode 100644 index 0000000..b0a1f08 --- /dev/null +++ b/rules/scripts/protolint.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +case "$MODE" in + diff) + echo "$FILES" | xargs "${lint_tool}" + exit $? + ;; + fix) + echo "$FILES" | xargs "${lint_tool}" -fix + ;; + *) + echo "Unknown mode: $MODE" + exit 2 +esac diff --git a/rules/util.bzl b/rules/util.bzl new file mode 100644 index 0000000..6f0eae2 --- /dev/null +++ b/rules/util.bzl @@ -0,0 +1,18 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +def _deb_package_impl(rctx): + rctx.download_and_extract(rctx.attr.url, sha256 = rctx.attr.sha256) + rctx.extract("data.tar.xz") + rctx.symlink("usr/bin/clang-format-14", "clang-format") + rctx.symlink(rctx.attr.build_file, "BUILD") + +deb_package = repository_rule( + implementation = _deb_package_impl, + attrs = { + "url": attr.string(mandatory = True), + "sha256": attr.string(mandatory = True), + "build_file": attr.label(mandatory = True), + }, +) diff --git a/src/ate/ate_api.h b/src/ate/ate_api.h index bd39e69..47af937 100644 --- a/src/ate/ate_api.h +++ b/src/ate/ate_api.h @@ -93,7 +93,7 @@ typedef struct Blob { * ate_client_ptr is an opaque pointer to an AteClient instance. */ typedef struct { -}* ate_client_ptr; +} * ate_client_ptr; typedef struct { // Endpoint address in IP or DNS format including port number. For example: diff --git a/third_party/lint/BUILD.clang-format.bazel b/third_party/lint/BUILD.clang-format.bazel new file mode 100644 index 0000000..c55986a --- /dev/null +++ b/third_party/lint/BUILD.clang-format.bazel @@ -0,0 +1,7 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +package(default_visibility = ["//visibility:public"]) + +exports_files(["clang-format"]) diff --git a/third_party/lint/repos.bzl b/third_party/lint/repos.bzl index 6a455e3..f49872f 100644 --- a/third_party/lint/repos.bzl +++ b/third_party/lint/repos.bzl @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +load("@//rules:util.bzl", "deb_package") def lint_repos(lowrisc_lint = None): http_archive( @@ -17,3 +18,9 @@ def lint_repos(lowrisc_lint = None): build_file = Label("//third_party/lint:BUILD.protolint.bazel"), url = "https://github.com/yoheimuta/protolint/releases/download/v0.50.5/protolint_0.50.5_linux_amd64.tar.gz", ) + deb_package( + name = "clang-format", + url = "http://ftp.us.debian.org/debian/pool/main/l/llvm-toolchain-14/clang-format-14_14.0.6-12_amd64.deb", + sha256 = "3fbcadd614577667f6ff5958cc70b01fcf0b6a27f723a01516ced294b83d1002", + build_file = Label("//third_party/lint:BUILD.clang-format.bazel"), + )