Skip to content

Commit

Permalink
Avoid B015,B018 for last expression in a cell (#8815)
Browse files Browse the repository at this point in the history
## Summary

This PR updates `B015` and `B018` to ignore last top-level expressions
in each cell of a Jupyter Notebook.

Part of #8669

## Test Plan

Add test cases for both rules and update the snapshots.
  • Loading branch information
dhruvmanila authored Nov 22, 2023
1 parent 727e389 commit 5b726f7
Show file tree
Hide file tree
Showing 11 changed files with 454 additions and 1 deletion.
149 changes: 149 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B015.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": 1,
"id": "33faf7ad-a3fd-4ac4-a0c3-52e507ed49df",
"metadata": {},
"outputs": [],
"source": [
"x = 1"
]
},
{
"cell_type": "code",
"execution_count": 2,
"id": "481fb4bf-c1b9-47da-927f-3cfdfe4b49ec",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"True"
]
},
"execution_count": 2,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Simple case\n",
"x == 1"
]
},
{
"cell_type": "code",
"execution_count": 3,
"id": "2f0c65a5-0a0e-4080-afce-5a8ed0d706df",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"True"
]
},
"execution_count": 3,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Only skip the last expression\n",
"x == 1 # B018\n",
"x == 1"
]
},
{
"cell_type": "code",
"execution_count": 4,
"id": "5a3fd75d-26d9-44f7-b013-1684aabfd0ae",
"metadata": {},
"outputs": [],
"source": [
"# Nested expressions isn't relevant\n",
"if True:\n",
" x == 1"
]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "e00e1afa-b76c-4774-be2a-7223641579e4",
"metadata": {},
"outputs": [],
"source": [
"# Semicolons shouldn't affect the output\n",
"x == 1;"
]
},
{
"cell_type": "code",
"execution_count": 6,
"id": "05eab5b9-e2ba-4954-8ef3-b035a79573fe",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"True"
]
},
"execution_count": 6,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Semicolons with multiple expressions\n",
"x == 1; x == 1"
]
},
{
"cell_type": "code",
"execution_count": 7,
"id": "9cbbddc5-83fc-4fdb-81ab-53a3912ae898",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"True"
]
},
"execution_count": 7,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Comments, newlines and whitespace\n",
"x == 1 # comment\n",
"\n",
"# another comment"
]
}
],
"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
}
149 changes: 149 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B018.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": 1,
"id": "33faf7ad-a3fd-4ac4-a0c3-52e507ed49df",
"metadata": {},
"outputs": [],
"source": [
"x = 1"
]
},
{
"cell_type": "code",
"execution_count": 2,
"id": "481fb4bf-c1b9-47da-927f-3cfdfe4b49ec",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"1"
]
},
"execution_count": 2,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Simple case\n",
"x"
]
},
{
"cell_type": "code",
"execution_count": 3,
"id": "2f0c65a5-0a0e-4080-afce-5a8ed0d706df",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"1"
]
},
"execution_count": 3,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Only skip the last expression\n",
"x # B018\n",
"x"
]
},
{
"cell_type": "code",
"execution_count": 4,
"id": "5a3fd75d-26d9-44f7-b013-1684aabfd0ae",
"metadata": {},
"outputs": [],
"source": [
"# Nested expressions isn't relevant\n",
"if True:\n",
" x"
]
},
{
"cell_type": "code",
"execution_count": 5,
"id": "e00e1afa-b76c-4774-be2a-7223641579e4",
"metadata": {},
"outputs": [],
"source": [
"# Semicolons shouldn't affect the output\n",
"x;"
]
},
{
"cell_type": "code",
"execution_count": 6,
"id": "05eab5b9-e2ba-4954-8ef3-b035a79573fe",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"1"
]
},
"execution_count": 6,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Semicolons with multiple expressions\n",
"x; x"
]
},
{
"cell_type": "code",
"execution_count": 7,
"id": "9cbbddc5-83fc-4fdb-81ab-53a3912ae898",
"metadata": {},
"outputs": [
{
"data": {
"text/plain": [
"1"
]
},
"execution_count": 7,
"metadata": {},
"output_type": "execute_result"
}
],
"source": [
"# Comments, newlines and whitespace\n",
"x # comment\n",
"\n",
"# another comment"
]
}
],
"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
}
12 changes: 12 additions & 0 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use ruff_python_ast::{
use ruff_text_size::{Ranged, TextRange, TextSize};

use ruff_diagnostics::{Diagnostic, IsolationLevel};
use ruff_notebook::CellOffsets;
use ruff_python_ast::all::{extract_all_names, DunderAllFlags};
use ruff_python_ast::helpers::{
collect_import_from_member, extract_handled_exceptions, to_module_path,
Expand Down Expand Up @@ -78,6 +79,8 @@ pub(crate) struct Checker<'a> {
module_path: Option<&'a [String]>,
/// The [`PySourceType`] of the current file.
pub(crate) source_type: PySourceType,
/// The [`CellOffsets`] for the current file, if it's a Jupyter notebook.
cell_offsets: Option<&'a CellOffsets>,
/// The [`flags::Noqa`] for the current analysis (i.e., whether to respect suppression
/// comments).
noqa: flags::Noqa,
Expand Down Expand Up @@ -120,6 +123,7 @@ impl<'a> Checker<'a> {
indexer: &'a Indexer,
importer: Importer<'a>,
source_type: PySourceType,
cell_offsets: Option<&'a CellOffsets>,
) -> Checker<'a> {
Checker {
settings,
Expand All @@ -137,6 +141,7 @@ impl<'a> Checker<'a> {
deferred: Deferred::default(),
diagnostics: Vec::default(),
flake8_bugbear_seen: Vec::default(),
cell_offsets,
}
}
}
Expand Down Expand Up @@ -225,6 +230,11 @@ impl<'a> Checker<'a> {
self.package
}

/// The [`CellOffsets`] for the current file, if it's a Jupyter notebook.
pub(crate) const fn cell_offsets(&self) -> Option<&'a CellOffsets> {
self.cell_offsets
}

/// Returns whether the given rule should be checked.
#[inline]
pub(crate) const fn enabled(&self, rule: Rule) -> bool {
Expand Down Expand Up @@ -1942,6 +1952,7 @@ pub(crate) fn check_ast(
path: &Path,
package: Option<&Path>,
source_type: PySourceType,
cell_offsets: Option<&CellOffsets>,
) -> Vec<Diagnostic> {
let module_path = package.and_then(|package| to_module_path(package, path));
let module = Module {
Expand Down Expand Up @@ -1970,6 +1981,7 @@ pub(crate) fn check_ast(
indexer,
Importer::new(python_ast, locator, stylist),
source_type,
cell_offsets,
);
checker.bind_builtins();

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ pub fn check_path(
path,
package,
source_type,
cell_offsets,
));
}
if use_imports {
Expand Down
28 changes: 28 additions & 0 deletions crates/ruff_linter/src/rules/flake8_bugbear/helpers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
use ruff_notebook::CellOffsets;
use ruff_python_semantic::SemanticModel;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};

/// Return `true` if the statement containing the current expression is the last
/// top-level expression in the cell. This assumes that the source is a Jupyter
/// Notebook.
pub(super) fn at_last_top_level_expression_in_cell(
semantic: &SemanticModel,
locator: &Locator,
cell_offsets: Option<&CellOffsets>,
) -> bool {
if !semantic.at_top_level() {
return false;
}
let current_statement_end = semantic.current_statement().end();
cell_offsets
.and_then(|cell_offsets| cell_offsets.containing_range(current_statement_end))
.is_some_and(|cell_range| {
SimpleTokenizer::new(
locator.contents(),
TextRange::new(current_statement_end, cell_range.end()),
)
.all(|token| token.kind() == SimpleTokenKind::Semi || token.kind().is_trivia())
})
}
Loading

0 comments on commit 5b726f7

Please sign in to comment.