-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add "no-curly-quotes" rule to the codebase. #56
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
fa24ec0
no-curly-quotes
EliSchleifer d887844
add
EliSchleifer 93ccf0d
0c7cf4b
2684bc7
516c0c7
beffd1f
d965904
d11f98e
45cada6
7ffb852
cd0da65
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,3 +12,6 @@ enabled = false | |
[neveredit] | ||
enabled = false | ||
paths = [] | ||
|
||
[nocurlyquotes] | ||
enabled = false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
pub mod if_change_then_change; | ||
pub mod never_edit; | ||
pub mod no_curly_quotes; | ||
pub mod pls_no_land; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
use std::fs::File; | ||
use std::io::BufRead; | ||
use std::io::BufReader; | ||
use std::path::PathBuf; | ||
|
||
use crate::run::Run; | ||
|
||
use anyhow::Context; | ||
use log::debug; | ||
use log::trace; | ||
use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; | ||
|
||
use crate::diagnostic::{Diagnostic, Position, Range, Replacement, Severity}; | ||
|
||
pub fn no_curly_quotes(run: &Run, _upstream: &str) -> anyhow::Result<Vec<Diagnostic>> { | ||
let config = &run.config.nocurlyquotes; | ||
|
||
if !config.enabled { | ||
trace!("'nocurlyquotes' is disabled"); | ||
return Ok(vec![]); | ||
} | ||
|
||
debug!("scanning {} files for curly quotes", run.paths.len()); | ||
|
||
// Scan files in parallel | ||
let results: Result<Vec<_>, _> = run.paths.par_iter().map(no_curly_quotes_impl).collect(); | ||
|
||
match results { | ||
Ok(v) => Ok(v.into_iter().flatten().collect()), | ||
Err(e) => Err(e), | ||
} | ||
} | ||
|
||
const DOUBLE_CURLY_QUOTES: [char; 4] = ['\u{201C}', '\u{201D}', '\u{201E}', '\u{201F}']; | ||
const SINGLE_CURLY_QUOTES: [char; 2] = ['\u{2018}', '\u{2019}']; | ||
|
||
fn no_curly_quotes_impl(path: &PathBuf) -> anyhow::Result<Vec<Diagnostic>> { | ||
let in_file = File::open(path).with_context(|| format!("failed to open: {:#?}", path))?; | ||
let in_buf = BufReader::new(in_file); | ||
|
||
trace!("scanning contents of {}", path.display()); | ||
|
||
let lines_view = in_buf | ||
.lines() | ||
.collect::<std::io::Result<Vec<String>>>() | ||
.with_context(|| format!("failed to read lines of text from {:#?}", path))?; | ||
|
||
let mut ret = Vec::new(); | ||
|
||
for (i, line) in lines_view.iter().enumerate() { | ||
let mut char_issues = Vec::new(); | ||
|
||
for (pos, c) in line.char_indices() { | ||
if SINGLE_CURLY_QUOTES.contains(&c) { | ||
let char_pos = line[..pos].chars().count() as u64; | ||
char_issues.push((char_pos, "'")); | ||
} | ||
if DOUBLE_CURLY_QUOTES.contains(&c) { | ||
let char_pos = line[..pos].chars().count() as u64; | ||
char_issues.push((char_pos, "\"")); | ||
} | ||
} | ||
|
||
if char_issues.is_empty() { | ||
continue; | ||
} | ||
|
||
// Build an array of replacements for each character in char_positions | ||
let replacements: Vec<Replacement> = char_issues | ||
.iter() | ||
.map(|&(char_pos, rchar)| Replacement { | ||
deleted_region: Range { | ||
start: Position { | ||
line: i as u64, | ||
character: char_pos, | ||
}, | ||
end: Position { | ||
line: i as u64, | ||
character: char_pos + 1, | ||
}, | ||
}, | ||
inserted_content: rchar.to_string(), | ||
}) | ||
.collect(); | ||
|
||
ret.push(Diagnostic { | ||
path: path.to_str().unwrap().to_string(), | ||
range: Some(Range { | ||
start: Position { | ||
line: i as u64, | ||
character: char_issues.first().unwrap().0, | ||
}, | ||
end: Position { | ||
line: i as u64, | ||
character: char_issues.last().unwrap().0 + 1, | ||
}, | ||
}), | ||
severity: Severity::Error, | ||
code: "no-curly-quotes".to_string(), | ||
message: format!("Found curly quote on line {}", i + 1), | ||
replacements: Some(replacements), | ||
}); | ||
} | ||
|
||
Ok(ret) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
use spectral::prelude::*; | ||
|
||
mod integration_testing; | ||
use integration_testing::TestRepo; | ||
|
||
const TOML_ON: &str = r#" | ||
[nocurlyquotes] | ||
enabled = true | ||
"#; | ||
|
||
const CURLY_QUOTES: &str = r#" | ||
the opening double quote ( “ ) U+201C | ||
the closing double quote ( ” ) U+201D | ||
the opening single quote ( ‘ ) U+2018 | ||
the closing single quote ( ’) U+2019 | ||
the double low quotation ( „ ) U+201E | ||
the double high reversed ( ‟ ) U+201F | ||
// | ||
"#; | ||
|
||
#[test] | ||
fn honor_disabled_in_config() -> anyhow::Result<()> { | ||
let test_repo: TestRepo = TestRepo::make().unwrap(); | ||
|
||
test_repo.write("src/curly.txt", "empty_file".as_bytes()); | ||
test_repo.git_add_all()?; | ||
test_repo.git_commit_all("create curly quote file"); | ||
test_repo.write("src/curly.txt", CURLY_QUOTES.as_bytes()); | ||
|
||
// disable nocurlyquotes | ||
let toml_off = r#" | ||
[nocurlyquotes] | ||
enabled = false | ||
"#; | ||
|
||
test_repo.set_toolbox_toml(TOML_ON); | ||
let mut horton = test_repo.run_horton()?; | ||
assert_that(&horton.has_result("no-curly-quotes", "", Some("src/curly.txt"))).is_true(); | ||
|
||
test_repo.set_toolbox_toml(toml_off); | ||
horton = test_repo.run_horton()?; | ||
|
||
assert_that(&horton.exit_code).contains_value(0); | ||
assert_that(&horton.has_result("no-curly-quotes", "", Some("src/curly.txt"))).is_false(); | ||
assert_that(&horton.has_result("toolbox-perf", "1 files processed", None)).is_true(); | ||
|
||
Ok(()) | ||
} | ||
|
||
#[test] | ||
fn assert_find_curly_quotes() { | ||
let test_repo = TestRepo::make().unwrap(); | ||
test_repo.set_toolbox_toml(TOML_ON); | ||
test_repo.write("revision.foo", "//".as_bytes()); | ||
test_repo.git_commit_all("create revision.foo"); | ||
|
||
{ | ||
test_repo.write("revision.foo", CURLY_QUOTES.as_bytes()); | ||
let horton = test_repo.run_horton().unwrap(); | ||
assert_that(&horton.exit_code).contains_value(0); | ||
assert_that(&horton.has_result( | ||
"no-curly-quotes", | ||
"Found curly quote on line 2", | ||
Some("revision.foo"), | ||
)) | ||
.is_true(); | ||
assert_that(&horton.has_result( | ||
"no-curly-quotes", | ||
"Found curly quote on line 3", | ||
Some("revision.foo"), | ||
)) | ||
.is_true(); | ||
assert_that(&horton.has_result( | ||
"no-curly-quotes", | ||
"Found curly quote on line 4", | ||
Some("revision.foo"), | ||
)) | ||
.is_true(); | ||
assert_that(&horton.has_result( | ||
"no-curly-quotes", | ||
"Found curly quote on line 5", | ||
Some("revision.foo"), | ||
)) | ||
.is_true(); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the replacements 1-indexed and the diagnostics are 0-indexed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not, the test you added asserts the message, not the diagnostic location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch - I shoud standardize this in the diagnostics.rs file where we make every 1 index assuming input is 0 except for startLine and endLine...whoops.