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

Add flake8-pie PIE796: prefer-unique-enum #1923

Merged
merged 4 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,7 @@ For more, see [flake8-pie](https://pypi.org/project/flake8-pie/0.16.0/) on PyPI.
| ---- | ---- | ------- | --- |
| PIE790 | NoUnnecessaryPass | Unnecessary `pass` statement | 🛠 |
| PIE794 | DupeClassFieldDefinitions | Class field `...` is defined multiple times | 🛠 |
| PIE796 | PreferUniqueEnums | Enum contains duplicate value: `...` | |
| PIE807 | PreferListBuiltin | Prefer `list()` over useless lambda | 🛠 |

### flake8-commas (COM)
Expand Down
54 changes: 54 additions & 0 deletions resources/test/fixtures/flake8_pie/PIE796.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
class FakeEnum(enum.Enum):
A = "A"
B = "B"
C = "B" # PIE796


class FakeEnum2(Enum):
A = 1
B = 2
C = 2 # PIE796


class FakeEnum3(str, Enum):
A = "1"
B = "2"
C = "2" # PIE796

class FakeEnum4(Enum):
A = 1.0
B = 2.5
C = 2.5 # PIE796


class FakeEnum5(Enum):
A = 1.0
B = True
C = False
D = False # PIE796


class FakeEnum6(Enum):
A = 1
B = 2
C = None
D = None # PIE796


@enum.unique
class FakeEnum7(enum.Enum):
A = "A"
B = "B"
C = "C"

@unique
class FakeEnum8(Enum):
A = 1
B = 2
C = 2 # PIE796

class FakeEnum9(enum.Enum):
A = "A"
B = "B"
C = "C"

1 change: 1 addition & 0 deletions ruff.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,7 @@
"PIE79",
"PIE790",
"PIE794",
"PIE796",
"PIE8",
"PIE80",
"PIE807",
Expand Down
4 changes: 4 additions & 0 deletions src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,10 @@ where
flake8_pie::rules::dupe_class_field_definitions(self, stmt, body);
}

if self.settings.enabled.contains(&RuleCode::PIE796) {
flake8_pie::rules::prefer_unique_enums(self, stmt, body);
}

self.check_builtin_shadowing(name, stmt, false);

for expr in bases {
Expand Down
1 change: 1 addition & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ ruff_macros::define_rule_mapping!(
// flake8-pie
PIE790 => violations::NoUnnecessaryPass,
PIE794 => violations::DupeClassFieldDefinitions,
PIE796 => violations::PreferUniqueEnums,
PIE807 => violations::PreferListBuiltin,
// flake8-commas
COM812 => violations::TrailingCommaMissing,
Expand Down
1 change: 1 addition & 0 deletions src/rules/flake8_pie/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod tests {

#[test_case(RuleCode::PIE790, Path::new("PIE790.py"); "PIE790")]
#[test_case(RuleCode::PIE794, Path::new("PIE794.py"); "PIE794")]
#[test_case(RuleCode::PIE796, Path::new("PIE796.py"); "PIE796")]
#[test_case(RuleCode::PIE807, Path::new("PIE807.py"); "PIE807")]
fn rules(rule_code: RuleCode, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy());
Expand Down
37 changes: 37 additions & 0 deletions src/rules/flake8_pie/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use log::error;
use rustc_hash::FxHashSet;
use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind};

use crate::ast::comparable::ComparableExpr;
use crate::ast::helpers::unparse_expr;
use crate::ast::types::{Range, RefEquality};
use crate::autofix::helpers::delete_stmt;
use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -106,6 +108,41 @@ pub fn dupe_class_field_definitions<'a, 'b>(
}
}

/// PIE796
pub fn prefer_unique_enums<'a, 'b>(checker: &mut Checker<'a>, parent: &'b Stmt, body: &'b [Stmt])
where
'b: 'a,
{
let StmtKind::ClassDef { bases, .. } = &parent.node else {
return;
};

if !bases.iter().any(|expr| {
checker
.resolve_call_path(expr)
Copy link
Member

Choose a reason for hiding this comment

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

I tweaked this logic to use resolve_call_path, which validates that the expression is bound to enum.Enum (i.e., it tracks imports, follows aliases, and so on).

.map_or(false, |call_path| call_path == ["enum", "Enum"])
}) {
return;
}

let mut seen_targets: FxHashSet<ComparableExpr> = FxHashSet::default();
for stmt in body {
let StmtKind::Assign { value, .. } = &stmt.node else {
continue;
};

if !seen_targets.insert(ComparableExpr::from(value)) {
let diagnostic = Diagnostic::new(
violations::PreferUniqueEnums {
value: unparse_expr(value, checker.stylist),
},
Range::from_located(stmt),
);
checker.diagnostics.push(diagnostic);
}
}
}

/// PIE807
pub fn prefer_list_builtin(checker: &mut Checker, expr: &Expr) {
let ExprKind::Lambda { args, body } = &expr.node else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
source: src/rules/flake8_pie/mod.rs
expression: diagnostics
---
- kind:
PreferUniqueEnums: "\"B\""
location:
row: 4
column: 4
end_location:
row: 4
column: 11
fix: ~
parent: ~
- kind:
PreferUniqueEnums: "2"
location:
row: 10
column: 4
end_location:
row: 10
column: 9
fix: ~
parent: ~
- kind:
PreferUniqueEnums: "\"2\""
location:
row: 16
column: 4
end_location:
row: 16
column: 11
fix: ~
parent: ~
- kind:
PreferUniqueEnums: "2.5"
location:
row: 21
column: 4
end_location:
row: 21
column: 11
fix: ~
parent: ~
- kind:
PreferUniqueEnums: "False"
location:
row: 28
column: 4
end_location:
row: 28
column: 13
fix: ~
parent: ~
- kind:
PreferUniqueEnums: None
location:
row: 35
column: 4
end_location:
row: 35
column: 12
fix: ~
parent: ~
- kind:
PreferUniqueEnums: "2"
location:
row: 48
column: 4
end_location:
row: 48
column: 9
fix: ~
parent: ~

18 changes: 18 additions & 0 deletions src/violations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5970,6 +5970,24 @@ impl AlwaysAutofixableViolation for DupeClassFieldDefinitions {
}
}

define_violation!(
pub struct PreferUniqueEnums {
pub value: String,
}
Copy link
Member

Choose a reason for hiding this comment

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

(Tweaked this to use named fields which we're trying to prefer going forwards.)

);
impl Violation for PreferUniqueEnums {
fn message(&self) -> String {
let PreferUniqueEnums { value } = self;
format!("Enum contains duplicate value: `{value}`")
}

fn placeholder() -> Self {
PreferUniqueEnums {
value: "...".to_string(),
}
}
}

define_violation!(
pub struct PreferListBuiltin;
);
Expand Down