Fix panic on access to definitions after analyzing definitions (#23588)

Summary
--

This PR fixes #23587 by removing this `std::mem::take` call:


https://github.com/astral-sh/ruff/blob/a62ba8c6e2bac0b899d90fd30a1b26c07aac44bb/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs#L120

This was previously moving the `Definitions` out of the `Checker` and
causing
this operation to panic if the definitions were accessed in a later
analysis
phase:


https://github.com/astral-sh/ruff/blob/a62ba8c6e2bac0b899d90fd30a1b26c07aac44bb/crates/ruff_linter/src/checkers/ast/analyze/definitions.rs#L120

which was apparently never done before `PLR1712` was added.

I don't see any reason why this really needed to be a move as it only
took a
couple of lifetimes to handle it with borrowing, so this seems like the
easiest
fix.

I also changed the indexing operation to a `get` call (with a
`debug_assert`)
since the method already returns an `Option`, but doing that alone would
prevent
`PLR1712` from firing in the module scope.

Test Plan
--

A new test based on the issue that also covers the module scope
mentioned above

Author: ntBre

crates/ruff_linter/resources/test/fixtures/pylint/swap_with_temporary_variable_1.py

@@ -0,0 +1,8 @@
+"""
+Test that PLR1712 fires in the module-global scope.
+"""
+
+x, y = 1, 2
+temp = x  # PLR1712
+x = y
+y = temp

crates/ruff_linter/src/checkers/ast/analyze/definitions.rs

@@ -117,12 +117,15 @@ pub(crate) fn definitions(checker: &mut Checker) {
             })
     };
 
-    let definitions = std::mem::take(&mut checker.semantic.definitions);
     let mut overloaded_name: Option<&str> = None;
     for ContextualizedDefinition {
         definition,
         visibility,
-    } in definitions.resolve(exports.as_deref()).iter()
+    } in checker
+        .semantic
+        .definitions
+        .resolve(exports.as_deref())
+        .iter()
     {
         let docstring = docstrings::extraction::extract_docstring(definition);
 

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -474,4 +474,18 @@ mod tests {
         assert_diagnostics!(diagnostics);
         Ok(())
     }
+
+    /// Regression test for <https://github.com/astral-sh/ruff/issues/23587>.
+    #[test]
+    fn conflict_with_definition_rules() -> Result<()> {
+        let diagnostics = test_path(
+            Path::new("pylint/swap_with_temporary_variable_1.py"),
+            &LinterSettings::for_rules(vec![
+                Rule::SwapWithTemporaryVariable,
+                Rule::MissingTypeFunctionArgument,
+            ]),
+        )?;
+        assert_diagnostics!(diagnostics);
+        Ok(())
+    }
 }

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__conflict_with_definition_rules.snap

@@ -0,0 +1,21 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+PLR1712 [*] Unnecessary temporary variable
+ --> swap_with_temporary_variable_1.py:6:1
+  |
+5 |   x, y = 1, 2
+6 | / temp = x  # PLR1712
+7 | | x = y
+8 | | y = temp
+  | |________^
+  |
+help: Use `x, y = y, x` instead
+3 | """
+4 | 
+5 | x, y = 1, 2
+  - temp = x  # PLR1712
+  - x = y
+  - y = temp
+6 + x, y = y, x
+note: This is an unsafe fix and may change runtime behavior

crates/ruff_python_semantic/src/definition.rs

@@ -214,11 +214,11 @@ impl<'a> Definitions<'a> {
     }
 
     /// Resolve the visibility of each definition in the collection.
-    pub fn resolve(self, exports: Option<&[DunderAllName]>) -> ContextualizedDefinitions<'a> {
+    pub fn resolve(&'a self, exports: Option<&[DunderAllName]>) -> ContextualizedDefinitions<'a> {
         let mut definitions: IndexVec<DefinitionId, ContextualizedDefinition<'a>> =
             IndexVec::with_capacity(self.len());
 
-        for definition in self {
+        for definition in self.iter() {
             // Determine the visibility of the next definition, taking into account its parent's
             // visibility.
             let visibility = {
@@ -282,7 +282,11 @@ impl<'a> Definitions<'a> {
 
     /// Returns a reference to the Python AST.
     pub fn python_ast(&self) -> Option<&'a [Stmt]> {
-        let module = self[DefinitionId::module()].as_module()?;
+        let Some(definition) = self.get(DefinitionId::module()) else {
+            debug_assert!(false, "Module definition unavailable");
+            return None;
+        };
+        let module = definition.as_module()?;
         Some(module.python_ast)
     }
 }
@@ -306,7 +310,7 @@ impl<'a> IntoIterator for Definitions<'a> {
 
 /// A [`Definition`] in a Python program with its resolved [`Visibility`].
 pub struct ContextualizedDefinition<'a> {
-    pub definition: Definition<'a>,
+    pub definition: &'a Definition<'a>,
     pub visibility: Visibility,
 }