Skip to content

New lint: missing_workspace_lints #15092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6051,6 +6051,7 @@ Released 2018-09-13
[`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop
[`missing_trait_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_trait_methods
[`missing_transmute_annotations`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_transmute_annotations
[`missing_workspace_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_workspace_lints
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_attributes_style`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_attributes_style
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
Expand Down
53 changes: 53 additions & 0 deletions clippy_lints/src/cargo/missing_workspace_lints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
use super::MISSING_WORKSPACE_LINTS;
use cargo_metadata::Metadata;
use clippy_utils::diagnostics::span_lint;
use rustc_lint::LateContext;
use rustc_span::DUMMY_SP;
use serde::Deserialize;
use std::path::Path;

type Lints = toml::Table;

#[derive(Deserialize, Debug, Default)]
struct Workspace {
#[serde(default)]
lints: Lints,
}

#[derive(Deserialize, Debug)]
struct CargoToml {
#[serde(default)]
lints: Lints,
#[serde(default)]
workspace: Workspace,
}

pub fn check(cx: &LateContext<'_>, metadata: &Metadata) {
if let Ok(file) = cx.tcx.sess.source_map().load_file(Path::new("Cargo.toml"))
&& let Some(src) = file.src.as_deref()
&& let Ok(cargo_toml) = toml::from_str::<CargoToml>(src)
// if `[workspace.lints]` exists,
&& !cargo_toml.workspace.lints.is_empty()
{
// for each project that is included in the workspace,
for package in &metadata.packages {
// if the project's Cargo.toml doesn't have lints.workspace = true
if let Ok(file) = cx.tcx.sess.source_map().load_file(package.manifest_path.as_std_path())
&& let Some(src) = file.src.as_deref()
&& let Ok(cargo_toml) = toml::from_str::<CargoToml>(src)
&& !cargo_toml.lints.contains_key("workspace")
{
// TODO: Make real span
span_lint(
cx,
MISSING_WORKSPACE_LINTS,
DUMMY_SP,
format!(
"Your project {} is in a workspace with lints configured, but workspace.lints is not configured.",
package.name
),
);
}
}
}
}
41 changes: 41 additions & 0 deletions clippy_lints/src/cargo/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
mod common_metadata;
mod feature_name;
mod lint_groups_priority;
mod missing_workspace_lints;
mod multiple_crate_versions;
mod wildcard_dependencies;

Expand Down Expand Up @@ -213,6 +214,43 @@ declare_clippy_lint! {
"a lint group in `Cargo.toml` at the same priority as a lint"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for lint groups with the same priority as lints in the `Cargo.toml`
/// [`[lints]` table](https://doc.rust-lang.org/cargo/reference/manifest.html#the-lints-section).
///
/// This lint will be removed once [cargo#12918](https://github.com/rust-lang/cargo/issues/12918)
/// is resolved.
///
/// ### Why is this bad?
/// The order of lints in the `[lints]` is ignored, to have a lint override a group the
/// `priority` field needs to be used, otherwise the sort order is undefined.
///
/// ### Known problems
/// Does not check lints inherited using `lints.workspace = true`
///
/// ### Example
/// ```toml
/// # Passed as `--allow=clippy::similar_names --warn=clippy::pedantic`
/// # which results in `similar_names` being `warn`
/// [lints.clippy]
/// pedantic = "warn"
/// similar_names = "allow"
/// ```
/// Use instead:
/// ```toml
/// # Passed as `--warn=clippy::pedantic --allow=clippy::similar_names`
/// # which results in `similar_names` being `allow`
/// [lints.clippy]
/// pedantic = { level = "warn", priority = -1 }
/// similar_names = "allow"
/// ```
#[clippy::version = "1.88.0"]
pub MISSING_WORKSPACE_LINTS,
cargo,
"a workspace defines a lint but workspaces.lint is not set to true"
}

pub struct Cargo {
allowed_duplicate_crates: FxHashSet<String>,
ignore_publish: bool,
Expand All @@ -225,6 +263,7 @@ impl_lint_pass!(Cargo => [
MULTIPLE_CRATE_VERSIONS,
WILDCARD_DEPENDENCIES,
LINT_GROUPS_PRIORITY,
MISSING_WORKSPACE_LINTS
]);

impl Cargo {
Expand All @@ -243,6 +282,7 @@ impl LateLintPass<'_> for Cargo {
REDUNDANT_FEATURE_NAMES,
NEGATIVE_FEATURE_NAMES,
WILDCARD_DEPENDENCIES,
MISSING_WORKSPACE_LINTS,
];
static WITH_DEPS_LINTS: &[&Lint] = &[MULTIPLE_CRATE_VERSIONS];

Expand All @@ -257,6 +297,7 @@ impl LateLintPass<'_> for Cargo {
common_metadata::check(cx, &metadata, self.ignore_publish);
feature_name::check(cx, &metadata);
wildcard_dependencies::check(cx, &metadata);
missing_workspace_lints::check(cx, &metadata);
},
Err(e) => {
for lint in NO_DEPS_LINTS {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::byte_char_slices::BYTE_CHAR_SLICES_INFO,
crate::cargo::CARGO_COMMON_METADATA_INFO,
crate::cargo::LINT_GROUPS_PRIORITY_INFO,
crate::cargo::MISSING_WORKSPACE_LINTS_INFO,
crate::cargo::MULTIPLE_CRATE_VERSIONS_INFO,
crate::cargo::NEGATIVE_FEATURE_NAMES_INFO,
crate::cargo::REDUNDANT_FEATURE_NAMES_INFO,
Expand Down
59 changes: 59 additions & 0 deletions tests/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,65 @@ fn test_module_style_with_dep_in_subdir() {
assert!(output.status.success());
}

#[test]
fn test_missing_workspace_lints() {
if IS_RUSTC_TEST_SUITE {
return;
}
let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let target_dir = root.join("target").join("workspace_test");
let cwd = root.join("tests/workspace_test");

// Make sure we start with a clean state
Command::new("cargo")
.current_dir(&cwd)
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clean")
.args(["-p", "fail-missing-workspace-lints"])
.args(["-p", "pass-missing-workspace-lints"])
.output()
.unwrap();

let output = Command::new(&*CARGO_CLIPPY_PATH)
.current_dir(&cwd)
.env("CARGO_INCREMENTAL", "0")
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clippy")
.args(["-p", "pass-missing-workspace-lints"])
.arg("--")
.arg("--no-deps")
.arg("-Dclippy::missing-workspace-lints")
.arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
.output()
.unwrap();

println!("status: {}", output.status);
println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
assert!(output.status.success());

let output_fail = Command::new(&*CARGO_CLIPPY_PATH)
.current_dir(&cwd)
.env("CARGO_INCREMENTAL", "0")
.env("CARGO_TARGET_DIR", &target_dir)
.arg("clippy")
.args(["-p", "fail-missing-workspace-lints"])
.arg("--")
.arg("--no-deps")
.arg("-Dclippy::missing-workspace-lints")
.arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
.output()
.unwrap();

println!("status: {}", output_fail.status);
println!("stdout: {}", String::from_utf8_lossy(&output_fail.stdout));
println!("stderr: {}", String::from_utf8_lossy(&output_fail.stderr));
assert!(!output_fail.status.success());
assert!(String::from_utf8(output.stderr).unwrap().contains(
"warning: Your project is in a workspace with lints configured, but workspace.lints is not configured."
));
}

#[test]
fn test_no_deps_ignores_path_deps_in_workspaces() {
if IS_RUSTC_TEST_SUITE {
Expand Down
14 changes: 13 additions & 1 deletion tests/workspace_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,17 @@ name = "workspace_test"
version = "0.1.0"
edition = "2018"

[lints]
workspace = true

[workspace]
members = ["subcrate", "module_style/pass_no_mod_with_dep_in_subdir", "module_style/pass_mod_with_dep_in_subdir"]
members = [
"missing_workspace_lints/pass_missing_workspace_lints",
"missing_workspace_lints/fail_missing_workspace_lints",
"subcrate",
"module_style/pass_no_mod_with_dep_in_subdir",
"module_style/pass_mod_with_dep_in_subdir",
]

[workspace.lints.clippy]
absolute_paths = "allow"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[package]
name = "fail-missing-workspace-lints"
version = "0.1.0"
publish = false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "pass-missing-workspace-lints"
version = "0.1.0"
publish = false

[lints]
workspace = true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
Loading