Skip to content

Commit

Permalink
bazel: Setup clippy (#31460)
Browse files Browse the repository at this point in the history
_Stacked on top of_:
#31459

This PR enables clippy to be run with Bazel. Specifically it does two
things:

1. Updates `cargo-gazelle` to generate an `extract_cargo_lints` rule for
each target, and specifies this as the lint config for `rust_library`,
`rust_binary`, and `rust_test` rules.
2. Runs `bin/bazel gen` to regen all of our build files.

### Motivation

Improve the Bazel build by supporting clippy

### Tips for reviewers

* Only review the first commit, the second commit is the result of
running `bin/bazel gen`

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered. ([trigger-ci for additional test/nightly
runs](https://trigger-ci.dev.materialize.com/))
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [ ] If this PR includes major [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note),
I have pinged the relevant PM to schedule a changelog post.
  • Loading branch information
ParkMyCar authored Feb 14, 2025
1 parent 3b07d8e commit 8d0a888
Show file tree
Hide file tree
Showing 117 changed files with 1,106 additions and 43 deletions.
2 changes: 2 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
# This file exists so the root of our repository, e.g. the root `Cargo.toml`, can be referenced by Bazel.

exports_files(["Cargo.toml"])
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ no_effect = "warn"
unnecessary_unwrap = "warn"
dbg_macro = "warn"
todo = "warn"
wildcard_dependencies = "warn"
zero_prefixed_literal = "warn"
borrowed_box = "warn"
deref_addrof = "warn"
Expand Down
16 changes: 10 additions & 6 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -559,18 +559,18 @@ crates_repository(
},
cargo_config = "//:.cargo/config.toml",
cargo_lockfile = "//:Cargo.lock",
generator_urls = {
"aarch64-apple-darwin": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-aarch64-apple-darwin".format(RULES_RUST_VERSION),
"x86_64-apple-darwin": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-x86_64-apple-darwin".format(RULES_RUST_VERSION),
"aarch64-unknown-linux-gnu": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-aarch64-unknown-linux-gnu".format(RULES_RUST_VERSION),
"x86_64-unknown-linux-gnu": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-x86_64-unknown-linux-gnu".format(RULES_RUST_VERSION),
},
generator_sha256s = {
"aarch64-apple-darwin": "8204746334a17823bd6a54ce2c3821b0bdca96576700d568e2ca2bd8224dc0ea",
"x86_64-apple-darwin": "2ee14b230d32c05415852b7a388b76e700c87c506459e5b31ced19d6c131b6d0",
"aarch64-unknown-linux-gnu": "3792feb084bd43b9a7a9cd75be86ee9910b46db59360d6b29c9cca2f8889a0aa",
"x86_64-unknown-linux-gnu": "2b9d07f34694f63f0cc704989ad6ec148ff8d126579832f4f4d88edea75875b2",
},
generator_urls = {
"aarch64-apple-darwin": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-aarch64-apple-darwin".format(RULES_RUST_VERSION),
"x86_64-apple-darwin": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-x86_64-apple-darwin".format(RULES_RUST_VERSION),
"aarch64-unknown-linux-gnu": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-aarch64-unknown-linux-gnu".format(RULES_RUST_VERSION),
"x86_64-unknown-linux-gnu": "https://github.com/MaterializeInc/rules_rust/releases/download/v{0}/cargo-bazel-x86_64-unknown-linux-gnu".format(RULES_RUST_VERSION),
},
# When `isolated` is true, Bazel will create a new `$CARGO_HOME`, i.e. it
# won't use `~/.cargo`, when re-pinning. This is nice but not totally
# necessary, and it makes re-pinning painfully slow, so we disable it.
Expand Down Expand Up @@ -710,6 +710,10 @@ crate_repositories()

crate_universe_dependencies()

load("@rules_rust//cargo:deps.bzl", "cargo_dependencies")

cargo_dependencies()

# Third-Party Rust Tools
#
# A few crates bind to an external C/C++ library using `cxx`. To build these
Expand Down
10 changes: 10 additions & 0 deletions misc/bazel/cargo-gazelle/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# by the Apache License, Version 2.0.

load("@crates_io//:defs.bzl", "aliases", "all_crate_deps")
load("@rules_rust//cargo:defs.bzl", "extract_cargo_lints")
load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_doc_test", "rust_library", "rust_test")

package(default_visibility = ["//visibility:public"])
Expand All @@ -24,6 +25,7 @@ rust_library(
compile_data = [],
crate_features = ["default"],
data = [],
lint_config = ":lints",
proc_macro_deps = [] + all_crate_deps(proc_macro = True),
rustc_env = {},
rustc_flags = [],
Expand All @@ -50,6 +52,7 @@ rust_test(
crate_features = ["default"],
data = [],
env = {},
lint_config = ":lints",
proc_macro_deps = [] + all_crate_deps(
proc_macro = True,
proc_macro_dev = True,
Expand Down Expand Up @@ -84,9 +87,16 @@ rust_binary(
data = [],
env = {},
features = [],
lint_config = ":lints",
proc_macro_deps = [] + all_crate_deps(proc_macro = True),
rustc_env = {},
rustc_flags = ["-Copt-level=3"],
version = "0.0.0",
deps = [":cargo_gazelle"] + all_crate_deps(normal = True),
)

extract_cargo_lints(
name = "lints",
manifest = "Cargo.toml",
workspace = "@//:Cargo.toml",
)
8 changes: 5 additions & 3 deletions misc/bazel/cargo-gazelle/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ use cargo_gazelle::args::Args;
use cargo_gazelle::config::{CrateConfig, GlobalConfig};
use cargo_gazelle::context::CrateContext;
use cargo_gazelle::header::BazelHeader;
use cargo_gazelle::targets::{CargoBuildScript, RustBinary, RustLibrary, RustTarget, RustTest};
use cargo_gazelle::targets::{
CargoBuildScript, ExtractCargoLints, RustBinary, RustLibrary, RustTarget, RustTest,
};
use cargo_gazelle::BazelBuildFile;
use cargo_toml::Manifest;
use clap::Parser;
Expand Down Expand Up @@ -181,18 +183,17 @@ fn generage_build_bazel<'a>(

let build_script = CargoBuildScript::generate(config, &crate_context, &crate_config, package)?;
let library = RustLibrary::generate(config, package, &crate_config, build_script.as_ref())?;

let integration_tests: Vec<_> = package
.build_targets()
.filter(|target| matches!(target.id(), BuildTargetId::Test(_)))
.map(|target| RustTest::integration(config, package, &crate_config, &target))
.collect::<Result<_, _>>()?;

let binaries: Vec<_> = package
.build_targets()
.filter(|target| matches!(target.id(), BuildTargetId::Binary(_)))
.map(|target| RustBinary::generate(config, package, &crate_config, &target))
.collect::<Result<_, _>>()?;
let lints = ExtractCargoLints::generate();

#[allow(clippy::as_conversions)]
let targets: Vec<Box<dyn RustTarget>> = [Box::new(library) as Box<dyn RustTarget>]
Expand All @@ -213,6 +214,7 @@ fn generage_build_bazel<'a>(
.map(|t| Box::new(t) as Box<dyn RustTarget>),
)
.chain(additive_content.map(|t| Box::new(t) as Box<dyn RustTarget>))
.chain(std::iter::once(lints).map(|t| Box::new(t) as Box<dyn RustTarget>))
.collect();

Ok(Some(BazelBuildFile {
Expand Down
4 changes: 3 additions & 1 deletion misc/bazel/cargo-gazelle/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub enum Rule {
RustDocTest,
RustBinary,
CargoBuildScript,
ExtractCargoLints,
// TODO(parkmycar): Include these rules. The tricky part is they need to
// get imported from the crates_universe repository that is created.
// Aliases,
Expand All @@ -34,7 +35,7 @@ impl Rule {
| Rule::RustTest
| Rule::RustDocTest
| Rule::RustBinary => Module::Rust,
Rule::CargoBuildScript => Module::Cargo,
Rule::CargoBuildScript | Rule::ExtractCargoLints => Module::Cargo,
}
}
}
Expand All @@ -48,6 +49,7 @@ impl ToBazelDefinition for Rule {
Rule::RustDocTest => "rust_doc_test",
Rule::RustBinary => "rust_binary",
Rule::CargoBuildScript => "cargo_build_script",
Rule::ExtractCargoLints => "extract_cargo_lints",
};
let s = QuotedString::new(s);
s.format(writer)
Expand Down
60 changes: 60 additions & 0 deletions misc/bazel/cargo-gazelle/src/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct RustLibrary {
is_proc_macro: bool,
features: Field<List<QuotedString>>,
aliases: Field<Aliases>,
lint_config: Field<QuotedString>,
deps: Field<List<QuotedString>>,
proc_macro_deps: Field<List<QuotedString>>,
data: Field<List<QuotedString>>,
Expand Down Expand Up @@ -147,6 +148,9 @@ impl RustLibrary {
proc_macro_deps = proc_macro_deps.concat_other(select);
}

// TODO(parkmycar): Make the lint_config configurable.
let lint_config = QuotedString::new(":lints");

// For every library we also generate the tests targets.
let unit_test = RustTest::library(config, metadata, crate_config, features.clone())?;
let doc_tests = RustDocTest::generate(config, metadata, crate_config)?;
Expand Down Expand Up @@ -206,6 +210,7 @@ impl RustLibrary {
is_proc_macro: metadata.is_proc_macro(),
features: Field::new("crate_features", features),
aliases: Field::new("aliases", Aliases::default().normal().proc_macro()),
lint_config: Field::new("lint_config", lint_config),
deps: Field::new("deps", deps),
proc_macro_deps: Field::new("proc_macro_deps", proc_macro_deps),
data: Field::new("data", data),
Expand Down Expand Up @@ -239,6 +244,7 @@ impl ToBazelDefinition for RustLibrary {

self.features.format(&mut w)?;
self.aliases.format(&mut w)?;
self.lint_config.format(&mut w)?;
self.deps.format(&mut w)?;
self.proc_macro_deps.format(&mut w)?;
self.data.format(&mut w)?;
Expand All @@ -265,6 +271,7 @@ pub struct RustBinary {
version: Field<QuotedString>,
crate_root: Field<QuotedString>,
features: Field<List<QuotedString>>,
lint_config: Field<QuotedString>,
aliases: Field<Aliases>,
deps: Field<List<QuotedString>>,
proc_macro_deps: Field<List<QuotedString>>,
Expand Down Expand Up @@ -346,6 +353,9 @@ impl RustBinary {
proc_macro_deps = proc_macro_deps.concat_other(select);
}

// TODO(parkmycar): Make the lint_config configurable.
let lint_config = QuotedString::new(":lints");

// Add the library crate as a dep if it isn't already.
if maybe_library.is_some() {
let dep = format!(":{}", metadata.name().to_case(Case::Snake));
Expand Down Expand Up @@ -382,6 +392,7 @@ impl RustBinary {
crate_root: Field::new("crate_root", binary_path),
features: Field::new("features", List::empty()),
aliases: Field::new("aliases", Aliases::default().normal().proc_macro()),
lint_config: Field::new("lint_config", lint_config),
deps: Field::new("deps", deps),
proc_macro_deps: Field::new("proc_macro_deps", proc_macro_deps),
data: Field::new("data", data),
Expand Down Expand Up @@ -409,6 +420,7 @@ impl ToBazelDefinition for RustBinary {

self.features.format(&mut w)?;
self.aliases.format(&mut w)?;
self.lint_config.format(&mut w)?;
self.deps.format(&mut w)?;
self.proc_macro_deps.format(&mut w)?;
self.compile_data.format(&mut w)?;
Expand All @@ -431,6 +443,7 @@ pub struct RustTest {
kind: RustTestKind,
features: Field<List<QuotedString>>,
aliases: Field<Aliases>,
lint_config: Field<QuotedString>,
deps: Field<List<QuotedString>>,
proc_macro_deps: Field<List<QuotedString>>,
size: Field<RustTestSize>,
Expand Down Expand Up @@ -488,6 +501,9 @@ impl RustTest {
proc_macro_deps = proc_macro_deps.concat_other(select);
}

// TODO(parkmycar): Make the lint_config configurable.
let lint_config = QuotedString::new(":lints");

if matches!(kind, RustTestKind::Integration { .. }) {
let dep = format!(":{crate_name}");
if metadata.is_proc_macro() {
Expand Down Expand Up @@ -529,6 +545,7 @@ impl RustTest {
kind,
features: Field::new("crate_features", crate_features),
aliases: Field::new("aliases", aliases),
lint_config: Field::new("lint_config", lint_config),
deps: Field::new("deps", deps),
proc_macro_deps: Field::new("proc_macro_deps", proc_macro_deps),
size: Field::new("size", size),
Expand Down Expand Up @@ -604,6 +621,7 @@ impl ToBazelDefinition for RustTest {
self.kind.format(&mut w)?;
self.features.format(&mut w)?;
self.aliases.format(&mut w)?;
self.lint_config.format(&mut w)?;
self.deps.format(&mut w)?;
self.proc_macro_deps.format(&mut w)?;
self.size.format(&mut w)?;
Expand Down Expand Up @@ -953,6 +971,48 @@ impl ToBazelDefinition for CargoBuildScript {
}
}

/// Reads lint config from a Cargo.toml file.
#[derive(Debug)]
pub struct ExtractCargoLints {
name: Field<QuotedString>,
manifest: Field<QuotedString>,
workspace: Field<QuotedString>,
}

impl RustTarget for ExtractCargoLints {
fn rules(&self) -> Vec<Rule> {
vec![Rule::ExtractCargoLints]
}
}

impl ExtractCargoLints {
pub fn generate() -> Self {
ExtractCargoLints {
name: Field::new("name", QuotedString::new("lints")),
manifest: Field::new("manifest", QuotedString::new("Cargo.toml")),
workspace: Field::new("workspace", QuotedString::new("@//:Cargo.toml")),
}
}
}

impl ToBazelDefinition for ExtractCargoLints {
fn format(&self, writer: &mut dyn fmt::Write) -> Result<(), fmt::Error> {
let mut w = AutoIndentingWriter::new(writer);

writeln!(w, "extract_cargo_lints(")?;
{
let mut w = w.indent();

self.name.format(&mut w)?;
self.manifest.format(&mut w)?;
self.workspace.format(&mut w)?;
}
writeln!(w, ")")?;

Ok(())
}
}

/// An opaque blob of text that we treat as a target.
#[derive(Debug)]
pub struct AdditiveContent(String);
Expand Down
4 changes: 3 additions & 1 deletion misc/python/materialize/cli/gen-lints.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@
"todo",
# Wildcard dependencies are, by definition, incorrect. It is impossible
# to be compatible with all future breaking changes in a crate.
"wildcard_dependencies",
#
# TODO(parkmycar): Re-enable this lint when it's supported in Bazel.
# "wildcard_dependencies",
# Zero-prefixed literals may be incorrectly interpreted as octal literals.
"zero_prefixed_literal",
# Purely redundant tokens.
Expand Down
1 change: 0 additions & 1 deletion misc/wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ no_effect = "warn"
unnecessary_unwrap = "warn"
dbg_macro = "warn"
todo = "warn"
wildcard_dependencies = "warn"
zero_prefixed_literal = "warn"
borrowed_box = "warn"
deref_addrof = "warn"
Expand Down
9 changes: 9 additions & 0 deletions src/adapter-types/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# by the Apache License, Version 2.0.

load("@crates_io//:defs.bzl", "aliases", "all_crate_deps")
load("@rules_rust//cargo:defs.bzl", "extract_cargo_lints")
load("@rules_rust//rust:defs.bzl", "rust_doc_test", "rust_library", "rust_test")

package(default_visibility = ["//visibility:public"])
Expand All @@ -24,6 +25,7 @@ rust_library(
compile_data = [],
crate_features = ["default"],
data = [],
lint_config = ":lints",
proc_macro_deps = [] + all_crate_deps(proc_macro = True),
rustc_env = {},
rustc_flags = [],
Expand Down Expand Up @@ -55,6 +57,7 @@ rust_test(
crate_features = ["default"],
data = [],
env = {},
lint_config = ":lints",
proc_macro_deps = [] + all_crate_deps(
proc_macro = True,
proc_macro_dev = True,
Expand Down Expand Up @@ -86,3 +89,9 @@ rust_doc_test(
normal_dev = True,
),
)

extract_cargo_lints(
name = "lints",
manifest = "Cargo.toml",
workspace = "@//:Cargo.toml",
)
Loading

0 comments on commit 8d0a888

Please sign in to comment.