Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
546be5692a | ||
|
|
43e1f20b28 | ||
|
|
97388cefda | ||
|
|
63ce579989 |
@@ -1,5 +1,5 @@
|
||||
repos:
|
||||
- repo: https://github.com/charliermarsh/ruff
|
||||
rev: v0.0.34
|
||||
rev: v0.0.35
|
||||
hooks:
|
||||
- id: lint
|
||||
|
||||
2
Cargo.lock
generated
2
Cargo.lock
generated
@@ -1744,7 +1744,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "ruff"
|
||||
version = "0.0.34"
|
||||
version = "0.0.35"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"bincode",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "ruff"
|
||||
version = "0.0.34"
|
||||
version = "0.0.35"
|
||||
edition = "2021"
|
||||
|
||||
[lib]
|
||||
|
||||
@@ -57,7 +57,7 @@ ruff also works with [Pre-Commit](https://pre-commit.com) (requires Cargo on sys
|
||||
```yaml
|
||||
repos:
|
||||
- repo: https://github.com/charliermarsh/ruff
|
||||
rev: v0.0.34
|
||||
rev: v0.0.35
|
||||
hooks:
|
||||
- id: lint
|
||||
```
|
||||
@@ -86,7 +86,7 @@ ruff path/to/code/ --select F401 F403
|
||||
See `ruff --help` for more:
|
||||
|
||||
```shell
|
||||
ruff (v0.0.34)
|
||||
ruff (v0.0.35)
|
||||
An extremely fast Python linter.
|
||||
|
||||
USAGE:
|
||||
@@ -124,7 +124,7 @@ ruff's goal is to achieve feature-parity with Flake8 when used (1) without any p
|
||||
stylistic checks; limiting to Python 3 obviates the need for certain compatibility checks.)
|
||||
|
||||
Under those conditions, Flake8 implements about 58 rules, give or take. At time of writing, ruff
|
||||
implements 31 rules. (Note that these 31 rules likely cover a disproportionate share of errors:
|
||||
implements 33 rules. (Note that these 33 rules likely cover a disproportionate share of errors:
|
||||
unused imports, undefined variables, etc.)
|
||||
|
||||
Of the unimplemented rules, ruff is missing:
|
||||
@@ -154,6 +154,8 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
|
||||
| E714 | NotIsTest | Test for object identity should be `is not` |
|
||||
| E731 | DoNotAssignLambda | Do not assign a lambda expression, use a def |
|
||||
| E741 | AmbiguousVariableName | ambiguous variable name '...' |
|
||||
| E742 | AmbiguousClassName | ambiguous class name '...' |
|
||||
| E743 | AmbiguousFunctionName | ambiguous function name '...' |
|
||||
| E902 | IOError | No such file or directory: `...` |
|
||||
| F401 | UnusedImport | `...` imported but unused |
|
||||
| F403 | ImportStarUsage | Unable to detect undefined names |
|
||||
|
||||
@@ -4,6 +4,8 @@ use ruff::checks::{CheckKind, RejectedCmpop};
|
||||
fn main() {
|
||||
let mut check_kinds: Vec<CheckKind> = vec![
|
||||
CheckKind::AmbiguousVariableName("...".to_string()),
|
||||
CheckKind::AmbiguousClassName("...".to_string()),
|
||||
CheckKind::AmbiguousFunctionName("...".to_string()),
|
||||
CheckKind::AssertTuple,
|
||||
CheckKind::DefaultExceptNotLast,
|
||||
CheckKind::DoNotAssignLambda,
|
||||
|
||||
14
resources/test/fixtures/E742.py
vendored
Normal file
14
resources/test/fixtures/E742.py
vendored
Normal file
@@ -0,0 +1,14 @@
|
||||
class l:
|
||||
pass
|
||||
|
||||
|
||||
class I:
|
||||
pass
|
||||
|
||||
|
||||
class O:
|
||||
pass
|
||||
|
||||
|
||||
class X:
|
||||
pass
|
||||
15
resources/test/fixtures/E743.py
vendored
Normal file
15
resources/test/fixtures/E743.py
vendored
Normal file
@@ -0,0 +1,15 @@
|
||||
def l():
|
||||
pass
|
||||
|
||||
|
||||
def I():
|
||||
pass
|
||||
|
||||
|
||||
class X:
|
||||
def O(self):
|
||||
pass
|
||||
|
||||
|
||||
def x():
|
||||
pass
|
||||
8
resources/test/fixtures/F841.py
vendored
8
resources/test/fixtures/F841.py
vendored
@@ -14,3 +14,11 @@ def f():
|
||||
x = 1
|
||||
y = 2
|
||||
z = x + y
|
||||
|
||||
|
||||
def g():
|
||||
foo = (1, 2)
|
||||
(a, b) = (1, 2)
|
||||
|
||||
bar = (1, 2)
|
||||
(c, d) = bar
|
||||
|
||||
2
resources/test/fixtures/pyproject.toml
vendored
2
resources/test/fixtures/pyproject.toml
vendored
@@ -10,6 +10,8 @@ select = [
|
||||
"E714",
|
||||
"E731",
|
||||
"E741",
|
||||
"E742",
|
||||
"E743",
|
||||
"E902",
|
||||
"F401",
|
||||
"F403",
|
||||
|
||||
@@ -109,6 +109,30 @@ pub fn check_ambiguous_variable_name(name: &str, location: Location) -> Option<C
|
||||
}
|
||||
}
|
||||
|
||||
/// Check AmbiguousClassName compliance.
|
||||
pub fn check_ambiguous_class_name(name: &str, location: Location) -> Option<Check> {
|
||||
if is_ambiguous_name(name) {
|
||||
Some(Check::new(
|
||||
CheckKind::AmbiguousClassName(name.to_string()),
|
||||
location,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Check AmbiguousFunctionName compliance.
|
||||
pub fn check_ambiguous_function_name(name: &str, location: Location) -> Option<Check> {
|
||||
if is_ambiguous_name(name) {
|
||||
Some(Check::new(
|
||||
CheckKind::AmbiguousFunctionName(name.to_string()),
|
||||
location,
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Check UselessObjectInheritance compliance.
|
||||
pub fn check_useless_object_inheritance(
|
||||
stmt: &Stmt,
|
||||
|
||||
@@ -100,6 +100,24 @@ pub fn in_nested_block(parent_stack: &[usize], parents: &[&Stmt]) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Check if a node represents an unpacking assignment.
|
||||
pub fn is_unpacking_assignment(stmt: &Stmt) -> bool {
|
||||
if let StmtKind::Assign { targets, value, .. } = &stmt.node {
|
||||
for child in targets {
|
||||
match &child.node {
|
||||
ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. } => {}
|
||||
_ => return false,
|
||||
}
|
||||
}
|
||||
match &value.node {
|
||||
ExprKind::Set { .. } | ExprKind::List { .. } | ExprKind::Tuple { .. } => return false,
|
||||
_ => {}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Struct used to efficiently slice source code at (row, column) Locations.
|
||||
pub struct SourceCodeLocator<'a> {
|
||||
content: &'a str,
|
||||
|
||||
@@ -35,8 +35,10 @@ impl Scope {
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub enum BindingKind {
|
||||
Annotation,
|
||||
Argument,
|
||||
Assignment,
|
||||
Binding,
|
||||
Builtin,
|
||||
ClassDefinition,
|
||||
Definition,
|
||||
|
||||
@@ -208,6 +208,12 @@ where
|
||||
args,
|
||||
..
|
||||
} => {
|
||||
if self.settings.select.contains(&CheckCode::E743) {
|
||||
if let Some(check) = checks::check_ambiguous_function_name(name, stmt.location)
|
||||
{
|
||||
self.checks.push(check);
|
||||
}
|
||||
}
|
||||
for expr in decorator_list {
|
||||
self.visit_expr(expr);
|
||||
}
|
||||
@@ -296,6 +302,12 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
if self.settings.select.contains(&CheckCode::E742) {
|
||||
if let Some(check) = checks::check_ambiguous_class_name(name, stmt.location) {
|
||||
self.checks.push(check);
|
||||
}
|
||||
}
|
||||
|
||||
for expr in bases {
|
||||
self.visit_expr(expr)
|
||||
}
|
||||
@@ -541,7 +553,7 @@ where
|
||||
self.in_literal = true;
|
||||
}
|
||||
}
|
||||
ExprKind::Tuple { elts, ctx } => {
|
||||
ExprKind::Tuple { elts, ctx } | ExprKind::List { elts, ctx } => {
|
||||
if matches!(ctx, ExprContext::Store) {
|
||||
let check_too_many_expressions =
|
||||
self.settings.select.contains(&CheckCode::F621);
|
||||
@@ -569,7 +581,7 @@ where
|
||||
}
|
||||
let parent =
|
||||
self.parents[*(self.parent_stack.last().expect("No parent found."))];
|
||||
self.handle_node_store(expr, Some(parent));
|
||||
self.handle_node_store(expr, parent);
|
||||
}
|
||||
ExprContext::Del => self.handle_node_delete(expr),
|
||||
},
|
||||
@@ -823,7 +835,7 @@ where
|
||||
ctx: ExprContext::Store,
|
||||
},
|
||||
),
|
||||
Some(parent),
|
||||
parent,
|
||||
);
|
||||
self.parents.push(parent);
|
||||
}
|
||||
@@ -841,7 +853,7 @@ where
|
||||
ctx: ExprContext::Store,
|
||||
},
|
||||
),
|
||||
Some(parent),
|
||||
parent,
|
||||
);
|
||||
self.parents.push(parent);
|
||||
|
||||
@@ -1011,7 +1023,7 @@ impl<'a> Checker<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_node_store(&mut self, expr: &Expr, parent: Option<&Stmt>) {
|
||||
fn handle_node_store(&mut self, expr: &Expr, parent: &Stmt) {
|
||||
if let ExprKind::Name { id, .. } = &expr.node {
|
||||
let current =
|
||||
&self.scopes[*(self.scope_stack.last().expect("No current scope found."))];
|
||||
@@ -1041,35 +1053,59 @@ impl<'a> Checker<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
// TODO(charlie): Handle alternate binding types (like `Annotation`).
|
||||
if id == "__all__"
|
||||
&& matches!(current.kind, ScopeKind::Module)
|
||||
&& parent
|
||||
.map(|stmt| {
|
||||
matches!(stmt.node, StmtKind::Assign { .. })
|
||||
|| matches!(stmt.node, StmtKind::AugAssign { .. })
|
||||
|| matches!(stmt.node, StmtKind::AnnAssign { .. })
|
||||
})
|
||||
.unwrap_or_default()
|
||||
if matches!(parent.node, StmtKind::AnnAssign { value: None, .. }) {
|
||||
self.add_binding(
|
||||
id.to_string(),
|
||||
Binding {
|
||||
kind: BindingKind::Annotation,
|
||||
used: None,
|
||||
location: expr.location,
|
||||
},
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// TODO(charlie): Include comprehensions here.
|
||||
if matches!(parent.node, StmtKind::For { .. })
|
||||
|| matches!(parent.node, StmtKind::AsyncFor { .. })
|
||||
|| operations::is_unpacking_assignment(parent)
|
||||
{
|
||||
self.add_binding(
|
||||
id.to_string(),
|
||||
Binding {
|
||||
kind: BindingKind::Export(extract_all_names(parent.unwrap(), current)),
|
||||
kind: BindingKind::Binding,
|
||||
used: None,
|
||||
location: expr.location,
|
||||
},
|
||||
);
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
|
||||
if id == "__all__"
|
||||
&& matches!(current.kind, ScopeKind::Module)
|
||||
&& (matches!(parent.node, StmtKind::Assign { .. })
|
||||
|| matches!(parent.node, StmtKind::AugAssign { .. })
|
||||
|| matches!(parent.node, StmtKind::AnnAssign { .. }))
|
||||
{
|
||||
self.add_binding(
|
||||
id.to_string(),
|
||||
Binding {
|
||||
kind: BindingKind::Assignment,
|
||||
kind: BindingKind::Export(extract_all_names(parent, current)),
|
||||
used: None,
|
||||
location: expr.location,
|
||||
},
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
self.add_binding(
|
||||
id.to_string(),
|
||||
Binding {
|
||||
kind: BindingKind::Assignment,
|
||||
used: None,
|
||||
location: expr.location,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,8 @@ pub enum CheckCode {
|
||||
E714,
|
||||
E731,
|
||||
E741,
|
||||
E742,
|
||||
E743,
|
||||
E902,
|
||||
F401,
|
||||
F403,
|
||||
@@ -54,6 +56,8 @@ impl FromStr for CheckCode {
|
||||
"E714" => Ok(CheckCode::E714),
|
||||
"E731" => Ok(CheckCode::E731),
|
||||
"E741" => Ok(CheckCode::E741),
|
||||
"E742" => Ok(CheckCode::E742),
|
||||
"E743" => Ok(CheckCode::E743),
|
||||
"E902" => Ok(CheckCode::E902),
|
||||
"F401" => Ok(CheckCode::F401),
|
||||
"F403" => Ok(CheckCode::F403),
|
||||
@@ -93,6 +97,8 @@ impl CheckCode {
|
||||
CheckCode::E714 => "E714",
|
||||
CheckCode::E731 => "E731",
|
||||
CheckCode::E741 => "E741",
|
||||
CheckCode::E742 => "E742",
|
||||
CheckCode::E743 => "E743",
|
||||
CheckCode::E902 => "E902",
|
||||
CheckCode::F401 => "F401",
|
||||
CheckCode::F403 => "F403",
|
||||
@@ -130,6 +136,8 @@ impl CheckCode {
|
||||
CheckCode::E714 => &LintSource::AST,
|
||||
CheckCode::E731 => &LintSource::AST,
|
||||
CheckCode::E741 => &LintSource::AST,
|
||||
CheckCode::E742 => &LintSource::AST,
|
||||
CheckCode::E743 => &LintSource::AST,
|
||||
CheckCode::E902 => &LintSource::FileSystem,
|
||||
CheckCode::F401 => &LintSource::AST,
|
||||
CheckCode::F403 => &LintSource::AST,
|
||||
@@ -174,6 +182,8 @@ pub enum RejectedCmpop {
|
||||
pub enum CheckKind {
|
||||
AssertTuple,
|
||||
AmbiguousVariableName(String),
|
||||
AmbiguousClassName(String),
|
||||
AmbiguousFunctionName(String),
|
||||
DefaultExceptNotLast,
|
||||
DoNotAssignLambda,
|
||||
DuplicateArgumentName,
|
||||
@@ -211,6 +221,8 @@ impl CheckKind {
|
||||
match self {
|
||||
CheckKind::AssertTuple => "AssertTuple",
|
||||
CheckKind::AmbiguousVariableName(_) => "AmbiguousVariableName",
|
||||
CheckKind::AmbiguousClassName(_) => "AmbiguousClassName",
|
||||
CheckKind::AmbiguousFunctionName(_) => "AmbiguousFunctionName",
|
||||
CheckKind::DefaultExceptNotLast => "DefaultExceptNotLast",
|
||||
CheckKind::DuplicateArgumentName => "DuplicateArgumentName",
|
||||
CheckKind::FStringMissingPlaceholders => "FStringMissingPlaceholders",
|
||||
@@ -260,6 +272,8 @@ impl CheckKind {
|
||||
CheckKind::LineTooLong => &CheckCode::E501,
|
||||
CheckKind::DoNotAssignLambda => &CheckCode::E731,
|
||||
CheckKind::AmbiguousVariableName(_) => &CheckCode::E741,
|
||||
CheckKind::AmbiguousClassName(_) => &CheckCode::E742,
|
||||
CheckKind::AmbiguousFunctionName(_) => &CheckCode::E743,
|
||||
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
|
||||
CheckKind::MultiValueRepeatedKeyLiteral => &CheckCode::F601,
|
||||
CheckKind::MultiValueRepeatedKeyVariable(_) => &CheckCode::F602,
|
||||
@@ -312,6 +326,12 @@ impl CheckKind {
|
||||
CheckKind::DoNotAssignLambda => {
|
||||
"Do not assign a lambda expression, use a def".to_string()
|
||||
}
|
||||
CheckKind::AmbiguousClassName(name) => {
|
||||
format!("ambiguous class name '{}'", name)
|
||||
}
|
||||
CheckKind::AmbiguousFunctionName(name) => {
|
||||
format!("ambiguous function name '{}'", name)
|
||||
}
|
||||
CheckKind::AmbiguousVariableName(name) => {
|
||||
format!("ambiguous variable name '{}'", name)
|
||||
}
|
||||
@@ -388,6 +408,8 @@ impl CheckKind {
|
||||
/// Whether the check kind is (potentially) fixable.
|
||||
pub fn fixable(&self) -> bool {
|
||||
match self {
|
||||
CheckKind::AmbiguousClassName(_) => true,
|
||||
CheckKind::AmbiguousFunctionName(_) => true,
|
||||
CheckKind::AmbiguousVariableName(_) => false,
|
||||
CheckKind::AssertTuple => false,
|
||||
CheckKind::DefaultExceptNotLast => false,
|
||||
|
||||
@@ -441,6 +441,82 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e742() -> Result<()> {
|
||||
let mut actual = check_path(
|
||||
Path::new("./resources/test/fixtures/E742.py"),
|
||||
&settings::Settings {
|
||||
line_length: 88,
|
||||
exclude: vec![],
|
||||
select: BTreeSet::from([CheckCode::E742]),
|
||||
},
|
||||
&fixer::Mode::Generate,
|
||||
)?;
|
||||
actual.sort_by_key(|check| check.location);
|
||||
let expected = vec![
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousClassName("l".to_string()),
|
||||
location: Location::new(1, 1),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousClassName("I".to_string()),
|
||||
location: Location::new(5, 1),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousClassName("O".to_string()),
|
||||
location: Location::new(9, 1),
|
||||
fix: None,
|
||||
},
|
||||
];
|
||||
|
||||
assert_eq!(actual.len(), expected.len());
|
||||
for i in 0..actual.len() {
|
||||
assert_eq!(actual[i], expected[i]);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn e743() -> Result<()> {
|
||||
let mut actual = check_path(
|
||||
Path::new("./resources/test/fixtures/E743.py"),
|
||||
&settings::Settings {
|
||||
line_length: 88,
|
||||
exclude: vec![],
|
||||
select: BTreeSet::from([CheckCode::E743]),
|
||||
},
|
||||
&fixer::Mode::Generate,
|
||||
)?;
|
||||
actual.sort_by_key(|check| check.location);
|
||||
let expected = vec![
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousFunctionName("l".to_string()),
|
||||
location: Location::new(1, 1),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousFunctionName("I".to_string()),
|
||||
location: Location::new(5, 1),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::AmbiguousFunctionName("O".to_string()),
|
||||
location: Location::new(10, 5),
|
||||
fix: None,
|
||||
},
|
||||
];
|
||||
|
||||
assert_eq!(actual.len(), expected.len());
|
||||
for i in 0..actual.len() {
|
||||
assert_eq!(actual[i], expected[i]);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn f401() -> Result<()> {
|
||||
let mut actual = check_path(
|
||||
@@ -1008,6 +1084,21 @@ mod tests {
|
||||
location: Location::new(16, 5),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::UnusedVariable("foo".to_string()),
|
||||
location: Location::new(20, 5),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::UnusedVariable("a".to_string()),
|
||||
location: Location::new(21, 6),
|
||||
fix: None,
|
||||
},
|
||||
Check {
|
||||
kind: CheckKind::UnusedVariable("b".to_string()),
|
||||
location: Location::new(21, 9),
|
||||
fix: None,
|
||||
},
|
||||
];
|
||||
assert_eq!(actual.len(), expected.len());
|
||||
for i in 0..actual.len() {
|
||||
|
||||
@@ -267,6 +267,8 @@ other-attribute = 1
|
||||
CheckCode::E714,
|
||||
CheckCode::E731,
|
||||
CheckCode::E741,
|
||||
CheckCode::E742,
|
||||
CheckCode::E743,
|
||||
CheckCode::E902,
|
||||
CheckCode::F401,
|
||||
CheckCode::F403,
|
||||
|
||||
@@ -52,6 +52,8 @@ impl Settings {
|
||||
CheckCode::E714,
|
||||
CheckCode::E731,
|
||||
CheckCode::E741,
|
||||
CheckCode::E742,
|
||||
CheckCode::E743,
|
||||
CheckCode::E902,
|
||||
CheckCode::F401,
|
||||
CheckCode::F403,
|
||||
|
||||
Reference in New Issue
Block a user