Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Implement E711 and E712

This commit is contained in:
Charlie Marsh 2022-09-05 23:19:05 -04:00
parent 5ffb9c08d5
commit 7318bf8b12
12 changed files with 265 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@ -1758,6 +1758,7 @@ dependencies = [
"fern",
"filetime",
"glob",
"itertools",
"log",
"notify",
"once_cell",

1
Cargo.toml
View File

@ -19,6 +19,7 @@ dirs = { version = "4.0.0" }
fern = { version = "0.6.1" }
filetime = { version = "0.2.17" }
glob = { version = "0.3.0" }
itertools = "0.10.3"
log = { version = "0.4.17" }
notify = { version = "4.0.17" }
once_cell = { version = "1.13.1" }

2
README.md
View File

@ -122,6 +122,8 @@ lint rules that are obviated by Black (e.g., stylistic rules).
| ---- | ----- | ------- |
| E402 | ModuleImportNotAtTopOfFile | Module level import not at top of file |
| E501 | LineTooLong | Line too long |
| E711 | NoneComparison | Comparison to `None` should be `cond is None` |
| E712 | TrueFalseComparison | Comparison to `True` should be `cond is True` |
| E731 | DoNotAssignLambda | Do not assign a lambda expression, use a def |
| E902 | IOError | No such file or directory: `...` |
| F401 | UnusedImport | `...` imported but unused |

8
examples/generate_rules_table.rs
View File

@ -1,21 +1,23 @@
/// Generate a Markdown-compatible table of supported lint rules.
use ruff::checks::CheckKind;
use ruff::checks::{CheckKind, RejectedCmpop};
fn main() {
let mut check_kinds: Vec<CheckKind> = vec![
CheckKind::AssertTuple,
CheckKind::DefaultExceptNotLast,
CheckKind::DoNotAssignLambda,
CheckKind::DuplicateArgumentName,
CheckKind::FStringMissingPlaceholders,
CheckKind::IfTuple,
CheckKind::IOError("...".to_string()),
CheckKind::IfTuple,
CheckKind::ImportStarUsage,
CheckKind::LineTooLong,
CheckKind::DoNotAssignLambda,
CheckKind::ModuleImportNotAtTopOfFile,
CheckKind::NoAssertEquals,
CheckKind::NoneComparison(RejectedCmpop::Eq),
CheckKind::RaiseNotImplemented,
CheckKind::ReturnOutsideFunction,
CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq),
CheckKind::UndefinedExport("...".to_string()),
CheckKind::UndefinedLocal("...".to_string()),
CheckKind::UndefinedName("...".to_string()),

11
resources/test/fixtures/E711.py vendored Normal file
View File

@ -0,0 +1,11 @@
if var == None:
pass
if None != var:
pass
if var is None:
pass
if None is not var:
pass

14
resources/test/fixtures/E712.py vendored Normal file
View File

@ -0,0 +1,14 @@
if var == True:
pass
if False != var:
pass
if var != False != True:
pass
if var is True:
pass
if False is not var:
pass

2
resources/test/fixtures/pyproject.toml vendored
View File

@ -4,6 +4,8 @@ exclude = ["excluded.py", "**/migrations"]
select = [
"E402",
"E501",
"E711",
"E712",
"E731",
"E902",
"F401",

105
src/check_ast.rs
View File

@ -1,8 +1,9 @@
use std::collections::BTreeSet;
use std::path::Path;
use itertools::izip;
use rustpython_parser::ast::{
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprContext, ExprKind,
Location, Stmt, StmtKind, Suite,
};
use rustpython_parser::parser;
@ -11,7 +12,7 @@ use crate::ast_ops::{
extract_all_names, Binding, BindingKind, Scope, ScopeKind, SourceCodeLocator,
};
use crate::builtins::{BUILTINS, MAGIC_GLOBALS};
use crate::checks::{Check, CheckCode, CheckKind, Fix};
use crate::checks::{Check, CheckCode, CheckKind, Fix, RejectedCmpop};
use crate::settings::Settings;
use crate::visitor::{walk_excepthandler, Visitor};
use crate::{autofix, fixer, visitor};
@ -522,6 +523,106 @@ impl Visitor for Checker<'_> {
}
self.in_f_string = true;
}
ExprKind::Compare {
left,
ops,
comparators,
} => {
let op = ops.first().unwrap();
let comparator = left;
// Check `left`.
if self.settings.select.contains(&CheckCode::E711)
&& matches!(
comparator.node,
ExprKind::Constant {
value: Constant::None,
kind: None
}
)
{
if matches!(op, Cmpop::Eq) {
self.checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
comparator.location,
));
}
if matches!(op, Cmpop::NotEq) {
self.checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
comparator.location,
));
}
}
if self.settings.select.contains(&CheckCode::E712) {
if let ExprKind::Constant {
value: Constant::Bool(value),
kind: None,
} = comparator.node
{
if matches!(op, Cmpop::Eq) {
self.checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
comparator.location,
));
}
if matches!(op, Cmpop::NotEq) {
self.checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
comparator.location,
));
}
}
}
// Check each comparator in order.
for (op, comparator) in izip!(ops, comparators) {
if self.settings.select.contains(&CheckCode::E711)
&& matches!(
comparator.node,
ExprKind::Constant {
value: Constant::None,
kind: None
}
)
{
if matches!(op, Cmpop::Eq) {
self.checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::Eq),
comparator.location,
));
}
if matches!(op, Cmpop::NotEq) {
self.checks.push(Check::new(
CheckKind::NoneComparison(RejectedCmpop::NotEq),
comparator.location,
));
}
}
if self.settings.select.contains(&CheckCode::E712) {
if let ExprKind::Constant {
value: Constant::Bool(value),
kind: None,
} = comparator.node
{
if matches!(op, Cmpop::Eq) {
self.checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::Eq),
comparator.location,
));
}
if matches!(op, Cmpop::NotEq) {
self.checks.push(Check::new(
CheckKind::TrueFalseComparison(value, RejectedCmpop::NotEq),
comparator.location,
));
}
}
}
}
}
ExprKind::Constant {
value: Constant::Str(value),
..

48
src/checks.rs
View File

@ -10,6 +10,8 @@ use serde::{Deserialize, Serialize};
pub enum CheckCode {
E402,
E501,
E711,
E712,
E731,
E902,
F401,
@ -37,6 +39,8 @@ impl FromStr for CheckCode {
match s {
"E402" => Ok(CheckCode::E402),
"E501" => Ok(CheckCode::E501),
"E711" => Ok(CheckCode::E711),
"E712" => Ok(CheckCode::E712),
"E731" => Ok(CheckCode::E731),
"E902" => Ok(CheckCode::E902),
"F401" => Ok(CheckCode::F401),
@ -65,6 +69,8 @@ impl CheckCode {
match self {
CheckCode::E402 => "E402",
CheckCode::E501 => "E501",
CheckCode::E711 => "E711",
CheckCode::E712 => "E712",
CheckCode::E731 => "E731",
CheckCode::E902 => "E902",
CheckCode::F401 => "F401",
@ -91,6 +97,8 @@ impl CheckCode {
match self {
CheckCode::E402 => &LintSource::AST,
CheckCode::E501 => &LintSource::Lines,
CheckCode::E711 => &LintSource::AST,
CheckCode::E712 => &LintSource::AST,
CheckCode::E731 => &LintSource::AST,
CheckCode::E902 => &LintSource::FileSystem,
CheckCode::F401 => &LintSource::AST,
@ -120,6 +128,12 @@ pub enum LintSource {
FileSystem,
}
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum RejectedCmpop {
Eq,
NotEq,
}
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum CheckKind {
AssertTuple,
@ -133,8 +147,10 @@ pub enum CheckKind {
DoNotAssignLambda,
ModuleImportNotAtTopOfFile,
NoAssertEquals,
NoneComparison(RejectedCmpop),
RaiseNotImplemented,
ReturnOutsideFunction,
TrueFalseComparison(bool, RejectedCmpop),
UndefinedExport(String),
UndefinedLocal(String),
UndefinedName(String),
@ -159,8 +175,10 @@ impl CheckKind {
CheckKind::DoNotAssignLambda => "DoNotAssignLambda",
CheckKind::ModuleImportNotAtTopOfFile => "ModuleImportNotAtTopOfFile",
CheckKind::NoAssertEquals => "NoAssertEquals",
CheckKind::NoneComparison(_) => "NoneComparison",
CheckKind::RaiseNotImplemented => "RaiseNotImplemented",
CheckKind::ReturnOutsideFunction => "ReturnOutsideFunction",
CheckKind::TrueFalseComparison(_, _) => "TrueFalseComparison",
CheckKind::UndefinedExport(_) => "UndefinedExport",
CheckKind::UndefinedLocal(_) => "UndefinedLocal",
CheckKind::UndefinedName(_) => "UndefinedName",
@ -185,8 +203,10 @@ impl CheckKind {
CheckKind::DoNotAssignLambda => &CheckCode::E731,
CheckKind::ModuleImportNotAtTopOfFile => &CheckCode::E402,
CheckKind::NoAssertEquals => &CheckCode::R002,
CheckKind::NoneComparison(_) => &CheckCode::E711,
CheckKind::RaiseNotImplemented => &CheckCode::F901,
CheckKind::ReturnOutsideFunction => &CheckCode::F706,
CheckKind::TrueFalseComparison(_, _) => &CheckCode::E712,
CheckKind::UndefinedExport(_) => &CheckCode::F822,
CheckKind::UndefinedLocal(_) => &CheckCode::F823,
CheckKind::UndefinedName(_) => &CheckCode::F821,
@ -227,12 +247,36 @@ impl CheckKind {
CheckKind::NoAssertEquals => {
"`assertEquals` is deprecated, use `assertEqual` instead".to_string()
}
CheckKind::NoneComparison(op) => match op {
RejectedCmpop::Eq => "Comparison to `None` should be `cond is None`".to_string(),
RejectedCmpop::NotEq => {
"Comparison to `None` should be `cond is not None`".to_string()
}
},
CheckKind::RaiseNotImplemented => {
"`raise NotImplemented` should be `raise NotImplementedError`".to_string()
}
CheckKind::ReturnOutsideFunction => {
"a `return` statement outside of a function/method".to_string()
}
CheckKind::TrueFalseComparison(value, op) => match *value {
true => match op {
RejectedCmpop::Eq => {
"Comparison to `True` should be `cond is True`".to_string()
}
RejectedCmpop::NotEq => {
"Comparison to `True` should be `cond is not True`".to_string()
}
},
false => match op {
RejectedCmpop::Eq => {
"Comparison to `False` should be `cond is False`".to_string()
}
RejectedCmpop::NotEq => {
"Comparison to `False` should be `cond is not False`".to_string()
}
},
},
CheckKind::UndefinedExport(name) => {
format!("Undefined name `{name}` in `__all__`")
}
@ -262,15 +306,17 @@ impl CheckKind {
CheckKind::DefaultExceptNotLast => false,
CheckKind::DuplicateArgumentName => false,
CheckKind::FStringMissingPlaceholders => false,
CheckKind::IfTuple => false,
CheckKind::IOError(_) => false,
CheckKind::IfTuple => false,
CheckKind::ImportStarUsage => false,
CheckKind::DoNotAssignLambda => false,
CheckKind::LineTooLong => false,
CheckKind::ModuleImportNotAtTopOfFile => false,
CheckKind::NoAssertEquals => true,
CheckKind::NoneComparison(_) => false,
CheckKind::RaiseNotImplemented => false,
CheckKind::ReturnOutsideFunction => false,
CheckKind::TrueFalseComparison(_, _) => false,
CheckKind::UndefinedExport(_) => false,
CheckKind::UndefinedLocal(_) => false,
CheckKind::UndefinedName(_) => false,

2
src/lib.rs
View File

@ -1,3 +1,5 @@
extern crate core;
mod ast_ops;
mod autofix;
mod builtins;

76
src/linter.rs
View File

@ -84,7 +84,7 @@ mod tests {
use anyhow::Result;
use rustpython_parser::ast::Location;
use crate::checks::{Check, CheckCode, CheckKind, Fix};
use crate::checks::{Check, CheckCode, CheckKind, Fix, RejectedCmpop};
use crate::linter::check_path;
use crate::{autofix, settings};
@ -136,6 +136,79 @@ mod tests {
Ok(())
}
#[test]
fn e711() -> Result<()> {
let actual = check_path(
Path::new("./resources/test/fixtures/E711.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::E711]),
},
&autofix::Mode::Generate,
)?;
let expected = vec![
Check {
kind: CheckKind::NoneComparison(RejectedCmpop::Eq),
location: Location::new(1, 11),
fix: None,
},
Check {
kind: CheckKind::NoneComparison(RejectedCmpop::NotEq),
location: Location::new(4, 4),
fix: None,
},
];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test]
fn e712() -> Result<()> {
let actual = check_path(
Path::new("./resources/test/fixtures/E712.py"),
&settings::Settings {
line_length: 88,
exclude: vec![],
select: BTreeSet::from([CheckCode::E712]),
},
&autofix::Mode::Generate,
)?;
let expected = vec![
Check {
kind: CheckKind::TrueFalseComparison(true, RejectedCmpop::Eq),
location: Location::new(1, 11),
fix: None,
},
Check {
kind: CheckKind::TrueFalseComparison(false, RejectedCmpop::NotEq),
location: Location::new(4, 4),
fix: None,
},
Check {
kind: CheckKind::TrueFalseComparison(false, RejectedCmpop::NotEq),
location: Location::new(7, 11),
fix: None,
},
Check {
kind: CheckKind::TrueFalseComparison(true, RejectedCmpop::NotEq),
location: Location::new(7, 20),
fix: None,
},
];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);
}
Ok(())
}
#[test]
fn e731() -> Result<()> {
let actual = check_path(
@ -152,6 +225,7 @@ mod tests {
location: Location::new(1, 1),
fix: None,
}];
assert_eq!(actual.len(), expected.len());
for i in 0..actual.len() {
assert_eq!(actual[i], expected[i]);

2
src/pyproject.rs
View File

@ -239,6 +239,8 @@ other-attribute = 1
select: Some(BTreeSet::from([
CheckCode::E402,
CheckCode::E501,
CheckCode::E711,
CheckCode::E712,
CheckCode::E731,
CheckCode::E902,
CheckCode::F401,