Skip to content

Commit

Permalink
Merge branch 'master' into add-quiet-arg
Browse files Browse the repository at this point in the history
  • Loading branch information
mgrachev committed Aug 6, 2020
2 parents 925d0b3 + cf17699 commit 687dadd
Show file tree
Hide file tree
Showing 12 changed files with 509 additions and 29 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]
### 🚀 Added
- Add --quiet argument [#242](https://github.com/dotenv-linter/dotenv-linter/pull/242) ([@wesleimp](https://github.com/wesleimp), [@mgrachev](https://github.com/mgrachev))
- Add total problems to output and `--quiet` argument [#242](https://github.com/dotenv-linter/dotenv-linter/pull/242) ([@wesleimp](https://github.com/wesleimp), [@mgrachev](https://github.com/mgrachev))
- Add autofix feature (for LowercaseKey check) [#228](https://github.com/dotenv-linter/dotenv-linter/pull/228) ([@evgeniy-r](https://github.com/evgeniy-r))
- Add installation CI test for Windows (via `install.sh`) [#235](https://github.com/dotenv-linter/dotenv-linter/pull/235) ([@DDtKey](https://github.com/DDtKey))

### 🔧 Changed
Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ TrailingWhitespace
UnorderedKey
```

`dotenv-linter` can also automatically fix warnings in the files. Currently only one kind of warnings is fixed
(`LowercaseKey`). You should use the argument `--fix` (or its short version `-f`) for this (will be available in [v2.2.0](https://github.com/dotenv-linter/dotenv-linter/issues/238):

```shell script
$ dotenv-linter -f
Fixed warnings:
.env:3 LowercaseKey: The foo key should be in uppercase
Unfixed warnings:
.env:2 DuplicatedKey: The BAR key is duplicated
```

## ✅ Checks

### Duplicated Key
Expand Down
20 changes: 10 additions & 10 deletions src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ pub fn available_check_names() -> Vec<String> {
.collect()
}

pub fn run(lines: Vec<LineEntry>, skip_checks: &[&str]) -> Vec<Warning> {
pub fn run(lines: &[LineEntry], skip_checks: &[&str]) -> Vec<Warning> {
let mut checks = checklist();
checks.retain(|c| !skip_checks.contains(&c.name()));

let mut warnings: Vec<Warning> = Vec::new();

for line in &lines {
for line in lines {
let is_comment = line.is_comment();
for ch in &mut checks {
if is_comment && ch.skip_comments() {
Expand Down Expand Up @@ -101,7 +101,7 @@ mod tests {
let expected: Vec<Warning> = Vec::new();
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(empty, &skip_checks));
assert_eq!(expected, run(&empty, &skip_checks));
}

#[test]
Expand All @@ -110,7 +110,7 @@ mod tests {
let expected: Vec<Warning> = Vec::new();
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -122,7 +122,7 @@ mod tests {
let expected: Vec<Warning> = Vec::new();
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -131,7 +131,7 @@ mod tests {
let expected: Vec<Warning> = Vec::new();
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -146,7 +146,7 @@ mod tests {
let expected: Vec<Warning> = vec![warning];
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -161,7 +161,7 @@ mod tests {
let expected: Vec<Warning> = vec![warning];
let skip_checks: Vec<&str> = Vec::new();

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -177,7 +177,7 @@ mod tests {
let expected: Vec<Warning> = vec![warning];
let skip_checks: Vec<&str> = vec!["KeyWithoutValue"];

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand All @@ -187,7 +187,7 @@ mod tests {
let expected: Vec<Warning> = Vec::new();
let skip_checks: Vec<&str> = vec!["KeyWithoutValue", "EndingBlankLine"];

assert_eq!(expected, run(lines, &skip_checks));
assert_eq!(expected, run(&lines, &skip_checks));
}

#[test]
Expand Down
25 changes: 19 additions & 6 deletions src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use std::path::PathBuf;

#[derive(Clone, Debug, PartialEq)]
pub struct Warning {
check_name: String,
pub check_name: String,
line: LineEntry,
message: String,
pub is_fixed: bool,
}

impl Warning {
Expand All @@ -16,8 +17,21 @@ impl Warning {
line,
check_name,
message,
is_fixed: false,
}
}

pub fn line_number(&self) -> usize {
self.line.number
}

pub fn mark_as_fixed(&mut self) {
self.is_fixed = true;
}

pub fn mark_as_unfixed(&mut self) {
self.is_fixed = false;
}
}

impl fmt::Display for Warning {
Expand Down Expand Up @@ -174,8 +188,6 @@ mod tests {

mod from {
use super::*;
use std::env::temp_dir;
use std::fs::remove_file;

#[test]
fn path_without_file_test() {
Expand All @@ -186,22 +198,23 @@ mod tests {
#[test]
fn path_with_file_test() {
let file_name = String::from(".env");
let path = temp_dir().join(&file_name);
let dir = tempfile::tempdir().expect("create temp dir");
let path = dir.path().join(&file_name);
fs::File::create(&path).expect("create testfile");

let f = FileEntry::from(path.clone());
assert_eq!(
Some((
FileEntry {
path: path.clone(),
path,
file_name,
total_lines: 0
},
vec![]
)),
f
);
remove_file(path).expect("temp file deleted");
dir.close().expect("temp dir deleted");
}
}

Expand Down
167 changes: 167 additions & 0 deletions src/fixes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
use crate::common::*;

mod lowercase_key;

trait Fix {
fn name(&self) -> &str;

fn fix_warnings(
&self,
warnings: Vec<&mut Warning>,
lines: &mut Vec<LineEntry>,
) -> Option<usize> {
let mut count: usize = 0;
for warning in warnings {
let line = lines.get_mut(warning.line_number() - 1)?;
if self.fix_line(line).is_some() {
warning.mark_as_fixed();
count += 1;
}
}

Some(count)
}

fn fix_line(&self, _line: &mut LineEntry) -> Option<()> {
None
}
}

// TODO: skip fixes (like checks)
// The fix order is matter
fn fixlist() -> Vec<Box<dyn Fix>> {
vec![
// At first we run the fixers that handle a single line entry (they use default
// implementation of the fix_warnings() function)
Box::new(lowercase_key::LowercaseKeyFixer::default()),
// Then we should run the fixers that handle the line entry collection at whole.
// And at the end we should run the fixer for ExtraBlankLine check (because the previous
// fixers can create additional extra blank lines).
]
}

pub fn run(warnings: &mut [Warning], lines: &mut Vec<LineEntry>) -> usize {
if warnings.is_empty() {
return 0;
}

let mut count = 0;
for fixer in fixlist() {
// We can optimize it: create check_name:warnings map in advance
let fixer_warnings: Vec<&mut Warning> = warnings
.iter_mut()
.filter(|w| w.check_name == fixer.name())
.collect();

if !fixer_warnings.is_empty() {
match fixer.fix_warnings(fixer_warnings, lines) {
Some(fixer_count) => count += fixer_count,
None => {
for warning in warnings {
warning.mark_as_unfixed();
}
return 0;
}
}
}
}

count
}

#[cfg(test)]
mod tests {
use super::*;
use std::path::PathBuf;

fn line_entry(number: usize, total_lines: usize, str: &str) -> LineEntry {
LineEntry {
number,
file: FileEntry {
path: PathBuf::from(".env"),
file_name: ".env".to_string(),
total_lines,
},
raw_string: String::from(str),
}
}

fn blank_line_entry(number: usize, total_lines: usize) -> LineEntry {
LineEntry {
number,
file: FileEntry {
path: PathBuf::from(".env"),
file_name: ".env".to_string(),
total_lines,
},
raw_string: String::from("\n"),
}
}

#[test]
fn run_with_empty_warnings_test() {
let mut lines = vec![line_entry(1, 2, "A=B"), blank_line_entry(2, 2)];
let mut warnings: Vec<Warning> = Vec::new();

assert_eq!(0, run(&mut warnings, &mut lines));
}

#[test]
fn run_with_fixable_warning_test() {
let mut lines = vec![
line_entry(1, 3, "A=B"),
line_entry(2, 3, "c=d"),
blank_line_entry(3, 3),
];
let mut warnings = vec![Warning::new(
lines[1].clone(),
"LowercaseKey",
String::from("The c key should be in uppercase"),
)];

assert_eq!(1, run(&mut warnings, &mut lines));
assert_eq!("C=d", lines[1].raw_string);
assert!(warnings[0].is_fixed);
}

#[test]
fn run_with_unfixable_warning_test() {
let mut lines = vec![
line_entry(1, 3, "A=B"),
line_entry(2, 3, "C"),
blank_line_entry(3, 3),
];
let mut warnings = vec![Warning::new(
lines[1].clone(),
"KeyWithoutValue",
String::from("The C key should be with a value or have an equal sign"),
)];

assert_eq!(0, run(&mut warnings, &mut lines));
assert!(!warnings[0].is_fixed);
}

#[test]
fn run_when_lines_do_not_fit_numbers_test() {
let mut lines = vec![
line_entry(1, 3, "a=B"),
line_entry(4, 3, "c=D"),
blank_line_entry(3, 3),
];
let mut warnings = vec![
Warning::new(
lines[0].clone(),
"LowercaseKey",
String::from("The a key should be in uppercase"),
),
Warning::new(
lines[1].clone(),
"LowercaseKey",
String::from("The c key should be in uppercase"),
),
];

assert_eq!(0, run(&mut warnings, &mut lines));
assert!(!warnings[0].is_fixed);
}
}

0 comments on commit 687dadd

Please sign in to comment.