Inline AST helpers for new literal nodes (#8374)

A small refactor to inline the `is_const_none` now that there's a
dedicated `ExprNoneLiteral` node.
This commit is contained in:
Dhruv Manilawala 2023-10-31 16:36:54 +05:30 committed by GitHub
parent 982ae6ff08
commit 8977b6ae11
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 63 additions and 63 deletions

View file

@ -1,7 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast as ast;
use ruff_python_ast::helpers::is_const_none;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -64,7 +64,7 @@ pub(crate) fn request_without_timeout(checker: &mut Checker, call: &ast::ExprCal
})
{
if let Some(keyword) = call.arguments.find_keyword("timeout") {
if is_const_none(&keyword.value) {
if keyword.value.is_none_literal_expr() {
checker.diagnostics.push(Diagnostic::new(
RequestWithoutTimeout { implicit: false },
keyword.range(),

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast, Arguments, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
@ -81,14 +81,14 @@ fn is_infinite_iterator(arg: &Expr, semantic: &SemanticModel) -> bool {
}
// Ex) `itertools.repeat(1, None)`
if args.len() == 2 && is_const_none(&args[1]) {
if args.len() == 2 && args[1].is_none_literal_expr() {
return true;
}
// Ex) `iterools.repeat(1, times=None)`
for keyword in keywords {
if keyword.arg.as_ref().is_some_and(|name| name == "times") {
if is_const_none(&keyword.value) {
if keyword.value.is_none_literal_expr() {
return true;
}
}

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast};
use ruff_text_size::Ranged;
@ -86,7 +86,7 @@ pub(crate) fn call_datetime_fromtimestamp(checker: &mut Checker, call: &ast::Exp
}
// none args
if call.arguments.args.len() > 1 && is_const_none(&call.arguments.args[1]) {
if call.arguments.args.len() > 1 && call.arguments.args[1].is_none_literal_expr() {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeFromtimestamp, call.range()));

View file

@ -1,7 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast};
use ruff_python_ast::{self as ast, Expr};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -77,7 +77,12 @@ pub(crate) fn call_datetime_now_without_tzinfo(checker: &mut Checker, call: &ast
}
// none args
if call.arguments.args.first().is_some_and(is_const_none) {
if call
.arguments
.args
.first()
.is_some_and(Expr::is_none_literal_expr)
{
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, call.range()));

View file

@ -1,7 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast};
use ruff_python_ast::{self as ast, Expr};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -73,7 +73,12 @@ pub(crate) fn call_datetime_without_tzinfo(checker: &mut Checker, call: &ast::Ex
}
// Positional arg: is constant None.
if call.arguments.args.get(7).is_some_and(is_const_none) {
if call
.arguments
.args
.get(7)
.is_some_and(Expr::is_none_literal_expr)
{
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeWithoutTzinfo, call.range()));

View file

@ -1,4 +1,3 @@
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{Arguments, Expr, ExprAttribute};
use crate::checkers::ast::Checker;
@ -15,5 +14,5 @@ pub(super) fn parent_expr_is_astimezone(checker: &Checker) -> bool {
pub(super) fn has_non_none_keyword(arguments: &Arguments, keyword: &str) -> bool {
arguments
.find_keyword(keyword)
.is_some_and(|keyword| !is_const_none(&keyword.value))
.is_some_and(|keyword| !keyword.value.is_none_literal_expr())
}

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast};
use ruff_text_size::Ranged;
@ -86,7 +86,7 @@ pub(crate) fn print_call(checker: &mut Checker, call: &ast::ExprCall) {
// If the print call has a `file=` argument (that isn't `None`, `"sys.stdout"`,
// or `"sys.stderr"`), don't trigger T201.
if let Some(keyword) = call.arguments.find_keyword("file") {
if !is_const_none(&keyword.value) {
if !keyword.value.is_none_literal_expr() {
if checker.semantic().resolve_call_path(&keyword.value).map_or(
true,
|call_path| {

View file

@ -8,7 +8,7 @@ use smallvec::SmallVec;
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
@ -245,7 +245,7 @@ fn non_none_annotation_element<'a>(
// E.g., `typing.Union` or `typing.Optional`
if let Expr::Subscript(ExprSubscript { value, slice, .. }) = annotation {
if semantic.match_typing_expr(value, "Optional") {
return if is_const_none(slice) {
return if slice.is_none_literal_expr() {
None
} else {
Some(slice)
@ -264,7 +264,7 @@ fn non_none_annotation_element<'a>(
return None;
};
return match (is_const_none(left), is_const_none(right)) {
return match (left.is_none_literal_expr(), right.is_none_literal_expr()) {
(false, true) => Some(left),
(true, false) => Some(right),
(true, true) => None,
@ -280,11 +280,11 @@ fn non_none_annotation_element<'a>(
..
}) = annotation
{
if !is_const_none(left) {
if !left.is_none_literal_expr() {
return Some(left);
}
if !is_const_none(right) {
if !right.is_none_literal_expr() {
return Some(right);
}

View file

@ -6,7 +6,7 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
use ruff_diagnostics::{AlwaysFixableViolation, Violation};
use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::helpers::{is_const_false, is_const_true};
use ruff_python_ast::stmt_if::elif_else_range;
use ruff_python_ast::visitor::Visitor;
@ -341,7 +341,7 @@ fn unnecessary_return_none(checker: &mut Checker, stack: &Stack) {
let Some(expr) = stmt.value.as_deref() else {
continue;
};
if !is_const_none(expr) {
if !expr.is_none_literal_expr() {
continue;
}
let mut diagnostic = Diagnostic::new(UnnecessaryReturnNone, stmt.range);
@ -682,7 +682,7 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex
} else {
if checker.enabled(Rule::UnnecessaryReturnNone) {
// Skip functions that have a return annotation that is not `None`.
if returns.map_or(true, is_const_none) {
if returns.map_or(true, Expr::is_none_literal_expr) {
unnecessary_return_none(checker, &stack);
}
}

View file

@ -4,7 +4,6 @@ use ruff_text_size::Ranged;
use crate::fix::snippet::SourceCodeSnippet;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use crate::checkers::ast::Checker;
@ -260,7 +259,7 @@ pub(crate) fn dict_get_with_none_default(checker: &mut Checker, expr: &Expr) {
let Some(default) = args.get(1) else {
return;
};
if !is_const_none(default) {
if !default.is_none_literal_expr() {
return;
}

View file

@ -3,7 +3,7 @@ use rustc_hash::FxHashMap;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers;
use ruff_python_ast::helpers::{generate_comparison, is_const_none};
use ruff_python_ast::helpers::generate_comparison;
use ruff_python_ast::{self as ast, CmpOp, Expr};
use ruff_text_size::Ranged;
@ -148,7 +148,7 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
if !helpers::is_constant_non_singleton(next) {
if let Some(op) = EqCmpOp::try_from(*op) {
if checker.enabled(Rule::NoneComparison) && is_const_none(comparator) {
if checker.enabled(Rule::NoneComparison) && comparator.is_none_literal_expr() {
match op {
EqCmpOp::Eq => {
let diagnostic = Diagnostic::new(NoneComparison(op), comparator.range());
@ -204,7 +204,7 @@ pub(crate) fn literal_comparisons(checker: &mut Checker, compare: &ast::ExprComp
continue;
};
if checker.enabled(Rule::NoneComparison) && is_const_none(next) {
if checker.enabled(Rule::NoneComparison) && next.is_none_literal_expr() {
match op {
EqCmpOp::Eq => {
let diagnostic = Diagnostic::new(NoneComparison(op), next.range());

View file

@ -1,7 +1,7 @@
use itertools::Itertools;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast, CmpOp, Expr};
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
@ -99,7 +99,7 @@ fn deprecated_type_comparison(checker: &mut Checker, compare: &ast::ExprCompare)
if arguments
.args
.first()
.is_some_and(|arg| !arg.is_name_expr() && !is_const_none(arg))
.is_some_and(|arg| !arg.is_name_expr() && !arg.is_none_literal_expr())
{
checker.diagnostics.push(Diagnostic::new(
TypeComparison {
@ -191,7 +191,7 @@ fn is_type(expr: &Expr, semantic: &SemanticModel) -> bool {
arguments
.args
.first()
.is_some_and(|arg| !arg.is_name_expr() && !is_const_none(arg))
.is_some_and(|arg| !arg.is_name_expr() && !arg.is_none_literal_expr())
}
Expr::Name(ast::ExprName { id, .. }) => {
// Ex) `type(obj) == int`

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_text_size::Ranged;
@ -80,7 +80,7 @@ fn has_eq_without_hash(body: &[Stmt]) -> bool {
//
// __hash__ = None
// ```
if id == "__hash__" && is_const_none(value) {
if id == "__hash__" && value.is_none_literal_expr() {
has_hash = true;
}
}

View file

@ -2,7 +2,7 @@ use ruff_python_ast::{self as ast, Stmt};
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -48,7 +48,7 @@ impl Violation for ReturnInInit {
pub(crate) fn return_in_init(checker: &mut Checker, stmt: &Stmt) {
if let Stmt::Return(ast::StmtReturn { value, range: _ }) = stmt {
if let Some(expr) = value {
if is_const_none(expr) {
if expr.is_none_literal_expr() {
// Explicit `return None`.
return;
}

View file

@ -1,6 +1,6 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast};
use ruff_text_size::Ranged;
@ -58,7 +58,7 @@ pub(crate) fn subprocess_popen_preexec_fn(checker: &mut Checker, call: &ast::Exp
if let Some(keyword) = call
.arguments
.find_keyword("preexec_fn")
.filter(|keyword| !is_const_none(&keyword.value))
.filter(|keyword| !keyword.value.is_none_literal_expr())
{
checker
.diagnostics

View file

@ -2,7 +2,7 @@ use ruff_python_ast::{self as ast, Expr, Stmt};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::{is_const_none, ReturnStatementVisitor};
use ruff_python_ast::helpers::ReturnStatementVisitor;
use ruff_python_ast::statement_visitor::StatementVisitor;
use ruff_text_size::Ranged;
@ -51,7 +51,7 @@ pub(crate) fn useless_return(
returns: Option<&Expr>,
) {
// Skip functions that have a return annotation that is not `None`.
if !returns.map_or(true, is_const_none) {
if !returns.map_or(true, Expr::is_none_literal_expr) {
return;
}
@ -84,7 +84,10 @@ pub(crate) fn useless_return(
};
// Verify that the return statement is either bare or returns `None`.
if !value.as_ref().map_or(true, |expr| is_const_none(expr)) {
if !value
.as_ref()
.map_or(true, |expr| expr.is_none_literal_expr())
{
return;
};

View file

@ -3,7 +3,6 @@ use ruff_text_size::{Ranged, TextRange};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use crate::checkers::ast::Checker;
use crate::importer::ImportRequest;
@ -85,7 +84,7 @@ pub(crate) fn lru_cache_with_maxsize_none(checker: &mut Checker, decorator_list:
value,
range: _,
} = &keywords[0];
if arg.as_ref().is_some_and(|arg| arg == "maxsize") && is_const_none(value) {
if arg.as_ref().is_some_and(|arg| arg == "maxsize") && value.is_none_literal_expr() {
let mut diagnostic = Diagnostic::new(
LRUCacheWithMaxsizeNone,
TextRange::new(func.end(), decorator.end()),

View file

@ -4,7 +4,7 @@ use anyhow::Result;
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::is_const_none;
use ruff_python_ast::{self as ast, Expr, Operator, ParameterWithDefault, Parameters};
use ruff_python_parser::typing::parse_type_annotation;
use ruff_text_size::{Ranged, TextRange};
@ -174,7 +174,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, parameters: &Parameters)
.chain(&parameters.kwonlyargs)
{
let Some(default) = default else { continue };
if !is_const_none(default) {
if !default.is_none_literal_expr() {
continue;
}
let Some(annotation) = &parameter.annotation else {

View file

@ -576,17 +576,12 @@ pub const fn is_singleton(expr: &Expr) -> bool {
)
}
/// Return `true` if the [`Expr`] is a constant or tuple of constants.
/// Return `true` if the [`Expr`] is a literal or tuple of literals.
pub fn is_constant(expr: &Expr) -> bool {
match expr {
Expr::StringLiteral(_)
| Expr::BytesLiteral(_)
| Expr::NumberLiteral(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| Expr::EllipsisLiteral(_) => true,
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(is_constant),
_ => false,
if let Expr::Tuple(ast::ExprTuple { elts, .. }) = expr {
elts.iter().all(is_constant)
} else {
expr.is_literal_expr()
}
}
@ -595,12 +590,7 @@ pub fn is_constant_non_singleton(expr: &Expr) -> bool {
is_constant(expr) && !is_singleton(expr)
}
/// Return `true` if an [`Expr`] is `None`.
pub const fn is_const_none(expr: &Expr) -> bool {
expr.is_none_literal_expr()
}
/// Return `true` if an [`Expr`] is `True`.
/// Return `true` if an [`Expr`] is a literal `True`.
pub const fn is_const_true(expr: &Expr) -> bool {
matches!(
expr,
@ -608,7 +598,7 @@ pub const fn is_const_true(expr: &Expr) -> bool {
)
}
/// Return `true` if an [`Expr`] is `False`.
/// Return `true` if an [`Expr`] is a literal `False`.
pub const fn is_const_false(expr: &Expr) -> bool {
matches!(
expr,