Skip to content

Commit

Permalink
Don't treat annotations as redefinitions in .pyi files (#10512)
Browse files Browse the repository at this point in the history
## Summary

In #10341, we fixed some false
positives in `.pyi` files, but introduced others. This PR effectively
reverts the change in #10341 and fixes it in a slightly different way.
Instead of changing the _bindings_ we generate in the semantic model in
`.pyi` files, we instead change how we _resolve_ them.

Closes #10509.
  • Loading branch information
charliermarsh authored Mar 21, 2024
1 parent 60fd98e commit caa1450
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
"""Regression test for: https://github.com/astral-sh/ruff/issues/10509"""

from foo import Bar as Bar

class Eggs:
Bar: int # OK

Bar = 1 # F811
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1836,7 +1836,7 @@ impl<'a> Checker<'a> {
if matches!(
parent,
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
) && !(self.semantic.in_annotation() || self.source_type.is_stub())
) && !self.semantic.in_annotation()
{
self.add_binding(id, expr.range(), BindingKind::Annotation, flags);
return;
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pyflakes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ mod tests {
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_28.py"))]
#[test_case(Rule::RedefinedWhileUnused, Path::new("F811_29.pyi"))]
#[test_case(Rule::UndefinedName, Path::new("F821_0.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_1.py"))]
#[test_case(Rule::UndefinedName, Path::new("F821_2.py"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: crates/ruff_linter/src/rules/pyflakes/mod.rs
---
F811_29.pyi:8:1: F811 Redefinition of unused `Bar` from line 3
|
6 | Bar: int # OK
7 |
8 | Bar = 1 # F811
| ^^^ F811
|
= help: Remove definition: `Bar`
26 changes: 19 additions & 7 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,10 @@ impl<'a> SemanticModel<'a> {
//
// The `name` in `print(name)` should be treated as unresolved, but the `name` in
// `name: str` should be treated as used.
BindingKind::Annotation => continue,
//
// Stub files are an exception. In a stub file, it _is_ considered valid to
// resolve to a type annotation.
BindingKind::Annotation if !self.in_stub_file() => continue,

// If it's a deletion, don't treat it as resolved, since the name is now
// unbound. For example, given:
Expand Down Expand Up @@ -1570,6 +1573,11 @@ impl<'a> SemanticModel<'a> {
.intersects(SemanticModelFlags::FUTURE_ANNOTATIONS)
}

/// Return `true` if the model is in a stub file (i.e., a file with a `.pyi` extension).
pub const fn in_stub_file(&self) -> bool {
self.flags.intersects(SemanticModelFlags::STUB_FILE)
}

/// Return `true` if the model is in a named expression assignment (e.g., `x := 1`).
pub const fn in_named_expression_assignment(&self) -> bool {
self.flags
Expand Down Expand Up @@ -1675,7 +1683,7 @@ bitflags! {
/// Flags indicating the current model state.
#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)]
pub struct SemanticModelFlags: u32 {
/// The model is in a type annotation that will only be evaluated when running a type
/// The model is in a type annotation that will only be evaluated when running a type
/// checker.
///
/// For example, the model could be visiting `int` in:
Expand Down Expand Up @@ -1875,6 +1883,9 @@ bitflags! {
/// ```
const FUTURE_ANNOTATIONS = 1 << 15;

/// The model is in a Python stub file (i.e., a `.pyi` file).
const STUB_FILE = 1 << 16;

/// The model has traversed past the module docstring.
///
/// For example, the model could be visiting `x` in:
Expand All @@ -1883,7 +1894,7 @@ bitflags! {
///
/// x: int = 1
/// ```
const MODULE_DOCSTRING_BOUNDARY = 1 << 16;
const MODULE_DOCSTRING_BOUNDARY = 1 << 17;

/// The model is in a type parameter definition.
///
Expand All @@ -1893,23 +1904,23 @@ bitflags! {
///
/// Record = TypeVar("Record")
///
const TYPE_PARAM_DEFINITION = 1 << 17;
const TYPE_PARAM_DEFINITION = 1 << 18;

/// The model is in a named expression assignment.
///
/// For example, the model could be visiting `x` in:
/// ```python
/// if (x := 1): ...
/// ```
const NAMED_EXPRESSION_ASSIGNMENT = 1 << 18;
const NAMED_EXPRESSION_ASSIGNMENT = 1 << 19;

/// The model is in a comprehension variable assignment.
///
/// For example, the model could be visiting `x` in:
/// ```python
/// [_ for x in range(10)]
/// ```
const COMPREHENSION_ASSIGNMENT = 1 << 19;
const COMPREHENSION_ASSIGNMENT = 1 << 20;

/// The model is in a module / class / function docstring.
///
Expand All @@ -1928,7 +1939,7 @@ bitflags! {
/// """Function docstring."""
/// pass
/// ```
const DOCSTRING = 1 << 20;
const DOCSTRING = 1 << 21;

/// The context is in any type annotation.
const ANNOTATION = Self::TYPING_ONLY_ANNOTATION.bits() | Self::RUNTIME_EVALUATED_ANNOTATION.bits() | Self::RUNTIME_REQUIRED_ANNOTATION.bits();
Expand All @@ -1953,6 +1964,7 @@ impl SemanticModelFlags {
pub fn new(path: &Path) -> Self {
let mut flags = Self::default();
if is_python_stub_file(path) {
flags |= Self::STUB_FILE;
flags |= Self::FUTURE_ANNOTATIONS;
}
flags
Expand Down

0 comments on commit caa1450

Please sign in to comment.