Skip to content

Commit

Permalink
feat(core): added linter for #225
Browse files Browse the repository at this point in the history
  • Loading branch information
elijah-potter committed Jan 14, 2025
1 parent b3f9c62 commit 113bc36
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 3 deletions.
34 changes: 33 additions & 1 deletion harper-core/src/linting/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Display for LintKind {
}
}

#[derive(Debug, Clone, Serialize, Deserialize, Is)]
#[derive(Debug, Clone, Serialize, Deserialize, Is, PartialEq, Eq)]
pub enum Suggestion {
ReplaceWith(Vec<char>),
/// Insert the provided characters _after_ the offending text.
Expand All @@ -69,6 +69,25 @@ pub enum Suggestion {
}

impl Suggestion {
/// Construct an instance of [`Self::ReplaceWith`], but make the content match the case of the
/// provided template.
///
/// For example, if we want to replace "You're" with "You are", we can provide "you are" and
/// "You're".
pub fn replace_with_match_case(mut value: Vec<char>, template: &[char]) -> Self {
for (v, t) in value.iter_mut().zip(template.iter()) {
if v.is_ascii_uppercase() != t.is_ascii_uppercase() {
if t.is_uppercase() {
*v = v.to_ascii_uppercase();
} else {
*v = v.to_ascii_lowercase();
}
}
}

Self::ReplaceWith(value)
}

/// Apply a suggestion to a given text.
pub fn apply(&self, span: Span, source: &mut Vec<char>) {
match self {
Expand Down Expand Up @@ -130,4 +149,17 @@ mod tests {

assert_eq!(source_chars, "This, is a test".chars().collect::<Vec<_>>());
}

#[test]
fn suggestion_your_match_case() {
let template: Vec<_> = "You're".chars().collect();
let value: Vec<_> = "you are".chars().collect();

let correct = "You are".chars().collect();

assert_eq!(
Suggestion::replace_with_match_case(value, &template),
Suggestion::ReplaceWith(correct)
)
}
}
4 changes: 3 additions & 1 deletion harper-core/src/linting/lint_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use super::merge_words::MergeWords;
use super::multiple_sequential_pronouns::MultipleSequentialPronouns;
use super::number_suffix_capitalization::NumberSuffixCapitalization;
use super::plural_conjugate::PluralConjugate;
use super::pronoun_contraction::PronounContraction;
use super::proper_noun_capitalization_linters::{
AmazonNames, Americas, AppleNames, AzureNames, ChineseCommunistParty, GoogleNames, Holidays,
Koreas, MetaNames, MicrosoftNames, UnitedOrganizations,
Expand Down Expand Up @@ -184,7 +185,8 @@ create_lint_group_config!(
AzureNames => true,
MergeWords => true,
PluralConjugate => false,
OxfordComma => true
OxfordComma => true,
PronounContraction => true
);

impl<T: Dictionary + Default> Default for LintGroup<T> {
Expand Down
41 changes: 41 additions & 0 deletions harper-core/src/linting/merge_linters.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
macro_rules! merge_linters {
($name:ident => $($linter:ident),* => $desc:expr) => {
pub use merge_rule_hidden::$name;

mod merge_rule_hidden {
use paste::paste;
use crate::{Document, linting::{Lint, Linter}};

$(
use super::$linter;
)*

paste! {
#[derive(Default)]
pub struct $name {
$(
[< $linter:snake >]: $linter,
)*
}

impl Linter for $name {
fn lint(&mut self, document: &Document) -> Vec<Lint>{
let mut lints = Vec::new();

$(
lints.extend(self.[< $linter:snake >].lint(document));
)*

lints
}

fn description(&self) -> &'static str {
$desc
}
}
}
}
};
}

pub(crate) use merge_linters;
3 changes: 3 additions & 0 deletions harper-core/src/linting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ mod lint;
mod lint_group;
mod long_sentences;
mod matcher;
mod merge_linters;
mod merge_words;
mod multiple_sequential_pronouns;
mod number_suffix_capitalization;
mod oxford_comma;
mod pattern_linter;
mod plural_conjugate;
mod pronoun_contraction;
mod proper_noun_capitalization_linters;
mod repeated_words;
mod sentence_capitalization;
Expand Down Expand Up @@ -47,6 +49,7 @@ pub use number_suffix_capitalization::NumberSuffixCapitalization;
pub use oxford_comma::OxfordComma;
pub use pattern_linter::PatternLinter;
pub use plural_conjugate::PluralConjugate;
pub use pronoun_contraction::PronounContraction;
pub use proper_noun_capitalization_linters::{
AmazonNames, Americas, AppleNames, AzureNames, ChineseCommunistParty, GoogleNames, Holidays,
Koreas, MetaNames, MicrosoftNames, UnitedOrganizations,
Expand Down
70 changes: 70 additions & 0 deletions harper-core/src/linting/pronoun_contraction/avoid_contraction.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use crate::{
patterns::{Pattern, SequencePattern},
Token,
};

use super::super::{Lint, LintKind, PatternLinter, Suggestion};

pub struct AvoidContraction {
pattern: Box<dyn Pattern>,
}

impl Default for AvoidContraction {
fn default() -> Self {
let pattern = SequencePattern::aco("you're").then_whitespace().then_noun();

Self {
pattern: Box::new(pattern),
}
}
}

impl PatternLinter for AvoidContraction {
fn pattern(&self) -> &dyn Pattern {
self.pattern.as_ref()
}

fn match_to_lint(&self, matched_tokens: &[Token], source: &[char]) -> Lint {
let word = matched_tokens[0].span.get_content(source);

Lint {
span: matched_tokens[0].span,
lint_kind: LintKind::WordChoice,
suggestions: vec![Suggestion::replace_with_match_case(
vec!['y', 'o', 'u', 'r'],
word,
)],
message: "I appears you intended to use the possessive version of this word".to_owned(),
priority: 63,
}
}

fn description(&self) -> &'static str {
"This rule looks for situations where a contraction was used where it shouldn't."

This comment has been minimized.

Copy link
@ccoVeille

ccoVeille Jan 15, 2025

Contributor

Nitpicking and humor: I wouldn't have used a contraction in the description of a rule that aims to avoid contractions

-"This rule looks for situations where a contraction was used where it shouldn't."
+"This rule looks for situations where a contraction was used where it should not."
}
}

#[cfg(test)]
mod tests {
use crate::linting::tests::assert_suggestion_result;

use super::AvoidContraction;

#[test]
fn issue_139() {
assert_suggestion_result(
"it would be great if you're PR was merged into tower-lsp",
AvoidContraction::default(),
"it would be great if your PR was merged into tower-lsp",
);
}

#[test]
fn car() {
assert_suggestion_result(
"You're car is black.",
AvoidContraction::default(),
"Your car is black.",
);
}
}
9 changes: 9 additions & 0 deletions harper-core/src/linting/pronoun_contraction/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
use super::merge_linters::merge_linters;

mod avoid_contraction;
mod should_contract;

use avoid_contraction::AvoidContraction;
use should_contract::ShouldContract;

merge_linters! {PronounContraction => ShouldContract, AvoidContraction => "Choosing when to contract pronouns is a challenging art. This rule looks for faults." }
82 changes: 82 additions & 0 deletions harper-core/src/linting/pronoun_contraction/should_contract.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
use crate::{
patterns::{Pattern, SequencePattern, WordSet},
CharStringExt, Token,
};

use crate::linting::{LintKind, PatternLinter, Suggestion};
use crate::Lint;

pub struct ShouldContract {
pattern: Box<dyn Pattern>,
}

impl Default for ShouldContract {
fn default() -> Self {
Self {
pattern: Box::new(
SequencePattern::default()
.then_word_set(WordSet::all(&["your", "were"]))
.then_whitespace()
.t_aco("the")
.then_whitespace()
.then_noun(),
),
}
}
}

impl ShouldContract {
fn mistake_to_correct(mistake: &str) -> impl Iterator<Item = Vec<char>> {
match mistake.to_lowercase().as_str() {
"your" => vec!["you're", "you are"],
"were" => vec!["we're", "we are"],
_ => panic!("The pattern in this linter should make a fall-through impossible."),
}
.into_iter()
.map(|v| v.chars().collect())
}
}

impl PatternLinter for ShouldContract {
fn pattern(&self) -> &dyn Pattern {
self.pattern.as_ref()
}

fn match_to_lint(&self, matched_tokens: &[Token], source: &[char]) -> Lint {
let mistake = matched_tokens[0].span.get_content(source);

Lint {
span: matched_tokens[0].span,
lint_kind: LintKind::WordChoice,
suggestions: Self::mistake_to_correct(&mistake.to_lower().to_string())
.map(|v| Suggestion::replace_with_match_case(v, mistake))
.collect(),
message: "Use the contraction or separate the words instead.".to_string(),
priority: 31,
}
}

fn description(&self) -> &'static str {
"Neglecting the apostrophe when contracting pronouns with \"are\" (like \"your\" and \"you are\") is a fatal, but extremely common mistake to make."
}
}

#[cfg(test)]
mod tests {
use super::ShouldContract;
use crate::linting::tests::assert_suggestion_result;

#[test]
fn issue_225() {
assert_suggestion_result("Your the man", ShouldContract::default(), "You're the man");
}

#[test]
fn were_team() {
assert_suggestion_result(
"Were the best team.",
ShouldContract::default(),
"We're the best team.",
);
}
}
6 changes: 5 additions & 1 deletion harper-core/src/patterns/sequence_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use hashbrown::HashSet;
use paste::paste;

use super::whitespace_pattern::WhitespacePattern;
use super::{NounPhrase, Pattern, RepeatingPattern};
use super::{NounPhrase, Pattern, RepeatingPattern, WordSet};
use crate::{CharStringExt, Lrc, Token, TokenKind};

/// A pattern that checks that a sequence of other patterns match.
Expand Down Expand Up @@ -59,6 +59,10 @@ impl SequencePattern {
gen_then_from_is!(apostrophe);
gen_then_from_is!(hyphen);

pub fn then_word_set(self, set: WordSet) -> Self {
self.then(Box::new(set))
}

/// Add a pattern that looks for more complex ideas, like nouns with adjectives attached.
pub fn then_noun_phrase(self) -> Self {
self.then(Box::new(NounPhrase))
Expand Down

0 comments on commit 113bc36

Please sign in to comment.