Skip to content

Commit

Permalink
Compute NotebookIndex for Diagnostics on stdin (#7663)
Browse files Browse the repository at this point in the history
## Summary

This PR fixes the bug where the `NotebookIndex` was not being computed
when
using stdin as the input source.

## Test Plan

On `main`, the diagnostic output won't include the cell number when
using stdin
while it'll be included after this fix.

### `main`

```console
$ cat ~/playground/ruff/notebooks/test.ipynb | cargo run --bin ruff -- check --isolated --no-cache - --stdin-filename ~/playground/ruff/notebooks/test.ipynb
/Users/dhruv/playground/ruff/notebooks/test.ipynb:2:8: F401 [*] `math` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:7:8: F811 Redefinition of unused `random` from line 1
/Users/dhruv/playground/ruff/notebooks/test.ipynb:8:8: F401 [*] `pprint` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:12:4: F632 [*] Use `==` to compare constant literals
/Users/dhruv/playground/ruff/notebooks/test.ipynb:13:38: F632 [*] Use `==` to compare constant literals
Found 5 errors.
[*] 4 potentially fixable with the --fix option.
```

### `dhruv/notebook-index-stdin`

```console
$ cat ~/playground/ruff/notebooks/test.ipynb | cargo run --bin ruff -- check --isolated --no-cache - --stdin-filename ~/playground/ruff/notebooks/test.ipynb       
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 3:2:8: F401 [*] `math` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:1:8: F811 Redefinition of unused `random` from line 1
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 5:2:8: F401 [*] `pprint` imported but unused
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:2:4: F632 [*] Use `==` to compare constant literals
/Users/dhruv/playground/ruff/notebooks/test.ipynb:cell 6:3:38: F632 [*] Use `==` to compare constant literals
Found 5 errors.
[*] 4 potentially fixable with the --fix option.
```
  • Loading branch information
dhruvmanila committed Sep 29, 2023
1 parent 8c8988e commit b519b56
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
21 changes: 12 additions & 9 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::ops::AddAssign;
use std::os::unix::fs::PermissionsExt;
use std::path::Path;

use anyhow::{anyhow, Context, Result};
use anyhow::{Context, Result};
use colored::Colorize;
use filetime::FileTime;
use log::{debug, error, warn};
Expand Down Expand Up @@ -342,13 +342,7 @@ pub(crate) fn lint_path(
}

let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = source_kind {
FxHashMap::from_iter([(
path.to_str()
.ok_or_else(|| anyhow!("Unable to parse filename: {:?}", path))?
.to_string(),
// Index needs to be computed always to store in cache.
notebook.index().clone(),
)])
FxHashMap::from_iter([(path.to_string_lossy().to_string(), notebook.into_index())])
} else {
FxHashMap::default()
};
Expand Down Expand Up @@ -474,14 +468,23 @@ pub(crate) fn lint_stdin(
);
}

let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = source_kind {
FxHashMap::from_iter([(
path.map_or_else(|| "-".into(), |path| path.to_string_lossy().to_string()),
notebook.into_index(),
)])
} else {
FxHashMap::default()
};

Ok(Diagnostics {
messages,
fixed: FxHashMap::from_iter([(
fs::relativize_path(path.unwrap_or_else(|| Path::new("-"))),
fixed,
)]),
imports,
notebook_indexes: FxHashMap::default(),
notebook_indexes,
})
}

Expand Down
9 changes: 9 additions & 0 deletions crates/ruff_notebook/src/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,15 @@ impl Notebook {
self.index.get_or_init(|| self.build_index())
}

/// Return the Jupyter notebook index, consuming the notebook.
///
/// The index is built only once when required. This is only used to
/// report diagnostics, so by that time all of the fixes must have
/// been applied if `--fix` was passed.
pub fn into_index(mut self) -> NotebookIndex {
self.index.take().unwrap_or_else(|| self.build_index())
}

/// Return the cell offsets for the concatenated source code corresponding
/// the Jupyter notebook.
pub fn cell_offsets(&self) -> &[TextSize] {
Expand Down

0 comments on commit b519b56

Please sign in to comment.