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

Fix command. #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
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(())
}
}
78 changes: 57 additions & 21 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 @@
}
}

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

impl<H> Fixer<H>
where
H: Healer,
{
pub fn new(medic: H) -> Self {

Check warning on line 62 in cargo-scout-lib/src/scout/mod.rs

View check run for this annotation

Codecov / codecov/patch

cargo-scout-lib/src/scout/mod.rs#L62

Added line #L62 was not covered by tests
Self { medic }
}
pub fn run(&self, lints: Vec<Lint>) -> Result<(), crate::error::Error> {
println!("[Scout] - applying fixes");
self.medic.heal(lints)

Check warning on line 67 in cargo-scout-lib/src/scout/mod.rs

View check run for this annotation

Codecov / codecov/patch

cargo-scout-lib/src/scout/mod.rs#L65-L67

Added lines #L65 - L67 were not covered by tests
}
}

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 @@
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 @@
// 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>>,
times_called: RefCell<usize>,
lints: Vec<Lint>,
}
impl TestLinter {
pub fn new() -> Self {
Self {
lints_times_called: Rc::new(RefCell::new(0)),
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)),
times_called: RefCell::new(0),
lints,
}
}
Expand All @@ -140,10 +160,16 @@
&self,
_working_dir: impl Into<PathBuf>,
) -> Result<Vec<Lint>, crate::error::Error> {
*self.lints_times_called.borrow_mut() += 1;
*self.times_called.borrow_mut() += 1;
Ok(self.lints.clone())
}
}
impl Healer for TestLinter {
fn heal(&self, _lints: Vec<Lint>) -> Result<(), crate::error::Error> {
*self.times_called.borrow_mut() += 1;
Ok(())
}
}
struct TestConfig {
members: Vec<String>,
}
Expand All @@ -165,13 +191,12 @@
// 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.times_called.borrow());
Ok(())
}

Expand Down Expand Up @@ -213,13 +238,12 @@
// 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.times_called.borrow());
assert_eq!(expected_lints_from_diff, actual_lints_from_diff);
Ok(())
}
Expand All @@ -236,13 +260,12 @@
// 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.times_called.borrow());
Ok(())
}

Expand Down Expand Up @@ -270,14 +293,27 @@
]);
// 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.times_called.borrow());
Ok(())
}

#[test]
fn test_heal() -> Result<(), crate::error::Error> {
let fixer = TestLinter::new();
let config = TestConfig::new(Vec::new());
let vcs = TestVCS::new(Vec::new());

let expected_times_called = 1;
let scout = Scout::new(vcs, config, &fixer);
let lints = scout.run()?;
fixer.heal(lints)?;
assert_eq!(expected_times_called, *fixer.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 @@
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!(

Check warning on line 24 in cargo-scout-lib/src/utils.rs

View check run for this annotation

Codecov / codecov/patch

cargo-scout-lib/src/utils.rs#L24

Added line #L24 was not covered by tests
relative_path,
get_relative_file_path(get_absolute_file_path(relative_path)?)?
);
Ok(())
}
}
Loading