Skip to content

Commit

Permalink
Merge pull request #567 from leodasvacas/clippy-run
Browse files Browse the repository at this point in the history
Clippy run
  • Loading branch information
brson authored Jul 11, 2016
2 parents 712aed9 + e948836 commit 8984d8d
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 74 deletions.
23 changes: 8 additions & 15 deletions src/rustup/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,24 @@ use telemetry::{Telemetry, TelemetryEvent};
pub fn run_command_for_dir<S: AsRef<OsStr>>(cmd: Command,
args: &[S],
cfg: &Cfg) -> Result<()> {
let arg0 = env::args().next().map(|a| PathBuf::from(a));
let arg0 = env::args().next().map(PathBuf::from);
let arg0 = arg0.as_ref()
.and_then(|a| a.file_name())
.and_then(|a| a.to_str());
let arg0 = try!(arg0.ok_or(ErrorKind::NoExeName));
if (arg0 == "rustc" || arg0 == "rustc.exe") && try!(cfg.telemetry_enabled()) {
return telemetry_rustc(cmd, &args, &cfg);
return telemetry_rustc(cmd, args, cfg);
}

run_command_for_dir_without_telemetry(cmd, &args)
run_command_for_dir_without_telemetry(cmd, args)
}

fn telemetry_rustc<S: AsRef<OsStr>>(mut cmd: Command, args: &[S], cfg: &Cfg) -> Result<()> {
let now = Instant::now();

cmd.args(&args[1..]);

let has_color_args = (&args).iter().any(|e| {
let has_color_args = args.iter().any(|e| {
let e = e.as_ref().to_str().unwrap_or("");
e.starts_with("--color")
});
Expand Down Expand Up @@ -80,21 +80,14 @@ fn telemetry_rustc<S: AsRef<OsStr>>(mut cmd: Command, args: &[S], cfg: &Cfg) ->
buffer.clear();
let _ = handle.write(b.as_bytes());

let c = re.captures(&b);
match c {
None => continue,
Some(caps) => {
if caps.len() > 0 {
let _ = errors.push(caps.name("error").unwrap_or("").to_owned());
}
if let Some(caps) = re.captures(&b) {
if !caps.is_empty() {
errors.push(caps.name("error").unwrap_or("").to_owned());
}
};
}

let e = match errors.len() {
0 => None,
_ => Some(errors),
};
let e = if errors.is_empty() { None } else { Some(errors) };

let te = TelemetryEvent::RustcRun { duration_ms: ms,
exit_code: exit_code,
Expand Down
5 changes: 1 addition & 4 deletions src/rustup/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,10 +392,7 @@ impl Cfg {
}

pub fn set_telemetry(&self, telemetry_enabled: bool) -> Result<()> {
match telemetry_enabled {
true => self.enable_telemetry(),
false => self.disable_telemetry(),
}
if telemetry_enabled { self.enable_telemetry() } else { self.disable_telemetry() }
}

fn enable_telemetry(&self) -> Result<()> {
Expand Down
2 changes: 1 addition & 1 deletion src/rustup/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ impl<'a> InstallMethod<'a> {
Ok(true)
}
InstallMethod::Dist(toolchain, update_hash, dl_cfg) => {
let ref prefix = InstallPrefix::from(path.to_owned());
let prefix = &InstallPrefix::from(path.to_owned());
let maybe_new_hash =
try!(dist::update_from_dist(
dl_cfg,
Expand Down
80 changes: 35 additions & 45 deletions src/rustup/telemetry_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub struct TelemetryAnalysis {
rustc_error_statistics: RustcStatistics,
}

#[derive(Default)]
pub struct RustcStatistics {
rustc_execution_count: u32,
compile_time_ms_total: u64,
Expand All @@ -32,26 +33,15 @@ pub struct RustcStatistics {

impl RustcStatistics {
pub fn new() -> RustcStatistics {
RustcStatistics {
rustc_execution_count: 0u32,
compile_time_ms_total: 0u64,
compile_time_ms_mean: 0u64,
compile_time_ms_ntile_75: 0u64,
compile_time_ms_ntile_90: 0u64,
compile_time_ms_ntile_95: 0u64,
compile_time_ms_ntile_99: 0u64,
compile_time_ms_stdev: 0f64,
exit_codes_with_count: HashMap::new(),
error_codes_with_counts: HashMap::new()
}
Default::default()
}
}

impl fmt::Display for RustcStatistics {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let mut errors: String = String::new();
if self.error_codes_with_counts.len() > 0 {

if !self.error_codes_with_counts.is_empty() {
errors = " rustc errors\n".to_owned();
for (error, count) in &self.error_codes_with_counts {
errors = errors + &format!(" '{}': {}\n", error, count);
Expand All @@ -60,7 +50,7 @@ impl fmt::Display for RustcStatistics {

let mut exits: String = String::new();

if self.exit_codes_with_count.len() > 0 {
if !self.exit_codes_with_count.is_empty() {
exits = " rustc exit codes\n".to_owned();

for (exit, count) in &self.exit_codes_with_count {
Expand Down Expand Up @@ -116,7 +106,7 @@ rustc error statistics

impl TelemetryAnalysis {
pub fn new(telemetry_dir: PathBuf) -> TelemetryAnalysis {
TelemetryAnalysis {
TelemetryAnalysis {
telemetry_dir: telemetry_dir,
rustc_statistics: RustcStatistics::new(),
rustc_success_statistics: RustcStatistics::new(),
Expand Down Expand Up @@ -170,34 +160,34 @@ impl TelemetryAnalysis {
Ok(events)
}

pub fn analyze_telemetry_events(&mut self, events: &Vec<TelemetryEvent>) -> Result<()> {
pub fn analyze_telemetry_events(&mut self, events: &[TelemetryEvent]) -> Result<()> {
let mut rustc_durations = Vec::new();
let mut rustc_exit_codes = Vec::new();

let mut rustc_successful_durations = Vec::new();

let mut rustc_error_durations = Vec::new();
let mut error_list: Vec<Vec<String>> = Vec::new();
let mut error_codes_with_counts: HashMap<String, i32> = HashMap::new();

let mut toolchains = Vec::new();
let mut toolchains_with_errors = Vec::new();
let mut targets = Vec::new();

let mut updated_toolchains = Vec::new();
let mut updated_toolchains_with_errors = Vec::new();

for event in events {
match event {
&TelemetryEvent::RustcRun{ duration_ms, ref exit_code, ref errors } => {
match *event {
TelemetryEvent::RustcRun{ duration_ms, ref exit_code, ref errors } => {
self.rustc_statistics.rustc_execution_count += 1;
rustc_durations.push(duration_ms);

let exit_count = self.rustc_statistics.exit_codes_with_count.entry(*exit_code).or_insert(0);
*exit_count += 1;

rustc_exit_codes.push(exit_code);

if errors.is_some() {
let errors = errors.clone().unwrap();

Expand All @@ -207,19 +197,19 @@ impl TelemetryAnalysis {
}

error_list.push(errors);
rustc_error_durations.push(duration_ms);
rustc_error_durations.push(duration_ms);
} else {
rustc_successful_durations.push(duration_ms);
}
},
&TelemetryEvent::TargetAdd{ ref toolchain, ref target, success } => {
TelemetryEvent::TargetAdd{ ref toolchain, ref target, success } => {
toolchains.push(toolchain.to_owned());
targets.push(target.to_owned());
if !success {
toolchains_with_errors.push(toolchain.to_owned());
}
},
&TelemetryEvent::ToolchainUpdate{ ref toolchain, success } => {
TelemetryEvent::ToolchainUpdate{ ref toolchain, success } => {
updated_toolchains.push(toolchain.to_owned());
if !success {
updated_toolchains_with_errors.push(toolchain.to_owned());
Expand All @@ -244,27 +234,27 @@ impl TelemetryAnalysis {
}
}

pub fn compute_rustc_percentiles(values: &Vec<u64>) -> RustcStatistics {
pub fn compute_rustc_percentiles(values: &[u64]) -> RustcStatistics {
RustcStatistics {
rustc_execution_count: (values.len() as u32),
compile_time_ms_total: values.iter().fold(0, |sum, val| sum + val),
compile_time_ms_mean: mean(&values),
compile_time_ms_ntile_75: ntile(75, &values),
compile_time_ms_ntile_90: ntile(90, &values),
compile_time_ms_ntile_95: ntile(95, &values),
compile_time_ms_ntile_99: ntile(99, &values),
compile_time_ms_stdev: stdev(&values),
compile_time_ms_mean: mean(values),
compile_time_ms_ntile_75: ntile(75, values),
compile_time_ms_ntile_90: ntile(90, values),
compile_time_ms_ntile_95: ntile(95, values),
compile_time_ms_ntile_99: ntile(99, values),
compile_time_ms_stdev: stdev(values),
exit_codes_with_count: HashMap::new(),
error_codes_with_counts: HashMap::new()
}
}

pub fn ntile(percentile: i32, values: &Vec<u64>) -> u64 {
if values.len() == 0 {
pub fn ntile(percentile: i32, values: &[u64]) -> u64 {
if values.is_empty() {
return 0u64;
}

let mut values = values.clone();
let mut values = values.to_owned();
values.sort();

let count = values.len() as f32;
Expand All @@ -276,9 +266,9 @@ pub fn ntile(percentile: i32, values: &Vec<u64>) -> u64 {
values[n]
}

pub fn mean(values: &Vec<u64>) -> u64 {
if values.len() == 0 {
return 0u64;
pub fn mean(values: &[u64]) -> u64 {
if values.is_empty() {
return 0;
}

let count = values.len() as f64;
Expand All @@ -288,12 +278,12 @@ pub fn mean(values: &Vec<u64>) -> u64 {
(sum / count) as u64
}

pub fn variance(values: &Vec<u64>) -> f64 {
if values.len() == 0 {
pub fn variance(values: &[u64]) -> f64 {
if values.is_empty() {
return 0f64;
}

let mean = mean(&values);
let mean = mean(values);

let mut deviations: Vec<i64> = Vec::new();

Expand All @@ -308,12 +298,12 @@ pub fn variance(values: &Vec<u64>) -> f64 {
sum / (values.len() as f64)
}

pub fn stdev(values: &Vec<u64>) -> f64 {
if values.len() == 0 {
pub fn stdev(values: &[u64]) -> f64 {
if values.is_empty() {
return 0f64;
}

let variance = variance(&values);
let variance = variance(values);

variance.sqrt()
}
18 changes: 9 additions & 9 deletions src/rustup/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct Toolchain<'a> {
dist_handler: Box<Fn(rustup_dist::Notification) + 'a>,
}

/// Used by the list_component function
/// Used by the `list_component` function
pub struct ComponentStatus {
pub component: Component,
pub required: bool,
Expand Down Expand Up @@ -142,7 +142,7 @@ impl<'a> Toolchain<'a> {
}
}

fn download_cfg<'b>(&'b self) -> dist::DownloadCfg<'b> {
fn download_cfg(&self) -> dist::DownloadCfg {
dist::DownloadCfg {
dist_root: &self.cfg.dist_root_url,
temp_cfg: &self.cfg.temp_cfg,
Expand Down Expand Up @@ -220,7 +220,7 @@ impl<'a> Toolchain<'a> {
// installs, and do it all in a single transaction.
for installer in installers {
let installer_str = installer.to_str().unwrap_or("bogus");
match installer_str.rfind(".") {
match installer_str.rfind('.') {
Some(i) => {
let extension = &installer_str[i+1..];
if extension != "gz" {
Expand Down Expand Up @@ -276,7 +276,7 @@ impl<'a> Toolchain<'a> {
return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
}

let ref bin_path = self.path.join("bin").join(binary.as_ref());
let bin_path = &self.path.join("bin").join(binary.as_ref());
let mut cmd = Command::new(bin_path);
self.set_env(&mut cmd);
Ok(cmd)
Expand Down Expand Up @@ -356,7 +356,7 @@ impl<'a> Toolchain<'a> {
return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
}

let ref toolchain = self.name;
let toolchain = &self.name;
let ref toolchain = try!(ToolchainDesc::from_str(toolchain)
.chain_err(|| ErrorKind::ComponentsUnsupported(self.name.to_string())));
let prefix = InstallPrefix::from(self.path.to_owned());
Expand All @@ -376,7 +376,7 @@ impl<'a> Toolchain<'a> {

for component in &targ_pkg.components {
let installed = config.as_ref()
.map(|c| c.components.contains(&component))
.map(|c| c.components.contains(component))
.unwrap_or(false);

// Get the component so we can check if it is available
Expand All @@ -395,7 +395,7 @@ impl<'a> Toolchain<'a> {

for extension in &targ_pkg.extensions {
let installed = config.as_ref()
.map(|c| c.components.contains(&extension))
.map(|c| c.components.contains(extension))
.unwrap_or(false);

// Get the component so we can check if it is available
Expand Down Expand Up @@ -464,7 +464,7 @@ impl<'a> Toolchain<'a> {
return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
}

let ref toolchain = self.name;
let toolchain = &self.name;
let ref toolchain = try!(ToolchainDesc::from_str(toolchain)
.chain_err(|| ErrorKind::ComponentsUnsupported(self.name.to_string())));
let prefix = InstallPrefix::from(self.path.to_owned());
Expand Down Expand Up @@ -507,7 +507,7 @@ impl<'a> Toolchain<'a> {
return Err(ErrorKind::ToolchainNotInstalled(self.name.to_owned()).into());
}

let ref toolchain = self.name;
let toolchain = &self.name;
let ref toolchain = try!(ToolchainDesc::from_str(toolchain)
.chain_err(|| ErrorKind::ComponentsUnsupported(self.name.to_string())));
let prefix = InstallPrefix::from(self.path.to_owned());
Expand Down

0 comments on commit 8984d8d

Please sign in to comment.