Skip to content

Commit

Permalink
WIP: Tryouts on a fix command.
Browse files Browse the repository at this point in the history
  • Loading branch information
o0Ignition0o committed Jan 14, 2020
1 parent 7a80947 commit 71977d6
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 30 deletions.
11 changes: 5 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ before_script:
fi
script:
- cargo fmt -- --check
- cargo test --all
- cargo clippy --all-targets --tests -- -D clippy::pedantic
after_success: |
- |
if [[ "$TRAVIS_RUST_VERSION" == stable ]]; then
USE_SKEPTIC=1 cargo tarpaulin --out Xml
bash <(curl -s https://codecov.io/bash)
echo "Uploaded code coverage"
USE_SKEPTIC=1 cargo tarpaulin --out Xml && bash <(curl -s https://codecov.io/bash) && echo "Uploaded code coverage"
else
cargo test --all
fi
- cargo clippy --all-targets --tests -- -D clippy::pedantic
4 changes: 4 additions & 0 deletions cargo-scout-lib/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ pub enum Error {
Io(#[from] std::io::Error),
#[error("Git error: {0}")]
Git(#[from] git2::Error),
#[error("The provided command is invalid.")]
InvalidCommand,
#[error("Couldn't strip prefix from path: {0}")]
StripPrefix(#[from] std::path::StripPrefixError),
}
5 changes: 5 additions & 0 deletions cargo-scout-lib/src/healer/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
use crate::linter::Lint;

pub trait Healer {
fn heal(&self, lints: Vec<Lint>) -> Result<(), crate::error::Error>;
}
1 change: 1 addition & 0 deletions cargo-scout-lib/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod config;
pub mod error;
pub mod healer;
pub mod linter;
pub mod scout;
pub mod vcs;
Expand Down
3 changes: 2 additions & 1 deletion cargo-scout-lib/src/linter/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use serde::Serialize;
use std::path::PathBuf;

pub mod clippy;
Expand All @@ -21,7 +22,7 @@ pub struct Lint {
}

/// A `Location` has a file name, a start and an end line
#[derive(PartialEq, Clone, Debug)]
#[derive(Serialize, PartialEq, Clone, Debug)]
pub struct Location {
pub path: String,
pub lines: [u32; 2],
Expand Down
97 changes: 96 additions & 1 deletion cargo-scout-lib/src/linter/rustfmt.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
use crate::error::Error;
use crate::healer::Healer;
use crate::linter::{Lint, Linter, Location};
use crate::utils::get_absolute_file_path;
use serde::Deserialize;
use crate::utils::get_relative_file_path;
use serde::{Deserialize, Serialize};
use serde_json;
use std::path::{Path, PathBuf};
use std::process::Command;

#[derive(Default)]
pub struct RustFmt {}

#[derive(Serialize)]
struct FmtLocation {
file: String,
range: [u32; 2],
}

impl Linter for RustFmt {
fn lints(&self, working_dir: impl Into<PathBuf>) -> Result<Vec<Lint>, Error> {
let working_dir = working_dir.into();
Expand All @@ -21,6 +29,53 @@ impl Linter for RustFmt {
}
}

impl Healer for RustFmt {
// Skipped from code coverage
// because an external command
// cannot be easily unit tested
#[cfg_attr(tarpaulin, skip)]
fn heal(&self, lints: Vec<Lint>) -> Result<(), crate::error::Error> {
let l = &lints_as_json(&lints)?;
let fmt_fix = Command::new("cargo")
.args(&[
"+nightly",
"fmt",
"--",
"--unstable-features",
"--file-lines",
l,
"--skip-children",
])
.output()
.expect("failed to run rustfmt");

if fmt_fix.status.success() {
Ok(())
} else {
Err(crate::error::Error::Command(String::from_utf8(
fmt_fix.stderr,
)?))
}
}
}

fn lints_as_json(lints: &[Lint]) -> Result<String, crate::error::Error> {
let locations: Vec<FmtLocation> = lints
.iter()
.filter_map(|l| {
if let Ok(file) = get_relative_file_path(l.location.path.clone()) {
Some(FmtLocation {
file,
range: l.location.lines,
})
} else {
None
}
})
.collect();
Ok(serde_json::to_string(&locations)?)
}

impl RustFmt {
fn command_parameters() -> Vec<&'static str> {
vec!["+nightly", "fmt", "--", "--emit", "json"]
Expand Down Expand Up @@ -185,4 +240,44 @@ mod tests {

Ok(())
}

#[test]
fn test_lints_as_json() -> Result<(), crate::error::Error> {
let expected_output = r#"[{"file":"src/lib.rs","range":[1,2]},{"file":"foo_bar.rs","range":[11,19]},{"file":"baz.rs","range":[1,1]}]"#;

let lints_to_transform = vec![
Lint {
message: String::new(),
location: Location {
path: get_absolute_file_path("src/lib.rs")?,
lines: [1, 2],
},
},
Lint {
message: String::new(),
location: Location {
path: get_absolute_file_path("foo_bar.rs")?,
lines: [11, 19],
},
},
Lint {
message: String::new(),
location: Location {
path: get_absolute_file_path("baz.rs")?,
lines: [1, 1],
},
},
Lint {
message: "this one won't parse because the path is not absolute".to_string(),
location: Location {
path: "wont_pass.rs".to_string(),
lines: [42, 42],
},
},
];

let actual_output = lints_as_json(&lints_to_transform)?;
assert_eq!(expected_output, actual_output);
Ok(())
}
}
56 changes: 36 additions & 20 deletions cargo-scout-lib/src/scout/mod.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,27 @@
use crate::config::*;
use crate::healer::Healer;
use crate::linter::{Lint, Linter};
use crate::vcs::*;
use std::path::PathBuf;

pub struct Scout<V, C, L>
pub struct Scout<'s, V, C, L>
where
V: VCS,
C: Config,
L: Linter,
{
vcs: V,
config: C,
linter: L,
linter: &'s L,
}

impl<V, C, L> Scout<V, C, L>
impl<'s, V, C, L> Scout<'s, V, C, L>
where
V: VCS,
C: Config,
L: Linter,
{
pub fn new(vcs: V, config: C, linter: L) -> Self {
pub fn new(vcs: V, config: C, linter: &'s L) -> Self {
Self {
vcs,
config,
Expand All @@ -47,6 +48,26 @@ where
}
}

pub struct Fixer<H>
where
H: Healer,
{
medic: H,
}

impl<H> Fixer<H>
where
H: Healer,
{
pub fn new(medic: H) -> Self {
Self { medic }
}
pub fn run(&self, lints: Vec<Lint>) -> Result<(), crate::error::Error> {
println!("[Scout] - applying fixes");
self.medic.heal(lints)
}
}

fn diff_in_member(member: &PathBuf, sections: &[Section]) -> bool {
if let Some(m) = member.to_str() {
for s in sections {
Expand Down Expand Up @@ -94,7 +115,6 @@ mod scout_tests {
use std::cell::RefCell;
use std::clone::Clone;
use std::path::{Path, PathBuf};
use std::rc::Rc;
struct TestVCS {
sections: Vec<Section>,
sections_called: RefCell<bool>,
Expand All @@ -117,20 +137,20 @@ mod scout_tests {
// Using a RefCell here because lints
// takes &self and not &mut self.
// We use usize here because we will compare it to a Vec::len()
lints_times_called: Rc<RefCell<usize>>,
lints_times_called: RefCell<usize>,
lints: Vec<Lint>,
}
impl TestLinter {
pub fn new() -> Self {
Self {
lints_times_called: Rc::new(RefCell::new(0)),
lints_times_called: RefCell::new(0),
lints: Vec::new(),
}
}

pub fn with_lints(lints: Vec<Lint>) -> Self {
Self {
lints_times_called: Rc::new(RefCell::new(0)),
lints_times_called: RefCell::new(0),
lints,
}
}
Expand Down Expand Up @@ -165,13 +185,12 @@ mod scout_tests {
// No members so we won't have to iterate
let config = TestConfig::new(Vec::new());
let expected_times_called = 0;
let actual_times_called = Rc::clone(&linter.lints_times_called);
let scout = Scout::new(vcs, config, linter);
let scout = Scout::new(vcs, config, &linter);
// We don't check for the lints result here.
// It is already tested in the linter tests
// and in intersection tests
let _ = scout.run()?;
assert_eq!(expected_times_called, *actual_times_called.borrow());
assert_eq!(expected_times_called, *linter.lints_times_called.borrow());
Ok(())
}

Expand Down Expand Up @@ -213,13 +232,12 @@ mod scout_tests {
// The member matches the file name
let config = TestConfig::new(vec!["foo".to_string()]);
let expected_times_called = 1;
let actual_times_called = Rc::clone(&linter.lints_times_called);
let scout = Scout::new(vcs, config, linter);
let scout = Scout::new(vcs, config, &linter);
// We don't check for the lints result here.
// It is already tested in the linter tests
// and in intersection tests
let actual_lints_from_diff = scout.run()?;
assert_eq!(expected_times_called, *actual_times_called.borrow());
assert_eq!(expected_times_called, *linter.lints_times_called.borrow());
assert_eq!(expected_lints_from_diff, actual_lints_from_diff);
Ok(())
}
Expand All @@ -236,13 +254,12 @@ mod scout_tests {
// The member does not match the file name
let config = TestConfig::new(vec!["foo".to_string()]);
let expected_times_called = 0;
let actual_times_called = Rc::clone(&linter.lints_times_called);
let scout = Scout::new(vcs, config, linter);
let scout = Scout::new(vcs, config, &linter);
// We don't check for the lints result here.
// It is already tested in the linter tests
// and in intersection tests
let _ = scout.run()?;
assert_eq!(expected_times_called, *actual_times_called.borrow());
assert_eq!(expected_times_called, *linter.lints_times_called.borrow());
Ok(())
}

Expand Down Expand Up @@ -270,14 +287,13 @@ mod scout_tests {
]);
// We should run the linter on member1 and member2
let expected_times_called = 2;
let actual_times_called = Rc::clone(&linter.lints_times_called);
let scout = Scout::new(vcs, config, linter);
let scout = Scout::new(vcs, config, &linter);
// We don't check for the lints result here.
// It is already tested in the linter tests
// and in intersection tests
let _ = scout.run()?;

assert_eq!(expected_times_called, *actual_times_called.borrow());
assert_eq!(expected_times_called, *linter.lints_times_called.borrow());
Ok(())
}
}
Expand Down
22 changes: 22 additions & 0 deletions cargo-scout-lib/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,25 @@ pub fn get_absolute_file_path(file_path: impl AsRef<Path>) -> Result<String, Err
absolute_path.push(file_path);
Ok(absolute_path.to_string_lossy().to_string())
}

pub fn get_relative_file_path(file_path: impl AsRef<Path>) -> Result<String, Error> {
let current_dir = std::env::current_dir()?;
let relative_path = file_path
.as_ref()
.strip_prefix(current_dir.to_string_lossy().to_string())?;
Ok(relative_path.to_string_lossy().to_string())
}

#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_file_path_conversion() -> Result<(), crate::error::Error> {
let relative_path = "foo/bar.rs";
assert_eq!(
relative_path,
get_relative_file_path(get_absolute_file_path(relative_path)?)?
);
Ok(())
}
}
Loading

0 comments on commit 71977d6

Please sign in to comment.