Skip to content
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

multiple_crate_versions: skip dev and build deps #5636

Merged
merged 3 commits into from
May 26, 2020
Merged
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
17 changes: 12 additions & 5 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ fn create_test(lint: &LintData) -> io::Result<()> {

path.push("src");
fs::create_dir(&path)?;
write_file(path.join("main.rs"), get_test_file_contents(lint_name))?;
let header = format!("// compile-flags: --crate-name={}", lint_name);
write_file(path.join("main.rs"), get_test_file_contents(lint_name, Some(&header)))?;

Ok(())
}
Expand All @@ -90,7 +91,7 @@ fn create_test(lint: &LintData) -> io::Result<()> {
create_project_layout(lint.name, &test_dir, "pass", "This file should not trigger the lint")
} else {
let test_path = format!("tests/ui/{}.rs", lint.name);
let test_contents = get_test_file_contents(lint.name);
let test_contents = get_test_file_contents(lint.name, None);
write_file(lint.project_root.join(test_path), test_contents)
}
}
Expand Down Expand Up @@ -119,16 +120,22 @@ fn to_camel_case(name: &str) -> String {
.collect()
}

fn get_test_file_contents(lint_name: &str) -> String {
format!(
fn get_test_file_contents(lint_name: &str, header_commands: Option<&str>) -> String {
let mut contents = format!(
"#![warn(clippy::{})]

fn main() {{
// test code goes here
}}
",
lint_name
)
);

if let Some(header) = header_commands {
contents = format!("{}\n{}", header, contents);
}

contents
}

fn get_manifest_contents(lint_name: &str, hint: &str) -> String {
Expand Down
60 changes: 46 additions & 14 deletions clippy_lints/src/multiple_crate_versions.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
//! lint on multiple versions of a crate being used

use crate::utils::{run_lints, span_lint};
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_hir::{Crate, CRATE_HIR_ID};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::DUMMY_SP;

use cargo_metadata::{DependencyKind, MetadataCommand, Node, Package, PackageId};
use if_chain::if_chain;
use itertools::Itertools;

declare_clippy_lint! {
Expand Down Expand Up @@ -39,32 +42,61 @@ impl LateLintPass<'_, '_> for MultipleCrateVersions {
return;
}

let metadata = if let Ok(metadata) = cargo_metadata::MetadataCommand::new().exec() {
let metadata = if let Ok(metadata) = MetadataCommand::new().exec() {
metadata
} else {
span_lint(cx, MULTIPLE_CRATE_VERSIONS, DUMMY_SP, "could not read cargo metadata");

return;
};

let local_name = cx.tcx.crate_name(LOCAL_CRATE).as_str();
let mut packages = metadata.packages;
packages.sort_by(|a, b| a.name.cmp(&b.name));

for (name, group) in &packages.into_iter().group_by(|p| p.name.clone()) {
let group: Vec<cargo_metadata::Package> = group.collect();
if_chain! {
if let Some(resolve) = &metadata.resolve;
if let Some(local_id) = packages
.iter()
.find_map(|p| if p.name == *local_name { Some(&p.id) } else { None });
then {
for (name, group) in &packages.iter().group_by(|p| p.name.clone()) {
let group: Vec<&Package> = group.collect();

if group.len() <= 1 {
continue;
}

if group.len() > 1 {
let mut versions: Vec<_> = group.into_iter().map(|p| p.version).collect();
versions.sort();
let versions = versions.iter().join(", ");
if group.iter().all(|p| is_normal_dep(&resolve.nodes, local_id, &p.id)) {
let mut versions: Vec<_> = group.into_iter().map(|p| &p.version).collect();
versions.sort();
let versions = versions.iter().join(", ");

span_lint(
cx,
MULTIPLE_CRATE_VERSIONS,
DUMMY_SP,
&format!("multiple versions for dependency `{}`: {}", name, versions),
);
span_lint(
cx,
MULTIPLE_CRATE_VERSIONS,
DUMMY_SP,
&format!("multiple versions for dependency `{}`: {}", name, versions),
);
}
}
}
}
}
}

fn is_normal_dep(nodes: &[Node], local_id: &PackageId, dep_id: &PackageId) -> bool {
fn depends_on(node: &Node, dep_id: &PackageId) -> bool {
node.deps.iter().any(|dep| {
dep.pkg == *dep_id
&& dep
.dep_kinds
.iter()
.any(|info| matches!(info.kind, DependencyKind::Normal))
})
}

nodes
.iter()
.filter(|node| depends_on(node, dep_id))
.any(|node| node.id == *local_id || is_normal_dep(nodes, local_id, &node.id))
}
1 change: 1 addition & 0 deletions tests/ui-cargo/cargo_common_metadata/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
1 change: 1 addition & 0 deletions tests/ui-cargo/cargo_common_metadata/pass/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=cargo_common_metadata
#![warn(clippy::cargo_common_metadata)]

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Should not lint for dev or build dependencies. See issue 5041.

[package]
name = "multiple_crate_versions"
version = "0.1.0"
publish = false

# One of the versions of winapi is only a dev dependency: allowed
[dependencies]
ctrlc = "=3.1.0"
[dev-dependencies]
ansi_term = "=0.11.0"

# Both versions of winapi are a build dependency: allowed
[build-dependencies]
ctrlc = "=3.1.0"
ansi_term = "=0.11.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// compile-flags: --crate-name=multiple_crate_versions
#![warn(clippy::multiple_crate_versions)]

fn main() {}
1 change: 1 addition & 0 deletions tests/ui-cargo/multiple_crate_versions/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=multiple_crate_versions
#![warn(clippy::multiple_crate_versions)]

fn main() {}
1 change: 1 addition & 0 deletions tests/ui-cargo/multiple_crate_versions/pass/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=multiple_crate_versions
#![warn(clippy::multiple_crate_versions)]

fn main() {}
1 change: 1 addition & 0 deletions tests/ui-cargo/wildcard_dependencies/fail/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=wildcard_dependencies
#![warn(clippy::wildcard_dependencies)]

fn main() {}
1 change: 1 addition & 0 deletions tests/ui-cargo/wildcard_dependencies/pass/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --crate-name=wildcard_dependencies
#![warn(clippy::wildcard_dependencies)]

fn main() {}