feat: move 'could of' etc from matcher.rs to its own linter

harper-core/src/linting/lint_group.rs

@@ -33,6 +33,7 @@ use super::linking_verbs::LinkingVerbs;
 use super::long_sentences::LongSentences;
 use super::matcher::Matcher;
 use super::merge_words::MergeWords;
+use super::modal_of::ModalOf;
 use super::multiple_sequential_pronouns::MultipleSequentialPronouns;
 use super::nobody::Nobody;
 use super::number_suffix_capitalization::NumberSuffixCapitalization;
@@ -335,7 +336,8 @@ create_lint_group_config!(
     SpecialAttention => true,
     Everywhere => true,
     ThanOthers => true,
-    SupposedTo => true
+    SupposedTo => true,
+    ModalOf => true,
 );
 
 impl<T: Dictionary + Default> Default for LintGroup<T> {

harper-core/src/linting/matcher.rs

@@ -201,18 +201,6 @@ impl Matcher {
 
         // wrong set phrases and collocations
         triggers.extend(pt! {
-            "could", "of" => "could have",
-            "could", "of" => "could've",
-            "couldn't", "of" => "couldn't have",
-            "had", "of" => "had have",
-            "had", "of" => "had've",
-            "hadn't", "of" => "hadn't have",
-            "should", "of" => "should have",
-            "should", "of" => "should've",
-            "shouldn't", "of" => "shouldn't have",
-            "would", "of" => "would have",
-            "would", "of" => "would've",
-            "wouldn't", "of" => "wouldn't have",
             "discuss", "about" => "discuss",
             "discussed", "about" => "discussed",
             "discusses", "about" => "discusses",

harper-core/src/linting/mod.rs

@@ -30,6 +30,7 @@ mod long_sentences;
 mod matcher;
 mod merge_linters;
 mod merge_words;
+mod modal_of;
 mod multiple_sequential_pronouns;
 mod no_oxford_comma;
 mod nobody;
@@ -96,6 +97,7 @@ pub use lint_kind::LintKind;
 pub use long_sentences::LongSentences;
 pub use matcher::Matcher;
 pub use merge_words::MergeWords;
+pub use modal_of::ModalOf;
 pub use multiple_sequential_pronouns::MultipleSequentialPronouns;
 pub use no_oxford_comma::NoOxfordComma;
 pub use nobody::Nobody;

harper-core/src/linting/modal_of.rs

@@ -0,0 +1,226 @@
+use crate::{
+    patterns::{OwnedPatternExt, Pattern, SequencePattern, WordSet},
+    Lrc, Token, TokenStringExt,
+};
+
+use super::{Lint, LintKind, PatternLinter, Suggestion};
+
+pub struct ModalOf {
+    pattern: Box<dyn Pattern>,
+}
+
+impl Default for ModalOf {
+    fn default() -> Self {
+        // Note 1. "shan't of" is plausible but very unlikely
+        // Note 2. "had of" has trickier false positives and is less common anyway
+        // "The only other report we've had of this kind of problem ..."
+        // "The code I had of this used to work fine ..."
+        let modals = ["could", "might", "must", "should", "would"];
+        let mut words = WordSet::all(&modals);
+        modals.iter().for_each(|word| {
+            words.add(&format!("{}n't", word));
+        });
+
+        let modal_of = Lrc::new(
+            SequencePattern::default()
+                .then(Box::new(words))
+                .then_whitespace()
+                .then_exact_word("of"),
+        );
+
+        let ws_course = Lrc::new(
+            SequencePattern::default()
+                .then_whitespace()
+                .then_exact_word("course"),
+        );
+
+        Self {
+            pattern: Box::new(
+                SequencePattern::default()
+                    .then(Box::new(modal_of.clone()))
+                    .then(Box::new(ws_course.clone()))
+                    .or(Box::new(modal_of.clone())),
+            ),
+        }
+    }
+}
+
+impl PatternLinter for ModalOf {
+    fn pattern(&self) -> &dyn Pattern {
+        self.pattern.as_ref()
+    }
+
+    fn match_to_lint(&self, matched_toks: &[Token], source_chars: &[char]) -> Option<Lint> {
+        if matched_toks.len() != 3 {
+            return None;
+        }
+
+        let span_modal_of = matched_toks[0..3].span().unwrap();
+        let span_modal = matched_toks[0].span;
+
+        let modal_have = format!("{} have", span_modal.get_content_string(source_chars))
+            .chars()
+            .collect();
+        let modal_ws_of = span_modal_of.get_content(source_chars);
+
+        Some(Lint {
+            span: span_modal_of,
+            lint_kind: LintKind::WordChoice,
+            suggestions: vec![Suggestion::replace_with_match_case(modal_have, modal_ws_of)],
+            message: "Use `have` rather than `of` here.".to_string(),
+            priority: 126,
+        })
+    }
+
+    fn description(&self) -> &'static str {
+        "Detects `of` mistakenly used with `would`, `could`, `should`, etc."
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::ModalOf;
+    use crate::linting::tests::{assert_lint_count, assert_suggestion_result};
+
+    // atomic unit tests
+
+    #[test]
+    fn test_lowercase() {
+        assert_suggestion_result("could of", ModalOf::default(), "could have");
+    }
+
+    #[test]
+    fn test_negative() {
+        assert_suggestion_result("mightn't of", ModalOf::default(), "mightn't have");
+    }
+
+    #[test]
+    fn test_uppercase_negative() {
+        assert_suggestion_result("Mustn't of", ModalOf::default(), "Mustn't have");
+    }
+
+    #[test]
+    fn test_false_positive() {
+        assert_lint_count("should of course", ModalOf::default(), 0);
+    }
+
+    #[test]
+    fn test_false_positive_capital_negative() {
+        assert_lint_count("Wouldn't of course", ModalOf::default(), 0);
+    }
+
+    // real-world tests
+
+    #[test]
+    fn test_buggy_implementation() {
+        assert_lint_count(
+            "... could of just been a buggy implementation",
+            ModalOf::default(),
+            1,
+        );
+    }
+
+    #[test]
+    fn test_missed_one() {
+        assert_lint_count(
+            "We already have a function ... that nedb can understand so we might of missed one.",
+            ModalOf::default(),
+            1,
+        );
+    }
+
+    #[test]
+    fn test_user_option() {
+        assert_lint_count(
+            "im more likely to believe you might of left in the 'user' option",
+            ModalOf::default(),
+            1,
+        );
+    }
+
+    #[test]
+    fn catches_must_of() {
+        assert_suggestion_result(
+            "Ah I must of missed that part.",
+            ModalOf::default(),
+            "Ah I must have missed that part.",
+        );
+    }
+
+    #[test]
+    fn catches_should_of() {
+        assert_lint_count(
+            "Yeah I should of just mentioned it should of been a for of.",
+            ModalOf::default(),
+            2,
+        );
+    }
+
+    #[test]
+    fn catches_would_of() {
+        assert_suggestion_result(
+            "now this issue would of caused hundreds of thousands of extra lines",
+            ModalOf::default(),
+            "now this issue would have caused hundreds of thousands of extra lines",
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_you_could_of_course() {
+        assert_lint_count(
+            "You could of course explicit the else with each possibility",
+            ModalOf::default(),
+            0,
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_compiler_could_of_course() {
+        assert_lint_count(
+            "The compiler could of course detect this too",
+            ModalOf::default(),
+            0,
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_might_of_course_be() {
+        assert_lint_count("There might of course be other places where not implementing the IMemberSource might break ...", ModalOf::default(), 0);
+    }
+
+    #[test]
+    fn doesnt_catch_not_a_must_of_course() {
+        assert_lint_count(
+            "Not a must of course if the convention should be .ts",
+            ModalOf::default(),
+            0,
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_must_of_course_also() {
+        assert_lint_count(
+            "the schedular must of course also have run through",
+            ModalOf::default(),
+            0,
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_should_of_course_not() {
+        assert_lint_count(
+            "not being local should of course not be supported",
+            ModalOf::default(),
+            0,
+        );
+    }
+
+    #[test]
+    fn doesnt_catch_would_of_course_just() {
+        assert_lint_count(
+            "I would of course just test this by compiling with MATX_MULTI_GPU=ON",
+            ModalOf::default(),
+            0,
+        );
+    }
+}