Skip to content

Commit

Permalink
Auto merge of rust-lang#5636 - ebroto:issue_5041, r=phansch
Browse files Browse the repository at this point in the history
multiple_crate_versions: skip dev and build deps

changelog: multiple_crate_versions: skip dev and build deps

Closes rust-lang#5041
  • Loading branch information
bors committed May 26, 2020
2 parents 2a6cfa7 + ec0a00e commit 182ac89
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 19 deletions.
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() {}

0 comments on commit 182ac89

Please sign in to comment.