Skip to content

Commit

Permalink
DTZ rules: Clarify error messages and docs (#10621)
Browse files Browse the repository at this point in the history
- Clearly state in the documentation that passing `tz=None` is just as bad as not passing a `tz=` argument, from the perspective of these rules.
- Clearly state in the error messages exactly what the user is doing wrong, if the user is passing `tz=None` rather than failing to pass a `tz=` argument at all.
- Make error messages more concise, and separate out the suggested remedy from the thing that the user is identified as doing wrong.

Co-authored-by: Christian Clauss <cclauss@me.com>
  • Loading branch information
AlexWaygood and cclauss committed Mar 27, 2024
1 parent f9d0c6d commit abbefae
Show file tree
Hide file tree
Showing 19 changed files with 254 additions and 218 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::checkers::ast::Checker;
/// always use timezone-aware objects.
///
/// `datetime.date.fromtimestamp(ts)` returns a naive datetime object.
/// Instead, use `datetime.datetime.fromtimestamp(ts, tz=)` to return a
/// Instead, use `datetime.datetime.fromtimestamp(ts, tz=...)` to create a
/// timezone-aware object.
///
/// ## Example
Expand Down Expand Up @@ -50,10 +50,11 @@ pub struct CallDateFromtimestamp;
impl Violation for CallDateFromtimestamp {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"The use of `datetime.date.fromtimestamp()` is not allowed, use \
`datetime.datetime.fromtimestamp(ts, tz=).date()` instead"
)
format!("`datetime.date.fromtimestamp()` used")
}

fn fix_title(&self) -> Option<String> {
Some("Use `datetime.datetime.fromtimestamp(ts, tz=...).date()` instead".to_string())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::checkers::ast::Checker;
/// always use timezone-aware objects.
///
/// `datetime.date.today` returns a naive datetime object. Instead, use
/// `datetime.datetime.now(tz=).date()` to return a timezone-aware object.
/// `datetime.datetime.now(tz=...).date()` to create a timezone-aware object.
///
/// ## Example
/// ```python
Expand Down Expand Up @@ -49,10 +49,11 @@ pub struct CallDateToday;
impl Violation for CallDateToday {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"The use of `datetime.date.today()` is not allowed, use \
`datetime.datetime.now(tz=).date()` instead"
)
format!("`datetime.date.today()` used")
}

fn fix_title(&self) -> Option<String> {
Some("Use `datetime.datetime.now(tz=...).date()` instead".to_string())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,14 @@ use ruff_macros::{derive_message_formats, violation};

use ruff_python_ast::{self as ast};
use ruff_python_semantic::Modules;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_datetimez::rules::helpers::has_non_none_keyword;

use super::helpers;
use super::helpers::{self, DatetimeModuleAntipattern};

/// ## What it does
/// Checks for usage of `datetime.datetime.fromtimestamp()` without a `tz`
/// argument.
/// Checks for usage of `datetime.datetime.fromtimestamp()` that do not specify
/// a timezone.
///
/// ## Why is this bad?
/// Python datetime objects can be naive or timezone-aware. While an aware
Expand All @@ -21,9 +19,10 @@ use super::helpers;
/// datetime objects. Since this can lead to errors, it is recommended to
/// always use timezone-aware objects.
///
/// `datetime.datetime.fromtimestamp(ts)` returns a naive datetime object.
/// Instead, use `datetime.datetime.fromtimestamp(ts, tz=)` to return a
/// timezone-aware object.
/// `datetime.datetime.fromtimestamp(ts)` or
/// `datetime.datetime.fromtimestampe(ts, tz=None)` returns a naive datetime
/// object. Instead, use `datetime.datetime.fromtimestamp(ts, tz=<timezone>)`
/// to create a timezone-aware object.
///
/// ## Example
/// ```python
Expand All @@ -39,7 +38,7 @@ use super::helpers;
/// datetime.datetime.fromtimestamp(946684800, tz=datetime.timezone.utc)
/// ```
///
/// Or, for Python 3.11 and later:
/// Or, on Python 3.11 and later:
/// ```python
/// import datetime
///
Expand All @@ -49,14 +48,24 @@ use super::helpers;
/// ## References
/// - [Python documentation: Aware and Naive Objects](https://docs.python.org/3/library/datetime.html#aware-and-naive-objects)
#[violation]
pub struct CallDatetimeFromtimestamp;
pub struct CallDatetimeFromtimestamp(DatetimeModuleAntipattern);

impl Violation for CallDatetimeFromtimestamp {
#[derive_message_formats]
fn message(&self) -> String {
format!(
"The use of `datetime.datetime.fromtimestamp()` without `tz` argument is not allowed"
)
let CallDatetimeFromtimestamp(antipattern) = self;
match antipattern {
DatetimeModuleAntipattern::NoTzArgumentPassed => {
format!("`datetime.datetime.fromtimestamp()` called without a `tz` argument")
}
DatetimeModuleAntipattern::NonePassedToTzArgument => {
format!("`tz=None` passed to `datetime.datetime.fromtimestamp()`")
}
}
}

fn fix_title(&self) -> Option<String> {
Some("Pass a `datetime.timezone` object to the `tz` parameter".to_string())
}
}

Expand All @@ -82,26 +91,14 @@ pub(crate) fn call_datetime_fromtimestamp(checker: &mut Checker, call: &ast::Exp
return;
}

// no args / no args unqualified
if call.arguments.args.len() < 2 && call.arguments.keywords.is_empty() {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeFromtimestamp, call.range()));
return;
}

// none args
if call.arguments.args.len() > 1 && call.arguments.args[1].is_none_literal_expr() {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeFromtimestamp, call.range()));
return;
}
let antipattern = match call.arguments.find_argument("tz", 1) {
Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
Some(_) => return,
None => DatetimeModuleAntipattern::NoTzArgumentPassed,
};

// wrong keywords / none keyword
if !call.arguments.keywords.is_empty() && !has_non_none_keyword(&call.arguments, "tz") {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeFromtimestamp, call.range()));
}
checker.diagnostics.push(Diagnostic::new(
CallDatetimeFromtimestamp(antipattern),
call.range,
));
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};

use ruff_python_ast::{self as ast, Expr};
use ruff_python_ast as ast;
use ruff_python_semantic::Modules;
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
use crate::rules::flake8_datetimez::rules::helpers::has_non_none_keyword;

use super::helpers;
use super::helpers::{self, DatetimeModuleAntipattern};

/// ## What it does
/// Checks for usage of `datetime.datetime.now()` without a `tz` argument.
/// Checks for usages of `datetime.datetime.now()` that do not specify a timezone.
///
/// ## Why is this bad?
/// Python datetime objects can be naive or timezone-aware. While an aware
Expand All @@ -20,8 +18,9 @@ use super::helpers;
/// datetime objects. Since this can lead to errors, it is recommended to
/// always use timezone-aware objects.
///
/// `datetime.datetime.now()` returns a naive datetime object. Instead, use
/// `datetime.datetime.now(tz=)` to return a timezone-aware object.
/// `datetime.datetime.now()` or `datetime.datetime.now(tz=None)` returns a naive
/// datetime object. Instead, use `datetime.datetime.now(tz=<timezone>)` to create
/// a timezone-aware object.
///
/// ## Example
/// ```python
Expand All @@ -47,12 +46,24 @@ use super::helpers;
/// ## References
/// - [Python documentation: Aware and Naive Objects](https://docs.python.org/3/library/datetime.html#aware-and-naive-objects)
#[violation]
pub struct CallDatetimeNowWithoutTzinfo;
pub struct CallDatetimeNowWithoutTzinfo(DatetimeModuleAntipattern);

impl Violation for CallDatetimeNowWithoutTzinfo {
#[derive_message_formats]
fn message(&self) -> String {
format!("The use of `datetime.datetime.now()` without `tz` argument is not allowed")
let CallDatetimeNowWithoutTzinfo(antipattern) = self;
match antipattern {
DatetimeModuleAntipattern::NoTzArgumentPassed => {
format!("`datetime.datetime.now()` called without a `tz` argument")
}
DatetimeModuleAntipattern::NonePassedToTzArgument => {
format!("`tz=None` passed to `datetime.datetime.now()`")
}
}
}

fn fix_title(&self) -> Option<String> {
Some("Pass a `datetime.timezone` object to the `tz` parameter".to_string())
}
}

Expand All @@ -75,31 +86,14 @@ pub(crate) fn call_datetime_now_without_tzinfo(checker: &mut Checker, call: &ast
return;
}

// no args / no args unqualified
if call.arguments.args.is_empty() && call.arguments.keywords.is_empty() {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, call.range()));
return;
}
let antipattern = match call.arguments.find_argument("tz", 0) {
Some(ast::Expr::NoneLiteral(_)) => DatetimeModuleAntipattern::NonePassedToTzArgument,
Some(_) => return,
None => DatetimeModuleAntipattern::NoTzArgumentPassed,
};

// none args
if call
.arguments
.args
.first()
.is_some_and(Expr::is_none_literal_expr)
{
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, call.range()));
return;
}

// wrong keywords / none keyword
if !call.arguments.keywords.is_empty() && !has_non_none_keyword(&call.arguments, "tz") {
checker
.diagnostics
.push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, call.range()));
}
checker.diagnostics.push(Diagnostic::new(
CallDatetimeNowWithoutTzinfo(antipattern),
call.range,
));
}

0 comments on commit abbefae

Please sign in to comment.