mirror of
https://github.com/astral-sh/ruff.git
synced 2025-07-24 05:26:23 +00:00
[red-knot] Disallow todo_type! without custom message (#17083)
## Summary Disallow empty `todo_type!()`s without a custom message. They can lead to spurious diffs in `mypy_primer` where the only thing that's changed is the file/line information.
This commit is contained in:
David Peter
GitHub
parent
fa80e10aac
commit
d9616e61b0
3 changed files with 24 additions and 53 deletions
|
|
@ -175,18 +175,12 @@ impl AttributeKind {
|
|||
/// Meta data for `Type::Todo`, which represents a known limitation in red-knot.
|
||||
#[cfg(debug_assertions)]
|
||||
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
|
||||
pub enum TodoType {
|
||||
FileAndLine(&'static str, u32),
|
||||
Message(&'static str),
|
||||
}
|
||||
pub struct TodoType(pub &'static str);
|
||||
|
||||
#[cfg(debug_assertions)]
|
||||
impl std::fmt::Display for TodoType {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
TodoType::FileAndLine(file, line) => write!(f, "[{file}:{line}]"),
|
||||
TodoType::Message(msg) => write!(f, "({msg})"),
|
||||
}
|
||||
write!(f, "({msg})", msg = self.0)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -203,24 +197,18 @@ impl std::fmt::Display for TodoType {
|
|||
|
||||
/// Create a `Type::Todo` variant to represent a known limitation in the type system.
|
||||
///
|
||||
/// It can be used with a custom message (preferred): `todo_type!("PEP 604 not supported")`,
|
||||
/// or simply using `todo_type!()`, which will include information about the file and line.
|
||||
/// It can be created by specifying a custom message: `todo_type!("PEP 604 not supported")`.
|
||||
#[cfg(debug_assertions)]
|
||||
macro_rules! todo_type {
|
||||
() => {
|
||||
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(
|
||||
$crate::types::TodoType::FileAndLine(file!(), line!()),
|
||||
))
|
||||
};
|
||||
($message:literal) => {
|
||||
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(
|
||||
$crate::types::TodoType::Message($message),
|
||||
))
|
||||
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo($crate::types::TodoType(
|
||||
$message,
|
||||
)))
|
||||
};
|
||||
($message:ident) => {
|
||||
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(
|
||||
$crate::types::TodoType::Message($message),
|
||||
))
|
||||
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo($crate::types::TodoType(
|
||||
$message,
|
||||
)))
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -5625,16 +5613,12 @@ pub(crate) mod tests {
|
|||
|
||||
let todo1 = todo_type!("1");
|
||||
let todo2 = todo_type!("2");
|
||||
let todo3 = todo_type!();
|
||||
let todo4 = todo_type!();
|
||||
|
||||
let int = KnownClass::Int.to_instance(&db);
|
||||
|
||||
assert!(int.is_assignable_to(&db, todo1));
|
||||
assert!(int.is_assignable_to(&db, todo3));
|
||||
|
||||
assert!(todo1.is_assignable_to(&db, int));
|
||||
assert!(todo3.is_assignable_to(&db, int));
|
||||
|
||||
// We lose information when combining several `Todo` types. This is an
|
||||
// acknowledged limitation of the current implementation. We can not
|
||||
|
|
@ -5646,21 +5630,17 @@ pub(crate) mod tests {
|
|||
// salsa, but that would mean we would have to pass in `db` everywhere.
|
||||
|
||||
// A union of several `Todo` types collapses to a single `Todo` type:
|
||||
assert!(UnionType::from_elements(&db, vec![todo1, todo2, todo3, todo4]).is_todo());
|
||||
assert!(UnionType::from_elements(&db, vec![todo1, todo2]).is_todo());
|
||||
|
||||
// And similar for intersection types:
|
||||
assert!(IntersectionBuilder::new(&db)
|
||||
.add_positive(todo1)
|
||||
.add_positive(todo2)
|
||||
.add_positive(todo3)
|
||||
.add_positive(todo4)
|
||||
.build()
|
||||
.is_todo());
|
||||
assert!(IntersectionBuilder::new(&db)
|
||||
.add_positive(todo1)
|
||||
.add_negative(todo2)
|
||||
.add_positive(todo3)
|
||||
.add_negative(todo4)
|
||||
.build()
|
||||
.is_todo());
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3596,8 +3596,7 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
|
||||
self.infer_first_comprehension_iter(generators);
|
||||
|
||||
// TODO generator type
|
||||
todo_type!()
|
||||
todo_type!("generator type")
|
||||
}
|
||||
|
||||
fn infer_list_comprehension_expression(&mut self, listcomp: &ast::ExprListComp) -> Type<'db> {
|
||||
|
|
@ -3609,8 +3608,7 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
|
||||
self.infer_first_comprehension_iter(generators);
|
||||
|
||||
// TODO list type
|
||||
todo_type!()
|
||||
todo_type!("list comprehension type")
|
||||
}
|
||||
|
||||
fn infer_dict_comprehension_expression(&mut self, dictcomp: &ast::ExprDictComp) -> Type<'db> {
|
||||
|
|
@ -3623,8 +3621,7 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
|
||||
self.infer_first_comprehension_iter(generators);
|
||||
|
||||
// TODO dict type
|
||||
todo_type!()
|
||||
todo_type!("dict comprehension type")
|
||||
}
|
||||
|
||||
fn infer_set_comprehension_expression(&mut self, setcomp: &ast::ExprSetComp) -> Type<'db> {
|
||||
|
|
@ -3636,8 +3633,7 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
|
||||
self.infer_first_comprehension_iter(generators);
|
||||
|
||||
// TODO set type
|
||||
todo_type!()
|
||||
todo_type!("set comprehension type")
|
||||
}
|
||||
|
||||
fn infer_generator_expression_scope(&mut self, generator: &ast::ExprGenerator) {
|
||||
|
|
@ -5979,7 +5975,9 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
}
|
||||
}
|
||||
ast::ExprContext::Invalid => TypeAndQualifiers::unknown(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => todo_type!().into(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => {
|
||||
todo_type!("Name expression annotation in Store/Del context").into()
|
||||
}
|
||||
},
|
||||
|
||||
ast::Expr::Subscript(subscript @ ast::ExprSubscript { value, slice, .. }) => {
|
||||
|
|
@ -6136,7 +6134,9 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
.in_type_expression(self.db())
|
||||
.unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)),
|
||||
ast::ExprContext::Invalid => Type::unknown(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => {
|
||||
todo_type!("Name expression annotation in Store/Del context")
|
||||
}
|
||||
},
|
||||
|
||||
ast::Expr::Attribute(attribute_expression) => match attribute_expression.ctx {
|
||||
|
|
@ -6145,7 +6145,9 @@ impl<'db> TypeInferenceBuilder<'db> {
|
|||
.in_type_expression(self.db())
|
||||
.unwrap_or_else(|error| error.into_fallback_type(&self.context, expression)),
|
||||
ast::ExprContext::Invalid => Type::unknown(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => todo_type!(),
|
||||
ast::ExprContext::Store | ast::ExprContext::Del => {
|
||||
todo_type!("Attribute expression annotation in Store/Del context")
|
||||
}
|
||||
},
|
||||
|
||||
ast::Expr::NoneLiteral(_literal) => Type::none(self.db()),
|
||||
|
|
|
|||
|
|
@ -325,18 +325,7 @@ fn dynamic_elements_ordering(left: DynamicType, right: DynamicType) -> Ordering
|
|||
(_, DynamicType::Unknown) => Ordering::Greater,
|
||||
|
||||
#[cfg(debug_assertions)]
|
||||
(DynamicType::Todo(left), DynamicType::Todo(right)) => match (left, right) {
|
||||
(
|
||||
TodoType::FileAndLine(left_file, left_line),
|
||||
TodoType::FileAndLine(right_file, right_line),
|
||||
) => left_file
|
||||
.cmp(right_file)
|
||||
.then_with(|| left_line.cmp(&right_line)),
|
||||
(TodoType::FileAndLine(..), _) => Ordering::Less,
|
||||
(_, TodoType::FileAndLine(..)) => Ordering::Greater,
|
||||
|
||||
(TodoType::Message(left), TodoType::Message(right)) => left.cmp(right),
|
||||
},
|
||||
(DynamicType::Todo(TodoType(left)), DynamicType::Todo(TodoType(right))) => left.cmp(right),
|
||||
|
||||
#[cfg(not(debug_assertions))]
|
||||
(DynamicType::Todo(TodoType), DynamicType::Todo(TodoType)) => Ordering::Equal,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue