Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hippietrail
Copy link
Contributor

Addresses the false positives brought to light in #681

Addresses the false positives brought to light in Automattic#681
@hippietrail hippietrail changed the title feat: move 'could of' etc from mather to own linter feat: move 'could of' etc from matcher.rs to its own linter Feb 19, 2025
Copy link
Collaborator

@elijah-potter elijah-potter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicky stuff, this looks good overall. Keep up the great work!

}

#[test]
fn test_for_of() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would replace "test" with whether the rule should correct or allow the phrase.

For example: catches_missed_part. That way, the intent is better conveyed.


#[test]
fn test_missed_part() {
assert_lint_count("Ah I must of missed that part.", ModalOf::default(), 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For test cases where the rule should match, try to use assert_suggestion_result instead.

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your priorities here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants