Add flags

crates/ruff/resources/test/fixtures/flake8_pyi/PYI025.pyi

@@ -10,7 +10,21 @@ def f():
 def f():
     from collections.abc import Container, Sized, Set, ValuesView  # PYI025
 
-    GLOBAL: Set[int] = set()
+def f():
+    if True:
+        from collections.abc import Set
+    else:
+        Set = 1
+
+    x: Set = set()
+
+    x: Set
+
+    del Set
+
+    def f():
+        print(Set)
 
-    class Class:
-        member: Set[int]
+        def Set():
+            pass
+        print(Set)

crates/ruff/src/checkers/ast/mod.rs

@@ -833,24 +833,28 @@ where
                             BindingKind::SubmoduleImportation(SubmoduleImportation {
                                 qualified_name,
                             }),
-                            BindingFlags::empty(),
+                            BindingFlags::EXTERNAL,
                         );
                     } else {
+                        let mut flags = BindingFlags::EXTERNAL;
+                        if alias.asname.is_some() {
+                            flags |= BindingFlags::ALIAS;
+                        }
+                        if alias
+                            .asname
+                            .as_ref()
+                            .map_or(false, |asname| asname == &alias.name)
+                        {
+                            flags |= BindingFlags::EXPLICIT_EXPORT;
+                        }
+
                         let name = alias.asname.as_ref().unwrap_or(&alias.name);
                         let qualified_name = &alias.name;
                         self.add_binding(
                             name,
                             alias.identifier(self.locator),
                             BindingKind::Importation(Importation { qualified_name }),
-                            if alias
-                                .asname
-                                .as_ref()
-                                .map_or(false, |asname| asname == &alias.name)
-                            {
-                                BindingFlags::EXPLICIT_EXPORT
-                            } else {
-                                BindingFlags::empty()
-                            },
+                            flags,
                         );
 
                         if let Some(asname) = &alias.asname {
@@ -1135,6 +1139,18 @@ where
                             }
                         }
 
+                        let mut flags = BindingFlags::EXTERNAL;
+                        if alias.asname.is_some() {
+                            flags |= BindingFlags::ALIAS;
+                        }
+                        if alias
+                            .asname
+                            .as_ref()
+                            .map_or(false, |asname| asname == &alias.name)
+                        {
+                            flags |= BindingFlags::EXPLICIT_EXPORT;
+                        }
+
                         // Given `from foo import bar`, `name` would be "bar" and `qualified_name` would
                         // be "foo.bar". Given `from foo import bar as baz`, `name` would be "baz"
                         // and `qualified_name` would be "foo.bar".
@@ -1145,15 +1161,7 @@ where
                             name,
                             alias.identifier(self.locator),
                             BindingKind::FromImportation(FromImportation { qualified_name }),
-                            if alias
-                                .asname
-                                .as_ref()
-                                .map_or(false, |asname| asname == &alias.name)
-                            {
-                                BindingFlags::EXPLICIT_EXPORT
-                            } else {
-                                BindingFlags::empty()
-                            },
+                            flags,
                         );
                     }
                     if self.enabled(Rule::RelativeImports) {

crates/ruff/src/importer/mod.rs

@@ -1,4 +1,7 @@
-//! Add and modify import statements to make module members available during fix execution.
+//! Code modification struct to add and modify import statements.
+//!
+//! Enables rules to make module members available (that may be not yet be imported) during fix
+//! execution.
 
 use std::error::Error;
 

crates/ruff/src/lib.rs

@@ -29,6 +29,7 @@ mod noqa;
 pub mod packaging;
 pub mod pyproject_toml;
 pub mod registry;
+mod renamer;
 pub mod resolver;
 mod rule_redirects;
 mod rule_selector;

crates/ruff/src/renamer.rs

@@ -0,0 +1,172 @@
+//! Code modification struct to support symbol renaming within a scope.
+
+use anyhow::{anyhow, Result};
+
+use ruff_diagnostics::Edit;
+use ruff_python_semantic::{Binding, BindingKind, Scope, SemanticModel};
+
+pub(crate) struct Renamer;
+
+impl Renamer {
+    /// Rename a symbol (from `name` to `target`) within a [`Scope`].
+    ///
+    /// The renaming algorithm is as follows:
+    ///
+    /// 1. Start with the first [`Binding`] in the scope, for the given name. For example, in the
+    ///    following snippet, we'd start by examining the `x = 1` binding:
+    ///
+    /// ```python
+    /// if True:
+    ///     x = 1
+    ///     print(x)
+    /// else:
+    ///     x = 2
+    ///     print(x)
+    ///
+    /// print(x)
+    /// ```
+    ///
+    /// 2. Rename the [`Binding`]. In most cases, this is a simple replacement. For example,
+    ///    renaming `x` to `y` above would require replacing `x = 1` with `y = 1`. After the
+    ///    first replacement in the snippet above, we'd have:
+    ///
+    /// ```python
+    /// if True:
+    ///     y = 1
+    ///     print(x)
+    /// else:
+    ///     x = 2
+    ///     print(x)
+    ///
+    /// print(x)
+    /// ```
+    ///
+    ///    Note that, when renaming imports, we need to instead rename (or add) an alias. For
+    ///    example, to rename `pandas` to `pd`, we may need to rewrite `import pandas` to
+    ///    `import pandas as pd`, rather than `import pd`.
+    ///
+    /// 3. Rename every reference to the [`Binding`]. For example, renaming the references to the
+    ///    `x = 1` binding above would give us:
+    ///
+    /// ```python
+    /// if True:
+    ///     y = 1
+    ///     print(y)
+    /// else:
+    ///     x = 2
+    ///     print(x)
+    ///
+    /// print(x)
+    /// ```
+    ///
+    /// 4. Rename every delayed annotation. (See [`SemanticModel::delayed_annotations`].)
+    ///
+    /// 5. Repeat the above process for every [`Binding`] in the scope with the given name.
+    ///    After renaming the `x = 2` binding, we'd have:
+    ///
+    /// ```python
+    /// if True:
+    ///     y = 1
+    ///     print(y)
+    /// else:
+    ///     y = 2
+    ///     print(y)
+    ///
+    /// print(y)
+    /// ```
+    ///
+    /// `global` and `nonlocal` declarations add some additional complexity. If we're renaming a
+    /// name that's declared as `global` or `nonlocal` in a child scope, we need to rename the name
+    /// in that scope too, repeating the above process.
+    ///
+    /// If we're renaming a name that's declared as `global` or `nonlocal` in the current scope,
+    /// then we need to identify the scope in which the name is declared, and perform the rename
+    /// in that scope instead (which will in turn trigger the above process on the current scope).
+    ///
+    /// `global` and `nonlocal` declarations are not yet supported.
+    pub(crate) fn rename(
+        name: &str,
+        target: &str,
+        scope: &Scope,
+        semantic: &SemanticModel,
+    ) -> Result<(Edit, Vec<Edit>)> {
+        let mut edits = vec![];
+
+        // Iterate over every binding to the name in the scope.
+        for binding_id in scope.get_all(name) {
+            let binding = semantic.binding(binding_id);
+
+            // Rename the binding.
+            if let Some(edit) = Renamer::rename_binding(binding, target) {
+                edits.push(edit);
+
+                // Rename any delayed annotations.
+                if let Some(annotations) = semantic.delayed_annotations(binding_id) {
+                    edits.extend(annotations.iter().filter_map(|annotation_id| {
+                        let annotation = semantic.binding(*annotation_id);
+                        Renamer::rename_binding(annotation, target)
+                    }));
+                }
+
+                // Rename the references to the binding.
+                edits.extend(binding.references().map(|reference_id| {
+                    let reference = semantic.reference(reference_id);
+                    Edit::range_replacement(target.to_string(), reference.range())
+                }));
+            }
+        }
+
+        // Deduplicate any edits. In some cases, a reference can be both a read _and_ a write. For
+        // example, `x += 1` is both a read of and a write to `x`.
+        edits.sort();
+        edits.dedup();
+
+        let edit = edits
+            .pop()
+            .ok_or(anyhow!("Unable to rename any references to `{name}`"))?;
+        Ok((edit, edits))
+    }
+
+    /// Rename a [`Binding`] reference.
+    fn rename_binding(binding: &Binding, target: &str) -> Option<Edit> {
+        match &binding.kind {
+            BindingKind::Importation(_) | BindingKind::FromImportation(_) => {
+                if binding.is_alias() {
+                    // Ex) Rename `import pandas as alias` to `import pandas as pd`.
+                    Some(Edit::range_replacement(target.to_string(), binding.range))
+                } else {
+                    // Ex) Rename `import pandas` to `import pandas as pd`.
+                    Some(Edit::insertion(
+                        format!(" as {target}"),
+                        binding.range.end(),
+                    ))
+                }
+            }
+            BindingKind::SubmoduleImportation(import) => {
+                // Ex) Rename `import pandas.core` to `import pandas as pd`.
+                let module_name = import.qualified_name.split('.').next().unwrap();
+                Some(Edit::range_replacement(
+                    format!("{module_name} as {target}"),
+                    binding.range,
+                ))
+            }
+            // Avoid renaming builtins and other "special" bindings.
+            BindingKind::FutureImportation | BindingKind::Builtin | BindingKind::Export(_) => None,
+            // By default, replace the binding's name with the target name.
+            BindingKind::Annotation
+            | BindingKind::Argument
+            | BindingKind::NamedExprAssignment
+            | BindingKind::UnpackedAssignment
+            | BindingKind::Assignment
+            | BindingKind::LoopVar
+            | BindingKind::Global
+            | BindingKind::Nonlocal
+            | BindingKind::ClassDefinition
+            | BindingKind::FunctionDefinition
+            | BindingKind::Deletion
+            | BindingKind::UnboundException => {
+                Some(Edit::range_replacement(target.to_string(), binding.range))
+            }
+        }
+    }
+}

crates/ruff/src/rules/flake8_pyi/rules/unaliased_collections_abc_set_import.rs

@@ -1,10 +1,10 @@
-use itertools::Itertools;
-use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_semantic::{BindingKind, FromImportation, Scope};
 
 use crate::checkers::ast::Checker;
 use crate::registry::AsRule;
+use crate::renamer::Renamer;
 
 /// ## What it does
 /// Checks for `from collections.abc import Set` imports that do not alias
@@ -66,19 +66,11 @@ pub(crate) fn unaliased_collections_abc_set_import(
         let mut diagnostic = Diagnostic::new(UnaliasedCollectionsAbcSetImport, binding.range);
         if checker.patch(diagnostic.kind.rule()) {
             if checker.semantic().is_available("AbstractSet") {
-                // Create an edit to alias `Set` to `AbstractSet`.
-                let binding_edit =
-                    Edit::insertion(" as AbstractSet".to_string(), binding.range.end());
-
-                // Create an edit to replace all references to `Set` with `AbstractSet`.
-                let reference_edits: Vec<Edit> = binding
-                    .references()
-                    .map(|reference_id| checker.semantic().reference(reference_id).range())
-                    .dedup()
-                    .map(|range| Edit::range_replacement("AbstractSet".to_string(), range))
-                    .collect();
-
-                diagnostic.set_fix(Fix::suggested_edits(binding_edit, reference_edits));
+                diagnostic.try_set_fix(|| {
+                    let (edit, rest) =
+                        Renamer::rename(name, "AbstractSet", scope, checker.semantic())?;
+                    Ok(Fix::suggested_edits(edit, rest))
+                });
             }
         }
         diagnostics.push(diagnostic);

crates/ruff/src/rules/flake8_pyi/snapshots/ruff__rules__flake8_pyi__tests__PYI025_PYI025.pyi.snap

@@ -27,7 +27,7 @@ PYI025.pyi:11:51: PYI025 [*] Use `from collections.abc import Set as AbstractSet
 11 |     from collections.abc import Container, Sized, Set, ValuesView  # PYI025
    |                                                   ^^^ PYI025
 12 | 
-13 |     GLOBAL: Set[int] = set()
+13 | def f():
    |
    = help: Alias `Set` to `AbstractSet`
 
@@ -38,11 +38,44 @@ PYI025.pyi:11:51: PYI025 [*] Use `from collections.abc import Set as AbstractSet
 11    |-    from collections.abc import Container, Sized, Set, ValuesView  # PYI025
    11 |+    from collections.abc import Container, Sized, Set as AbstractSet, ValuesView  # PYI025
 12 12 | 
-13    |-    GLOBAL: Set[int] = set()
-   13 |+    GLOBAL: AbstractSet[int] = set()
-14 14 | 
-15 15 |     class Class:
-16    |-        member: Set[int]
-   16 |+        member: AbstractSet[int]
+13 13 | def f():
+14 14 |     if True:
+
+PYI025.pyi:15:37: PYI025 [*] Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin
+   |
+13 | def f():
+14 |     if True:
+15 |         from collections.abc import Set
+   |                                     ^^^ PYI025
+16 |     else:
+17 |         Set = 1
+   |
+   = help: Alias `Set` to `AbstractSet`
+
+ℹ Suggested fix
+12 12 | 
+13 13 | def f():
+14 14 |     if True:
+15    |-        from collections.abc import Set
+   15 |+        from collections.abc import Set as AbstractSet
+16 16 |     else:
+17    |-        Set = 1
+   17 |+        AbstractSet = 1
+18 18 | 
+19    |-    x: Set = set()
+   19 |+    x: AbstractSet = set()
+20 20 | 
+21    |-    x: Set
+   21 |+    x: AbstractSet
+22 22 | 
+23    |-    del Set
+   23 |+    del AbstractSet
+24 24 | 
+25 25 |     def f():
+26    |-        print(Set)
+   26 |+        print(AbstractSet)
+27 27 | 
+28 28 |         def Set():
+29 29 |             pass