language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-30 13:51:37 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[red-knot] AnnAssign with no RHS is not a Definition (#13247)

Browse source 
My plan for handling declared types is to introduce a `Declaration` in
addition to `Definition`. A `Declaration` is an annotation of a name
with a type; a `Definition` is an actual runtime assignment of a value
to a name. A few things (an annotated function parameter, an
annotated-assignment with an RHS) are both a `Definition` and a
`Declaration`.

This more cleanly separates type inference (only cares about
`Definition`) from declared types (only impacted by a `Declaration`),
and I think it will work out better than trying to squeeze everything
into `Definition`. One of the tests in this PR
(`annotation_only_assignment_transparent_to_local_inference`)
demonstrates one reason why. The statement `x: int` should have no
effect on local inference of the type of `x`; whatever the locally
inferred type of `x` was before `x: int` should still be the inferred
type after `x: int`. This is actually quite hard to do if `x: int` is
considered a `Definition`, because a core assumption of the use-def map
is that a `Definition` replaces the previous value. To achieve this
would require some hackery to effectively treat `x: int` sort of as if
it were `x: int = x`, but it's not really even equivalent to that, so
this approach gets quite ugly.

As a first step in this plan, this PR stops treating AnnAssign with no
RHS as a `Definition`, which fixes behavior in a couple added tests.

This actually makes things temporarily worse for the ellipsis-type test,
since it is defined in typeshed only using annotated assignments with no
RHS. This will be fixed properly by the upcoming addition of
declarations, which should also treat a declared type as sufficient to
import a name, at least from a stub.
This commit is contained in:
Carl Meyer 2024-09-05 08:55:00 -07:00 • committed by GitHub
parent 65cc6ec41d
commit 2a3775e525
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 68 additions and 25 deletions
Download patch file Download diff file Expand all files Collapse all files

30
crates/red_knot_python_semantic/src/semantic_index/builder.rs
View file

@ -498,7 +498,6 @@ where
}
ast::Stmt::AnnAssign(node) => {
debug_assert!(self.current_assignment.is_none());
// TODO deferred annotation visiting
self.visit_expr(&node.annotation);
if let Some(value) = &node.value {
self.visit_expr(value);
@ -633,21 +632,22 @@ where
match expr {
ast::Expr::Name(name_node @ ast::ExprName { id, ctx, .. }) => {
let mut flags = match ctx {
ast::ExprContext::Load => SymbolFlags::IS_USED,
ast::ExprContext::Store => SymbolFlags::IS_DEFINED,
ast::ExprContext::Del => SymbolFlags::IS_DEFINED,
ast::ExprContext::Invalid => SymbolFlags::empty(),
let flags = match (ctx, self.current_assignment) {
(ast::ExprContext::Store, Some(CurrentAssignment::AugAssign(_))) => {
// For augmented assignment, the target expression is also used.
SymbolFlags::IS_DEFINED | SymbolFlags::IS_USED
}
(ast::ExprContext::Store, Some(CurrentAssignment::AnnAssign(ann_assign)))
if ann_assign.value.is_none() =>
{
// An annotated assignment that doesn't assign a value is not a Definition
SymbolFlags::empty()
}
(ast::ExprContext::Load, _) => SymbolFlags::IS_USED,
(ast::ExprContext::Store, _) => SymbolFlags::IS_DEFINED,
(ast::ExprContext::Del, _) => SymbolFlags::IS_DEFINED,
(ast::ExprContext::Invalid, _) => SymbolFlags::empty(),
};
if matches!(
self.current_assignment,
Some(CurrentAssignment::AugAssign(_))
) && !ctx.is_invalid()
{
// For augmented assignment, the target expression is also used, so we should
// record that as a use.
flags |= SymbolFlags::IS_USED;
}
let symbol = self.add_or_update_symbol(id.clone(), flags);
if flags.contains(SymbolFlags::IS_DEFINED) {
match self.current_assignment {

62
crates/red_knot_python_semantic/src/types/infer.rs
View file

@ -928,10 +928,11 @@ impl<'db> TypeInferenceBuilder<'db> {
}
fn infer_annotated_assignment_statement(&mut self, assignment: &ast::StmtAnnAssign) {
if let ast::Expr::Name(_) = assignment.target.as_ref() {
// assignments to non-Names are not Definitions, and neither are annotated assignments
// without an RHS
if assignment.value.is_some() && matches!(*assignment.target, ast::Expr::Name(_)) {
self.infer_definition(assignment);
} else {
// currently we don't consider assignments to non-Names to be Definitions
self.infer_annotated_assignment(assignment);
}
}
@ -941,11 +942,13 @@ impl<'db> TypeInferenceBuilder<'db> {
assignment: &ast::StmtAnnAssign,
definition: Definition<'db>,
) {
let ty = self.infer_annotated_assignment(assignment);
let ty = self
.infer_annotated_assignment(assignment)
.expect("Only annotated assignments with a RHS should create a Definition");
self.types.definitions.insert(definition, ty);
}
fn infer_annotated_assignment(&mut self, assignment: &ast::StmtAnnAssign) -> Type<'db> {
fn infer_annotated_assignment(&mut self, assignment: &ast::StmtAnnAssign) -> Option<Type<'db>> {
let ast::StmtAnnAssign {
range: _,
target,
@ -954,13 +957,13 @@ impl<'db> TypeInferenceBuilder<'db> {
simple: _,
} = assignment;
self.infer_optional_expression(value.as_deref());
let value_ty = self.infer_optional_expression(value.as_deref());
let annotation_ty = self.infer_expression(annotation);
self.infer_expression(annotation);
self.infer_expression(target);
annotation_ty
value_ty
}
fn infer_augmented_assignment_statement(&mut self, assignment: &ast::StmtAugAssign) {
@ -1890,8 +1893,6 @@ impl<'db> TypeInferenceBuilder<'db> {
let Some(enclosing_symbol) = enclosing_symbol_table.symbol_by_name(name) else {
continue;
};
// TODO a "definition" that is just an annotated-assignment with no RHS should not
// count as "is_defined" here.
if enclosing_symbol.is_defined() {
// We can return early here, because the nearest function-like scope that
// defines a name must be the only source for the nonlocal reference (at
@ -2909,7 +2910,7 @@ mod tests {
// TODO: update this once `infer_ellipsis_literal_expression` correctly
// infers `types.EllipsisType`.
assert_public_ty(&db, "src/a.py", "x", "Unknown | Literal[EllipsisType]");
assert_public_ty(&db, "src/a.py", "x", "Unbound");
Ok(())
}
@ -3972,6 +3973,47 @@ mod tests {
Ok(())
}
#[test]
fn nonlocal_name_reference_skips_annotation_only_assignment() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_dedented(
"/src/a.py",
"
def f():
x = 1
def g():
// it's pretty weird to have an annotated assignment in a function where the
// name is otherwise not defined; maybe should be an error?
x: int
def h():
y = x
",
)?;
assert_scope_ty(&db, "/src/a.py", &["f", "g", "h"], "y", "Literal[1]");
Ok(())
}
#[test]
fn annotation_only_assignment_transparent_to_local_inference() -> anyhow::Result<()> {
let mut db = setup_db();
db.write_dedented(
"/src/a.py",
"
x = 1
x: int
y = x
",
)?;
assert_public_ty(&db, "/src/a.py", "y", "Literal[1]");
Ok(())
}
// Incremental inference tests
fn first_public_def<'db>(db: &'db TestDb, file: File, name: &str) -> Definition<'db> {