From d4f86d715496e05f9c24c0ac6f06552467f910e5 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Tue, 12 Nov 2024 00:02:05 +0000 Subject: [PATCH] [`flake8-datetimez`] Usages of `datetime.max`/`datetime.min` (`DTZ901`) --- .../test/fixtures/flake8_datetimez/DTZ901.py | 30 +++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_datetimez/mod.rs | 15 +++ .../rules/flake8_datetimez/rules/helpers.rs | 6 +- .../src/rules/flake8_datetimez/rules/mod.rs | 2 + .../rules/use_datetime_max_min.rs | 113 ++++++++++++++++++ ...e8_datetimez__tests__DTZ901_DTZ901.py.snap | 71 +++++++++++ 8 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/flake8_datetimez/DTZ901.py create mode 100644 crates/ruff_linter/src/rules/flake8_datetimez/rules/use_datetime_max_min.rs create mode 100644 crates/ruff_linter/src/rules/flake8_datetimez/snapshots/ruff_linter__rules__flake8_datetimez__tests__DTZ901_DTZ901.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_datetimez/DTZ901.py b/crates/ruff_linter/resources/test/fixtures/flake8_datetimez/DTZ901.py new file mode 100644 index 0000000000000..704ab974023bd --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/flake8_datetimez/DTZ901.py @@ -0,0 +1,30 @@ +import datetime + + +# Error +datetime.datetime.max +datetime.datetime.min + +datetime.datetime.max.replace(year=...) +datetime.datetime.min.replace(hour=...) + + +# No error +datetime.datetime.max.replace(tzinfo=...) +datetime.datetime.min.replace(tzinfo=...) + + +from datetime import datetime + + +# Error +datetime.max +datetime.min + +datetime.max.replace(year=...) +datetime.min.replace(hour=...) + + +# No error +datetime.max.replace(tzinfo=...) +datetime.min.replace(tzinfo=...) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 42d50c40e8d3b..46da644efa3b5 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -339,6 +339,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::SixPY3) { flake8_2020::rules::name_or_attribute(checker, expr); } + if checker.enabled(Rule::UseDatetimeMaxMin) { + flake8_datetimez::rules::use_datetime_max_min(checker, expr); + } if checker.enabled(Rule::BannedApi) { flake8_tidy_imports::rules::banned_attribute_access(checker, expr); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 8e1e90fd34514..2d6bdb607deb5 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -706,6 +706,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Flake8Datetimez, "007") => (RuleGroup::Stable, rules::flake8_datetimez::rules::CallDatetimeStrptimeWithoutZone), (Flake8Datetimez, "011") => (RuleGroup::Stable, rules::flake8_datetimez::rules::CallDateToday), (Flake8Datetimez, "012") => (RuleGroup::Stable, rules::flake8_datetimez::rules::CallDateFromtimestamp), + (Flake8Datetimez, "901") => (RuleGroup::Preview, rules::flake8_datetimez::rules::UseDatetimeMaxMin), // pygrep-hooks (PygrepHooks, "001") => (RuleGroup::Removed, rules::pygrep_hooks::rules::Eval), diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/mod.rs b/crates/ruff_linter/src/rules/flake8_datetimez/mod.rs index f803eb3e3c638..c16f86747c40e 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_datetimez/mod.rs @@ -9,6 +9,7 @@ mod tests { use test_case::test_case; use crate::registry::Rule; + use crate::settings::types::PreviewMode; use crate::test::test_path; use crate::{assert_messages, settings}; @@ -30,4 +31,18 @@ mod tests { assert_messages!(snapshot, diagnostics); Ok(()) } + + #[test_case(Rule::UseDatetimeMaxMin, Path::new("DTZ901.py"))] + fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { + let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); + let diagnostics = test_path( + Path::new("flake8_datetimez").join(path).as_path(), + &settings::LinterSettings { + preview: PreviewMode::Enabled, + ..settings::LinterSettings::for_rule(rule_code) + }, + )?; + assert_messages!(snapshot, diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/helpers.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/helpers.rs index a580c9bc95e76..551c082b101b4 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_datetimez/rules/helpers.rs @@ -8,10 +8,10 @@ pub(super) enum DatetimeModuleAntipattern { NonePassedToTzArgument, } -/// Check if the parent expression is a call to `astimezone`. This assumes that -/// the current expression is a `datetime.datetime` object. +/// Check if the parent expression is a call to `astimezone`. +/// This assumes that the current expression is a `datetime.datetime` object. pub(super) fn parent_expr_is_astimezone(checker: &Checker) -> bool { - checker.semantic().current_expression_parent().is_some_and( |parent| { + checker.semantic().current_expression_parent().is_some_and(|parent| { matches!(parent, Expr::Attribute(ExprAttribute { attr, .. }) if attr.as_str() == "astimezone") }) } diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/mod.rs index 87646ca77565a..f5f26e0cd803b 100644 --- a/crates/ruff_linter/src/rules/flake8_datetimez/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_datetimez/rules/mod.rs @@ -7,6 +7,7 @@ pub(crate) use call_datetime_today::*; pub(crate) use call_datetime_utcfromtimestamp::*; pub(crate) use call_datetime_utcnow::*; pub(crate) use call_datetime_without_tzinfo::*; +pub(crate) use use_datetime_max_min::*; mod call_date_fromtimestamp; mod call_date_today; @@ -18,3 +19,4 @@ mod call_datetime_utcfromtimestamp; mod call_datetime_utcnow; mod call_datetime_without_tzinfo; mod helpers; +mod use_datetime_max_min; diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/rules/use_datetime_max_min.rs b/crates/ruff_linter/src/rules/flake8_datetimez/rules/use_datetime_max_min.rs new file mode 100644 index 0000000000000..1e4e0090fd043 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_datetimez/rules/use_datetime_max_min.rs @@ -0,0 +1,113 @@ +use crate::checkers::ast::Checker; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, ExprAttribute, ExprCall}; +use ruff_python_semantic::Modules; +use ruff_text_size::Ranged; +use std::fmt::{Display, Formatter}; + +#[derive(Debug, Eq, PartialEq)] +enum MaxMin { + /// `datetime.datetime.max` + Max, + /// `datetime.datetime.min` + Min, +} + +impl MaxMin { + fn from(attr: &str) -> Self { + match attr { + "max" => Self::Max, + "min" => Self::Min, + _ => panic!("Unexpected argument for MaxMin"), + } + } +} + +impl Display for MaxMin { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let name = match self { + MaxMin::Max => "max", + MaxMin::Min => "min", + }; + + write!(f, "{name}") + } +} + +/// ## What it does +/// Checks for usages of `datetime.datetime.max` and `datetime.datetime.min`. +/// +/// ## Why is this bad? +/// `datetime.max` and `datetime.min` are constants with no timezone information. +/// Therefore, operations on them might fail unexpectedly: +/// +/// ```python +/// # Timezone: UTC-14 +/// datetime.max.timestamp() # ValueError: year 10000 is out of range +/// datetime.min.timestamp() # ValueError: year 0 is out of range +/// ``` +/// +/// ## Example +/// ```python +/// datetime.max +/// ``` +/// +/// Use instead: +/// ```python +/// datetime.max.replace(tzinfo=datetime.UTC) +/// ``` +#[violation] +pub struct UseDatetimeMaxMin(MaxMin); + +impl Violation for UseDatetimeMaxMin { + #[derive_message_formats] + fn message(&self) -> String { + format!("`datetime.datetime.{}` used", self.0) + } +} + +/// DTZ901 +pub(crate) fn use_datetime_max_min(checker: &mut Checker, expr: &Expr) { + let semantic = checker.semantic(); + + if !semantic.seen_module(Modules::DATETIME) { + return; + } + + let Some(qualified_name) = semantic.resolve_qualified_name(expr) else { + return; + }; + + let maxmin = match qualified_name.segments() { + ["datetime", "datetime", attr @ ("max" | "min")] => MaxMin::from(attr), + _ => return, + }; + + if followed_by_replace_tzinfo(checker) { + return; + } + + let diagnostic = Diagnostic::new(UseDatetimeMaxMin(maxmin), expr.range()); + + checker.diagnostics.push(diagnostic); +} + +/// Check if the current expression has the pattern `foo.replace(tzinfo=bar)`. +pub(super) fn followed_by_replace_tzinfo(checker: &Checker) -> bool { + let semantic = checker.semantic(); + + let Some(parent) = semantic.current_expression_parent() else { + return false; + }; + let Some(grandparent) = semantic.current_expression_grandparent() else { + return false; + }; + + match (parent, grandparent) { + (Expr::Attribute(ExprAttribute { attr, .. }), Expr::Call(ExprCall { arguments, .. })) => { + attr.as_str() == "replace" && matches!(arguments.find_keyword("tzinfo"), Some(..)) + } + _ => false, + } +} diff --git a/crates/ruff_linter/src/rules/flake8_datetimez/snapshots/ruff_linter__rules__flake8_datetimez__tests__DTZ901_DTZ901.py.snap b/crates/ruff_linter/src/rules/flake8_datetimez/snapshots/ruff_linter__rules__flake8_datetimez__tests__DTZ901_DTZ901.py.snap new file mode 100644 index 0000000000000..1efea5e111aed --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_datetimez/snapshots/ruff_linter__rules__flake8_datetimez__tests__DTZ901_DTZ901.py.snap @@ -0,0 +1,71 @@ +--- +source: crates/ruff_linter/src/rules/flake8_datetimez/mod.rs +snapshot_kind: text +--- +DTZ901.py:5:1: DTZ901 `datetime.datetime.max` used + | +4 | # Error +5 | datetime.datetime.max + | ^^^^^^^^^^^^^^^^^^^^^ DTZ901 +6 | datetime.datetime.min + | + +DTZ901.py:6:1: DTZ901 `datetime.datetime.min` used + | +4 | # Error +5 | datetime.datetime.max +6 | datetime.datetime.min + | ^^^^^^^^^^^^^^^^^^^^^ DTZ901 +7 | +8 | datetime.datetime.max.replace(year=...) + | + +DTZ901.py:8:1: DTZ901 `datetime.datetime.max` used + | +6 | datetime.datetime.min +7 | +8 | datetime.datetime.max.replace(year=...) + | ^^^^^^^^^^^^^^^^^^^^^ DTZ901 +9 | datetime.datetime.min.replace(hour=...) + | + +DTZ901.py:9:1: DTZ901 `datetime.datetime.min` used + | +8 | datetime.datetime.max.replace(year=...) +9 | datetime.datetime.min.replace(hour=...) + | ^^^^^^^^^^^^^^^^^^^^^ DTZ901 + | + +DTZ901.py:21:1: DTZ901 `datetime.datetime.max` used + | +20 | # Error +21 | datetime.max + | ^^^^^^^^^^^^ DTZ901 +22 | datetime.min + | + +DTZ901.py:22:1: DTZ901 `datetime.datetime.min` used + | +20 | # Error +21 | datetime.max +22 | datetime.min + | ^^^^^^^^^^^^ DTZ901 +23 | +24 | datetime.max.replace(year=...) + | + +DTZ901.py:24:1: DTZ901 `datetime.datetime.max` used + | +22 | datetime.min +23 | +24 | datetime.max.replace(year=...) + | ^^^^^^^^^^^^ DTZ901 +25 | datetime.min.replace(hour=...) + | + +DTZ901.py:25:1: DTZ901 `datetime.datetime.min` used + | +24 | datetime.max.replace(year=...) +25 | datetime.min.replace(hour=...) + | ^^^^^^^^^^^^ DTZ901 + |