Compare commits

..

6 Commits

Author SHA1 Message Date
Charlie Marsh
5a06fb28fd Bump version to 0.0.66 2022-10-10 10:03:59 -04:00
Charlie Marsh
46750a3e17 Flag unimplemented error codes in M001 (#388) 2022-10-10 10:03:40 -04:00
Charlie Marsh
9cc902b802 Avoid F821 false-positives with NameError (#386) 2022-10-10 09:39:59 -04:00
Harutaka Kawamura
c2a36ebd1e Implement C410 (#382) 2022-10-09 23:33:55 -04:00
Charlie Marsh
34ca225393 Rename flakes8 to flake8 2022-10-09 23:02:44 -04:00
Harutaka Kawamura
38c30905e6 Implement C409 (#381) 2022-10-09 22:34:52 -04:00
16 changed files with 415 additions and 88 deletions

2
Cargo.lock generated
View File

@@ -1907,7 +1907,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.0.65"
version = "0.0.66"
dependencies = [
"anyhow",
"bincode",

View File

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

View File

@@ -286,6 +286,8 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| C405 | UnnecessaryLiteralSet | Unnecessary <list/tuple> literal - rewrite as a set literal | | |
| C406 | UnnecessaryLiteralDict | Unnecessary <list/tuple> literal - rewrite as a dict literal | | |
| C408 | UnnecessaryCollectionCall | Unnecessary <dict/list/tuple> call - rewrite as a literal | | |
| C409 | UnnecessaryLiteralWithinTupleCall | Unnecessary <list/tuple> literal passed to tuple() - remove the outer call to tuple() | | |
| C410 | UnnecessaryLiteralWithinListCall | Unnecessary <list/tuple> literal passed to list() - rewrite as a list literal | | |
| C415 | UnnecessarySubscriptReversal | Unnecessary subscript reversal of iterable within <reversed/set/sorted>() | | |
| T201 | PrintFound | `print` found | | 🛠 |
| T203 | PPrintFound | `pprint` found | | 🛠 |

3
resources/test/fixtures/C409.py vendored Normal file
View File

@@ -0,0 +1,3 @@
t1 = tuple([1, 2])
t2 = tuple((1, 2))
t3 = tuple([])

4
resources/test/fixtures/C410.py vendored Normal file
View File

@@ -0,0 +1,4 @@
l1 = list([1, 2])
l2 = list((1, 2))
l3 = list([])
l4 = list(())

View File

@@ -122,3 +122,12 @@ class PEP593Test:
dict["foo", "bar"], # Expected to fail as undefined.
123,
]
def in_ipython_notebook() -> bool:
try:
# autoimported by notebooks
get_ipython() # type: ignore[name-defined]
except NameError:
return False # not in notebook
return True

View File

@@ -15,6 +15,9 @@ def f() -> None:
# Invalid
d = 1 # noqa: F841, E501
# Invalid (and unimplemented)
d = 1 # noqa: F841, W191
# Valid
_ = """Lorem ipsum dolor sit amet.

View File

@@ -757,7 +757,7 @@ pub fn is_super_call_with_arguments(func: &Expr, args: &Vec<Expr>) -> bool {
}
}
// flakes8-comprehensions
// flake8-comprehensions
/// Check `list(generator)` compliance.
pub fn unnecessary_generator_list(expr: &Expr, func: &Expr, args: &Vec<Expr>) -> Option<Check> {
if args.len() == 1 {
@@ -969,6 +969,66 @@ pub fn unnecessary_collection_call(
None
}
pub fn unnecessary_literal_within_tuple_call(
expr: &Expr,
func: &Expr,
args: &[Expr],
) -> Option<Check> {
if let ExprKind::Name { id, .. } = &func.node {
if id == "tuple" {
if let Some(arg) = args.first() {
match &arg.node {
ExprKind::Tuple { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralWithinTupleCall("tuple".to_string()),
Range::from_located(expr),
));
}
ExprKind::List { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralWithinTupleCall("list".to_string()),
Range::from_located(expr),
));
}
_ => {}
}
}
}
}
None
}
pub fn unnecessary_literal_within_list_call(
expr: &Expr,
func: &Expr,
args: &[Expr],
) -> Option<Check> {
if let ExprKind::Name { id, .. } = &func.node {
if id == "list" {
if let Some(arg) = args.first() {
match &arg.node {
ExprKind::Tuple { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralWithinListCall("tuple".to_string()),
Range::from_located(expr),
));
}
ExprKind::List { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralWithinListCall("list".to_string()),
Range::from_located(expr),
));
}
_ => {}
}
}
}
}
None
}
pub fn unnecessary_subscript_reversal(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if let Some(first_arg) = args.first() {
if let ExprKind::Name { id, .. } = &func.node {

View File

@@ -1,6 +1,6 @@
use once_cell::sync::Lazy;
use regex::Regex;
use rustpython_ast::{Expr, ExprKind, StmtKind};
use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind, StmtKind};
use crate::python::typing;
@@ -35,6 +35,22 @@ pub fn match_annotated_subscript(expr: &Expr) -> Option<SubscriptKind> {
}
}
fn node_name(expr: &Expr) -> Option<&str> {
if let ExprKind::Name { id, .. } = &expr.node {
Some(id)
} else {
None
}
}
pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool {
match &expr.node {
ExprKind::Attribute { attr, .. } => target == attr,
ExprKind::Name { id, .. } => target == id,
_ => false,
}
}
pub fn is_assignment_to_a_dunder(node: &StmtKind) -> bool {
// Check whether it's an assignment to a dunder, with or without a type annotation.
// This is what pycodestyle (as of 2.9.1) does.
@@ -65,10 +81,27 @@ pub fn is_assignment_to_a_dunder(node: &StmtKind) -> bool {
}
}
pub fn match_name_or_attr(expr: &Expr, target: &str) -> bool {
match &expr.node {
ExprKind::Attribute { attr, .. } => target == attr,
ExprKind::Name { id, .. } => target == id,
_ => false,
/// Extract the names of all handled exceptions.
/// Note that, for now, this only matches on ExprKind::Name, and so won't catch exceptions like
/// `module.CustomException`. (But will catch all builtin exceptions.)
pub fn extract_handler_names(handlers: &[Excepthandler]) -> Vec<&str> {
let mut handler_names = vec![];
for handler in handlers {
match &handler.node {
ExcepthandlerKind::ExceptHandler { type_, .. } => {
if let Some(type_) = type_ {
if let ExprKind::Tuple { elts, .. } = &type_.node {
for type_ in elts {
if let Some(name) = node_name(type_) {
handler_names.push(name);
}
}
} else if let Some(name) = node_name(type_) {
handler_names.push(name);
}
}
}
}
}
handler_names
}

View File

@@ -10,7 +10,7 @@ use rustpython_parser::ast::{
};
use rustpython_parser::parser;
use crate::ast::helpers::{match_name_or_attr, SubscriptKind};
use crate::ast::helpers::{extract_handler_names, match_name_or_attr, SubscriptKind};
use crate::ast::operations::{extract_all_names, SourceCodeLocator};
use crate::ast::relocate::relocate_expr;
use crate::ast::types::{
@@ -60,6 +60,7 @@ pub struct Checker<'a> {
seen_docstring: bool,
futures_allowed: bool,
annotations_future_enabled: bool,
except_handlers: Vec<Vec<&'a str>>,
}
impl<'a> Checker<'a> {
@@ -74,25 +75,26 @@ impl<'a> Checker<'a> {
autofix,
path,
locator: SourceCodeLocator::new(content),
checks: vec![],
parents: vec![],
parent_stack: vec![],
scopes: vec![],
scope_stack: vec![],
dead_scopes: vec![],
deferred_string_annotations: vec![],
deferred_annotations: vec![],
deferred_functions: vec![],
deferred_lambdas: vec![],
deferred_assignments: vec![],
in_f_string: None,
in_annotation: false,
in_literal: false,
seen_non_import: false,
seen_docstring: false,
futures_allowed: true,
annotations_future_enabled: false,
checks: Default::default(),
deletions: Default::default(),
parents: Default::default(),
parent_stack: Default::default(),
scopes: Default::default(),
scope_stack: Default::default(),
dead_scopes: Default::default(),
deferred_string_annotations: Default::default(),
deferred_annotations: Default::default(),
deferred_functions: Default::default(),
deferred_lambdas: Default::default(),
deferred_assignments: Default::default(),
in_f_string: None,
in_annotation: Default::default(),
in_literal: Default::default(),
seen_non_import: Default::default(),
seen_docstring: Default::default(),
futures_allowed: true,
annotations_future_enabled: Default::default(),
except_handlers: Default::default(),
}
}
}
@@ -601,6 +603,27 @@ where
self.visit_stmt(stmt);
}
}
StmtKind::Try {
body,
handlers,
orelse,
finalbody,
} => {
self.except_handlers.push(extract_handler_names(handlers));
for stmt in body {
self.visit_stmt(stmt);
}
self.except_handlers.pop();
for excepthandler in handlers {
self.visit_excepthandler(excepthandler)
}
for stmt in orelse {
self.visit_stmt(stmt);
}
for stmt in finalbody {
self.visit_stmt(stmt);
}
}
_ => visitor::walk_stmt(self, stmt),
};
@@ -796,6 +819,21 @@ where
};
}
if self.settings.enabled.contains(&CheckCode::C409) {
if let Some(check) =
checks::unnecessary_literal_within_tuple_call(expr, func, args)
{
self.checks.push(check);
};
}
if self.settings.enabled.contains(&CheckCode::C410) {
if let Some(check) =
checks::unnecessary_literal_within_list_call(expr, func, args)
{
self.checks.push(check);
};
}
if self.settings.enabled.contains(&CheckCode::C415) {
if let Some(check) = checks::unnecessary_subscript_reversal(expr, func, args) {
self.checks.push(check);
@@ -1471,6 +1509,14 @@ impl<'a> Checker<'a> {
if self.path.ends_with("__init__.py") && id == "__path__" {
return;
}
// Avoid flagging if NameError is handled.
if let Some(handler_names) = self.except_handlers.last() {
if handler_names.contains(&"NameError") {
return;
}
}
self.checks.push(Check::new(
CheckKind::UndefinedName(id.clone()),
self.locate_check(Range::from_located(expr)),

View File

@@ -184,15 +184,15 @@ pub fn check_lines(
let mut valid_codes = vec![];
for code in codes {
if !matches.contains(&code) {
invalid_codes.push(code);
invalid_codes.push(code.to_string());
} else {
valid_codes.push(code);
valid_codes.push(code.to_string());
}
}
if !invalid_codes.is_empty() {
let mut check = Check::new(
CheckKind::UnusedNOQA(Some(invalid_codes.join(", "))),
CheckKind::UnusedNOQA(Some(invalid_codes)),
Range {
location: Location::new(row + 1, start + 1),
end_location: Location::new(row + 1, end + 1),

View File

@@ -1,6 +1,7 @@
use itertools::Itertools;
use rustpython_parser::ast::Location;
use serde::{Deserialize, Serialize};
use std::str::FromStr;
use strum_macros::{AsRefStr, EnumIter, EnumString};
use crate::ast::checks::Primitive;
@@ -129,6 +130,8 @@ pub enum CheckCode {
C405,
C406,
C408,
C409,
C410,
C415,
// flake8-print
T201,
@@ -210,7 +213,7 @@ pub enum CheckKind {
BuiltinVariableShadowing(String),
BuiltinArgumentShadowing(String),
BuiltinAttributeShadowing(String),
// flakes8-comprehensions
// flake8-comprehensions
UnnecessaryGeneratorList,
UnnecessaryGeneratorSet,
UnnecessaryGeneratorDict,
@@ -219,6 +222,8 @@ pub enum CheckKind {
UnnecessaryLiteralSet(String),
UnnecessaryLiteralDict(String),
UnnecessaryCollectionCall(String),
UnnecessaryLiteralWithinTupleCall(String),
UnnecessaryLiteralWithinListCall(String),
UnnecessarySubscriptReversal(String),
// flake8-print
PrintFound,
@@ -233,7 +238,7 @@ pub enum CheckKind {
UsePEP604Annotation,
SuperCallWithParameters,
// Meta
UnusedNOQA(Option<String>),
UnusedNOQA(Option<Vec<String>>),
}
impl CheckCode {
@@ -312,6 +317,12 @@ impl CheckCode {
CheckCode::C408 => {
CheckKind::UnnecessaryCollectionCall("<dict/list/tuple>".to_string())
}
CheckCode::C409 => {
CheckKind::UnnecessaryLiteralWithinTupleCall("<list/tuple>".to_string())
}
CheckCode::C410 => {
CheckKind::UnnecessaryLiteralWithinListCall("<list/tuple>".to_string())
}
CheckCode::C415 => {
CheckKind::UnnecessarySubscriptReversal("<reversed/set/sorted>".to_string())
}
@@ -398,6 +409,8 @@ impl CheckKind {
CheckKind::UnnecessaryLiteralSet(_) => &CheckCode::C405,
CheckKind::UnnecessaryLiteralDict(_) => &CheckCode::C406,
CheckKind::UnnecessaryCollectionCall(_) => &CheckCode::C408,
CheckKind::UnnecessaryLiteralWithinTupleCall(..) => &CheckCode::C409,
CheckKind::UnnecessaryLiteralWithinListCall(..) => &CheckCode::C410,
CheckKind::UnnecessarySubscriptReversal(_) => &CheckCode::C415,
// flake8-print
CheckKind::PrintFound => &CheckCode::T201,
@@ -584,6 +597,28 @@ impl CheckKind {
CheckKind::UnnecessaryCollectionCall(obj_type) => {
format!("Unnecessary {obj_type} call - rewrite as a literal")
}
CheckKind::UnnecessaryLiteralWithinTupleCall(literal) => {
if literal == "list" {
format!(
"Unnecessary {literal} literal passed to tuple() - rewrite as a tuple literal"
)
} else {
format!(
"Unnecessary {literal} literal passed to tuple() - remove the outer call to tuple()"
)
}
}
CheckKind::UnnecessaryLiteralWithinListCall(literal) => {
if literal == "list" {
format!(
"Unnecessary {literal} literal passed to list() - remove the outer call to list()"
)
} else {
format!(
"Unnecessary {literal} literal passed to list() - rewrite as a list literal"
)
}
}
CheckKind::UnnecessarySubscriptReversal(func) => {
format!("Unnecessary subscript reversal of iterable within {func}()")
}
@@ -616,9 +651,21 @@ impl CheckKind {
"Use `super()` instead of `super(__class__, self)`".to_string()
}
// Meta
CheckKind::UnusedNOQA(code) => match code {
CheckKind::UnusedNOQA(codes) => match codes {
None => "Unused `noqa` directive".to_string(),
Some(code) => format!("Unused `noqa` directive for: {code}"),
Some(codes) => {
let codes = codes
.iter()
.map(|code| {
if CheckCode::from_str(code).is_ok() {
code.to_string()
} else {
format!("{code} (not implemented)")
}
})
.join(", ");
format!("Unused `noqa` directive for: {codes}")
}
},
}
}

View File

@@ -762,42 +762,6 @@ mod tests {
Ok(())
}
#[test]
fn m001() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/M001.py"),
&settings::Settings::for_rules(vec![CheckCode::M001, CheckCode::E501, CheckCode::F841]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn init() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/__init__.py"),
&settings::Settings::for_rules(vec![CheckCode::F821, CheckCode::F822]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn future_annotations() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/future_annotations.py"),
&settings::Settings::for_rules(vec![CheckCode::F401, CheckCode::F821]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn e999() -> Result<()> {
let mut checks = check_path(
@@ -942,6 +906,30 @@ mod tests {
Ok(())
}
#[test]
fn c409() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/C409.py"),
&settings::Settings::for_rule(CheckCode::C409),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn c410() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/C410.py"),
&settings::Settings::for_rule(CheckCode::C410),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn c415() -> Result<()> {
let mut checks = check_path(
@@ -1073,4 +1061,40 @@ mod tests {
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn m001() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/M001.py"),
&settings::Settings::for_rules(vec![CheckCode::M001, CheckCode::E501, CheckCode::F841]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn init() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/__init__.py"),
&settings::Settings::for_rules(vec![CheckCode::F821, CheckCode::F822]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
#[test]
fn future_annotations() -> Result<()> {
let mut checks = check_path(
Path::new("./resources/test/fixtures/future_annotations.py"),
&settings::Settings::for_rules(vec![CheckCode::F401, CheckCode::F821]),
&fixer::Mode::Generate,
)?;
checks.sort_by_key(|check| check.location);
insta::assert_yaml_snapshot!(checks);
Ok(())
}
}

View File

@@ -0,0 +1,32 @@
---
source: src/linter.rs
expression: checks
---
- kind:
UnnecessaryLiteralWithinTupleCall: list
location:
row: 1
column: 6
end_location:
row: 1
column: 19
fix: ~
- kind:
UnnecessaryLiteralWithinTupleCall: tuple
location:
row: 2
column: 6
end_location:
row: 2
column: 19
fix: ~
- kind:
UnnecessaryLiteralWithinTupleCall: list
location:
row: 3
column: 6
end_location:
row: 3
column: 15
fix: ~

View File

@@ -0,0 +1,41 @@
---
source: src/linter.rs
expression: checks
---
- kind:
UnnecessaryLiteralWithinListCall: list
location:
row: 1
column: 6
end_location:
row: 1
column: 18
fix: ~
- kind:
UnnecessaryLiteralWithinListCall: tuple
location:
row: 2
column: 6
end_location:
row: 2
column: 18
fix: ~
- kind:
UnnecessaryLiteralWithinListCall: list
location:
row: 3
column: 6
end_location:
row: 3
column: 14
fix: ~
- kind:
UnnecessaryLiteralWithinListCall: tuple
location:
row: 4
column: 6
end_location:
row: 4
column: 14
fix: ~

View File

@@ -20,7 +20,8 @@ expression: checks
column: 18
applied: false
- kind:
UnusedNOQA: E501
UnusedNOQA:
- E501
location:
row: 13
column: 10
@@ -37,7 +38,9 @@ expression: checks
column: 24
applied: false
- kind:
UnusedNOQA: E501
UnusedNOQA:
- F841
- E501
location:
row: 16
column: 10
@@ -45,7 +48,7 @@ expression: checks
row: 16
column: 30
fix:
content: " # noqa: F841"
content: ""
location:
row: 16
column: 10
@@ -54,54 +57,74 @@ expression: checks
column: 30
applied: false
- kind:
UnusedNOQA: F841
UnusedNOQA:
- W191
location:
row: 41
row: 19
column: 10
end_location:
row: 19
column: 30
fix:
content: " # noqa: F841"
location:
row: 19
column: 10
end_location:
row: 19
column: 30
applied: false
- kind:
UnusedNOQA:
- F841
location:
row: 44
column: 4
end_location:
row: 41
row: 44
column: 24
fix:
content: " # noqa: E501"
location:
row: 41
row: 44
column: 4
end_location:
row: 41
row: 44
column: 24
applied: false
- kind:
UnusedNOQA: E501
UnusedNOQA:
- E501
location:
row: 49
row: 52
column: 4
end_location:
row: 49
row: 52
column: 18
fix:
content: ""
location:
row: 49
row: 52
column: 4
end_location:
row: 49
row: 52
column: 18
applied: false
- kind:
UnusedNOQA: ~
location:
row: 57
row: 60
column: 4
end_location:
row: 57
row: 60
column: 12
fix:
content: ""
location:
row: 57
row: 60
column: 4
end_location:
row: 57
row: 60
column: 12
applied: false