Skip to content

Commit

Permalink
Auto merge of #7791 - ehuss:rename-kind, r=alexcrichton
Browse files Browse the repository at this point in the history
Rename `Kind`

Rename `dependency::Kind` → `dependency::DepKind`
Rename `source_id::Kind` → `source_id::SourceKind`

I struggle when there are multiple types with the same name in the same code base. I think this makes it a little clearer what the type is.

I was tempted to also rename `registry::Kind`, but I could not think of a good name. That file is particularly hard for me to understand (locked vs normal sources, abstract trait, etc.), so I don't feel comfortable changing it. It's also localized in one file, so not as important.
  • Loading branch information
bors committed Jan 13, 2020
2 parents 735f648 + 0b653a4 commit ae08099
Show file tree
Hide file tree
Showing 10 changed files with 111 additions and 106 deletions.
32 changes: 16 additions & 16 deletions crates/resolver-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::fmt::Write;
use std::rc::Rc;
use std::time::Instant;

use cargo::core::dependency::Kind;
use cargo::core::dependency::DepKind;
use cargo::core::resolver::{self, ResolveOpts};
use cargo::core::source::{GitReference, SourceId};
use cargo::core::Resolve;
Expand Down Expand Up @@ -629,7 +629,7 @@ pub fn dep(name: &str) -> Dependency {
pub fn dep_req(name: &str, req: &str) -> Dependency {
Dependency::parse_no_deprecated(name, Some(req), registry_loc()).unwrap()
}
pub fn dep_req_kind(name: &str, req: &str, kind: Kind, public: bool) -> Dependency {
pub fn dep_req_kind(name: &str, req: &str, kind: DepKind, public: bool) -> Dependency {
let mut dep = dep_req(name, req);
dep.set_kind(kind);
dep.set_public(public);
Expand All @@ -642,7 +642,7 @@ pub fn dep_loc(name: &str, location: &str) -> Dependency {
let source_id = SourceId::for_git(&url, master).unwrap();
Dependency::parse_no_deprecated(name, Some("1.0.0"), source_id).unwrap()
}
pub fn dep_kind(name: &str, kind: Kind) -> Dependency {
pub fn dep_kind(name: &str, kind: DepKind) -> Dependency {
dep(name).set_kind(kind).clone()
}

Expand Down Expand Up @@ -677,12 +677,12 @@ impl fmt::Debug for PrettyPrintRegistry {
} else {
write!(f, "pkg!((\"{}\", \"{}\") => [", s.name(), s.version())?;
for d in s.dependencies() {
if d.kind() == Kind::Normal
if d.kind() == DepKind::Normal
&& &d.version_req().to_string() == "*"
&& !d.is_public()
{
write!(f, "dep(\"{}\"),", d.name_in_toml())?;
} else if d.kind() == Kind::Normal && !d.is_public() {
} else if d.kind() == DepKind::Normal && !d.is_public() {
write!(
f,
"dep_req(\"{}\", \"{}\"),",
Expand All @@ -696,9 +696,9 @@ impl fmt::Debug for PrettyPrintRegistry {
d.name_in_toml(),
d.version_req(),
match d.kind() {
Kind::Development => "Kind::Development",
Kind::Build => "Kind::Build",
Kind::Normal => "Kind::Normal",
DepKind::Development => "DepKind::Development",
DepKind::Build => "DepKind::Build",
DepKind::Normal => "DepKind::Normal",
},
d.is_public()
)?;
Expand All @@ -725,8 +725,8 @@ fn meta_test_deep_pretty_print_registry() {
pkg!(("bar", "2.0.0") => [dep_req("baz", "=1.0.1")]),
pkg!(("baz", "1.0.2") => [dep_req("other", "2")]),
pkg!(("baz", "1.0.1")),
pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", Kind::Build, false)]),
pkg!(("cat", "1.0.3") => [dep_req_kind("other", "2", Kind::Development, false)]),
pkg!(("cat", "1.0.2") => [dep_req_kind("other", "2", DepKind::Build, false)]),
pkg!(("cat", "1.0.3") => [dep_req_kind("other", "2", DepKind::Development, false)]),
pkg!(("dep_req", "1.0.0")),
pkg!(("dep_req", "2.0.0")),
])
Expand All @@ -738,8 +738,8 @@ fn meta_test_deep_pretty_print_registry() {
pkg!((\"bar\", \"2.0.0\") => [dep_req(\"baz\", \"= 1.0.1\"),]),\
pkg!((\"baz\", \"1.0.2\") => [dep_req(\"other\", \"^2\"),]),\
pkg!((\"baz\", \"1.0.1\")),\
pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", Kind::Build, false),]),\
pkg!((\"cat\", \"1.0.3\") => [dep_req_kind(\"other\", \"^2\", Kind::Development, false),]),\
pkg!((\"cat\", \"1.0.2\") => [dep_req_kind(\"other\", \"^2\", DepKind::Build, false),]),\
pkg!((\"cat\", \"1.0.3\") => [dep_req_kind(\"other\", \"^2\", DepKind::Development, false),]),\
pkg!((\"dep_req\", \"1.0.0\")),\
pkg!((\"dep_req\", \"2.0.0\")),]"
)
Expand Down Expand Up @@ -848,10 +848,10 @@ pub fn registry_strategy(
format!(">={}, <={}", s[c].0, s[d].0)
},
match k {
0 => Kind::Normal,
1 => Kind::Build,
// => Kind::Development, // Development has no impact so don't gen
_ => panic!("bad index for Kind"),
0 => DepKind::Normal,
1 => DepKind::Build,
// => DepKind::Development, // Development has no impact so don't gen
_ => panic!("bad index for DepKind"),
},
p && k == 0,
))
Expand Down
36 changes: 20 additions & 16 deletions crates/resolver-tests/tests/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use cargo::core::dependency::Kind;
use cargo::core::dependency::DepKind;
use cargo::core::{enable_nightly_features, Dependency};
use cargo::util::{is_ci, Config};

Expand Down Expand Up @@ -232,7 +232,7 @@ fn pub_fail() {
let input = vec![
pkg!(("a", "0.0.4")),
pkg!(("a", "0.0.5")),
pkg!(("e", "0.0.6") => [dep_req_kind("a", "<= 0.0.4", Kind::Normal, true),]),
pkg!(("e", "0.0.6") => [dep_req_kind("a", "<= 0.0.4", DepKind::Normal, true),]),
pkg!(("kB", "0.0.3") => [dep_req("a", ">= 0.0.5"),dep("e"),]),
];
let reg = registry(input);
Expand All @@ -244,7 +244,7 @@ fn basic_public_dependency() {
let reg = registry(vec![
pkg!(("A", "0.1.0")),
pkg!(("A", "0.2.0")),
pkg!("B" => [dep_req_kind("A", "0.1", Kind::Normal, true)]),
pkg!("B" => [dep_req_kind("A", "0.1", DepKind::Normal, true)]),
pkg!("C" => [dep("A"), dep("B")]),
]);

Expand Down Expand Up @@ -279,7 +279,7 @@ fn public_dependency_filling_in() {
pkg!(("a", "0.1.1")),
pkg!(("b", "0.0.0") => [dep("bad")]),
pkg!(("b", "0.0.1") => [dep("bad")]),
pkg!(("b", "0.0.2") => [dep_req_kind("a", "=0.0.6", Kind::Normal, true)]),
pkg!(("b", "0.0.2") => [dep_req_kind("a", "=0.0.6", DepKind::Normal, true)]),
pkg!("c" => [dep_req("b", ">=0.0.1")]),
pkg!("d" => [dep("c"), dep("a"), dep("b")]),
]);
Expand Down Expand Up @@ -315,7 +315,7 @@ fn public_dependency_filling_in_and_update() {
let reg = registry(vec![
pkg!(("A", "0.0.0")),
pkg!(("A", "0.0.2")),
pkg!("B" => [dep_req_kind("A", "=0.0.0", Kind::Normal, true),]),
pkg!("B" => [dep_req_kind("A", "=0.0.0", DepKind::Normal, true),]),
pkg!("C" => [dep("A"),dep("B")]),
pkg!("D" => [dep("B"),dep("C")]),
]);
Expand All @@ -341,7 +341,7 @@ fn public_dependency_skipping() {
pkg!(("a", "0.2.0")),
pkg!(("a", "2.0.0")),
pkg!(("b", "0.0.0") => [dep("bad")]),
pkg!(("b", "0.2.1") => [dep_req_kind("a", "0.2.0", Kind::Normal, true)]),
pkg!(("b", "0.2.1") => [dep_req_kind("a", "0.2.0", DepKind::Normal, true)]),
pkg!("c" => [dep("a"),dep("b")]),
];
let reg = registry(input);
Expand All @@ -361,7 +361,7 @@ fn public_dependency_skipping_in_backtracking() {
pkg!(("A", "0.0.3") => [dep("bad")]),
pkg!(("A", "0.0.4")),
pkg!(("A", "0.0.5")),
pkg!("B" => [dep_req_kind("A", ">= 0.0.3", Kind::Normal, true)]),
pkg!("B" => [dep_req_kind("A", ">= 0.0.3", DepKind::Normal, true)]),
pkg!("C" => [dep_req("A", "<= 0.0.4"), dep("B")]),
];
let reg = registry(input);
Expand All @@ -374,9 +374,9 @@ fn public_sat_topological_order() {
let input = vec![
pkg!(("a", "0.0.1")),
pkg!(("a", "0.0.0")),
pkg!(("b", "0.0.1") => [dep_req_kind("a", "= 0.0.1", Kind::Normal, true),]),
pkg!(("b", "0.0.1") => [dep_req_kind("a", "= 0.0.1", DepKind::Normal, true),]),
pkg!(("b", "0.0.0") => [dep("bad"),]),
pkg!("A" => [dep_req("a", "= 0.0.0"),dep_req_kind("b", "*", Kind::Normal, true)]),
pkg!("A" => [dep_req("a", "= 0.0.0"),dep_req_kind("b", "*", DepKind::Normal, true)]),
];

let reg = registry(input);
Expand All @@ -388,7 +388,7 @@ fn public_sat_unused_makes_things_pub() {
let input = vec![
pkg!(("a", "0.0.1")),
pkg!(("a", "0.0.0")),
pkg!(("b", "8.0.1") => [dep_req_kind("a", "= 0.0.1", Kind::Normal, true),]),
pkg!(("b", "8.0.1") => [dep_req_kind("a", "= 0.0.1", DepKind::Normal, true),]),
pkg!(("b", "8.0.0") => [dep_req("a", "= 0.0.1"),]),
pkg!("c" => [dep_req("b", "= 8.0.0"),dep_req("a", "= 0.0.0"),]),
];
Expand All @@ -403,8 +403,8 @@ fn public_sat_unused_makes_things_pub_2() {
pkg!(("c", "0.0.2")),
pkg!(("c", "0.0.1")),
pkg!(("a-sys", "0.0.2")),
pkg!(("a-sys", "0.0.1") => [dep_req_kind("c", "= 0.0.1", Kind::Normal, true),]),
pkg!("P" => [dep_req_kind("a-sys", "*", Kind::Normal, true),dep_req("c", "= 0.0.1"),]),
pkg!(("a-sys", "0.0.1") => [dep_req_kind("c", "= 0.0.1", DepKind::Normal, true),]),
pkg!("P" => [dep_req_kind("a-sys", "*", DepKind::Normal, true),dep_req("c", "= 0.0.1"),]),
pkg!("A" => [dep("P"),dep_req("c", "= 0.0.2"),]),
];
let reg = registry(input);
Expand Down Expand Up @@ -492,13 +492,17 @@ fn test_resolving_with_same_name() {
#[test]
fn test_resolving_with_dev_deps() {
let reg = registry(vec![
pkg!("foo" => ["bar", dep_kind("baz", Kind::Development)]),
pkg!("baz" => ["bat", dep_kind("bam", Kind::Development)]),
pkg!("foo" => ["bar", dep_kind("baz", DepKind::Development)]),
pkg!("baz" => ["bat", dep_kind("bam", DepKind::Development)]),
pkg!("bar"),
pkg!("bat"),
]);

let res = resolve(vec![dep("foo"), dep_kind("baz", Kind::Development)], &reg).unwrap();
let res = resolve(
vec![dep("foo"), dep_kind("baz", DepKind::Development)],
&reg,
)
.unwrap();

assert_same(&res, &names(&["root", "foo", "bar", "baz", "bat"]));
}
Expand Down Expand Up @@ -1061,7 +1065,7 @@ fn resolving_with_public_constrained_sibling() {
dep_req("backtrack_trap1", "1.0"),
dep_req("backtrack_trap2", "1.0"),
dep_req("constrained", "<=60")]),
pkg!(("bar", "1.0.0") => [dep_req_kind("constrained", ">=60", Kind::Normal, true)]),
pkg!(("bar", "1.0.0") => [dep_req_kind("constrained", ">=60", DepKind::Normal, true)]),
];
// Bump these to make the test harder, but you'll also need to
// change the version constraints on `constrained` above. To correctly
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

use crate::core::compiler::Unit;
use crate::core::compiler::{BuildContext, CompileKind, CompileMode};
use crate::core::dependency::Kind as DepKind;
use crate::core::dependency::DepKind;
use crate::core::package::Downloads;
use crate::core::profiles::{Profile, UnitFor};
use crate::core::resolver::Resolve;
Expand Down
30 changes: 15 additions & 15 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ struct Inner {
registry_id: Option<SourceId>,
req: VersionReq,
specified_req: bool,
kind: Kind,
kind: DepKind,
only_match_name: bool,
explicit_name_in_toml: Option<InternedString>,

Expand All @@ -51,7 +51,7 @@ struct SerializedDependency<'a> {
name: &'a str,
source: SourceId,
req: String,
kind: Kind,
kind: DepKind,
rename: Option<&'a str>,

optional: bool,
Expand Down Expand Up @@ -86,7 +86,7 @@ impl ser::Serialize for Dependency {
}

#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Clone, Debug, Copy)]
pub enum Kind {
pub enum DepKind {
Normal,
Development,
Build,
Expand Down Expand Up @@ -138,15 +138,15 @@ this warning.
}
}

impl ser::Serialize for Kind {
impl ser::Serialize for DepKind {
fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
where
S: ser::Serializer,
{
match *self {
Kind::Normal => None,
Kind::Development => Some("dev"),
Kind::Build => Some("build"),
DepKind::Normal => None,
DepKind::Development => Some("dev"),
DepKind::Build => Some("build"),
}
.serialize(s)
}
Expand Down Expand Up @@ -208,7 +208,7 @@ impl Dependency {
source_id,
registry_id: None,
req: VersionReq::any(),
kind: Kind::Normal,
kind: DepKind::Normal,
only_match_name: true,
optional: false,
public: false,
Expand Down Expand Up @@ -284,7 +284,7 @@ impl Dependency {
self
}

pub fn kind(&self) -> Kind {
pub fn kind(&self) -> DepKind {
self.inner.kind
}

Expand All @@ -296,7 +296,7 @@ impl Dependency {
pub fn set_public(&mut self, public: bool) -> &mut Dependency {
if public {
// Setting 'public' only makes sense for normal dependencies
assert_eq!(self.kind(), Kind::Normal);
assert_eq!(self.kind(), DepKind::Normal);
}
Rc::make_mut(&mut self.inner).public = public;
self
Expand All @@ -320,10 +320,10 @@ impl Dependency {
self.inner.explicit_name_in_toml
}

pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency {
pub fn set_kind(&mut self, kind: DepKind) -> &mut Dependency {
if self.is_public() {
// Setting 'public' only makes sense for normal dependencies
assert_eq!(kind, Kind::Normal);
assert_eq!(kind, DepKind::Normal);
}
Rc::make_mut(&mut self.inner).kind = kind;
self
Expand Down Expand Up @@ -400,14 +400,14 @@ impl Dependency {
/// Returns `false` if the dependency is only used to build the local package.
pub fn is_transitive(&self) -> bool {
match self.inner.kind {
Kind::Normal | Kind::Build => true,
Kind::Development => false,
DepKind::Normal | DepKind::Build => true,
DepKind::Development => false,
}
}

pub fn is_build(&self) -> bool {
match self.inner.kind {
Kind::Build => true,
DepKind::Build => true,
_ => false,
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/resolver/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet};
use std::fmt;
use std::iter::FromIterator;

use crate::core::dependency::Kind;
use crate::core::dependency::DepKind;
use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target};
use crate::util::errors::CargoResult;
use crate::util::Graph;
Expand Down Expand Up @@ -87,7 +87,7 @@ impl Resolve {
.edges(p)
.filter(|(_, deps)| {
deps.iter()
.any(|d| d.kind() == Kind::Normal && d.is_public())
.any(|d| d.kind() == DepKind::Normal && d.is_public())
})
.map(|(dep_package, _)| *dep_package)
.collect::<HashSet<PackageId>>();
Expand Down
Loading

0 comments on commit ae08099

Please sign in to comment.