mirror of https://github.com/astral-sh/ruff
Add handling for rules that are both deprecated and redirected
This commit is contained in:
parent
c7e40e6dc1
commit
e5bf2301bb
|
|
@ -1204,6 +1204,26 @@ class Foo:
|
||||||
"###);
|
"###);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn deprecated_redirected() {
|
||||||
|
// Selection of a redirected deprecated rule without preview enabled should still work
|
||||||
|
// but a warning should be displayed
|
||||||
|
let mut cmd = RuffCheck::default().args(["--select", "PGH001"]).build();
|
||||||
|
assert_cmd_snapshot!(cmd
|
||||||
|
.pass_stdin(r###"
|
||||||
|
x = eval(input("Enter a number: "))
|
||||||
|
"###), @r###"
|
||||||
|
success: false
|
||||||
|
exit_code: 1
|
||||||
|
----- stdout -----
|
||||||
|
-:2:5: S307 Use of possibly insecure function; consider using `ast.literal_eval`
|
||||||
|
Found 1 error.
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
warning: Rule `PGH001` is deprecated and will be removed in a future release. Use `S307` instead.
|
||||||
|
"###);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn deprecated_direct_preview_enabled() {
|
fn deprecated_direct_preview_enabled() {
|
||||||
// Direct selection of a deprecated rule in preview should fail
|
// Direct selection of a deprecated rule in preview should fail
|
||||||
|
|
@ -1228,6 +1248,25 @@ def reciprocal(n):
|
||||||
"###);
|
"###);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn deprecated_redirect_preview_enabled() {
|
||||||
|
// Direct selection of a deprecated rule in preview should fail
|
||||||
|
let mut cmd = RuffCheck::default()
|
||||||
|
.args(["--select", "PGH001", "--preview"])
|
||||||
|
.build();
|
||||||
|
assert_cmd_snapshot!(cmd
|
||||||
|
.pass_stdin(r###"
|
||||||
|
x = eval(input("Enter a number: "))
|
||||||
|
"###), @r###"
|
||||||
|
success: false
|
||||||
|
exit_code: 2
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
ruff failed
|
||||||
|
Cause: Selection of deprecated rule `PGH001` is not allowed when preview mode is enabled. Use `S307` instead.
|
||||||
|
"###);
|
||||||
|
}
|
||||||
#[test]
|
#[test]
|
||||||
fn deprecated_indirect_preview_enabled() {
|
fn deprecated_indirect_preview_enabled() {
|
||||||
// `TRY200` is deprecated and should be off by default in preview.
|
// `TRY200` is deprecated and should be off by default in preview.
|
||||||
|
|
@ -1250,6 +1289,24 @@ def reciprocal(n):
|
||||||
"###);
|
"###);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn deprecated_indirect_redirect_preview_enabled() {
|
||||||
|
// e.g. `PGH001` is deprecated and redirected and should be off by default in preview.
|
||||||
|
let mut cmd = RuffCheck::default()
|
||||||
|
.args(["--select", "PGH", "--preview"])
|
||||||
|
.build();
|
||||||
|
assert_cmd_snapshot!(cmd
|
||||||
|
.pass_stdin(r###"
|
||||||
|
x = eval(input("Enter a number: "))
|
||||||
|
"###), @r###"
|
||||||
|
success: true
|
||||||
|
exit_code: 0
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
"###);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn deprecated_multiple_direct_preview_enabled() {
|
fn deprecated_multiple_direct_preview_enabled() {
|
||||||
// Direct selection of the deprecated rules in preview should fail with
|
// Direct selection of the deprecated rules in preview should fail with
|
||||||
|
|
@ -1278,6 +1335,46 @@ def reciprocal(n):
|
||||||
"###);
|
"###);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn deprecated_multiple_direct_and_redirected_preview_enabled() {
|
||||||
|
// Direct selection of the deprecated rules in preview should fail with
|
||||||
|
// a message listing all of the rule codes
|
||||||
|
// The redirected rules should include an alternative
|
||||||
|
let mut cmd = RuffCheck::default()
|
||||||
|
.args([
|
||||||
|
"--select",
|
||||||
|
"ANN101",
|
||||||
|
"--select",
|
||||||
|
"ANN102",
|
||||||
|
"--select",
|
||||||
|
"PGH001",
|
||||||
|
"--preview",
|
||||||
|
])
|
||||||
|
.build();
|
||||||
|
assert_cmd_snapshot!(cmd
|
||||||
|
.pass_stdin(r###"
|
||||||
|
x = eval(input("Enter a number: "))
|
||||||
|
|
||||||
|
def reciprocal(n):
|
||||||
|
try:
|
||||||
|
return 1 / n
|
||||||
|
except ZeroDivisionError:
|
||||||
|
raise ValueError
|
||||||
|
"###), @r###"
|
||||||
|
success: false
|
||||||
|
exit_code: 2
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
ruff failed
|
||||||
|
Cause: Selection of deprecated rules is not allowed when preview mode is enabled. Remove selection of:
|
||||||
|
- ANN102
|
||||||
|
- ANN101
|
||||||
|
- PGH001 (use `S307` instead)
|
||||||
|
|
||||||
|
"###);
|
||||||
|
}
|
||||||
|
|
||||||
/// An unreadable pyproject.toml in non-isolated mode causes ruff to hard-error trying to build up
|
/// An unreadable pyproject.toml in non-isolated mode causes ruff to hard-error trying to build up
|
||||||
/// configuration globs
|
/// configuration globs
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
|
|
|
||||||
|
|
@ -21,6 +21,9 @@ static REDIRECTS: Lazy<HashMap<&'static str, &'static str>> = Lazy::new(|| {
|
||||||
("C9", "C90"),
|
("C9", "C90"),
|
||||||
("T1", "T10"),
|
("T1", "T10"),
|
||||||
("T2", "T20"),
|
("T2", "T20"),
|
||||||
|
// The PGH category is deprecated
|
||||||
|
("PGH001", "S307"),
|
||||||
|
("PGH002", "G010"),
|
||||||
// TODO(charlie): Remove by 2023-02-01.
|
// TODO(charlie): Remove by 2023-02-01.
|
||||||
("R", "RET"),
|
("R", "RET"),
|
||||||
("R5", "RET5"),
|
("R5", "RET5"),
|
||||||
|
|
|
||||||
|
|
@ -126,6 +126,41 @@ impl RuleSelector {
|
||||||
RuleSelector::Linter(l) => (l.common_prefix(), ""),
|
RuleSelector::Linter(l) => (l.common_prefix(), ""),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Parse [`RuleSelector`] from a string; but do not follow redirects.
|
||||||
|
pub fn from_str_no_redirect(s: &str) -> Result<Self, ParseError> {
|
||||||
|
match s {
|
||||||
|
"ALL" => Ok(Self::All),
|
||||||
|
#[allow(deprecated)]
|
||||||
|
"NURSERY" => Ok(Self::Nursery),
|
||||||
|
"C" => Ok(Self::C),
|
||||||
|
"T" => Ok(Self::T),
|
||||||
|
_ => {
|
||||||
|
let (linter, code) =
|
||||||
|
Linter::parse_code(s).ok_or_else(|| ParseError::Unknown(s.to_string()))?;
|
||||||
|
|
||||||
|
if code.is_empty() {
|
||||||
|
return Ok(Self::Linter(linter));
|
||||||
|
}
|
||||||
|
|
||||||
|
// Does the selector select a single rule?
|
||||||
|
let prefix = RuleCodePrefix::parse(&linter, code)
|
||||||
|
.map_err(|_| ParseError::Unknown(s.to_string()))?;
|
||||||
|
|
||||||
|
if is_single_rule_selector(&prefix) {
|
||||||
|
Ok(Self::Rule {
|
||||||
|
prefix,
|
||||||
|
redirected_from: None,
|
||||||
|
})
|
||||||
|
} else {
|
||||||
|
Ok(Self::Prefix {
|
||||||
|
prefix,
|
||||||
|
redirected_from: None,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Serialize for RuleSelector {
|
impl Serialize for RuleSelector {
|
||||||
|
|
|
||||||
|
|
@ -756,6 +756,7 @@ impl LintConfiguration {
|
||||||
let mut redirects = FxHashMap::default();
|
let mut redirects = FxHashMap::default();
|
||||||
let mut deprecated_nursery_selectors = FxHashSet::default();
|
let mut deprecated_nursery_selectors = FxHashSet::default();
|
||||||
let mut deprecated_selectors = FxHashSet::default();
|
let mut deprecated_selectors = FxHashSet::default();
|
||||||
|
let mut deprecated_redirected_selectors = FxHashSet::default();
|
||||||
let mut removed_selectors = FxHashSet::default();
|
let mut removed_selectors = FxHashSet::default();
|
||||||
let mut ignored_preview_selectors = FxHashSet::default();
|
let mut ignored_preview_selectors = FxHashSet::default();
|
||||||
|
|
||||||
|
|
@ -916,9 +917,31 @@ impl LintConfiguration {
|
||||||
|
|
||||||
// Deprecated rules
|
// Deprecated rules
|
||||||
if kind.is_enable() {
|
if kind.is_enable() {
|
||||||
if let RuleSelector::Rule { prefix, .. } = selector {
|
if let RuleSelector::Rule {
|
||||||
|
prefix,
|
||||||
|
redirected_from: None,
|
||||||
|
} = selector
|
||||||
|
{
|
||||||
if prefix.rules().any(|rule| rule.is_deprecated()) {
|
if prefix.rules().any(|rule| rule.is_deprecated()) {
|
||||||
deprecated_selectors.insert(selector);
|
deprecated_selectors.insert(selector.clone());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if let RuleSelector::Rule {
|
||||||
|
prefix: redirected_to,
|
||||||
|
redirected_from: Some(redirected_from),
|
||||||
|
} = selector
|
||||||
|
{
|
||||||
|
if let Ok(redirected_selector) =
|
||||||
|
RuleSelector::from_str_no_redirect(redirected_from)
|
||||||
|
{
|
||||||
|
if let RuleSelector::Rule { ref prefix, .. }
|
||||||
|
| RuleSelector::Prefix { ref prefix, .. } = redirected_selector
|
||||||
|
{
|
||||||
|
if prefix.rules().all(|rule| rule.is_deprecated()) {
|
||||||
|
deprecated_redirected_selectors
|
||||||
|
.insert((redirected_selector, redirected_to));
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -1001,21 +1024,56 @@ impl LintConfiguration {
|
||||||
"Rule `{prefix}{code}` is deprecated and will be removed in a future release.",
|
"Rule `{prefix}{code}` is deprecated and will be removed in a future release.",
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
for (selection, redirected_to) in deprecated_redirected_selectors {
|
||||||
|
let (prefix, code) = selection.prefix_and_code();
|
||||||
|
let (redirect_prefix, redirect_code) = (
|
||||||
|
redirected_to.linter().common_prefix(),
|
||||||
|
redirected_to.short_code(),
|
||||||
|
);
|
||||||
|
warn_user!(
|
||||||
|
"Rule `{prefix}{code}` is deprecated and will be removed in a future release. Use `{redirect_prefix}{redirect_code}` instead.",
|
||||||
|
);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
let deprecated_selectors = deprecated_selectors.iter().collect::<Vec<_>>();
|
// Combine the deprecated selectors and those that have been redirected
|
||||||
|
let deprecated_selectors = deprecated_selectors
|
||||||
|
.iter()
|
||||||
|
.map(|selector| (selector, None))
|
||||||
|
.chain(
|
||||||
|
deprecated_redirected_selectors
|
||||||
|
.iter()
|
||||||
|
.map(|(selector, redirect)| (selector, Some(redirect))),
|
||||||
|
)
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
|
||||||
match deprecated_selectors.as_slice() {
|
match deprecated_selectors.as_slice() {
|
||||||
[] => (),
|
[] => (),
|
||||||
[selection] => {
|
[(selection, redirect)] => {
|
||||||
let (prefix, code) = selection.prefix_and_code();
|
let (prefix, code) = selection.prefix_and_code();
|
||||||
return Err(anyhow!("Selection of deprecated rule `{prefix}{code}` is not allowed when preview mode is enabled."));
|
let err = if let Some(redirect) = redirect {
|
||||||
|
let (redirect_prefix, redirect_code) =
|
||||||
|
(redirect.linter().common_prefix(), redirect.short_code());
|
||||||
|
anyhow!("Selection of deprecated rule `{prefix}{code}` is not allowed when preview mode is enabled. Use `{redirect_prefix}{redirect_code}` instead.")
|
||||||
|
} else {
|
||||||
|
anyhow!("Selection of deprecated rule `{prefix}{code}` is not allowed when preview mode is enabled.")
|
||||||
|
};
|
||||||
|
return Err(err);
|
||||||
}
|
}
|
||||||
[..] => {
|
[..] => {
|
||||||
let mut message = "Selection of deprecated rules is not allowed when preview mode is enabled. Remove selection of:".to_string();
|
let mut message = "Selection of deprecated rules is not allowed when preview mode is enabled. Remove selection of:".to_string();
|
||||||
for selection in deprecated_selectors {
|
for (selection, redirect) in deprecated_selectors {
|
||||||
let (prefix, code) = selection.prefix_and_code();
|
let (prefix, code) = selection.prefix_and_code();
|
||||||
message.push_str("\n\t- ");
|
message.push_str("\n\t- ");
|
||||||
message.push_str(prefix);
|
message.push_str(prefix);
|
||||||
message.push_str(code);
|
message.push_str(code);
|
||||||
|
if let Some(redirect) = redirect {
|
||||||
|
let (redirect_prefix, redirect_code) =
|
||||||
|
(redirect.linter().common_prefix(), redirect.short_code());
|
||||||
|
message.push_str(
|
||||||
|
format!(" (use `{redirect_prefix}{redirect_code}` instead)")
|
||||||
|
.as_str(),
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
message.push('\n');
|
message.push('\n');
|
||||||
return Err(anyhow!(message));
|
return Err(anyhow!(message));
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue