Skip to content

Commit

Permalink
Get rid of nesting
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanrittenhouse
evanrittenhouse committed Apr 25, 2023
1 parent 67b251a commit 58bfb57
Showing 1 changed file with 69 additions and 75 deletions.
144 changes: 69 additions & 75 deletions crates/ruff/src/rules/flake8_todo/rules.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ impl Violation for MissingSpaceAfterColonInTodo {

// Capture groups correspond to checking:
// 1. Tag (used to match against `TODO` in capitalization and spelling, e.g. ToDo and FIXME)
// 2. Author exists (in parentheses) after tag, but before colon
// 2. Author exists (in parentheses) with 0 or 1 spaces between it and the tag, but before colon
// 3. Colon exists after author
// 4. Space exists after colon
// 5. Text exists after space
Expand Down Expand Up @@ -248,100 +248,90 @@ pub fn check_todos(
let mut prev_token_is_todo = false;

for (start, token, end) in tokens.iter().flatten() {
let Tok::Comment(comment) = token else {
continue;
};

let diagnostics_ref = &mut diagnostics;
let range = Range::new(*start, *end);

// The previous token is a TODO, so let's check if this token is a link
if prev_token_is_todo {
if ISSUE_LINK_REGEX_SET.is_match(comment) {
prev_token_is_todo = false;
// continuing here avoids an expensive call to captures_iter(). if this line is a
// link, we can skip checking if it's a TODO
continue;
}
let Some(comment) = token_opt else {
prev_token_is_todo = false;
continue;
};

diagnostics_ref.push(Diagnostic::new(MissingLinkInTodo, range));
}
let mut captures_opt = get_captured_matches(comment);
if captures_opt.peek().is_none() {
// If we didn't match the regex at all, we know that this token isn't a TODO. The regex
// defined above requires that the `tag` capture group is matched.
prev_token_is_todo = false;
continue;
};

// TODO: gotta be a way to remove this nest
if let Some(captures) = get_captured_matches(comment).peek() {
// Unwrap is safe because the "tag" capture group is required to enter the current
// block
let tag = captures.name("tag").unwrap().as_str();
if tag != "TODO" {
diagnostics_ref.push(Diagnostic::new(
InvalidTodoTag {
// Check for errors on the tag.
// Unwrap is safe because the "tag" capture group is required to get here.
let captures = captures_opt.peek().unwrap();
let tag = captures.name("tag").unwrap().as_str();
if tag != "TODO" {
diagnostics_ref.push(Diagnostic::new(
InvalidTodoTag {
tag: String::from(tag),
},
range,
));

if tag.to_uppercase() == "TODO" {
let invalid_capitalization = Diagnostic::new(
InvalidCapitalizationInTodo {
tag: String::from(tag),
},
range,
));

if tag.to_uppercase() == "TODO" {
let invalid_capitalization = Diagnostic::new(
InvalidCapitalizationInTodo {
tag: String::from(tag),
},
range,
)
.with_fix(
if should_autofix(autofix, settings, Rule::InvalidCapitalizationInTodo) {
// The TODO regex allows for any number of spaces, so let's find where the first "t"
// or "T" is. We know the unwrap is safe because of the mandatory regex
// match. We'll use position() since "#" is 2 bytes which could throw
// off an implementation that uses byte-indexing.
let first_t_location = comment
.chars()
.position(|c| c.to_string() == "t" || c.to_string() == "T")
.unwrap();
)
.with_fix(
if should_autofix(autofix, settings, Rule::InvalidCapitalizationInTodo) {
// The TODO regex allows for any number of spaces, so let's find where the first "t"
// or "T" is. We know the unwrap is safe because of the mandatory regex
// match. We'll use position() since "#" is 2 bytes which could throw
// off an implementation that uses byte-indexing.
let first_t_position = comment
.chars()
.position(|c| c.to_string() == "t" || c.to_string() == "T")
.unwrap();

Fix::new(vec![Edit::replacement(
"TODO".to_string(),
Location::new(range.location.row(), first_t_location),
Location::new(
range.location.row(),
// We know `tag` is 4 bytes long because we've already ensured
// it's some variant of "TODO"
first_t_location + tag.len(),
),
)])
} else {
Fix::empty()
},
);
Fix::new(vec![Edit::replacement(
"TODO".to_string(),
Location::new(range.location.row(), first_t_position),
Location::new(
range.location.row(),
// We know `tag` is 4 bytes long because we've already ensured
// it's some variant of "TODO", so we're guaranteed to have
// consistency.
first_t_position + tag.len(),
),
)])
} else {
Fix::empty()
},
);

diagnostics_ref.push(invalid_capitalization);
}
diagnostics_ref.push(invalid_capitalization);
}
}

for capture_group_index in 2..=NUM_CAPTURE_GROUPS {
if captures.get(capture_group_index).is_some() {
continue;
}

if let Some(diagnostic) =
get_regex_error(capture_group_index, &range, diagnostics_ref)
{
diagnostics_ref.push(diagnostic);
};
// Check the rest of the capture groups for errors
for capture_group_index in 2..=NUM_CAPTURE_GROUPS {
if captures.get(capture_group_index).is_some() {
continue;
}

prev_token_is_todo = true;
} else {
prev_token_is_todo = false;
if let Some(diagnostic) = get_regex_error(capture_group_index, &range, diagnostics_ref)
{
diagnostics_ref.push(diagnostic);
};
}

prev_token_is_todo = true;
}

diagnostics
}

fn get_captured_matches(text: &str) -> Peekable<CaptureMatches> {
TODO_REGEX.captures_iter(text).peekable()
}

/// Mapper for static regex errors caused by a capture group at index i (i > 1 since the tag
/// capture group could lead to multiple diagnostics being pushed)
fn get_regex_error(i: usize, range: &Range, diagnostics: &mut [Diagnostic]) -> Option<Diagnostic> {
Expand All @@ -366,3 +356,7 @@ fn get_regex_error(i: usize, range: &Range, diagnostics: &mut [Diagnostic]) -> O
fn should_autofix(autofix: flags::Autofix, settings: &Settings, rule: Rule) -> bool {
autofix.into() && settings.rules.should_fix(rule)
}

fn get_captured_matches(text: &str) -> Peekable<CaptureMatches> {
TODO_REGEX.captures_iter(text).peekable()
}

0 comments on commit 58bfb57

Please sign in to comment.