Skip to content

Commit

Permalink
Move Truthiness into ruff_python_ast (#4071)
Browse files Browse the repository at this point in the history
  • Loading branch information
JonathanPlasse authored Apr 24, 2023
1 parent 407af6e commit df77595
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 132 deletions.
4 changes: 1 addition & 3 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1615,9 +1615,7 @@ where
flake8_bugbear::rules::assert_false(self, stmt, test, msg.as_deref());
}
if self.settings.rules.enabled(Rule::PytestAssertAlwaysFalse) {
if let Some(diagnostic) = flake8_pytest_style::rules::assert_falsy(stmt, test) {
self.diagnostics.push(diagnostic);
}
flake8_pytest_style::rules::assert_falsy(self, stmt, test);
}
if self.settings.rules.enabled(Rule::PytestCompositeAssertion) {
flake8_pytest_style::rules::composite_condition(
Expand Down
78 changes: 5 additions & 73 deletions crates/ruff/src/rules/flake8_bandit/rules/shell_injection.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! Checks relating to shell injection.
use num_bigint::BigInt;
use once_cell::sync::Lazy;
use regex::Regex;
use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::Truthiness;
use ruff_python_ast::types::Range;
use ruff_python_semantic::context::Context;

Expand Down Expand Up @@ -130,74 +130,6 @@ fn get_call_kind(func: &Expr, context: &Context) -> Option<CallKind> {
})
}

#[derive(Copy, Clone, Debug)]
enum Truthiness {
// The `shell` keyword argument is set and evaluates to `False`.
Falsey,
// The `shell` keyword argument is set and evaluates to `True`.
Truthy,
// The `shell` keyword argument is set, but its value is unknown.
Unknown,
}

impl From<&Keyword> for Truthiness {
fn from(value: &Keyword) -> Self {
match &value.node.value.node {
ExprKind::Constant {
value: Constant::Bool(b),
..
} => {
if *b {
Truthiness::Truthy
} else {
Truthiness::Falsey
}
}
ExprKind::Constant {
value: Constant::Int(int),
..
} => {
if int == &BigInt::from(0u8) {
Truthiness::Falsey
} else {
Truthiness::Truthy
}
}
ExprKind::Constant {
value: Constant::Float(float),
..
} => {
if (float - 0.0).abs() < f64::EPSILON {
Truthiness::Falsey
} else {
Truthiness::Truthy
}
}
ExprKind::Constant {
value: Constant::None,
..
} => Truthiness::Falsey,
ExprKind::List { elts, .. }
| ExprKind::Set { elts, .. }
| ExprKind::Tuple { elts, .. } => {
if elts.is_empty() {
Truthiness::Falsey
} else {
Truthiness::Truthy
}
}
ExprKind::Dict { keys, .. } => {
if keys.is_empty() {
Truthiness::Falsey
} else {
Truthiness::Truthy
}
}
_ => Truthiness::Unknown,
}
}
}

#[derive(Copy, Clone, Debug)]
struct ShellKeyword<'a> {
/// Whether the `shell` keyword argument is set and evaluates to `True`.
Expand All @@ -207,7 +139,7 @@ struct ShellKeyword<'a> {
}

/// Return the `shell` keyword argument to the given function call, if any.
fn find_shell_keyword(keywords: &[Keyword]) -> Option<ShellKeyword> {
fn find_shell_keyword<'a>(ctx: &Context, keywords: &'a [Keyword]) -> Option<ShellKeyword<'a>> {
keywords
.iter()
.find(|keyword| {
Expand All @@ -218,7 +150,7 @@ fn find_shell_keyword(keywords: &[Keyword]) -> Option<ShellKeyword> {
.map_or(false, |arg| arg == "shell")
})
.map(|keyword| ShellKeyword {
truthiness: keyword.into(),
truthiness: Truthiness::from_expr(&keyword.node.value, |id| ctx.is_builtin(id)),
keyword,
})
}
Expand Down Expand Up @@ -255,7 +187,7 @@ pub fn shell_injection(checker: &mut Checker, func: &Expr, args: &[Expr], keywor

if matches!(call_kind, Some(CallKind::Subprocess)) {
if let Some(arg) = args.first() {
match find_shell_keyword(keywords) {
match find_shell_keyword(&checker.ctx, keywords) {
// S602
Some(ShellKeyword {
truthiness: Truthiness::Truthy,
Expand Down Expand Up @@ -308,7 +240,7 @@ pub fn shell_injection(checker: &mut Checker, func: &Expr, args: &[Expr], keywor
} else if let Some(ShellKeyword {
truthiness: Truthiness::Truthy,
keyword,
}) = find_shell_keyword(keywords)
}) = find_shell_keyword(&checker.ctx, keywords)
{
// S604
if checker
Expand Down
13 changes: 6 additions & 7 deletions crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustpython_parser::ast::{

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{has_comments_in, unparse_stmt};
use ruff_python_ast::helpers::{has_comments_in, unparse_stmt, Truthiness};
use ruff_python_ast::source_code::{Locator, Stylist};
use ruff_python_ast::types::Range;
use ruff_python_ast::visitor::Visitor;
Expand All @@ -22,7 +22,6 @@ use crate::checkers::ast::Checker;
use crate::cst::matchers::match_module;
use crate::registry::AsRule;

use super::helpers::is_falsy_constant;
use super::unittest_assert::UnittestAssert;

/// ## What it does
Expand Down Expand Up @@ -223,11 +222,11 @@ pub fn unittest_assertion(
}

/// PT015
pub fn assert_falsy(stmt: &Stmt, test: &Expr) -> Option<Diagnostic> {
if is_falsy_constant(test) {
Some(Diagnostic::new(PytestAssertAlwaysFalse, Range::from(stmt)))
} else {
None
pub fn assert_falsy(checker: &mut Checker, stmt: &Stmt, test: &Expr) {
if Truthiness::from_expr(test, |id| checker.ctx.is_builtin(id)).is_falsey() {
checker
.diagnostics
.push(Diagnostic::new(PytestAssertAlwaysFalse, Range::from(stmt)));
}
}

Expand Down
45 changes: 0 additions & 45 deletions crates/ruff/src/rules/flake8_pytest_style/rules/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use num_traits::identities::Zero;
use ruff_python_ast::call_path::{collect_call_path, CallPath};
use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};

use ruff_python_ast::helpers::map_callable;

use crate::checkers::ast::Checker;

const ITERABLE_INITIALIZERS: &[&str] = &["dict", "frozenset", "list", "tuple", "set"];

pub(super) fn get_mark_decorators(decorators: &[Expr]) -> impl Iterator<Item = (&Expr, CallPath)> {
decorators.iter().filter_map(|decorator| {
let Some(call_path) = collect_call_path(map_callable(decorator)) else {
Expand Down Expand Up @@ -61,48 +58,6 @@ pub(super) fn is_abstractmethod_decorator(decorator: &Expr, checker: &Checker) -
})
}

/// Check if the expression is a constant that evaluates to false.
pub(super) fn is_falsy_constant(expr: &Expr) -> bool {
match &expr.node {
ExprKind::Constant { value, .. } => match value {
Constant::Bool(value) => !*value,
Constant::None => true,
Constant::Str(string) => string.is_empty(),
Constant::Bytes(bytes) => bytes.is_empty(),
Constant::Int(int) => int.is_zero(),
Constant::Float(float) => *float == 0.0,
Constant::Complex { real, imag } => *real == 0.0 && *imag == 0.0,
Constant::Ellipsis => true,
Constant::Tuple(elts) => elts.is_empty(),
},
ExprKind::JoinedStr { values, .. } => values.is_empty(),
ExprKind::Tuple { elts, .. } | ExprKind::List { elts, .. } => elts.is_empty(),
ExprKind::Dict { keys, .. } => keys.is_empty(),
ExprKind::Call {
func,
args,
keywords,
} => {
if let ExprKind::Name { id, .. } = &func.node {
if ITERABLE_INITIALIZERS.contains(&id.as_str()) {
if args.is_empty() && keywords.is_empty() {
return true;
} else if !keywords.is_empty() {
return false;
} else if let Some(arg) = args.get(0) {
return is_falsy_constant(arg);
}
return false;
}
false
} else {
false
}
}
_ => false,
}
}

pub(super) fn is_pytest_parametrize(decorator: &Expr, checker: &Checker) -> bool {
checker
.ctx
Expand Down
105 changes: 101 additions & 4 deletions crates/ruff_python_ast/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::Path;

use itertools::Itertools;
use log::error;
use num_traits::Zero;
use once_cell::sync::Lazy;
use regex::Regex;
use rustc_hash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -50,6 +51,13 @@ pub fn unparse_constant(constant: &Constant, stylist: &Stylist) -> String {
generator.generate()
}

fn is_iterable_initializer<F>(id: &str, is_builtin: F) -> bool
where
F: Fn(&str) -> bool,
{
matches!(id, "list" | "tuple" | "set" | "dict" | "frozenset") && is_builtin(id)
}

/// Return `true` if the `Expr` contains an expression that appears to include a
/// side-effect (like a function call).
///
Expand All @@ -68,10 +76,7 @@ where
{
if args.is_empty() && keywords.is_empty() {
if let ExprKind::Name { id, .. } = &func.node {
if !matches!(id.as_str(), "set" | "list" | "tuple" | "dict" | "frozenset") {
return true;
}
if !is_builtin(id) {
if !is_iterable_initializer(id.as_str(), |id| is_builtin(id)) {
return true;
}
return false;
Expand Down Expand Up @@ -1422,6 +1427,98 @@ pub fn locate_cmpops(contents: &str) -> Vec<LocatedCmpop> {
ops
}

#[derive(Copy, Clone, Debug, PartialEq, is_macro::Is)]
pub enum Truthiness {
// An expression evaluates to `False`.
Falsey,
// An expression evaluates to `True`.
Truthy,
// An expression evaluates to an unknown value (e.g., a variable `x` of unknown type).
Unknown,
}

impl From<Option<bool>> for Truthiness {
fn from(value: Option<bool>) -> Self {
match value {
Some(true) => Truthiness::Truthy,
Some(false) => Truthiness::Falsey,
None => Truthiness::Unknown,
}
}
}

impl From<Truthiness> for Option<bool> {
fn from(truthiness: Truthiness) -> Self {
match truthiness {
Truthiness::Truthy => Some(true),
Truthiness::Falsey => Some(false),
Truthiness::Unknown => None,
}
}
}

impl Truthiness {
pub fn from_expr<F>(expr: &Expr, is_builtin: F) -> Self
where
F: Fn(&str) -> bool,
{
match &expr.node {
ExprKind::Constant { value, .. } => match value {
Constant::Bool(value) => Some(*value),
Constant::None => Some(false),
Constant::Str(string) => Some(!string.is_empty()),
Constant::Bytes(bytes) => Some(!bytes.is_empty()),
Constant::Int(int) => Some(!int.is_zero()),
Constant::Float(float) => Some(*float != 0.0),
Constant::Complex { real, imag } => Some(*real != 0.0 || *imag != 0.0),
Constant::Ellipsis => Some(true),
Constant::Tuple(elts) => Some(!elts.is_empty()),
},
ExprKind::JoinedStr { values, .. } => {
if values.is_empty() {
Some(false)
} else if values.iter().any(|value| {
let ExprKind::Constant { value: Constant::Str(string), .. } = &value.node else {
return false;
};
!string.is_empty()
}) {
Some(true)
} else {
None
}
}
ExprKind::List { elts, .. }
| ExprKind::Set { elts, .. }
| ExprKind::Tuple { elts, .. } => Some(!elts.is_empty()),
ExprKind::Dict { keys, .. } => Some(!keys.is_empty()),
ExprKind::Call {
func,
args,
keywords,
} => {
if let ExprKind::Name { id, .. } = &func.node {
if is_iterable_initializer(id.as_str(), |id| is_builtin(id)) {
if args.is_empty() && keywords.is_empty() {
Some(false)
} else if args.len() == 1 && keywords.is_empty() {
Self::from_expr(&args[0], is_builtin).into()
} else {
None
}
} else {
None
}
} else {
None
}
}
_ => None,
}
.into()
}
}

#[cfg(test)]
mod tests {
use std::borrow::Cow;
Expand Down

0 comments on commit df77595

Please sign in to comment.