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

some clippy-suggested improvements #3733

Merged
merged 3 commits into from
Feb 22, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 6 additions & 9 deletions src/cargo/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,12 @@ impl fmt::Display for VersionInfo {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "cargo {}.{}.{}",
self.major, self.minor, self.patch)?;
match self.cfg_info.as_ref().map(|ci| &ci.release_channel) {
Some(channel) => {
if channel != "stable" {
write!(f, "-{}", channel)?;
let empty = String::from("");
write!(f, "{}", self.pre_release.as_ref().unwrap_or(&empty))?;
}
},
None => (),
if let Some(channel) = self.cfg_info.as_ref().map(|ci| &ci.release_channel) {
if channel != "stable" {
write!(f, "-{}", channel)?;
let empty = String::from("");
write!(f, "{}", self.pre_release.as_ref().unwrap_or(&empty))?;
}
};

if let Some(ref cfg) = self.cfg_info {
Expand Down
13 changes: 5 additions & 8 deletions src/cargo/ops/cargo_rustc/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,16 +583,13 @@ pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {

let mut paths = Vec::new();
let mut deps = deps.split(' ').map(|s| s.trim()).filter(|s| !s.is_empty());
loop {
let mut file = match deps.next() {
Some(s) => s.to_string(),
None => break,
};
while file.ends_with("\\") {
while let Some(s) = deps.next() {
let mut file = s.to_string();
while file.ends_with('\\') {
file.pop();
file.push(' ');
file.push_str(deps.next().chain_error(|| {
internal(format!("malformed dep-info format, trailing \\"))
internal("malformed dep-info format, trailing \\".to_string())
})?);
}
paths.push(cwd.join(&file));
Expand All @@ -602,7 +599,7 @@ pub fn parse_dep_info(dep_info: &Path) -> CargoResult<Option<Vec<PathBuf>>> {

fn dep_info_mtime_if_fresh(dep_info: &Path) -> CargoResult<Option<FileTime>> {
if let Some(paths) = parse_dep_info(dep_info)? {
Ok(mtime_if_fresh(&dep_info, paths.iter()))
Ok(mtime_if_fresh(dep_info, paths.iter()))
} else {
Ok(None)
}
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_rustc/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl Layout {
// the target triple as a Path and then just use the file stem as the
// component for the directory name.
if let Some(triple) = triple {
path.push(Path::new(triple).file_stem().ok_or(human(format!("target was empty")))?);
path.push(Path::new(triple).file_stem().ok_or(human("target was empty".to_string()))?);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised clippy didn't suggest ok_or_else(|| human(…))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It did, but I didn't fix all of those.

}
path.push(dest);
Layout::at(ws.config(), path)
Expand Down
10 changes: 5 additions & 5 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
let pkgid = unit.pkg.package_id();
if !unit.target.is_lib() { continue }
if unit.profile.doc { continue }
if cx.compilation.libraries.contains_key(&pkgid) {
if cx.compilation.libraries.contains_key(pkgid) {
continue
}

Expand All @@ -182,9 +182,9 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
}
}

if let Some(feats) = cx.resolve.features(&unit.pkg.package_id()) {
if let Some(feats) = cx.resolve.features(unit.pkg.package_id()) {
cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
.or_insert(HashSet::new())
.or_insert_with(HashSet::new)
.extend(feats.iter().map(|feat| format!("feature=\"{}\"", feat)));
}

Expand All @@ -193,7 +193,7 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,

for (&(ref pkg, _), output) in cx.build_state.outputs.lock().unwrap().iter() {
cx.compilation.cfgs.entry(pkg.clone())
.or_insert(HashSet::new())
.or_insert_with(HashSet::new)
.extend(output.cfgs.iter().cloned());

for dir in output.library_paths.iter() {
Expand Down Expand Up @@ -344,7 +344,7 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
},
&mut |line| {
// stderr from rustc can have a mix of JSON and non-JSON output
if line.starts_with("{") {
if line.starts_with('{') {
// Handle JSON lines
let compiler_message = json::Json::from_str(line).map_err(|_| {
internal(&format!("compiler produced invalid json: `{}`", line))
Expand Down
11 changes: 4 additions & 7 deletions src/cargo/ops/cargo_rustc/output_depinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ fn render_filename<P: AsRef<Path>>(path: P, basedir: Option<&str>) -> CargoResul
fn add_deps_for_unit<'a, 'b>(deps: &mut HashSet<PathBuf>, context: &mut Context<'a, 'b>,
unit: &Unit<'a>, visited: &mut HashSet<Unit<'a>>) -> CargoResult<()>
{
if !visited.insert(unit.clone()) {
if !visited.insert(*unit) {
return Ok(());
}

Expand Down Expand Up @@ -76,13 +76,10 @@ pub fn output_depinfo<'a, 'b>(context: &mut Context<'a, 'b>, unit: &Unit<'a>) ->
// dep-info generation failed, so delete output file. This will usually
// cause the build system to always rerun the build rule, which is correct
// if inefficient.
match fs::remove_file(output_path) {
Err(err) => {
if err.kind() != ErrorKind::NotFound {
return Err(err.into());
}
if let Err(err) = fs::remove_file(output_path) {
if err.kind() != ErrorKind::NotFound {
return Err(err.into());
}
_ => ()
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ fn run_unit_tests(options: &TestOptions,
let mut errors = Vec::new();

for &(ref pkg, _, ref exe) in &compilation.tests {
let to_display = match util::without_prefix(exe, &cwd) {
let to_display = match util::without_prefix(exe, cwd) {
Some(path) => path,
None => &**exe,
};
Expand Down Expand Up @@ -145,7 +145,7 @@ fn run_doc_tests(options: &TestOptions,
p.arg("--test-args").arg(arg);
}

if let Some(cfgs) = compilation.cfgs.get(&package.package_id()) {
if let Some(cfgs) = compilation.cfgs.get(package.package_id()) {
for cfg in cfgs.iter() {
p.arg("--cfg").arg(cfg);
}
Expand Down
15 changes: 6 additions & 9 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,17 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()>
emit_package(root.as_table().unwrap(), &mut out);
}

let deps = e.toml.get(&"package".to_string()).unwrap().as_slice().unwrap();
let deps = e.toml[&"package".to_string()].as_slice().unwrap();
for dep in deps.iter() {
let dep = dep.as_table().unwrap();

out.push_str("[[package]]\n");
emit_package(dep, &mut out);
}

match e.toml.get(&"metadata".to_string()) {
Some(metadata) => {
out.push_str("[metadata]\n");
out.push_str(&metadata.to_string());
}
None => {}
if let Some(metadata) = e.toml.get(&"metadata".to_string()) {
out.push_str("[metadata]\n");
out.push_str(&metadata.to_string());
}

// If the lockfile contents haven't changed so don't rewrite it. This is
Expand Down Expand Up @@ -128,8 +125,8 @@ fn emit_package(dep: &toml::Table, out: &mut String) {
out.push_str(&format!("source = {}\n", lookup(dep, "source")));
}

if let Some(ref s) = dep.get("dependencies") {
let slice = Value::as_slice(*s).unwrap();
if let Some(s) = dep.get("dependencies") {
let slice = Value::as_slice(s).unwrap();

if !slice.is_empty() {
out.push_str("dependencies = [\n");
Expand Down
74 changes: 28 additions & 46 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> {
let (mut registry, reg_id) = registry(opts.config,
opts.token.clone(),
opts.index.clone())?;
verify_dependencies(&pkg, &reg_id)?;
verify_dependencies(pkg, &reg_id)?;

// Prepare a tarball, with a non-surpressable warning if metadata
// is missing since this is being put online.
Expand All @@ -66,7 +66,7 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> {

// Upload said tarball to the specified destination
opts.config.shell().status("Uploading", pkg.package_id().to_string())?;
transmit(opts.config, &pkg, tarball.file(), &mut registry, opts.dry_run)?;
transmit(opts.config, pkg, tarball.file(), &mut registry, opts.dry_run)?;

Ok(())
}
Expand Down Expand Up @@ -121,13 +121,10 @@ fn transmit(config: &Config,
Some(ref readme) => Some(paths::read(&pkg.root().join(readme))?),
None => None,
};
match *license_file {
Some(ref file) => {
if fs::metadata(&pkg.root().join(file)).is_err() {
bail!("the license file `{}` does not exist", file)
}
if let Some(ref file) = *license_file {
if fs::metadata(&pkg.root().join(file)).is_err() {
bail!("the license file `{}` does not exist", file)
}
None => {}
}

// Do not upload if performing a dry run
Expand Down Expand Up @@ -246,18 +243,13 @@ pub fn http_handle(config: &Config) -> CargoResult<Easy> {
/// Favor cargo's `http.proxy`, then git's `http.proxy`. Proxies specified
/// via environment variables are picked up by libcurl.
fn http_proxy(config: &Config) -> CargoResult<Option<String>> {
match config.get_string("http.proxy")? {
Some(s) => return Ok(Some(s.val)),
None => {}
if let Some(s) = config.get_string("http.proxy")? {
return Ok(Some(s.val))
}
match git2::Config::open_default() {
Ok(cfg) => {
match cfg.get_str("http.proxy") {
Ok(s) => return Ok(Some(s.to_string())),
Err(..) => {}
}
if let Ok(cfg) = git2::Config::open_default() {
if let Ok(s) = cfg.get_str("http.proxy") {
return Ok(Some(s.to_string()))
}
Err(..) => {}
}
Ok(None)
}
Expand All @@ -282,9 +274,8 @@ pub fn http_proxy_exists(config: &Config) -> CargoResult<bool> {
}

pub fn http_timeout(config: &Config) -> CargoResult<Option<i64>> {
match config.get_i64("http.timeout")? {
Some(s) => return Ok(Some(s.val)),
None => {}
if let Some(s) = config.get_i64("http.timeout")? {
return Ok(Some(s.val))
}
Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok()))
}
Expand All @@ -293,11 +284,8 @@ pub fn registry_login(config: &Config, token: String) -> CargoResult<()> {
let RegistryConfig { index, token: _ } = registry_configuration(config)?;
let mut map = HashMap::new();
let p = config.cwd().to_path_buf();
match index {
Some(index) => {
map.insert("index".to_string(), ConfigValue::String(index, p.clone()));
}
None => {}
if let Some(index) = index {
map.insert("index".to_string(), ConfigValue::String(index, p.clone()));
}
map.insert("token".to_string(), ConfigValue::String(token, p));

Expand Down Expand Up @@ -327,28 +315,22 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
let (mut registry, _) = registry(config, opts.token.clone(),
opts.index.clone())?;

match opts.to_add {
Some(ref v) => {
let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
config.shell().status("Owner", format!("adding {:?} to crate {}",
v, name))?;
registry.add_owners(&name, &v).map_err(|e| {
human(format!("failed to add owners to crate {}: {}", name, e))
})?;
}
None => {}
if let Some(ref v) = opts.to_add {
let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
config.shell().status("Owner", format!("adding {:?} to crate {}",
v, name))?;
registry.add_owners(&name, &v).map_err(|e| {
human(format!("failed to add owners to crate {}: {}", name, e))
})?;
}

match opts.to_remove {
Some(ref v) => {
let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
config.shell().status("Owner", format!("removing {:?} from crate {}",
v, name))?;
registry.remove_owners(&name, &v).map_err(|e| {
human(format!("failed to remove owners from crate {}: {}", name, e))
})?;
}
None => {}
if let Some(ref v) = opts.to_remove {
let v = v.iter().map(|s| &s[..]).collect::<Vec<_>>();
config.shell().status("Owner", format!("removing {:?} from crate {}",
v, name))?;
registry.remove_owners(&name, &v).map_err(|e| {
human(format!("failed to remove owners from crate {}: {}", name, e))
})?;
}

if opts.list {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn resolve_ws_precisely<'a>(ws: &Workspace<'a>,
let resolved_with_overrides =
ops::resolve_with_previous(&mut registry, ws,
method, Some(&resolve), None,
&specs)?;
specs)?;

for &(ref replace_spec, _) in ws.root_replace() {
if !resolved_with_overrides.replacements().keys().any(|r| replace_spec.matches(r)) {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/sources/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl<'cfg> Source for DirectorySource<'cfg> {
// crates and otherwise may conflict with a VCS
// (rust-lang/cargo#3414).
if let Some(s) = path.file_name().and_then(|s| s.to_str()) {
if s.starts_with(".") {
if s.starts_with('.') {
continue
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl<'cfg> Source for GitSource<'cfg> {

trace!("updating git source `{:?}`", self.remote);

let repo = self.remote.checkout(&db_path, &self.config)?;
let repo = self.remote.checkout(&db_path, self.config)?;
let rev = repo.rev_for(&self.reference)?;
(repo, rev)
} else {
Expand All @@ -166,7 +166,7 @@ impl<'cfg> Source for GitSource<'cfg> {
// in scope so the destructors here won't tamper with too much.
// Checkout is immutable, so we don't need to protect it with a lock once
// it is created.
repo.copy_to(actual_rev.clone(), &checkout_path, &self.config)?;
repo.copy_to(actual_rev.clone(), &checkout_path, self.config)?;

let source_id = self.source_id.with_precise(Some(actual_rev.to_string()));
let path_source = PathSource::new_recursive(&checkout_path,
Expand Down
Loading