Python/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff synced 2026-01-24 06:50:59 -05:00
Code Issues Packages Projects Releases Wiki Activity

Implement E711 and E712 (#110)

Browse Source
This commit is contained in:
Charlie Marsh
2022-09-06 10:14:36 -04:00
committed by GitHub
parent 5ffb9c08d5
commit e306fe0765
12 changed files with 265 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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,