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

Refactoring into modules and traits #48

Merged
merged 1 commit into from
Dec 14, 2019
Merged
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
88 changes: 0 additions & 88 deletions src/cargo.rs

This file was deleted.

1 change: 1 addition & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#[derive(Debug)]
pub enum Error {
ScoutBuilder,
CargoToml(cargo_toml::Error),
Command(String),
Utf8(std::string::FromUtf8Error),
Expand Down
78 changes: 18 additions & 60 deletions src/clippy.rs → src/linter/clippy.rs
Original file line number Diff line number Diff line change
@@ -1,56 +1,22 @@
use serde::Deserialize;
use crate::linter::{Lint, Linter};
use std::io::{self, Write};
use std::path::PathBuf;
use std::process::Command;

#[derive(Deserialize, PartialEq, Debug)]
pub struct LintCode {
pub code: String,
pub explanation: String,
}

#[derive(Deserialize, PartialEq, Debug)]
pub struct LintSpan {
pub file_name: String,
/// The line where the lint should be reported
///
/// GitHub provides a line_start and a line_end.
/// We should use the line_start in case of multi-line lints.
/// (Why?)
pub line_start: usize,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Lint {
/// The lint message
///
/// Example:
///
/// unused variable: `count`
pub package_id: String,
pub src_path: Option<String>,
pub message: Option<Message>,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Message {
pub rendered: String,
pub spans: Vec<Span>,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Span {
pub file_name: String,
pub line_start: u32,
pub line_end: u32,
}

pub struct Linter {
pub struct Clippy {
verbose: bool,
no_default_features: bool,
all_features: bool,
}

impl Linter {
impl Linter for Clippy {
fn get_lints(&self, working_dir: PathBuf) -> Result<Vec<Lint>, crate::error::Error> {
self.clippy(working_dir)
.map(|clippy_output| lints(clippy_output.as_ref()))
}
}

impl Clippy {
pub fn new() -> Self {
Self {
verbose: false,
Expand All @@ -74,10 +40,6 @@ impl Linter {
self
}

pub fn get_lints(&self) -> Result<Vec<Lint>, crate::error::Error> {
self.clippy().map(|output| lints(&output))
}

fn get_command_parameters(&self) -> Vec<&str> {
let mut params = vec!["clippy", "--message-format", "json"];
if self.verbose {
Expand All @@ -101,8 +63,9 @@ impl Linter {
envs
}

fn clippy(&self) -> Result<String, crate::error::Error> {
fn clippy(&self, path: impl AsRef<std::path::Path>) -> Result<String, crate::error::Error> {
let clippy_pedantic_output = Command::new("cargo")
.current_dir(path)
.args(self.get_command_parameters())
.envs(self.get_envs())
.output()
Expand Down Expand Up @@ -157,11 +120,10 @@ pub fn lints(clippy_output: &str) -> Vec<Lint> {

#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_set_verbose() {
use crate::clippy::Linter;

let mut linter = Linter::new();
let mut linter = Clippy::new();
assert_eq!(false, linter.verbose);

let l2 = linter.set_verbose(true);
Expand All @@ -172,9 +134,7 @@ mod tests {
}
#[test]
fn test_get_envs() {
use crate::clippy::Linter;

let mut linter = Linter::new();
let mut linter = Clippy::new();
let mut expected_envs = vec![];
assert_eq!(expected_envs, linter.get_envs());

Expand All @@ -184,9 +144,7 @@ mod tests {
}
#[test]
fn test_get_command_parameters() {
use crate::clippy::Linter;

let mut linter = Linter::new();
let mut linter = Clippy::new();
let expected_command_parameters = vec![
"clippy",
"--message-format",
Expand Down Expand Up @@ -248,7 +206,7 @@ mod tests {
}
#[test]
fn test_lints() {
use crate::clippy::{lints, Lint, Message, Span};
use crate::linter::{Message, Span};
let expected_lints = vec![Lint {
package_id: "cargo-scout".to_string(),
src_path: Some("test/foo/bar.rs".to_string()),
Expand Down
50 changes: 50 additions & 0 deletions src/linter/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use serde::Deserialize;
use std::path::PathBuf;

pub mod clippy;

pub trait Linter {
fn get_lints(&self, working_dir: PathBuf) -> Result<Vec<Lint>, crate::error::Error>;
}

#[derive(Deserialize, PartialEq, Debug)]
pub struct LintCode {
pub code: String,
pub explanation: String,
}

#[derive(Deserialize, PartialEq, Debug)]
pub struct LintSpan {
pub file_name: String,
/// The line where the lint should be reported
///
/// GitHub provides a line_start and a line_end.
/// We should use the line_start in case of multi-line lints.
/// (Why?)
pub line_start: usize,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Lint {
/// The lint message
///
/// Example:
///
/// unused variable: `count`
pub package_id: String,
pub src_path: Option<String>,
pub message: Option<Message>,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Message {
pub rendered: String,
pub spans: Vec<Span>,
}

#[derive(Deserialize, PartialEq, Debug, Clone)]
pub struct Span {
pub file_name: String,
pub line_start: u32,
pub line_end: u32,
}
46 changes: 27 additions & 19 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
use crate::linter::clippy::Clippy;
use crate::linter::Lint;
use std::env;
use structopt::StructOpt;

mod cargo;
mod clippy;
mod error;
mod git;
mod intersections;
mod linter;
mod project;
mod scout;

#[derive(Debug, StructOpt)]
#[structopt(
Expand Down Expand Up @@ -40,7 +42,7 @@ struct Options {
without_error: bool,
}

fn display_warnings(warnings: &[clippy::Lint]) {
fn display_warnings(warnings: &[Lint]) {
for w in warnings {
if let Some(m) = &w.message {
for l in m.rendered.split('\n') {
Expand All @@ -56,40 +58,46 @@ fn main() -> Result<(), error::Error> {

println!("Getting diff against target {}", opts.branch);
let diff_sections = git::get_sections(env::current_dir()?, &opts.branch)?;

println!("Checking Cargo manifest");
let manifest = cargo::Parser::from_manifest_path(opts.cargo_toml)?;
if manifest.is_workspace() {
println!("Running in workspace, please note feature flags are not supported yet.");
}
println!("Running clippy");
let clippy_lints = clippy::Linter::new()
let mut project = project::cargo::Project::from_manifest_path(opts.cargo_toml)?;
project.set_all_features(opts.all_features);
project.set_no_default_features(opts.no_default_features);

let mut clippy_linter = Clippy::new();
clippy_linter
.set_verbose(opts.verbose)
.set_no_default_features(opts.no_default_features)
.set_all_features(opts.all_features)
.get_lints()?;
.set_all_features(opts.all_features);

let warnings_caused_by_diff =
intersections::get_lints_from_diff(&clippy_lints, &diff_sections, opts.verbose);
let mut scout_builder = scout::Builder::new();
scout_builder
.set_project_config(Box::new(project))
.set_linter(Box::new(clippy_linter));
println!("Running clippy");
let scout = scout_builder.build()?;

return_warnings(&warnings_caused_by_diff, opts.without_error)
let relevant_lints = scout.run_for_diff(&diff_sections)?;
return_warnings(&relevant_lints, opts.without_error)
}

fn return_warnings(lints: &[clippy::Lint], without_error: bool) -> Result<(), error::Error> {
fn return_warnings(lints: &[linter::Lint], without_error: bool) -> Result<(), error::Error> {
if lints.is_empty() {
println!("No warnings raised by clippy::pedantic in your diff, you're good to go!");
Ok(())
} else {
display_warnings(&lints);
if without_error {
return Ok(());
Ok(())
} else {
Err(error::Error::NotClean)
}
Err(error::Error::NotClean)
}
}

#[cfg(test)]
mod tests {
use crate::clippy::Lint;
use crate::linter::Lint;
use crate::return_warnings;
#[test]
fn test_return_status_with_lints() {
Expand Down
Loading