Compare commits

..

9 Commits

Author SHA1 Message Date
Charlie Marsh
db3c847771 Bump version to 0.0.83 2022-10-25 21:24:13 -04:00
Charlie Marsh
4adbfc24a5 Increment flake8-bugbear to 6/32 2022-10-25 21:23:13 -04:00
Charlie Marsh
8f734a6562 Implement B002 (unary prefix increment) (#468) 2022-10-25 21:10:51 -04:00
Charlie Marsh
bcf7519eb3 Implement B017 (no assertRaises(Exception)) (#467) 2022-10-25 20:55:00 -04:00
Heyward Fann
66089052ee chore: typo on #283 link (#464) 2022-10-25 08:02:11 -04:00
Jeong YunWon
d50cc8ff65 Restyle flake8_comprehensions::check to reduce indent (#462) 2022-10-24 15:16:56 -04:00
Harutaka Kawamura
b75ea94f58 Fix uppercase and lowercase check (#461) 2022-10-22 12:49:13 -04:00
Harutaka Kawamura
2c24e2fd28 Enable N811, 812, 813, 814, 817 for Import (#460) 2022-10-22 08:31:02 -04:00
Charlie Marsh
f8dc208665 Tweak messages for pep8-naming rules 2022-10-21 17:06:16 -04:00
26 changed files with 684 additions and 408 deletions

2
Cargo.lock generated
View File

@@ -2045,7 +2045,7 @@ dependencies = [
[[package]]
name = "ruff"
version = "0.0.82"
version = "0.0.83"
dependencies = [
"anyhow",
"assert_cmd",

View File

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

View File

@@ -77,7 +77,7 @@ Ruff also works with [pre-commit](https://pre-commit.com):
```yaml
repos:
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.0.82
rev: v0.0.83
hooks:
- id: lint
```
@@ -348,12 +348,12 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| N803 | InvalidArgumentName | Argument name `...` should be lowercase | |
| N804 | InvalidFirstArgumentNameForClassMethod | First argument of a class method should be named `cls` | |
| N805 | InvalidFirstArgumentNameForMethod | First argument of a method should be named `self` | |
| N807 | DunderFunctionName | function name should not start and end with '__' | |
| N811 | ConstantImportedAsNonConstant | constant '...' imported as non constant '...' | |
| N812 | LowercaseImportedAsNonLowercase | lowercase '...' imported as non lowercase '...' | |
| N813 | CamelcaseImportedAsLowercase | camelcase '...' imported as lowercase '...' | |
| N814 | CamelcaseImportedAsConstant | camelcase '...' imported as constant '...' | |
| N817 | CamelcaseImportedAsAcronym | camelcase '...' imported as acronym '...' | |
| N807 | DunderFunctionName | Function name should not start and end with `__` | |
| N811 | ConstantImportedAsNonConstant | Constant `...` imported as non-constant `...` | |
| N812 | LowercaseImportedAsNonLowercase | Lowercase `...` imported as non-lowercase `...` | |
| N813 | CamelcaseImportedAsLowercase | Camelcase `...` imported as lowercase `...` | |
| N814 | CamelcaseImportedAsConstant | Camelcase `...` imported as constant `...` | |
| N817 | CamelcaseImportedAsAcronym | Camelcase `...` imported as acronym `...` | |
### flake8-comprehensions
@@ -380,8 +380,10 @@ The 🛠 emoji indicates that a rule is automatically fixable by the `--fix` com
| Code | Name | Message | Fix |
| ---- | ---- | ------- | --- |
| B002 | UnaryPrefixIncrement | Python does not support the unary prefix increment. | |
| B011 | DoNotAssertFalse | Do not `assert False` (`python -O` removes these calls), raise `AssertionError()` | 🛠 |
| B014 | DuplicateHandlerException | Exception handler with duplicate exception: `ValueError` | 🛠 |
| B017 | NoAssertRaisesException | `assertRaises(Exception):` should be considered evil. | |
| B025 | DuplicateTryBlockException | try-except block with duplicate exception `Exception` | |
### flake8-builtins
@@ -471,7 +473,7 @@ including:
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (6/32)
- [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34)
- [`autoflake`](https://pypi.org/project/autoflake/) (1/7)
@@ -491,7 +493,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl
- [`flake8-super`](https://pypi.org/project/flake8-super/)
- [`flake8-print`](https://pypi.org/project/flake8-print/)
- [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (3/32)
- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (6/32)
Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa),
and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (8/34).
@@ -512,7 +514,7 @@ on Rust at all.
### Can I write my own plugins for Ruff?
Ruff does not yet support third-party plugins, though a plugin system is within-scope for the
project. See [#283](https://github.com/charliermarsh/ruff/issues/2830) for more.
project. See [#283](https://github.com/charliermarsh/ruff/issues/283) for more.
### Does Ruff support NumPy- or Google-style docstrings?

View File

@@ -19,7 +19,7 @@ fn main() {
"| {} | {} | {} | {} |",
check_kind.code().as_ref(),
check_kind.as_ref(),
check_kind.body().replace("|", r"\|"),
check_kind.summary().replace("|", r"\|"),
fix_token
);
}

20
resources/test/fixtures/B002.py vendored Normal file
View File

@@ -0,0 +1,20 @@
"""
Should emit:
B002 - on lines 15 and 20
"""
def this_is_all_fine(n):
x = n + 1
y = 1 + n
z = +x + y
return +z
def this_is_buggy(n):
x = ++n
return x
def this_is_buggy_too(n):
return ++n

36
resources/test/fixtures/B017.py vendored Normal file
View File

@@ -0,0 +1,36 @@
"""
Should emit:
B017 - on lines 20
"""
import asyncio
import unittest
CONSTANT = True
def something_else() -> None:
for i in (1, 2, 3):
print(i)
class Foo:
pass
class Foobar(unittest.TestCase):
def evil_raises(self) -> None:
with self.assertRaises(Exception):
raise Exception("Evil I say!")
def context_manager_raises(self) -> None:
with self.assertRaises(Exception) as ex:
raise Exception("Context manager is good")
self.assertEqual("Context manager is good", str(ex.exception))
def regex_raises(self) -> None:
with self.assertRaisesRegex(Exception, "Regex is good"):
raise Exception("Regex is good")
def raises_with_absolute_reference(self):
with self.assertRaises(asyncio.CancelledError):
Foo()

View File

@@ -1 +1,3 @@
from mod import BAD as bad
import mod.CONST as const
from mod import CONSTANT as constant
from mod import ANOTHER_CONSTANT as another_constant

View File

@@ -1 +1,3 @@
from mod import bad as Bad
import modl.lowercase as Lower
from mod import lowercase as Lowercase
from mod import another_lowercase as AnotherLowercase

View File

@@ -1 +1,3 @@
import mod.Camel as camel
from mod import CamelCase as camelcase
from mod import AnotherCamelCase as another_camelcase

View File

@@ -1 +1,3 @@
import mod.Camel as CAMEL
from mod import CamelCase as CAMELCASE
from mod import AnotherCamelCase as ANOTHER_CAMELCASE

View File

@@ -1 +1,2 @@
import mod.CaMel as CM
from mod import CamelCase as CC

View File

@@ -413,6 +413,55 @@ where
},
)
}
if let Some(asname) = &alias.node.asname {
let name = alias.node.name.split('.').last().unwrap();
if self.settings.enabled.contains(&CheckCode::N811) {
if let Some(check) =
pep8_naming::checks::constant_imported_as_non_constant(
stmt, name, asname,
)
{
self.checks.push(check);
}
}
if self.settings.enabled.contains(&CheckCode::N812) {
if let Some(check) =
pep8_naming::checks::lowercase_imported_as_non_lowercase(
stmt, name, asname,
)
{
self.checks.push(check);
}
}
if self.settings.enabled.contains(&CheckCode::N813) {
if let Some(check) =
pep8_naming::checks::camelcase_imported_as_lowercase(
stmt, name, asname,
)
{
self.checks.push(check);
}
}
if self.settings.enabled.contains(&CheckCode::N814) {
if let Some(check) = pep8_naming::checks::camelcase_imported_as_constant(
stmt, name, asname,
) {
self.checks.push(check);
}
}
if self.settings.enabled.contains(&CheckCode::N817) {
if let Some(check) = pep8_naming::checks::camelcase_imported_as_acronym(
stmt, name, asname,
) {
self.checks.push(check);
}
}
}
}
}
StmtKind::ImportFrom {
@@ -618,6 +667,11 @@ where
flake8_bugbear::plugins::assert_false(self, stmt, test, msg);
}
}
StmtKind::With { items, .. } | StmtKind::AsyncWith { items, .. } => {
if self.settings.enabled.contains(&CheckCode::B017) {
flake8_bugbear::plugins::assert_raises_exception(self, stmt, items);
}
}
StmtKind::Try { handlers, .. } => {
if self.settings.enabled.contains(&CheckCode::F707) {
if let Some(check) = pyflakes::checks::default_except_not_last(handlers) {
@@ -1077,6 +1131,10 @@ where
self,
));
}
if self.settings.enabled.contains(&CheckCode::B002) {
flake8_bugbear::plugins::unary_prefix_increment(self, expr, op, operand);
}
}
ExprKind::Compare {
left,

View File

@@ -75,8 +75,10 @@ pub enum CheckCode {
A002,
A003,
// flake8-bugbear
B002,
B011,
B014,
B017,
B025,
// flake8-comprehensions
C400,
@@ -265,8 +267,10 @@ pub enum CheckKind {
BuiltinArgumentShadowing(String),
BuiltinAttributeShadowing(String),
// flake8-bugbear
UnaryPrefixIncrement,
DoNotAssertFalse,
DuplicateHandlerException(Vec<String>),
NoAssertRaisesException,
DuplicateTryBlockException(String),
// flake8-comprehensions
UnnecessaryGeneratorList,
@@ -424,8 +428,10 @@ impl CheckCode {
CheckCode::A002 => CheckKind::BuiltinArgumentShadowing("...".to_string()),
CheckCode::A003 => CheckKind::BuiltinAttributeShadowing("...".to_string()),
// flake8-bugbear
CheckCode::B002 => CheckKind::UnaryPrefixIncrement,
CheckCode::B011 => CheckKind::DoNotAssertFalse,
CheckCode::B014 => CheckKind::DuplicateHandlerException(vec!["ValueError".to_string()]),
CheckCode::B017 => CheckKind::NoAssertRaisesException,
CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()),
// flake8-comprehensions
CheckCode::C400 => CheckKind::UnnecessaryGeneratorList,
@@ -598,8 +604,10 @@ impl CheckCode {
CheckCode::A001 => CheckCategory::Flake8Builtins,
CheckCode::A002 => CheckCategory::Flake8Builtins,
CheckCode::A003 => CheckCategory::Flake8Builtins,
CheckCode::B002 => CheckCategory::Flake8Bugbear,
CheckCode::B011 => CheckCategory::Flake8Bugbear,
CheckCode::B014 => CheckCategory::Flake8Bugbear,
CheckCode::B017 => CheckCategory::Flake8Bugbear,
CheckCode::B025 => CheckCategory::Flake8Bugbear,
CheckCode::C400 => CheckCategory::Flake8Comprehensions,
CheckCode::C401 => CheckCategory::Flake8Comprehensions,
@@ -741,8 +749,10 @@ impl CheckKind {
CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
CheckKind::BuiltinAttributeShadowing(_) => &CheckCode::A003,
// flake8-bugbear
CheckKind::UnaryPrefixIncrement => &CheckCode::B002,
CheckKind::DoNotAssertFalse => &CheckCode::B011,
CheckKind::DuplicateHandlerException(_) => &CheckCode::B014,
CheckKind::NoAssertRaisesException => &CheckCode::B017,
CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025,
// flake8-comprehensions
CheckKind::UnnecessaryGeneratorList => &CheckCode::C400,
@@ -979,6 +989,7 @@ impl CheckKind {
format!("Class attribute `{name}` is shadowing a python builtin")
}
// flake8-bugbear
CheckKind::UnaryPrefixIncrement => "Python does not support the unary prefix increment. Writing `++n` is equivalent to `+(+(n))`, which equals `n`. You meant `n += 1`.".to_string(),
CheckKind::DoNotAssertFalse => {
"Do not `assert False` (`python -O` removes these calls), raise `AssertionError()`"
.to_string()
@@ -992,6 +1003,9 @@ impl CheckKind {
format!("Exception handler with duplicate exceptions: {names}")
}
}
CheckKind::NoAssertRaisesException => {
"`assertRaises(Exception):` should be considered evil. It can lead to your test passing even if the code being tested is never executed due to a typo. Either assert for a more specific exception (builtin or custom), use `assertRaisesRegex`, or use the context manager form of `assertRaises`.".to_string()
}
CheckKind::DuplicateTryBlockException(name) => {
format!("try-except block with duplicate exception `{name}`")
}
@@ -1221,22 +1235,22 @@ impl CheckKind {
"First argument of a method should be named `self`".to_string()
}
CheckKind::DunderFunctionName => {
"function name should not start and end with '__'".to_string()
"Function name should not start and end with `__`".to_string()
}
CheckKind::ConstantImportedAsNonConstant(name, asname) => {
format!("constant '{name}' imported as non constant '{asname}'")
format!("Constant `{name}` imported as non-constant `{asname}`")
}
CheckKind::LowercaseImportedAsNonLowercase(name, asname) => {
format!("lowercase '{name}' imported as non lowercase '{asname}'")
format!("Lowercase `{name}` imported as non-lowercase `{asname}`")
}
CheckKind::CamelcaseImportedAsLowercase(name, asname) => {
format!("camelcase '{name}' imported as lowercase '{asname}'")
format!("Camelcase `{name}` imported as lowercase `{asname}`")
}
CheckKind::CamelcaseImportedAsConstant(name, asname) => {
format!("camelcase '{name}' imported as constant '{asname}'")
format!("Camelcase `{name}` imported as constant `{asname}`")
}
CheckKind::CamelcaseImportedAsAcronym(name, asname) => {
format!("camelcase '{name}' imported as acronym '{asname}'")
format!("Camelcase `{name}` imported as acronym `{asname}`")
}
// Meta
CheckKind::UnusedNOQA(codes) => match codes {
@@ -1258,6 +1272,19 @@ impl CheckKind {
}
}
/// The summary text for the check. Typically a truncated form of the body text.
pub fn summary(&self) -> String {
match self {
CheckKind::UnaryPrefixIncrement => {
"Python does not support the unary prefix increment.".to_string()
}
CheckKind::NoAssertRaisesException => {
"`assertRaises(Exception):` should be considered evil.".to_string()
}
_ => self.body(),
}
}
/// Whether the check kind is (potentially) fixable.
pub fn fixable(&self) -> bool {
matches!(

View File

@@ -0,0 +1,25 @@
use rustpython_ast::{ExprKind, Stmt, Withitem};
use crate::ast::helpers::match_name_or_attr;
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
/// B017
pub fn assert_raises_exception(checker: &mut Checker, stmt: &Stmt, items: &[Withitem]) {
if let Some(item) = items.first() {
let item_context = &item.context_expr;
if let ExprKind::Call { func, args, .. } = &item_context.node {
if match_name_or_attr(func, "assertRaises")
&& args.len() == 1
&& match_name_or_attr(args.first().unwrap(), "Exception")
&& item.optional_vars.is_none()
{
checker.add_check(Check::new(
CheckKind::NoAssertRaisesException,
Range::from_located(stmt),
));
}
}
}
}

View File

@@ -1,6 +1,10 @@
pub use assert_false::assert_false;
pub use assert_raises_exception::assert_raises_exception;
pub use duplicate_exceptions::duplicate_exceptions;
pub use duplicate_exceptions::duplicate_handler_exceptions;
pub use unary_prefix_increment::unary_prefix_increment;
mod assert_false;
mod assert_raises_exception;
mod duplicate_exceptions;
mod unary_prefix_increment;

View File

@@ -0,0 +1,19 @@
use rustpython_ast::{Expr, ExprKind, Unaryop};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
/// B002
pub fn unary_prefix_increment(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) {
if matches!(op, Unaryop::UAdd) {
if let ExprKind::UnaryOp { op, .. } = &operand.node {
if matches!(op, Unaryop::UAdd) {
checker.add_check(Check::new(
CheckKind::UnaryPrefixIncrement,
Range::from_located(expr),
))
}
}
}
}

View File

@@ -4,57 +4,75 @@ use rustpython_ast::{Comprehension, Constant, Expr, ExprKind, KeywordData, Locat
use crate::ast::types::Range;
use crate::checks::{Check, CheckKind};
fn function_name(func: &Expr) -> Option<&str> {
if let ExprKind::Name { id, .. } = &func.node {
Some(id)
} else {
None
}
}
fn exactly_one_argument_with_matching_function<'a>(
name: &str,
func: &Expr,
args: &'a [Expr],
) -> Option<&'a ExprKind> {
if args.len() != 1 {
return None;
}
if function_name(func)? != name {
return None;
}
Some(&args[0].node)
}
fn first_argument_with_matching_function<'a>(
name: &str,
func: &Expr,
args: &'a [Expr],
) -> Option<&'a ExprKind> {
if function_name(func)? != name {
return None;
}
Some(&args.first()?.node)
}
/// Check `list(generator)` compliance.
pub fn unnecessary_generator_list(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "list" {
if let ExprKind::GeneratorExp { .. } = &args[0].node {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorList,
Range::from_located(expr),
));
}
}
}
let argument = exactly_one_argument_with_matching_function("list", func, args)?;
if let ExprKind::GeneratorExp { .. } = argument {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorList,
Range::from_located(expr),
));
}
None
}
/// Check `set(generator)` compliance.
pub fn unnecessary_generator_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "set" {
if let ExprKind::GeneratorExp { .. } = &args[0].node {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorSet,
Range::from_located(expr),
));
}
}
}
let argument = exactly_one_argument_with_matching_function("set", func, args)?;
if let ExprKind::GeneratorExp { .. } = argument {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorSet,
Range::from_located(expr),
));
}
None
}
/// Check `dict((x, y) for x, y in iterable)` compliance.
pub fn unnecessary_generator_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "dict" {
if let ExprKind::GeneratorExp { elt, .. } = &args[0].node {
match &elt.node {
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorDict,
Range::from_located(expr),
));
}
_ => {}
}
}
let argument = exactly_one_argument_with_matching_function("dict", func, args)?;
if let ExprKind::GeneratorExp { elt, .. } = argument {
match &elt.node {
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryGeneratorDict,
Range::from_located(expr),
));
}
_ => {}
}
}
None
@@ -66,17 +84,12 @@ pub fn unnecessary_list_comprehension_set(
func: &Expr,
args: &[Expr],
) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "set" {
if let ExprKind::ListComp { .. } = &args[0].node {
return Some(Check::new(
CheckKind::UnnecessaryListComprehensionSet,
Range::from_located(expr),
));
}
}
}
let argument = exactly_one_argument_with_matching_function("set", func, args)?;
if let ExprKind::ListComp { .. } = &argument {
return Some(Check::new(
CheckKind::UnnecessaryListComprehensionSet,
Range::from_located(expr),
));
}
None
}
@@ -87,21 +100,16 @@ pub fn unnecessary_list_comprehension_dict(
func: &Expr,
args: &[Expr],
) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "dict" {
if let ExprKind::ListComp { elt, .. } = &args[0].node {
match &elt.node {
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryListComprehensionDict,
Range::from_located(expr),
));
}
_ => {}
}
}
let argument = exactly_one_argument_with_matching_function("dict", func, args)?;
if let ExprKind::ListComp { elt, .. } = &argument {
match &elt.node {
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryListComprehensionDict,
Range::from_located(expr),
));
}
_ => {}
}
}
None
@@ -109,82 +117,38 @@ pub fn unnecessary_list_comprehension_dict(
/// Check `set([1, 2])` compliance.
pub fn unnecessary_literal_set(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "set" {
match &args[0].node {
ExprKind::List { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralSet("list".to_string()),
Range::from_located(expr),
));
}
ExprKind::Tuple { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralSet("tuple".to_string()),
Range::from_located(expr),
));
}
_ => {}
}
}
}
}
None
let argument = exactly_one_argument_with_matching_function("set", func, args)?;
let kind = match argument {
ExprKind::List { .. } => "list",
ExprKind::Tuple { .. } => "tuple",
_ => return None,
};
Some(Check::new(
CheckKind::UnnecessaryLiteralSet(kind.to_string()),
Range::from_located(expr),
))
}
/// Check `dict([(1, 2)])` compliance.
pub fn unnecessary_literal_dict(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if args.len() == 1 {
if let ExprKind::Name { id, .. } = &func.node {
if id == "dict" {
match &args[0].node {
ExprKind::Tuple { elts, .. } => {
if let Some(elt) = elts.first() {
match &elt.node {
// dict((1, 2), ...))
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralDict("tuple".to_string()),
Range::from_located(expr),
));
}
_ => {}
}
} else {
// dict(())
return Some(Check::new(
CheckKind::UnnecessaryLiteralDict("tuple".to_string()),
Range::from_located(expr),
));
}
}
ExprKind::List { elts, .. } => {
if let Some(elt) = elts.first() {
match &elt.node {
// dict([(1, 2), ...])
ExprKind::Tuple { elts, .. } if elts.len() == 2 => {
return Some(Check::new(
CheckKind::UnnecessaryLiteralDict("list".to_string()),
Range::from_located(expr),
));
}
_ => {}
}
} else {
// dict([])
return Some(Check::new(
CheckKind::UnnecessaryLiteralDict("list".to_string()),
Range::from_located(expr),
));
}
}
_ => {}
}
}
let argument = exactly_one_argument_with_matching_function("dict", func, args)?;
let (kind, elts) = match argument {
ExprKind::Tuple { elts, .. } => ("tuple", elts),
ExprKind::List { elts, .. } => ("list", elts),
_ => return None,
};
if let Some(elt) = elts.first() {
// dict((1, 2), ...)) or dict([(1, 2), ...])
if !matches!(&elt.node, ExprKind::Tuple { elts, .. } if elts.len() == 2) {
return None;
}
}
None
Some(Check::new(
CheckKind::UnnecessaryLiteralDict(kind.to_string()),
Range::from_located(expr),
))
}
pub fn unnecessary_collection_call(
@@ -193,26 +157,21 @@ pub fn unnecessary_collection_call(
args: &[Expr],
keywords: &[Located<KeywordData>],
) -> Option<Check> {
if args.is_empty() {
if let ExprKind::Name { id, .. } = &func.node {
if id == "list" || id == "tuple" {
// list() or tuple()
return Some(Check::new(
CheckKind::UnnecessaryCollectionCall(id.to_string()),
Range::from_located(expr),
));
} else if id == "dict" {
// dict() or dict(a=1)
if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) {
return Some(Check::new(
CheckKind::UnnecessaryCollectionCall(id.to_string()),
Range::from_located(expr),
));
}
}
}
if !args.is_empty() {
return None;
}
None
let id = function_name(func)?;
match id {
"list" | "tuple" => {
// list() or tuple()
}
"dict" if keywords.is_empty() || keywords.iter().all(|kw| kw.node.arg.is_some()) => (),
_ => return None,
};
Some(Check::new(
CheckKind::UnnecessaryCollectionCall(id.to_string()),
Range::from_located(expr),
))
}
pub fn unnecessary_literal_within_tuple_call(
@@ -220,29 +179,16 @@ pub fn unnecessary_literal_within_tuple_call(
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
let argument = first_argument_with_matching_function("tuple", func, args)?;
let argument_kind = match argument {
ExprKind::Tuple { .. } => "tuple",
ExprKind::List { .. } => "list",
_ => return None,
};
Some(Check::new(
CheckKind::UnnecessaryLiteralWithinTupleCall(argument_kind.to_string()),
Range::from_located(expr),
))
}
pub fn unnecessary_literal_within_list_call(
@@ -250,62 +196,40 @@ pub fn unnecessary_literal_within_list_call(
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
let argument = first_argument_with_matching_function("list", func, args)?;
let argument_kind = match argument {
ExprKind::Tuple { .. } => "tuple",
ExprKind::List { .. } => "list",
_ => return None,
};
Some(Check::new(
CheckKind::UnnecessaryLiteralWithinListCall(argument_kind.to_string()),
Range::from_located(expr),
))
}
pub fn unnecessary_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() {
if let ExprKind::ListComp { .. } = &arg.node {
return Some(Check::new(
CheckKind::UnnecessaryListCall,
Range::from_located(expr),
));
}
}
}
let argument = first_argument_with_matching_function("list", func, args)?;
if let ExprKind::ListComp { .. } = argument {
return Some(Check::new(
CheckKind::UnnecessaryListCall,
Range::from_located(expr),
));
}
None
}
pub fn unnecessary_call_around_sorted(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if let ExprKind::Name { id: outer, .. } = &func.node {
if outer == "list" || outer == "reversed" {
if let Some(arg) = args.first() {
if let ExprKind::Call { func, .. } = &arg.node {
if let ExprKind::Name { id: inner, .. } = &func.node {
if inner == "sorted" {
return Some(Check::new(
CheckKind::UnnecessaryCallAroundSorted(outer.to_string()),
Range::from_located(expr),
));
}
}
}
}
let outer = function_name(func)?;
if !(outer == "list" || outer == "reversed") {
return None;
}
if let ExprKind::Call { func, .. } = &args.first()?.node {
if function_name(func)? == "sorted" {
return Some(Check::new(
CheckKind::UnnecessaryCallAroundSorted(outer.to_string()),
Range::from_located(expr),
));
}
}
None
@@ -316,91 +240,65 @@ pub fn unnecessary_double_cast_or_process(
func: &Expr,
args: &[Expr],
) -> Option<Check> {
if let ExprKind::Name { id: outer, .. } = &func.node {
if outer == "list"
|| outer == "tuple"
|| outer == "set"
|| outer == "reversed"
|| outer == "sorted"
let outer = function_name(func)?;
if !["list", "tuple", "set", "reversed", "sorted"].contains(&outer) {
return None;
}
fn new_check(inner: &str, outer: &str, expr: &Expr) -> Check {
Check::new(
CheckKind::UnnecessaryDoubleCastOrProcess(inner.to_string(), outer.to_string()),
Range::from_located(expr),
)
}
if let ExprKind::Call { func, .. } = &args.first()?.node {
let inner = function_name(func)?;
// Ex) set(tuple(...))
if (outer == "set" || outer == "sorted")
&& (inner == "list" || inner == "tuple" || inner == "reversed" || inner == "sorted")
{
if let Some(arg) = args.first() {
if let ExprKind::Call { func, .. } = &arg.node {
if let ExprKind::Name { id: inner, .. } = &func.node {
// Ex) set(tuple(...))
if (outer == "set" || outer == "sorted")
&& (inner == "list"
|| inner == "tuple"
|| inner == "reversed"
|| inner == "sorted")
{
return Some(Check::new(
CheckKind::UnnecessaryDoubleCastOrProcess(
inner.to_string(),
outer.to_string(),
),
Range::from_located(expr),
));
}
return Some(new_check(inner, outer, expr));
}
// Ex) list(tuple(...))
if (outer == "list" || outer == "tuple")
&& (inner == "list" || inner == "tuple")
{
return Some(Check::new(
CheckKind::UnnecessaryDoubleCastOrProcess(
inner.to_string(),
outer.to_string(),
),
Range::from_located(expr),
));
}
// Ex) list(tuple(...))
if (outer == "list" || outer == "tuple") && (inner == "list" || inner == "tuple") {
return Some(new_check(inner, outer, expr));
}
// Ex) set(set(...))
if outer == "set" && inner == "set" {
return Some(Check::new(
CheckKind::UnnecessaryDoubleCastOrProcess(
inner.to_string(),
outer.to_string(),
),
Range::from_located(expr),
));
}
}
}
}
// Ex) set(set(...))
if outer == "set" && inner == "set" {
return Some(new_check(inner, outer, 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 {
if id == "set" || id == "sorted" || id == "reversed" {
if let ExprKind::Subscript { slice, .. } = &first_arg.node {
if let ExprKind::Slice { lower, upper, step } = &slice.node {
if lower.is_none() && upper.is_none() {
if let Some(step) = step {
if let ExprKind::UnaryOp {
op: Unaryop::USub,
operand,
} = &step.node
{
if let ExprKind::Constant {
value: Constant::Int(val),
..
} = &operand.node
{
if *val == BigInt::from(1) {
return Some(Check::new(
CheckKind::UnnecessarySubscriptReversal(
id.to_string(),
),
Range::from_located(expr),
));
}
}
}
let first_arg = args.first()?;
let id = function_name(func)?;
if !["set", "sorted", "reversed"].contains(&id) {
return None;
}
if let ExprKind::Subscript { slice, .. } = &first_arg.node {
if let ExprKind::Slice { lower, upper, step } = &slice.node {
if lower.is_none() && upper.is_none() {
if let Some(step) = step {
if let ExprKind::UnaryOp {
op: Unaryop::USub,
operand,
} = &step.node
{
if let ExprKind::Constant {
value: Constant::Int(val),
..
} = &operand.node
{
if *val == BigInt::from(1) {
return Some(Check::new(
CheckKind::UnnecessarySubscriptReversal(id.to_string()),
Range::from_located(expr),
));
}
}
}
@@ -416,90 +314,65 @@ pub fn unnecessary_comprehension(
elt: &Expr,
generators: &[Comprehension],
) -> Option<Check> {
if generators.len() == 1 {
let generator = &generators[0];
if generator.ifs.is_empty() && generator.is_async == 0 {
if let ExprKind::Name { id: elt_id, .. } = &elt.node {
if let ExprKind::Name { id: target_id, .. } = &generator.target.node {
if elt_id == target_id {
match &expr.node {
ExprKind::ListComp { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryComprehension("list".to_string()),
Range::from_located(expr),
))
}
ExprKind::SetComp { .. } => {
return Some(Check::new(
CheckKind::UnnecessaryComprehension("set".to_string()),
Range::from_located(expr),
))
}
_ => {}
};
}
}
}
}
if generators.len() != 1 {
return None;
}
None
let generator = &generators[0];
if !(generator.ifs.is_empty() && generator.is_async == 0) {
return None;
}
let elt_id = function_name(elt)?;
let target_id = function_name(&generator.target)?;
if elt_id != target_id {
return None;
}
let expr_kind = match &expr.node {
ExprKind::ListComp { .. } => "list",
ExprKind::SetComp { .. } => "set",
_ => return None,
};
Some(Check::new(
CheckKind::UnnecessaryComprehension(expr_kind.to_string()),
Range::from_located(expr),
))
}
pub fn unnecessary_map(expr: &Expr, func: &Expr, args: &[Expr]) -> Option<Check> {
if let ExprKind::Name { id, .. } = &func.node {
if id == "map" {
if args.len() == 2 {
if let ExprKind::Lambda { .. } = &args[0].node {
return Some(Check::new(
CheckKind::UnnecessaryMap("generator".to_string()),
Range::from_located(expr),
));
fn new_check(kind: &str, expr: &Expr) -> Check {
Check::new(
CheckKind::UnnecessaryMap(kind.to_string()),
Range::from_located(expr),
)
}
let id = function_name(func)?;
match id {
"map" => {
if args.len() == 2 && matches!(&args[0].node, ExprKind::Lambda { .. }) {
return Some(new_check("generator", expr));
}
}
"list" | "set" => {
if let ExprKind::Call { func, args, .. } = &args.first()?.node {
let argument = first_argument_with_matching_function("map", func, args)?;
if let ExprKind::Lambda { .. } = argument {
return Some(new_check(id, expr));
}
}
} else if id == "list" || id == "set" {
if let Some(arg) = args.first() {
if let ExprKind::Call { func, args, .. } = &arg.node {
if let ExprKind::Name { id: f, .. } = &func.node {
if f == "map" {
if let Some(arg) = args.first() {
if let ExprKind::Lambda { .. } = &arg.node {
return Some(Check::new(
CheckKind::UnnecessaryMap(id.to_string()),
Range::from_located(expr),
));
}
}
}
}
}
}
} else if id == "dict" {
}
"dict" => {
if args.len() == 1 {
if let ExprKind::Call { func, args, .. } = &args[0].node {
if let ExprKind::Name { id: f, .. } = &func.node {
if f == "map" {
if let Some(arg) = args.first() {
if let ExprKind::Lambda { body, .. } = &arg.node {
match &body.node {
ExprKind::Tuple { elts, .. }
| ExprKind::List { elts, .. }
if elts.len() == 2 =>
{
return Some(Check::new(
CheckKind::UnnecessaryMap(id.to_string()),
Range::from_located(expr),
))
}
_ => {}
}
}
}
let argument = first_argument_with_matching_function("map", func, args)?;
if let ExprKind::Lambda { body, .. } = &argument {
if matches!(&body.node, ExprKind::Tuple { elts, .. } | ExprKind::List { elts, .. } if elts.len() == 2)
{
return Some(new_check(id, expr));
}
}
}
}
}
_ => (),
}
None
}

View File

@@ -245,8 +245,10 @@ mod tests {
#[test_case(CheckCode::A001, Path::new("A001.py"); "A001")]
#[test_case(CheckCode::A002, Path::new("A002.py"); "A002")]
#[test_case(CheckCode::A003, Path::new("A003.py"); "A003")]
#[test_case(CheckCode::B002, Path::new("B002.py"); "B002")]
#[test_case(CheckCode::B011, Path::new("B011.py"); "B011")]
#[test_case(CheckCode::B014, Path::new("B014.py"); "B014")]
#[test_case(CheckCode::B017, Path::new("B017.py"); "B017")]
#[test_case(CheckCode::B025, Path::new("B025.py"); "B025")]
#[test_case(CheckCode::C400, Path::new("C400.py"); "C400")]
#[test_case(CheckCode::C401, Path::new("C401.py"); "C401")]

View File

@@ -114,12 +114,36 @@ pub fn dunder_function_name(func_def: &Stmt, scope: &Scope, name: &str) -> Optio
None
}
fn is_lower(s: &str) -> bool {
let mut cased = false;
for c in s.chars() {
if c.is_uppercase() {
return false;
} else if !cased && c.is_lowercase() {
cased = true;
}
}
cased
}
fn is_upper(s: &str) -> bool {
let mut cased = false;
for c in s.chars() {
if c.is_lowercase() {
return false;
} else if (!cased) && c.is_uppercase() {
cased = true;
}
}
cased
}
pub fn constant_imported_as_non_constant(
import_from: &Stmt,
name: &str,
asname: &str,
) -> Option<Check> {
if name.chars().all(|c| c.is_uppercase()) && !asname.chars().all(|c| c.is_uppercase()) {
if is_upper(name) && !is_upper(asname) {
return Some(Check::new(
CheckKind::ConstantImportedAsNonConstant(name.to_string(), asname.to_string()),
Range::from_located(import_from),
@@ -133,7 +157,7 @@ pub fn lowercase_imported_as_non_lowercase(
name: &str,
asname: &str,
) -> Option<Check> {
if name.chars().all(|c| c.is_lowercase()) && asname.to_lowercase() != asname {
if is_lower(name) && asname.to_lowercase() != asname {
return Some(Check::new(
CheckKind::LowercaseImportedAsNonLowercase(name.to_string(), asname.to_string()),
Range::from_located(import_from),
@@ -143,9 +167,8 @@ pub fn lowercase_imported_as_non_lowercase(
}
fn is_camelcase(name: &str) -> bool {
!name.chars().all(|c| c.is_uppercase()) && !name.chars().all(|c| c.is_lowercase())
!is_lower(name) && !is_upper(name) && !name.contains('_')
}
fn is_acronym(name: &str, asname: &str) -> bool {
name.chars().filter(|c| c.is_uppercase()).join("") == asname
}
@@ -155,7 +178,7 @@ pub fn camelcase_imported_as_lowercase(
name: &str,
asname: &str,
) -> Option<Check> {
if is_camelcase(name) && asname.chars().all(|c| c.is_lowercase()) {
if is_camelcase(name) && is_lower(asname) {
return Some(Check::new(
CheckKind::CamelcaseImportedAsLowercase(name.to_string(), asname.to_string()),
Range::from_located(import_from),
@@ -169,7 +192,7 @@ pub fn camelcase_imported_as_constant(
name: &str,
asname: &str,
) -> Option<Check> {
if is_camelcase(name) && asname.chars().all(|c| c.is_uppercase()) && !is_acronym(name, asname) {
if is_camelcase(name) && is_upper(asname) && !is_acronym(name, asname) {
return Some(Check::new(
CheckKind::CamelcaseImportedAsConstant(name.to_string(), asname.to_string()),
Range::from_located(import_from),
@@ -183,7 +206,7 @@ pub fn camelcase_imported_as_acronym(
name: &str,
asname: &str,
) -> Option<Check> {
if is_camelcase(name) && asname.chars().all(|c| c.is_uppercase()) && is_acronym(name, asname) {
if is_camelcase(name) && is_upper(asname) && is_acronym(name, asname) {
return Some(Check::new(
CheckKind::CamelcaseImportedAsAcronym(name.to_string(), asname.to_string()),
Range::from_located(import_from),
@@ -191,3 +214,48 @@ pub fn camelcase_imported_as_acronym(
}
None
}
#[cfg(test)]
mod tests {
use super::{is_acronym, is_camelcase, is_lower, is_upper};
#[test]
fn test_is_lower() -> () {
assert!(is_lower("abc"));
assert!(is_lower("a_b_c"));
assert!(is_lower("a2c"));
assert!(!is_lower("aBc"));
assert!(!is_lower("ABC"));
assert!(!is_lower(""));
assert!(!is_lower("_"));
}
#[test]
fn test_is_upper() -> () {
assert!(is_upper("ABC"));
assert!(is_upper("A_B_C"));
assert!(is_upper("A2C"));
assert!(!is_upper("aBc"));
assert!(!is_upper("abc"));
assert!(!is_upper(""));
assert!(!is_upper("_"));
}
#[test]
fn test_is_camelcase() -> () {
assert!(is_camelcase("Camel"));
assert!(is_camelcase("CamelCase"));
assert!(!is_camelcase("camel"));
assert!(!is_camelcase("camel_case"));
assert!(!is_camelcase("CAMEL"));
assert!(!is_camelcase("CAMEL_CASE"));
}
#[test]
fn test_is_acronym() -> () {
assert!(is_acronym("AB", "AB"));
assert!(is_acronym("AbcDef", "AD"));
assert!(!is_acronym("AbcDef", "Ad"));
assert!(!is_acronym("AbcDef", "AB"));
}
}

View File

@@ -0,0 +1,21 @@
---
source: src/linter.rs
expression: checks
---
- kind: UnaryPrefixIncrement
location:
row: 15
column: 9
end_location:
row: 15
column: 12
fix: ~
- kind: UnaryPrefixIncrement
location:
row: 20
column: 12
end_location:
row: 20
column: 15
fix: ~

View File

@@ -0,0 +1,13 @@
---
source: src/linter.rs
expression: checks
---
- kind: NoAssertRaisesException
location:
row: 22
column: 9
end_location:
row: 25
column: 5
fix: ~

View File

@@ -4,13 +4,35 @@ expression: checks
---
- kind:
ConstantImportedAsNonConstant:
- BAD
- bad
- CONST
- const
location:
row: 1
column: 1
end_location:
row: 1
column: 27
column: 26
fix: ~
- kind:
ConstantImportedAsNonConstant:
- CONSTANT
- constant
location:
row: 2
column: 1
end_location:
row: 2
column: 37
fix: ~
- kind:
ConstantImportedAsNonConstant:
- ANOTHER_CONSTANT
- another_constant
location:
row: 3
column: 1
end_location:
row: 3
column: 53
fix: ~

View File

@@ -4,13 +4,35 @@ expression: checks
---
- kind:
LowercaseImportedAsNonLowercase:
- bad
- Bad
- lowercase
- Lower
location:
row: 1
column: 1
end_location:
row: 1
column: 27
column: 31
fix: ~
- kind:
LowercaseImportedAsNonLowercase:
- lowercase
- Lowercase
location:
row: 2
column: 1
end_location:
row: 2
column: 39
fix: ~
- kind:
LowercaseImportedAsNonLowercase:
- another_lowercase
- AnotherLowercase
location:
row: 3
column: 1
end_location:
row: 3
column: 54
fix: ~

View File

@@ -4,13 +4,35 @@ expression: checks
---
- kind:
CamelcaseImportedAsLowercase:
- CamelCase
- camelcase
- Camel
- camel
location:
row: 1
column: 1
end_location:
row: 1
column: 26
fix: ~
- kind:
CamelcaseImportedAsLowercase:
- CamelCase
- camelcase
location:
row: 2
column: 1
end_location:
row: 2
column: 39
fix: ~
- kind:
CamelcaseImportedAsLowercase:
- AnotherCamelCase
- another_camelcase
location:
row: 3
column: 1
end_location:
row: 3
column: 54
fix: ~

View File

@@ -4,13 +4,35 @@ expression: checks
---
- kind:
CamelcaseImportedAsConstant:
- CamelCase
- CAMELCASE
- Camel
- CAMEL
location:
row: 1
column: 1
end_location:
row: 1
column: 26
fix: ~
- kind:
CamelcaseImportedAsConstant:
- CamelCase
- CAMELCASE
location:
row: 2
column: 1
end_location:
row: 2
column: 39
fix: ~
- kind:
CamelcaseImportedAsConstant:
- AnotherCamelCase
- ANOTHER_CAMELCASE
location:
row: 3
column: 1
end_location:
row: 3
column: 54
fix: ~

View File

@@ -4,13 +4,24 @@ expression: checks
---
- kind:
CamelcaseImportedAsAcronym:
- CamelCase
- CC
- CaMel
- CM
location:
row: 1
column: 1
end_location:
row: 1
column: 23
fix: ~
- kind:
CamelcaseImportedAsAcronym:
- CamelCase
- CC
location:
row: 2
column: 1
end_location:
row: 2
column: 32
fix: ~