Compare commits

..

4 Commits

Author SHA1 Message Date
Charlie Marsh
546be5692a Bump version to 0.0.35 2022-09-11 21:54:00 -04:00
Charlie Marsh
43e1f20b28 Allow unused assignments in for loops and unpacking (#163) 2022-09-11 21:53:45 -04:00
Harutaka Kawamura
97388cefda Implement E743 (#162) 2022-09-11 21:27:33 -04:00
Harutaka Kawamura
63ce579989 Implement E742 (#160) 2022-09-11 20:27:48 -04:00
17 changed files with 264 additions and 24 deletions

View File

@@ -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
View File

@@ -1744,7 +1744,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.0.34"
version = "0.0.35"
dependencies = [
"anyhow",
"bincode",

View File

@@ -1,6 +1,6 @@
[package]
name = "ruff"
version = "0.0.34"
version = "0.0.35"
edition = "2021"
[lib]

View File

@@ -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 |

View File

@@ -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
View 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
View File

@@ -0,0 +1,15 @@
def l():
pass
def I():
pass
class X:
def O(self):
pass
def x():
pass

View File

@@ -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

View File

@@ -10,6 +10,8 @@ select = [
"E714",
"E731",
"E741",
"E742",
"E743",
"E902",
"F401",
"F403",

View File

@@ -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,

View File

@@ -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,

View File

@@ -35,8 +35,10 @@ impl Scope {
#[derive(Clone, Debug)]
pub enum BindingKind {
Annotation,
Argument,
Assignment,
Binding,
Builtin,
ClassDefinition,
Definition,

View File

@@ -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,
},
);
}
}

View File

@@ -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,

View File

@@ -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() {

View File

@@ -267,6 +267,8 @@ other-attribute = 1
CheckCode::E714,
CheckCode::E731,
CheckCode::E741,
CheckCode::E742,
CheckCode::E743,
CheckCode::E902,
CheckCode::F401,
CheckCode::F403,

View File

@@ -52,6 +52,8 @@ impl Settings {
CheckCode::E714,
CheckCode::E731,
CheckCode::E741,
CheckCode::E742,
CheckCode::E743,
CheckCode::E902,
CheckCode::F401,
CheckCode::F403,