feat: oldest in the books→oldest in the book (#2337)

* feat: oldest in the books→oldest in the book

* fix: different `messages` when it's sure you're referring to the idiom vs not sure

harper-core/src/linting/lint_group.rs

@@ -122,6 +122,7 @@ use super::nominal_wants::NominalWants;
 use super::noun_verb_confusion::NounVerbConfusion;
 use super::number_suffix_capitalization::NumberSuffixCapitalization;
 use super::of_course::OfCourse;
+use super::oldest_in_the_book::OldestInTheBook;
 use super::on_floor::OnFloor;
 use super::once_or_twice::OnceOrTwice;
 use super::one_and_the_same::OneAndTheSame;
@@ -508,8 +509,8 @@ impl LintGroup {
         insert_expr_rule!(BeAllowed, true);
         insert_expr_rule!(BestOfAllTime, true);
         insert_expr_rule!(BoringWords, false);
-        insert_expr_rule!(BrandBrandish, true);
         insert_expr_rule!(Bought, true);
+        insert_expr_rule!(BrandBrandish, true);
         insert_expr_rule!(Cant, true);
         insert_struct_rule!(CapitalizePersonalPronouns, true);
         insert_expr_rule!(CautionaryTale, true);
@@ -592,6 +593,7 @@ impl LintGroup {
         insert_struct_rule!(NounVerbConfusion, true);
         insert_struct_rule!(NumberSuffixCapitalization, true);
         insert_expr_rule!(OfCourse, true);
+        insert_expr_rule!(OldestInTheBook, true);
         insert_expr_rule!(OnFloor, true);
         insert_expr_rule!(OnceOrTwice, true);
         insert_expr_rule!(OneAndTheSame, true);

harper-core/src/linting/mod.rs

@@ -130,6 +130,7 @@ mod nominal_wants;
 mod noun_verb_confusion;
 mod number_suffix_capitalization;
 mod of_course;
+mod oldest_in_the_book;
 mod on_floor;
 mod once_or_twice;
 mod one_and_the_same;
@@ -476,6 +477,23 @@ pub mod tests {
         }
     }
 
+    /// Asserts that the lint's message matches the expected message.
+    #[track_caller]
+    pub fn assert_lint_message(text: &str, mut linter: impl Linter, expected_message: &str) {
+        let test = Document::new_markdown_default_curated(text);
+        let lints = linter.lint(&test);
+
+        // Just check the first lint for now
+        if let Some(lint) = lints.first() {
+            if lint.message != expected_message {
+                panic!(
+                    "Expected lint message \"{expected_message}\", but got \"{}\"",
+                    lint.message
+                );
+            }
+        }
+    }
+
     fn transform_nth_str(text: &str, linter: &mut impl Linter, n: usize) -> String {
         let mut text_chars: Vec<char> = text.chars().collect();
 

harper-core/src/linting/oldest_in_the_book.rs

@@ -0,0 +1,179 @@
+use crate::{
+    CharStringExt, Lint, Token,
+    expr::{Expr, Repeating, SequenceExpr},
+    linting::{ExprLinter, LintKind, Suggestion, expr_linter::Chunk},
+};
+
+pub struct OldestInTheBook {
+    expr: Box<dyn Expr>,
+}
+
+impl Default for OldestInTheBook {
+    fn default() -> Self {
+        let adj = |t: &Token, s: &[char]| {
+            let k = &t.kind;
+            (k.is_np_member() || k.is_adjective())
+                && !k.is_noun()
+                && !t
+                    .span
+                    .get_content(s)
+                    .eq_ignore_ascii_case_chars(&['i', 'n'])
+        };
+
+        // Zero or more adjectives
+        let adjseq = Repeating::new(Box::new(SequenceExpr::default().then(adj).t_ws()), 0);
+
+        let noun = |t: &Token, s: &[char]| {
+            let k = &t.kind;
+            (k.is_np_member() || k.is_noun() || k.is_oov())
+                && !t
+                    .span
+                    .get_content(s)
+                    .eq_ignore_ascii_case_chars(&['i', 'n'])
+        };
+
+        // One or more nouns
+        let nounseq = SequenceExpr::default()
+            .then(noun)
+            .then_optional(Repeating::new(
+                Box::new(SequenceExpr::default().t_ws().then(noun)),
+                1,
+            ));
+
+        let noun_phrase = SequenceExpr::default().then_optional(adjseq).then(nounseq);
+
+        Self {
+            expr: Box::new(
+                SequenceExpr::fixed_phrase("oldest ")
+                    .then(noun_phrase)
+                    .then_fixed_phrase(" in the books"),
+            ),
+        }
+    }
+}
+
+impl ExprLinter for OldestInTheBook {
+    type Unit = Chunk;
+
+    fn expr(&self) -> &dyn Expr {
+        self.expr.as_ref()
+    }
+
+    fn match_to_lint_with_context(
+        &self,
+        toks: &[Token],
+        src: &[char],
+        _ctx: Option<(&[Token], &[Token])>,
+    ) -> Option<Lint> {
+        let np = &toks[2..toks.len() - 4];
+        let tricky = np.iter().any(|n| {
+            n.span
+                .get_content(src)
+                .eq_any_ignore_ascii_case_str(&["trick", "tricks"])
+        });
+
+        let message = if tricky {
+            "This idiom should use singular `book` instead of plural `books`."
+        } else {
+            "If this is a play on the idiom `oldest trick in the book`, it should use singular `book` instead of plural `books`."
+        }
+        .to_string();
+
+        Some(Lint {
+            span: toks.last()?.span,
+            lint_kind: LintKind::Usage,
+            suggestions: vec![Suggestion::replace_with_match_case_str(
+                "book",
+                toks.last()?.span.get_content(src),
+            )],
+            message,
+            ..Default::default()
+        })
+    }
+
+    fn description(&self) -> &'static str {
+        "Detects the idiom `oldest X in the books`, which should use singular `book`."
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::OldestInTheBook;
+    use crate::linting::tests::{assert_lint_message, assert_suggestion_result};
+
+    // Probable references to the idiom "oldest trick in the book"
+
+    #[test]
+    fn fix_delphi_mistake() {
+        assert_suggestion_result(
+            "This is the oldest Delphi mistake in the books and I'm sure you've made it before (we all have), and I'm sure you recognise it when you see it.",
+            OldestInTheBook::default(),
+            "This is the oldest Delphi mistake in the book and I'm sure you've made it before (we all have), and I'm sure you recognise it when you see it.",
+        );
+    }
+
+    #[test]
+    fn fix_trick() {
+        assert_suggestion_result(
+            "... oldest trick in the books, a restart and it works all the times(for now).",
+            OldestInTheBook::default(),
+            "... oldest trick in the book, a restart and it works all the times(for now).",
+        );
+    }
+
+    #[test]
+    fn fix_virus_trick() {
+        assert_suggestion_result(
+            "Once the OS is started the MBR is typically protected for virus reasons - this is one of the oldest virus tricks in the books - goes back to ...",
+            OldestInTheBook::default(),
+            "Once the OS is started the MBR is typically protected for virus reasons - this is one of the oldest virus tricks in the book - goes back to ...",
+        )
+    }
+
+    #[test]
+    fn fix_mistake() {
+        assert_suggestion_result(
+            "Ok, I realized now that I was making the oldest mistake in the books with my code, dividing my v by 2 instead of dividing it by 5.",
+            OldestInTheBook::default(),
+            "Ok, I realized now that I was making the oldest mistake in the book with my code, dividing my v by 2 instead of dividing it by 5.",
+        );
+    }
+
+    #[test]
+    fn fix_tricks() {
+        assert_suggestion_result(
+            "He enables the oldest tricks in the books, create fear from thing like prosperity (we really don't need Foxconn?)",
+            OldestInTheBook::default(),
+            "He enables the oldest tricks in the book, create fear from thing like prosperity (we really don't need Foxconn?)",
+        );
+    }
+
+    #[test]
+    fn fix_military_plays() {
+        assert_suggestion_result(
+            "Isnt that like one of the oldest military plays in the books?",
+            OldestInTheBook::default(),
+            "Isnt that like one of the oldest military plays in the book?",
+        );
+    }
+
+    // Test messages
+
+    #[test]
+    fn is_oldest_trick_in_the_books_ref_to_idom() {
+        assert_lint_message(
+            "This is one of the oldest trick in the books",
+            OldestInTheBook::default(),
+            "This idiom should use singular `book` instead of plural `books`.",
+        );
+    }
+
+    #[test]
+    fn is_chromatic_alterations_ref_to_idom() {
+        assert_lint_message(
+            "One of the oldest chromatic alterations in the books is the raising of the leading tone",
+            OldestInTheBook::default(),
+            "If this is a play on the idiom `oldest trick in the book`, it should use singular `book` instead of plural `books`.",
+        );
+    }
+}