diff --git a/harper-core/src/linting/lint_group.rs b/harper-core/src/linting/lint_group.rs index 19f921e1..3bf72e01 100644 --- a/harper-core/src/linting/lint_group.rs +++ b/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 Default for LintGroup { diff --git a/harper-core/src/linting/matcher.rs b/harper-core/src/linting/matcher.rs index f06a21f3..641a685c 100644 --- a/harper-core/src/linting/matcher.rs +++ b/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", diff --git a/harper-core/src/linting/mod.rs b/harper-core/src/linting/mod.rs index 25b3aeef..ef3cdce7 100644 --- a/harper-core/src/linting/mod.rs +++ b/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; diff --git a/harper-core/src/linting/modal_of.rs b/harper-core/src/linting/modal_of.rs new file mode 100644 index 00000000..e9e1a01e --- /dev/null +++ b/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, +} + +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 { + 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, + ); + } +}