Implement F601 and F602 (#122)

This commit is contained in:
Charlie Marsh
2022-09-07 12:57:50 -04:00
committed by GitHub
parent 5e9ea8bda2
commit 5deb63a05f
10 changed files with 167 additions and 0 deletions

View File

@@ -156,6 +156,8 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
| F401 | UnusedImport | `...` imported but unused |
| F403 | ImportStarUsage | Unable to detect undefined names |
| F541 | FStringMissingPlaceholders | f-string without any placeholders |
| F601 | MultiValueRepeatedKeyLiteral | Dictionary key literal repeated |
| F602 | MultiValueRepeatedKeyVariable | Dictionary key `...` repeated |
| F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` |
| F634 | IfTuple | If test is a tuple, which is always `True` |
| F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method |

View File

@@ -13,6 +13,8 @@ fn main() {
CheckKind::ImportStarUsage,
CheckKind::LineTooLong,
CheckKind::ModuleImportNotAtTopOfFile,
CheckKind::MultiValueRepeatedKeyLiteral,
CheckKind::MultiValueRepeatedKeyVariable("...".to_string()),
CheckKind::NoAssertEquals,
CheckKind::NoneComparison(RejectedCmpop::Eq),
CheckKind::NotInTest,

12
resources/test/fixtures/F601.py vendored Normal file
View File

@@ -0,0 +1,12 @@
x = {
"a": 1,
"a": 2,
"b": 3,
("a", "b"): 3,
("a", "b"): 4,
1.0: 2,
1: 0,
1: 3,
b"123": 1,
b"123": 4,
}

7
resources/test/fixtures/F602.py vendored Normal file
View File

@@ -0,0 +1,7 @@
a = 1
b = 2
x = {
a: 1,
a: 2,
b: 3,
}

View File

@@ -13,6 +13,8 @@ select = [
"F401",
"F403",
"F541",
"F601",
"F602",
"F631",
"F634",
"F704",

View File

@@ -56,6 +56,20 @@ impl Checker<'_> {
}
}
#[derive(Debug, PartialEq)]
enum DictionaryKey<'a> {
Constant(&'a Constant),
Variable(&'a String),
}
fn convert_to_value(expr: &Expr) -> Option<DictionaryKey> {
match &expr.node {
ExprKind::Constant { value, .. } => Some(DictionaryKey::Constant(value)),
ExprKind::Name { id, .. } => Some(DictionaryKey::Variable(id)),
_ => None,
}
}
impl Visitor for Checker<'_> {
fn visit_stmt(&mut self, stmt: &Stmt) {
match &stmt.node {
@@ -503,6 +517,48 @@ impl Visitor for Checker<'_> {
}
}
}
ExprKind::Dict { keys, .. } => {
if self.settings.select.contains(&CheckCode::F601)
|| self.settings.select.contains(&CheckCode::F602)
{
let num_keys = keys.len();
for i in 0..num_keys {
let k1 = &keys[i];
let v1 = convert_to_value(k1);
for k2 in keys.iter().take(num_keys).skip(i + 1) {
let v2 = convert_to_value(k2);
match (&v1, &v2) {
(
Some(DictionaryKey::Constant(v1)),
Some(DictionaryKey::Constant(v2)),
) => {
if self.settings.select.contains(&CheckCode::F601) && v1 == v2 {
self.checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyLiteral,
k2.location,
))
}
}
(
Some(DictionaryKey::Variable(v1)),
Some(DictionaryKey::Variable(v2)),
) => {
if self.settings.select.contains(&CheckCode::F602) && v1 == v2 {
self.checks.push(Check::new(
CheckKind::MultiValueRepeatedKeyVariable(
v2.to_string(),
),
k2.location,
))
}
}
_ => {}
}
}
}
}
}
ExprKind::GeneratorExp { .. }
| ExprKind::ListComp { .. }
| ExprKind::DictComp { .. }

View File

@@ -19,6 +19,8 @@ pub enum CheckCode {
F401,
F403,
F541,
F601,
F602,
F631,
F634,
F704,
@@ -50,6 +52,8 @@ impl FromStr for CheckCode {
"F401" => Ok(CheckCode::F401),
"F403" => Ok(CheckCode::F403),
"F541" => Ok(CheckCode::F541),
"F601" => Ok(CheckCode::F601),
"F602" => Ok(CheckCode::F602),
"F631" => Ok(CheckCode::F631),
"F634" => Ok(CheckCode::F634),
"F704" => Ok(CheckCode::F704),
@@ -82,6 +86,8 @@ impl CheckCode {
CheckCode::F401 => "F401",
CheckCode::F403 => "F403",
CheckCode::F541 => "F541",
CheckCode::F601 => "F601",
CheckCode::F602 => "F602",
CheckCode::F631 => "F631",
CheckCode::F634 => "F634",
CheckCode::F704 => "F704",
@@ -112,6 +118,8 @@ impl CheckCode {
CheckCode::F401 => &LintSource::AST,
CheckCode::F403 => &LintSource::AST,
CheckCode::F541 => &LintSource::AST,
CheckCode::F601 => &LintSource::AST,
CheckCode::F602 => &LintSource::AST,
CheckCode::F631 => &LintSource::AST,
CheckCode::F634 => &LintSource::AST,
CheckCode::F704 => &LintSource::AST,
@@ -154,6 +162,8 @@ pub enum CheckKind {
ImportStarUsage,
LineTooLong,
ModuleImportNotAtTopOfFile,
MultiValueRepeatedKeyLiteral,
MultiValueRepeatedKeyVariable(String),
NoAssertEquals,
NoneComparison(RejectedCmpop),
NotInTest,
@@ -184,6 +194,8 @@ impl CheckKind {
CheckKind::LineTooLong => "LineTooLong",
CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
CheckKind::MultiValueRepeatedKeyLiteral => "MultiValueRepeatedKeyLiteral",
CheckKind::MultiValueRepeatedKeyVariable(_) => "MultiValueRepeatedKeyVariable",
CheckKind::NoAssertEquals => "NoAssertEquals",
CheckKind::NoneComparison(_) => "NoneComparison",
CheckKind::NotInTest => "NotInTest",
@@ -214,6 +226,8 @@ impl CheckKind {
CheckKind::LineTooLong => &CheckCode::E501,
CheckKind::DoNotAssignLambda => &CheckCode::E731,
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
CheckKind::MultiValueRepeatedKeyLiteral => &CheckCode::F601,
CheckKind::MultiValueRepeatedKeyVariable(_) => &CheckCode::F602,
CheckKind::NoAssertEquals => &CheckCode::R002,
CheckKind::NoneComparison(_) => &CheckCode::E711,
CheckKind::NotInTest => &CheckCode::E713,
@@ -258,6 +272,12 @@ impl CheckKind {
CheckKind::ModuleImportNotAtTopOfFile => {
"Module level import not at top of file".to_string()
}
CheckKind::MultiValueRepeatedKeyLiteral => {
"Dictionary key literal repeated".to_string()
}
CheckKind::MultiValueRepeatedKeyVariable(name) => {
format!("Dictionary key `{name}` repeated")
}
CheckKind::NoAssertEquals => {
"`assertEquals` is deprecated, use `assertEqual` instead".to_string()
}
@@ -328,6 +348,8 @@ impl CheckKind {
CheckKind::DoNotAssignLambda => false,
CheckKind::LineTooLong => false,
CheckKind::ModuleImportNotAtTopOfFile => false,
CheckKind::MultiValueRepeatedKeyLiteral => false,
CheckKind::MultiValueRepeatedKeyVariable(_) => false,
CheckKind::NoAssertEquals => true,
CheckKind::NotInTest => false,
CheckKind::NotIsTest => false,

View File

@@ -391,6 +391,66 @@ mod tests {
Ok(())
}
#[test]
fn f601() -> Result<()> {
let actual = check_path(
Path::new("./resources/test/fixtures/F601.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::F601]),
},
&autofix::Mode::Generate,
)?;
let expected = vec![
Check {
kind: CheckKind::MultiValueRepeatedKeyLiteral,
location: Location::new(3, 6),
fix: None,
},
Check {
kind: CheckKind::MultiValueRepeatedKeyLiteral,
location: Location::new(9, 5),
fix: None,
},
Check {
kind: CheckKind::MultiValueRepeatedKeyLiteral,
location: Location::new(11, 7),
fix: None,
},
];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test]
fn f602() -> Result<()> {
let actual = check_path(
Path::new("./resources/test/fixtures/F602.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::F602]),
},
&autofix::Mode::Generate,
)?;
let expected = vec![Check {
kind: CheckKind::MultiValueRepeatedKeyVariable("a".to_string()),
location: Location::new(5, 5),
fix: None,
}];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test]
fn f631() -> Result<()> {
let actual = check_path(

View File

@@ -248,6 +248,8 @@ other-attribute = 1
CheckCode::F401,
CheckCode::F403,
CheckCode::F541,
CheckCode::F601,
CheckCode::F602,
CheckCode::F631,
CheckCode::F634,
CheckCode::F704,

View File

@@ -55,6 +55,8 @@ impl Settings {
CheckCode::F401,
CheckCode::F403,
CheckCode::F541,
CheckCode::F601,
CheckCode::F602,
CheckCode::F631,
CheckCode::F634,
CheckCode::F704,