Move Truthiness into ruff_python_ast (#4071)

This commit is contained in:
Jonathan Plasse 2023-04-24 06:54:31 +02:00 committed by GitHub
parent 407af6e0ae
commit df77595426
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 113 additions and 132 deletions

View file

@ -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(

View file

@ -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;
@ -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`.
@ -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| {
@ -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,
})
}
@ -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,
@ -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

View file

@ -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;
@ -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
@ -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)));
}
}

View file

@ -1,4 +1,3 @@
use num_traits::identities::Zero;
use ruff_python_ast::call_path::{collect_call_path, CallPath};
use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};
@ -6,8 +5,6 @@ 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 {
@ -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

View file

@ -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};
@ -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).
///
@ -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;
@ -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;