Add "no-curly-quotes" rule to the codebase. (#56)

Adds support for nocurlyquotes that will find instances of curlyquotes
in a file and replace with correct non-curly form. We had instances of
this in our code base and it caused some issues with marketing site.

Fun little rule. Interestingly - this rule exposes a bug in how we apply
on `trunk check` sarif fixes for utf-8 non-ascii characters. For now
this rule will stay disabled until we have a fix.

Fix bug where multiple fixes in a single issue were being turned into
multiple issues (which is not what the spec or code was intending to
do).

.trunk/config/toolbox.toml

@@ -12,3 +12,6 @@ enabled = false
 [neveredit]
 enabled = false
 paths = []
+
+[nocurlyquotes]
+enabled = false

src/config.rs

@@ -15,6 +15,9 @@ pub struct Conf {
 
     #[config(nested)]
     pub neveredit: NeverEditConf,
+
+    #[config(nested)]
+    pub nocurlyquotes: NoCurlyQuotesConf,
 }
 
 impl Conf {
@@ -49,3 +52,9 @@ pub struct NeverEditConf {
     #[config(default = [])]
     pub paths: Vec<String>,
 }
+
+#[derive(Config)]
+pub struct NoCurlyQuotesConf {
+    #[config(default = false)]
+    pub enabled: bool,
+}

src/diagnostic.rs

@@ -83,11 +83,7 @@ impl Diagnostic {
         }
 
         let fixes = if let Some(replacements) = &self.replacements {
-            let mut fixes = Vec::new();
-            for replacement in replacements {
-                fixes.push(replacement.to_fix(self));
-            }
-            Some(fixes)
+            Some(vec![self.build_fix(replacements)])
         } else {
             None
         };
@@ -109,36 +105,47 @@ impl Diagnostic {
             .build()
             .unwrap()
     }
-}
 
-impl Replacement {
-    pub fn to_fix(&self, diag: &Diagnostic) -> sarif::Fix {
+    pub fn build_fix(&self, replacements: &[Replacement]) -> sarif::Fix {
         sarif::FixBuilder::default()
             .artifact_changes([sarif::ArtifactChangeBuilder::default()
                 .artifact_location(
                     sarif::ArtifactLocationBuilder::default()
-                        .uri(diag.path.clone())
+                        .uri(self.path.clone())
                         .build()
                         .unwrap(),
                 )
-                .replacements(vec![sarif::ReplacementBuilder::default()
-                    .deleted_region(
-                        sarif::RegionBuilder::default()
-                            .start_line(self.deleted_region.start.line as i64)
-                            .start_column(self.deleted_region.start.character as i64 + 1)
-                            .end_line(self.deleted_region.end.line as i64)
-                            .end_column(self.deleted_region.end.character as i64 + 1)
-                            .build()
-                            .unwrap(),
-                    )
-                    .inserted_content(
-                        sarif::ArtifactContentBuilder::default()
-                            .text(self.inserted_content.clone())
-                            .build()
-                            .unwrap(),
-                    )
-                    .build()
-                    .unwrap()])
+                .replacements(
+                    replacements
+                        .iter()
+                        .map(|replacement| {
+                            sarif::ReplacementBuilder::default()
+                                .deleted_region(
+                                    sarif::RegionBuilder::default()
+                                        .start_line(
+                                            replacement.deleted_region.start.line as i64 + 1,
+                                        )
+                                        .start_column(
+                                            replacement.deleted_region.start.character as i64 + 1,
+                                        )
+                                        .end_line(replacement.deleted_region.end.line as i64 + 1)
+                                        .end_column(
+                                            replacement.deleted_region.end.character as i64 + 1,
+                                        )
+                                        .build()
+                                        .unwrap(),
+                                )
+                                .inserted_content(
+                                    sarif::ArtifactContentBuilder::default()
+                                        .text(replacement.inserted_content.clone())
+                                        .build()
+                                        .unwrap(),
+                                )
+                                .build()
+                                .unwrap()
+                        })
+                        .collect::<Vec<_>>(),
+                )
                 .build()
                 .unwrap()])
             .build()

src/main.rs

@@ -4,6 +4,7 @@ use horton::config::Conf;
 use horton::diagnostic;
 use horton::rules::if_change_then_change::ictc;
 use horton::rules::never_edit::never_edit;
+use horton::rules::no_curly_quotes::no_curly_quotes;
 use horton::rules::pls_no_land::pls_no_land;
 use horton::run::{Cli, OutputFormat, Run, Subcommands};
 
@@ -164,6 +165,12 @@ fn run() -> anyhow::Result<()> {
         Err(e) => return Err(e),
     }
 
+    let ncq_result = no_curly_quotes(&run, &cli.upstream);
+    match ncq_result {
+        Ok(result) => ret.diagnostics.extend(result),
+        Err(e) => return Err(e),
+    }
+
     let mut output_string = generate_line_string(&ret);
     if cli.output_format == OutputFormat::Sarif {
         output_string = generate_sarif_string(&ret, &run, &start)?;

src/rules/mod.rs

@@ -1,3 +1,4 @@
 pub mod if_change_then_change;
 pub mod never_edit;
+pub mod no_curly_quotes;
 pub mod pls_no_land;

src/rules/no_curly_quotes.rs

@@ -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)
+}

tests/no_curly_quote_test.rs

@@ -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();
+    }
+}