Add cell indexes to all diagnostics

crates/ruff_linter/resources/test/fixtures/pyflakes/F402.ipynb

@@ -0,0 +1,60 @@
+{
+ "cells": [
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "33faf7ad-a3fd-4ac4-a0c3-52e507ed49df",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "import os\n",
+    "import os.path as path"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "id": "481fb4bf-c1b9-47da-927f-3cfdfe4b49ec",
+   "metadata": {},
+   "outputs": [],
+   "source": [
+    "for os in range(3):\n",
+    "    pass"
+   ]
+  },
+  {
+   "cell_type": "code",
+   "execution_count": null,
+   "outputs": [],
+   "source": [
+    "for path in range(3):\n",
+    "    pass"
+   ],
+   "metadata": {
+    "collapsed": false
+   },
+   "id": "2f0c65a5-0a0e-4080-afce-5a8ed0d706df"
+  }
+ ],
+ "metadata": {
+  "kernelspec": {
+   "display_name": "Python (ruff-playground)",
+   "language": "python",
+   "name": "ruff-playground"
+  },
+  "language_info": {
+   "codemirror_mode": {
+    "name": "ipython",
+    "version": 3
+   },
+   "file_extension": ".py",
+   "mimetype": "text/x-python",
+   "name": "python",
+   "nbconvert_exporter": "python",
+   "pygments_lexer": "ipython3",
+   "version": "3.11.3"
+  }
+ },
+ "nbformat": 4,
+ "nbformat_minor": 5
+}

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

@@ -151,13 +151,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
                         continue;
                     }
 
-                    #[allow(deprecated)]
-                    let line = checker.locator.compute_line_index(shadowed.start());
-
                     checker.diagnostics.push(Diagnostic::new(
                         pyflakes::rules::ImportShadowedByLoopVar {
                             name: name.to_string(),
-                            line,
+                            row: checker.compute_source_row(shadowed.start()),
                         },
                         binding.range(),
                     ));
@@ -243,12 +240,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
                         continue;
                     }
 
-                    #[allow(deprecated)]
-                    let line = checker.locator.compute_line_index(shadowed.start());
                     let mut diagnostic = Diagnostic::new(
                         pyflakes::rules::RedefinedWhileUnused {
                             name: (*name).to_string(),
-                            line,
+                            row: checker.compute_source_row(shadowed.start()),
                         },
                         binding.range(),
                     );

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

@@ -37,7 +37,7 @@ use ruff_python_ast::{
 use ruff_text_size::{Ranged, TextRange, TextSize};
 
 use ruff_diagnostics::{Diagnostic, IsolationLevel};
-use ruff_notebook::CellOffsets;
+use ruff_notebook::{CellOffsets, NotebookIndex};
 use ruff_python_ast::all::{extract_all_names, DunderAllFlags};
 use ruff_python_ast::helpers::{
     collect_import_from_member, extract_handled_exceptions, to_module_path,
@@ -56,7 +56,7 @@ use ruff_python_semantic::{
     StarImport, SubmoduleImport,
 };
 use ruff_python_stdlib::builtins::{IPYTHON_BUILTINS, MAGIC_GLOBALS, PYTHON_BUILTINS};
-use ruff_source_file::Locator;
+use ruff_source_file::{Locator, OneIndexed, SourceRow};
 
 use crate::checkers::ast::annotation::AnnotationContext;
 use crate::checkers::ast::deferred::Deferred;
@@ -83,6 +83,8 @@ pub(crate) struct Checker<'a> {
     pub(crate) source_type: PySourceType,
     /// The [`CellOffsets`] for the current file, if it's a Jupyter notebook.
     cell_offsets: Option<&'a CellOffsets>,
+    /// The [`NotebookIndex`] for the current file, if it's a Jupyter notebook.
+    notebook_index: Option<&'a NotebookIndex>,
     /// The [`flags::Noqa`] for the current analysis (i.e., whether to respect suppression
     /// comments).
     noqa: flags::Noqa,
@@ -128,6 +130,7 @@ impl<'a> Checker<'a> {
         importer: Importer<'a>,
         source_type: PySourceType,
         cell_offsets: Option<&'a CellOffsets>,
+        notebook_index: Option<&'a NotebookIndex>,
     ) -> Checker<'a> {
         Checker {
             settings,
@@ -146,6 +149,7 @@ impl<'a> Checker<'a> {
             diagnostics: Vec::default(),
             flake8_bugbear_seen: Vec::default(),
             cell_offsets,
+            notebook_index,
             last_stmt_end: TextSize::default(),
         }
     }
@@ -198,6 +202,22 @@ impl<'a> Checker<'a> {
         }
     }
 
+    /// Returns the [`SourceRow`] for the given offset.
+    pub(crate) fn compute_source_row(&self, offset: TextSize) -> SourceRow {
+        #[allow(deprecated)]
+        let line_index = self.locator.compute_line_index(offset);
+
+        if let Some(notebook_index) = self.notebook_index {
+            let cell = notebook_index.cell(line_index).unwrap_or(OneIndexed::MIN);
+            let row = notebook_index
+                .cell_row(line_index)
+                .unwrap_or(OneIndexed::MIN);
+            SourceRow::Notebook(cell, row)
+        } else {
+            SourceRow::SourceFile(line_index)
+        }
+    }
+
     /// The [`Locator`] for the current file, which enables extraction of source code from byte
     /// offsets.
     pub(crate) const fn locator(&self) -> &'a Locator<'a> {
@@ -240,6 +260,11 @@ impl<'a> Checker<'a> {
         self.cell_offsets
     }
 
+    /// The [`NotebookIndex`] for the current file, if it's a Jupyter notebook.
+    pub(crate) const fn notebook_index(&self) -> Option<&'a NotebookIndex> {
+        self.notebook_index
+    }
+
     /// Returns whether the given rule should be checked.
     #[inline]
     pub(crate) const fn enabled(&self, rule: Rule) -> bool {
@@ -1984,6 +2009,7 @@ pub(crate) fn check_ast(
     package: Option<&Path>,
     source_type: PySourceType,
     cell_offsets: Option<&CellOffsets>,
+    notebook_index: Option<&NotebookIndex>,
 ) -> Vec<Diagnostic> {
     let module_path = package.and_then(|package| to_module_path(package, path));
     let module = Module {
@@ -2013,6 +2039,7 @@ pub(crate) fn check_ast(
         Importer::new(python_ast, locator, stylist),
         source_type,
         cell_offsets,
+        notebook_index,
     );
     checker.bind_builtins();
 

crates/ruff_linter/src/linter.rs

@@ -148,6 +148,7 @@ pub fn check_path(
         match tokens.into_ast_source(source_kind, source_type) {
             Ok(python_ast) => {
                 let cell_offsets = source_kind.as_ipy_notebook().map(Notebook::cell_offsets);
+                let notebook_index = source_kind.as_ipy_notebook().map(Notebook::index);
                 if use_ast {
                     diagnostics.extend(check_ast(
                         &python_ast,
@@ -161,6 +162,7 @@ pub fn check_path(
                         package,
                         source_type,
                         cell_offsets,
+                        notebook_index,
                     ));
                 }
                 if use_imports {

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

@@ -54,6 +54,7 @@ mod tests {
     #[test_case(Rule::UnusedImport, Path::new("F401_19.py"))]
     #[test_case(Rule::UnusedImport, Path::new("F401_20.py"))]
     #[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.py"))]
+    #[test_case(Rule::ImportShadowedByLoopVar, Path::new("F402.ipynb"))]
     #[test_case(Rule::UndefinedLocalWithImportStar, Path::new("F403.py"))]
     #[test_case(Rule::LateFutureImport, Path::new("F404_0.py"))]
     #[test_case(Rule::LateFutureImport, Path::new("F404_1.py"))]

crates/ruff_linter/src/rules/pyflakes/rules/imports.rs

@@ -1,6 +1,6 @@
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
-use ruff_source_file::OneIndexed;
+use ruff_source_file::{OneIndexed, SourceRow};
 
 /// ## What it does
 /// Checks for import bindings that are shadowed by loop variables.
@@ -32,14 +32,14 @@ use ruff_source_file::OneIndexed;
 #[violation]
 pub struct ImportShadowedByLoopVar {
     pub(crate) name: String,
-    pub(crate) line: OneIndexed,
+    pub(crate) row: SourceRow,
 }
 
 impl Violation for ImportShadowedByLoopVar {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let ImportShadowedByLoopVar { name, line } = self;
-        format!("Import `{name}` from line {line} shadowed by loop variable")
+        let ImportShadowedByLoopVar { name, row } = self;
+        format!("Import `{name}` from {row} shadowed by loop variable")
     }
 }
 

crates/ruff_linter/src/rules/pyflakes/rules/redefined_while_unused.rs

@@ -1,6 +1,6 @@
 use ruff_diagnostics::Violation;
 use ruff_macros::{derive_message_formats, violation};
-use ruff_source_file::OneIndexed;
+use ruff_source_file::{OneIndexed, SourceRow};
 
 /// ## What it does
 /// Checks for variable definitions that redefine (or "shadow") unused
@@ -25,13 +25,13 @@ use ruff_source_file::OneIndexed;
 #[violation]
 pub struct RedefinedWhileUnused {
     pub name: String,
-    pub line: OneIndexed,
+    pub row: SourceRow,
 }
 
 impl Violation for RedefinedWhileUnused {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let RedefinedWhileUnused { name, line } = self;
-        format!("Redefinition of unused `{name}` from line {line}")
+        let RedefinedWhileUnused { name, row } = self;
+        format!("Redefinition of unused `{name}` from {row}")
     }
 }

crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F402_F402.ipynb.snap

@@ -0,0 +1,23 @@
+---
+source: crates/ruff_linter/src/rules/pyflakes/mod.rs
+---
+F402.ipynb:3:5: F402 Import `os` from cell 1, line 1 shadowed by loop variable
+  |
+1 | import os
+2 | import os.path as path
+3 | for os in range(3):
+  |     ^^ F402
+4 |     pass
+5 | for path in range(3):
+  |
+
+F402.ipynb:5:5: F402 Import `path` from cell 1, line 2 shadowed by loop variable
+  |
+3 | for os in range(3):
+4 |     pass
+5 | for path in range(3):
+  |     ^^^^ F402
+6 |     pass
+  |
+
+

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

@@ -2,7 +2,7 @@ use ruff_python_ast::Expr;
 
 use ruff_diagnostics::{Diagnostic, Violation};
 use ruff_macros::{derive_message_formats, violation};
-use ruff_source_file::OneIndexed;
+use ruff_source_file::{OneIndexed, SourceRow};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -43,27 +43,25 @@ use crate::checkers::ast::Checker;
 #[violation]
 pub struct LoadBeforeGlobalDeclaration {
     name: String,
-    line: OneIndexed,
+    row: SourceRow,
 }
 
 impl Violation for LoadBeforeGlobalDeclaration {
     #[derive_message_formats]
     fn message(&self) -> String {
-        let LoadBeforeGlobalDeclaration { name, line } = self;
-        format!("Name `{name}` is used prior to global declaration on line {line}")
+        let LoadBeforeGlobalDeclaration { name, row } = self;
+        format!("Name `{name}` is used prior to global declaration on {row}")
     }
 }
 
 /// PLE0118
 pub(crate) fn load_before_global_declaration(checker: &mut Checker, name: &str, expr: &Expr) {
     if let Some(stmt) = checker.semantic().global(name) {
         if expr.start() < stmt.start() {
-            #[allow(deprecated)]
-            let location = checker.locator().compute_source_location(stmt.start());
             checker.diagnostics.push(Diagnostic::new(
                 LoadBeforeGlobalDeclaration {
                     name: name.to_string(),
-                    line: location.row,
+                    row: checker.compute_source_row(stmt.start()),
                 },
                 expr.range(),
             ));

crates/ruff_source_file/src/lib.rs

@@ -1,5 +1,5 @@
 use std::cmp::Ordering;
-use std::fmt::{Debug, Formatter};
+use std::fmt::{Debug, Display, Formatter};
 use std::sync::Arc;
 
 #[cfg(feature = "serde")]
@@ -253,3 +253,20 @@ impl Debug for SourceLocation {
             .finish()
     }
 }
+
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub enum SourceRow {
+    /// A row within a cell in a Jupyter Notebook.
+    Notebook(OneIndexed, OneIndexed),
+    /// A row within a source file.
+    SourceFile(OneIndexed),
+}
+
+impl Display for SourceRow {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match self {
+            SourceRow::Notebook(cell, line) => write!(f, "cell {cell}, line {line}"),
+            SourceRow::SourceFile(line) => write!(f, "line {line}"),
+        }
+    }
+}