Skip to content

Commit

Permalink
Flag [ as an invalid noqa suffix (#5982)
Browse files Browse the repository at this point in the history
Closes #5960.
  • Loading branch information
charliermarsh committed Jul 22, 2023
1 parent 32773e8 commit 6ff566f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 13 deletions.
41 changes: 29 additions & 12 deletions crates/ruff/src/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,8 @@ impl<'a> Directive<'a> {

// If the next character is `:`, then it's a list of codes. Otherwise, it's a directive
// to ignore all rules.
return Ok(Some(
if text[noqa_literal_end..]
.chars()
.next()
.map_or(false, |c| c == ':')
{
let directive = match text[noqa_literal_end..].chars().next() {
Some(':') => {
// E.g., `# noqa: F401, F841`.
let mut codes_start = noqa_literal_end;

Expand Down Expand Up @@ -120,17 +116,31 @@ impl<'a> Directive<'a> {
range: range.add(offset),
codes,
})
} else {
// E.g., `# noqa`.
}
None | Some('#') => {
// E.g., `# noqa` or `# noqa# ignore`.
let range = TextRange::new(
TextSize::try_from(comment_start).unwrap(),
TextSize::try_from(noqa_literal_end).unwrap(),
);
Self::All(All {
range: range.add(offset),
})
},
));
}
Some(c) if c.is_whitespace() => {
// E.g., `# noqa # ignore`.
let range = TextRange::new(
TextSize::try_from(comment_start).unwrap(),
TextSize::try_from(noqa_literal_end).unwrap(),
);
Self::All(All {
range: range.add(offset),
})
}
_ => return Err(ParseError::InvalidSuffix),
};

return Ok(Some(directive));
}

Ok(None)
Expand Down Expand Up @@ -237,7 +247,7 @@ impl FileExemption {
#[allow(deprecated)]
let line = locator.compute_line_index(range.start());
let path_display = relativize_path(path);
warn!("Invalid `# noqa` directive on {path_display}:{line}: {err}");
warn!("Invalid `# ruff: noqa` directive at {path_display}:{line}: {err}");
}
Ok(Some(ParsedFileExemption::All)) => {
return Some(Self::All);
Expand All @@ -250,7 +260,8 @@ impl FileExemption {
} else {
#[allow(deprecated)]
let line = locator.compute_line_index(range.start());
warn!("Invalid code provided to `# ruff: noqa` on line {line}: {code}");
let path_display = relativize_path(path);
warn!("Invalid code provided to `# ruff: noqa` at {path_display}:{line}: {code}");
None
}
}));
Expand Down Expand Up @@ -904,6 +915,12 @@ mod tests {
assert_debug_snapshot!(Directive::try_extract(source, TextSize::default()));
}

#[test]
fn noqa_invalid_suffix() {
let source = "# noqa[F401]";
assert_debug_snapshot!(Directive::try_extract(source, TextSize::default()));
}

#[test]
fn flake8_exemption_all() {
let source = "# flake8: noqa";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::registry::AsRule;
/// Checks for f-strings that do not contain any placeholder expressions.
///
/// ## Why is this bad?
/// F-strings are a convenient way to format strings, but they are not
/// f-strings are a convenient way to format strings, but they are not
/// necessary if there are no placeholder expressions to format. In this
/// case, a regular string should be used instead, as an f-string without
/// placeholders can be confusing for readers, who may expect such a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: crates/ruff/src/noqa.rs
expression: "Directive::try_extract(source, TextSize::default())"
---
Err(
InvalidSuffix,
)

0 comments on commit 6ff566f

Please sign in to comment.