Fix all lint warnings (#214)

As per `trunk check --all`.

README.md

@@ -71,7 +71,7 @@ llvm_register_toolchains()
 
 And add the following section to your .bazelrc file:
 
-```
+```sh
 # Not needed after https://github.com/bazelbuild/bazel/issues/7260 is closed
 build --incompatible_enable_cc_toolchain_resolution
 
@@ -91,7 +91,7 @@ attributes to `llvm_toolchain`.
 
 ## Advanced Usage
 
-#### Per host architecture LLVM version
+### Per host architecture LLVM version
 
 LLVM does not come with distributions for all host architectures in each
 version. In particular patch versions often come with few prebuilt packages.
@@ -104,7 +104,7 @@ specified explicitly. This is like providing `llvm_version = "15.0.6"`, just
 like in the example on the top. However, here we provide two more entries that
 map their respective target to a distinct version:
 
-```
+```starlark
 llvm_toolchain(
     name = "llvm_toolchain",
     llvm_versions = {
@@ -115,7 +115,7 @@ llvm_toolchain(
 )
 ```
 
-#### Customizations
+### Customizations
 
 We currently offer limited customizability through attributes of the
 [llvm_toolchain\_\* rules](toolchain/rules.bzl). You can send us a PR to add
@@ -127,7 +127,7 @@ new compiler features, etc., my advice would be to look at the toolchain
 configurations generated by this repo, and copy-paste/edit to make your own in
 any package in your own workspace.
 
-```
+```sh
 bazel query --output=build @llvm_toolchain//:all | grep -v -e '^#' -e '^  generator'
 ```
 
@@ -140,7 +140,7 @@ directory, and also create a wrapper script for clang such that the actual
 clang invocation is not through the symlinked path. See the files in the
 `@llvm_toolchain//:` package as a reference.
 
-```
+```sh
 # See generated files for reference.
 ls -lR "$(bazel info output_base)/external/llvm_toolchain"
 
@@ -158,7 +158,7 @@ See [bazel
 tutorial](https://docs.bazel.build/versions/main/tutorial/cc-toolchain-config.html)
 for how CC toolchains work in general.
 
-#### Selecting Toolchains
+### Selecting Toolchains
 
 If toolchains are registered (see Quickstart section above), you do not need to
 do anything special for bazel to find the toolchain. You may want to check once
@@ -172,7 +172,7 @@ For specifying unregistered toolchains on the command line, please use the
 We no longer support the `--crosstool_top=@llvm_toolchain//:toolchain` flag,
 and instead rely on the `--incompatible_enable_cc_toolchain_resolution` flag.
 
-#### Bring Your Own LLVM
+### Bring Your Own LLVM
 
 The following mechanisms are available for using an LLVM toolchain:
 
@@ -204,7 +204,7 @@ The following mechanisms are available for using an LLVM toolchain:
    a toolchain root specified through the `toolchain_roots` attribute with an
    empty key.
 
-#### Sysroots
+### Sysroots
 
 A sysroot can be specified through the `sysroot` attribute. This can be either
 a path on the user's system, or a bazel `filegroup` like label. One way to
@@ -213,7 +213,7 @@ entire filesystem for the image you want. Another way is to use the build
 scripts provided by the [Chromium
 project](https://chromium.googlesource.com/chromium/src/+/HEAD/docs/linux/sysroot.md).
 
-#### Cross-compilation
+### Cross-compilation
 
 The toolchain supports cross-compilation if you bring your own sysroot. When
 cross-compiling, we link against the libstdc++ from the sysroot
@@ -231,14 +231,14 @@ Then, when cross-compiling, explicitly specify the toolchain with the sysroot
 and the target platform. For example, see the [WORKSPACE](tests/WORKSPACE) file and
 the [test script](tests/scripts/run_xcompile_tests.sh) for cross-compilation.
 
-```
+```sh
 bazel build \
   --platforms=@com_grail_bazel_toolchain//platforms:linux-x86_64 \
   --extra_toolchains=@llvm_toolchain_with_sysroot//:cc-toolchain-x86_64-linux \
   //...
 ```
 
-#### Supporting New Target Platforms
+### Supporting New Target Platforms
 
 The following is a rough (untested) list of steps:
 
@@ -256,7 +256,7 @@ The following is a rough (untested) list of steps:
    `toolchain_roots` or `urls` attribute.
 6. Test your build.
 
-#### Sandbox
+### Sandbox
 
 Sandboxing the toolchain introduces a significant overhead (100ms per action,
 as of mid 2018). To overcome this, one can use
@@ -267,12 +267,12 @@ substitute templated paths to the toolchain as absolute paths. When running
 bazel actions, these paths will be available from inside the sandbox as part of
 the / read-only mount. Note that this will make your builds non-hermetic.
 
-#### Compatibility
+### Compatibility
 
 The toolchain is tested to work with `rules_go`, `rules_rust`, and
 `rules_foreign_cc`.
 
-#### Accessing tools
+### Accessing tools
 
 The LLVM distribution also provides several tools like `clang-format`. You can
 depend on these tools directly in the bin directory of the distribution. When

tests/scripts/bazel.sh

@@ -12,7 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-os="$(uname -s | tr "[:upper:]" "[:lower:]")"
+# shellcheck shell=bash
+
+short_uname="$(uname -s)"
+readonly short_uname
+
+os="$(echo "${short_uname}" | tr "[:upper:]" "[:lower:]")"
 readonly os
 
 arch="$(uname -m)"
@@ -47,7 +52,7 @@ common_test_args=(
 
 # TODO: Remove this once we no longer support bazel 6.x.
 # This feature isn't intentionally supported on macOS.
-if [[ $(uname -s) == 'Darwin' ]]; then
+if [[ ${short_uname} == 'Darwin' ]]; then
   common_test_args+=(--features=-supports_dynamic_linker)
 fi
 

tests/scripts/centos_test.sh

@@ -13,6 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# Disable the unreachable code warning because the test is disabled.
+# shellcheck disable=SC2317
+
 echo "This test is disabled because our supported versions of LLVM do not work with CentOS."
 exit 1
 

tests/scripts/run_external_tests.sh

@@ -23,13 +23,12 @@ cd "${scripts_dir}"
 
 # Generate some files needed for the tests.
 "${bazel}" query "${common_args[@]}" @io_bazel_rules_go//tests/core/cgo:dylib_test >/dev/null
-if [[ $USE_BZLMOD == "true" ]]; then
+if [[ ${USE_BZLMOD} == "true" ]]; then
   "$("${bazel}" info output_base)/external/rules_go~0.41.0/tests/core/cgo/generate_imported_dylib.sh"
 else
   "$("${bazel}" info output_base)/external/io_bazel_rules_go/tests/core/cgo/generate_imported_dylib.sh"
 fi
 
-set -x
 test_args=(
   "${common_test_args[@]}"
   # Fix LLVM version to be 14.0.0 because that's the last known version with
@@ -50,12 +49,14 @@ test_args=(
 # @rules_rust//test/unit/{native_deps,linkstamps,interleaved_cc_info}:all
 #   but under bzlmod the linkstamp tests fail due to the fact we are currently
 #   overriding rules_rust locally as its not yet released in the BCR
+# shellcheck disable=SC2207
+absl_targets=($("${bazel}" query "${common_args[@]}" 'attr(timeout, short, tests(@com_google_absl//absl/...))'))
 "${bazel}" --bazelrc=/dev/null test "${test_args[@]}" -- \
   //foreign:pcre \
   @openssl//:libssl \
   @rules_rust//test/unit/interleaved_cc_info:all \
   @io_bazel_rules_go//tests/core/cgo:all \
   -@io_bazel_rules_go//tests/core/cgo:cc_libs_test \
   -@io_bazel_rules_go//tests/core/cgo:external_includes_test \
-  $("${bazel}" query 'attr(timeout, short, tests(@com_google_absl//absl/...))') \
+  "${absl_targets[@]}" \
   -@com_google_absl//absl/time/internal/cctz:time_zone_format_test

tests/scripts/run_tests.sh

@@ -18,15 +18,15 @@ set -euo pipefail
 toolchain_name=""
 
 while getopts "t:h" opt; do
-  case "$opt" in
-  "t") toolchain_name="$OPTARG" ;;
+  case "${opt}" in
+  "t") toolchain_name="${OPTARG}" ;;
   "h")
     echo "Usage:"
     echo "-t - Toolchain name to use for testing; default is llvm_toolchain"
     exit 2
     ;;
-  "?")
-    echo "invalid option: -$OPTARG"
+  *)
+    echo "invalid option: -${OPTARG}"
     exit 1
     ;;
   esac

tests/scripts/suse_leap_test.sh

@@ -25,7 +25,7 @@ toolchain="@llvm_toolchain_13_0_0//:cc-toolchain-x86_64-linux"
 git_root=$(git rev-parse --show-toplevel)
 readonly git_root
 
-echo "git root: $git_root"
+echo "git root: ${git_root}"
 
 for image in "${images[@]}"; do
   docker pull "${image}"

tests/scripts/suse_tumbleweed_test.sh

@@ -25,7 +25,7 @@ toolchain="@llvm_toolchain_13_0_0//:cc-toolchain-x86_64-linux"
 git_root=$(git rev-parse --show-toplevel)
 readonly git_root
 
-echo "git root: $git_root"
+echo "git root: ${git_root}"
 
 for image in "${images[@]}"; do
   docker pull "${image}"

toolchain/cc_wrapper.sh.tpl

@@ -13,7 +13,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-#
+
 # OS X relpath is not really working. This is a wrapper script around gcc
 # to simulate relpath behavior.
 #
@@ -24,7 +24,9 @@
 #
 # See https://blogs.oracle.com/dipol/entry/dynamic_libraries_rpath_and_mac
 # on how to set those paths for Mach-O binaries.
-#
+
+# shellcheck disable=SC1083
+
 set -eu
 
 # See note in toolchain/internal/configure.bzl where we define
@@ -43,6 +45,6 @@ elif [[ ${BASH_SOURCE[0]} == "/"* ]]; then
   clang="${execroot_path}/%{toolchain_path_prefix}bin/clang"
   exec "${clang}" "${@}"
 else
-  echo >&2 "ERROR: could not find clang; PWD=\"$(pwd)\"; PATH=\"${PATH}\"."
+  echo >&2 "ERROR: could not find clang; PWD=\"${PWD}\"; PATH=\"${PATH}\"."
   exit 5
 fi

toolchain/internal/common.bzl

@@ -41,8 +41,8 @@ def _linux_dist(rctx):
     if res.return_code:
         fail("Failed to detect machine architecture: \n%s\n%s" % (res.stdout, res.stderr))
     info = {}
-    for l in res.stdout.splitlines():
-        parts = l.split("=", 1)
+    for line in res.stdout.splitlines():
+        parts = line.split("=", 1)
         info[parts[0]] = parts[1]
 
     distname = info["ID"].strip('\"')
@@ -122,12 +122,12 @@ def host_os_arch_dict_value(rctx, attr_name, debug = False):
 
     key2 = os_arch_pair(os(rctx), arch(rctx))
     if debug:
-        print("`%s` attribute missing for key '%s' in repository '%s'; checking with key '%s'" % (attr_name, key1, rctx.name, key2))
+        print("`%s` attribute missing for key '%s' in repository '%s'; checking with key '%s'" % (attr_name, key1, rctx.name, key2))  # buildifier: disable=print
     if key2 in d:
         return (key2, d.get(key2))
 
     if debug:
-        print("`%s` attribute missing for key '%s' in repository '%s'; checking with key ''" % (attr_name, key2, rctx.name))
+        print("`%s` attribute missing for key '%s' in repository '%s'; checking with key ''" % (attr_name, key2, rctx.name))  # buildifier: disable=print
     return ("", d.get(""))  # Fallback to empty key.
 
 def canonical_dir_path(path):
@@ -147,10 +147,10 @@ def pkg_path_from_label(label):
     else:
         return label.package
 
-def list_to_string(l):
-    if l == None:
+def list_to_string(ls):
+    if ls == None:
         return "None"
-    return "[{}]".format(", ".join(["\"{}\"".format(d) for d in l]))
+    return "[{}]".format(", ".join(["\"{}\"".format(d) for d in ls]))
 
 def attr_dict(attr):
     # Returns a mutable dict of attr values from the struct. This is useful to

toolchain/internal/configure.bzl

@@ -42,12 +42,6 @@ load(
 # workspace builds, there is never a @@ in labels.
 BZLMOD_ENABLED = "@@" in str(Label("//:unused"))
 
-def _include_dirs_str(rctx, key):
-    dirs = rctx.attr.cxx_builtin_include_directories.get(key)
-    if not dirs:
-        return ""
-    return ("\n" + 12 * " ").join(["\"%s\"," % d for d in dirs])
-
 def llvm_config_impl(rctx):
     _check_os_arch_keys(rctx.attr.sysroot)
     _check_os_arch_keys(rctx.attr.cxx_builtin_include_directories)

toolchain/internal/llvm_distributions.bzl

@@ -336,6 +336,9 @@ def _get_auth(ctx, urls):
 
 def download_llvm(rctx):
     urls = []
+    sha256 = None
+    strip_prefix = None
+    key = None
     update_sha256 = False
     if rctx.attr.urls:
         urls, sha256, strip_prefix, key = _urls(rctx)
@@ -359,7 +362,7 @@ def download_llvm(rctx):
 def _urls(rctx):
     (key, urls) = _host_os_arch_dict_value(rctx, "urls", debug = False)
     if not urls:
-        print("LLVM archive URLs missing and no default fallback provided; will try 'distribution' attribute")
+        print("LLVM archive URLs missing and no default fallback provided; will try 'distribution' attribute")  # buildifier: disable=print
 
     sha256 = rctx.attr.sha256.get(key, default = "")
     strip_prefix = rctx.attr.strip_prefix.get(key, default = "")
@@ -371,7 +374,7 @@ def _get_llvm_version(rctx):
         return rctx.attr.llvm_version
     if not rctx.attr.llvm_versions:
         fail("Neither 'llvm_version' nor 'llvm_versions' given.")
-    (key, llvm_version) = _host_os_arch_dict_value(rctx, "llvm_versions")
+    (_, llvm_version) = _host_os_arch_dict_value(rctx, "llvm_versions")
     if not llvm_version:
         fail("LLVM version string missing for ({os}, {arch})", os = _os(rctx), arch = _arch(rctx))
     return llvm_version

toolchain/internal/release_name.bzl

@@ -16,7 +16,7 @@ def _darwin_apple_suffix(llvm_version, arch):
     major_llvm_version = _major_llvm_version(llvm_version)
     patch_llvm_version = _patch_llvm_version(llvm_version)
     if major_llvm_version == 9:
-        "darwin-apple"
+        return "darwin-apple"
     elif major_llvm_version >= 15:
         if arch == "arm64":
             if major_llvm_version == 15 and patch_llvm_version <= 6:
@@ -93,6 +93,7 @@ def _linux(llvm_version, distname, version, arch):
 
     # NOTE: Many of these systems are untested because I do not have access to them.
     # If you find this mapping wrong, please send a Pull Request on Github.
+    os_name = None
     if arch in ["aarch64", "armv7a", "mips", "mipsel"]:
         os_name = "linux-gnu"
     elif distname == "freebsd":
@@ -134,7 +135,8 @@ def _linux(llvm_version, distname, version, arch):
             os_name = _ubuntu_osname(arch, "18.04", major_llvm_version, llvm_version)
         elif float(version) >= 9:
             os_name = _ubuntu_osname(arch, "20.04", major_llvm_version, llvm_version)
-    else:
+
+    if not os_name:
         fail("Unsupported linux distribution and version: %s, %s" % (distname, version))
 
     return "clang+llvm-{llvm_version}-{arch}-{os_name}.tar.xz".format(

toolchain/internal/repo.bzl

@@ -269,8 +269,8 @@ llvm_config_attrs.update({
 def llvm_repo_impl(rctx):
     os = _os(rctx)
     if os == "windows":
-        rctx.file("BUILD", executable = False)
-        return
+        rctx.file("BUILD.bazel", executable = False)
+        return None
 
     rctx.file(
         "BUILD.bazel",

toolchain/internal/sysroot.bzl

@@ -26,7 +26,7 @@ def _darwin_sdk_path(rctx):
     if exec_result.return_code:
         fail("Failed to detect OSX SDK path: \n%s\n%s" % (exec_result.stdout, exec_result.stderr))
     if exec_result.stderr:
-        print(exec_result.stderr)
+        print(exec_result.stderr)  # buildifier: disable=print
     return exec_result.stdout.strip()
 
 # Default sysroot path can be used when the user has not provided an explicit