From 650794ed7c112b2d1150d2cd99038c457001fc30 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 12 Feb 2024 18:54:18 +0100 Subject: [PATCH 1/3] Disable top-level docstring formatting for notebooks --- .../ruff/notebook_docstring.options.json | 5 +++ .../test/fixtures/ruff/notebook_docstring.py | 4 ++ crates/ruff_python_formatter/src/range.rs | 6 +-- .../src/statement/suite.rs | 32 +++++++++++++--- .../format@notebook_docstring.py.snap | 37 +++++++++++++++++++ 5 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json create mode 100644 crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py create mode 100644 crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json new file mode 100644 index 0000000000000..b4c9bc2c29ac2 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json @@ -0,0 +1,5 @@ +[ + { + "source_type": "Ipynb" + } +] diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py new file mode 100644 index 0000000000000..e83aebd6adde6 --- /dev/null +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py @@ -0,0 +1,4 @@ +""" + This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. + Ruff should leave it as is +"""; diff --git a/crates/ruff_python_formatter/src/range.rs b/crates/ruff_python_formatter/src/range.rs index 77a17c55873dc..c187c03b53ebc 100644 --- a/crates/ruff_python_formatter/src/range.rs +++ b/crates/ruff_python_formatter/src/range.rs @@ -214,9 +214,9 @@ impl<'ast> PreorderVisitor<'ast> for FindEnclosingNode<'_, 'ast> { // Don't pick potential docstrings as the closest enclosing node because `suite.rs` than fails to identify them as // docstrings and docstring formatting won't kick in. // Format the enclosing node instead and slice the formatted docstring from the result. - let is_maybe_docstring = node - .as_stmt_expr() - .is_some_and(|stmt| DocstringStmt::is_docstring_statement(stmt)); + let is_maybe_docstring = node.as_stmt_expr().is_some_and(|stmt| { + DocstringStmt::is_docstring_statement(stmt, self.context.options().source_type()) + }); if is_maybe_docstring { return TraversalSignal::Skip; diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index e05a9f6f59017..8658cce9582e3 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -103,7 +103,9 @@ impl FormatRule> for FormatSuite { } SuiteKind::Function => { - if let Some(docstring) = DocstringStmt::try_from_statement(first, self.kind) { + if let Some(docstring) = + DocstringStmt::try_from_statement(first, self.kind, source_type) + { SuiteChildStatement::Docstring(docstring) } else { SuiteChildStatement::Other(first) @@ -111,7 +113,9 @@ impl FormatRule> for FormatSuite { } SuiteKind::Class => { - if let Some(docstring) = DocstringStmt::try_from_statement(first, self.kind) { + if let Some(docstring) = + DocstringStmt::try_from_statement(first, self.kind, source_type) + { if !comments.has_leading(first) && lines_before(first.start(), source) > 1 && !source_type.is_stub() @@ -143,7 +147,9 @@ impl FormatRule> for FormatSuite { } SuiteKind::TopLevel => { if is_format_module_docstring_enabled(f.context()) { - if let Some(docstring) = DocstringStmt::try_from_statement(first, self.kind) { + if let Some(docstring) = + DocstringStmt::try_from_statement(first, self.kind, source_type) + { SuiteChildStatement::Docstring(docstring) } else { SuiteChildStatement::Other(first) @@ -184,7 +190,8 @@ impl FormatRule> for FormatSuite { true } else if is_module_docstring_newlines_enabled(f.context()) && self.kind == SuiteKind::TopLevel - && DocstringStmt::try_from_statement(first.statement(), self.kind).is_some() + && DocstringStmt::try_from_statement(first.statement(), self.kind, source_type) + .is_some() { // Only in preview mode, insert a newline after a module level docstring, but treat // it as a docstring otherwise. See: https://github.com/psf/black/pull/3932. @@ -734,7 +741,16 @@ pub(crate) struct DocstringStmt<'a> { impl<'a> DocstringStmt<'a> { /// Checks if the statement is a simple string that can be formatted as a docstring - fn try_from_statement(stmt: &'a Stmt, suite_kind: SuiteKind) -> Option> { + fn try_from_statement( + stmt: &'a Stmt, + suite_kind: SuiteKind, + source_type: PySourceType, + ) -> Option> { + // Notebooks don't have a concept of notebooks, never recognise them as docstrings. + if source_type.is_ipynb() && suite_kind == SuiteKind::TopLevel { + return None; + } + let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else { return None; }; @@ -752,7 +768,11 @@ impl<'a> DocstringStmt<'a> { } } - pub(crate) fn is_docstring_statement(stmt: &StmtExpr) -> bool { + pub(crate) fn is_docstring_statement(stmt: &StmtExpr, source_type: PySourceType) -> bool { + if source_type.is_ipynb() { + return false; + } + if let Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) = stmt.value.as_ref() { !value.is_implicit_concatenated() } else { diff --git a/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap new file mode 100644 index 0000000000000..12d3a8528ef96 --- /dev/null +++ b/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap @@ -0,0 +1,37 @@ +--- +source: crates/ruff_python_formatter/tests/fixtures.rs +input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py +--- +## Input +```python +""" + This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. + Ruff should leave it as is +"""; +``` + +## Outputs +### Output 1 +``` +indent-style = space +line-width = 88 +indent-width = 4 +quote-style = Double +line-ending = LineFeed +magic-trailing-comma = Respect +docstring-code = Disabled +docstring-code-line-width = "dynamic" +preview = Disabled +target_version = Py38 +source_type = Ipynb +``` + +```python +""" + This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. + Ruff should leave it as is +"""; +``` + + + From 03605196b68f052d2fef67c1141ee251d2b851af Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 12 Feb 2024 19:01:32 +0100 Subject: [PATCH 2/3] Fix comment --- crates/ruff_python_formatter/src/statement/suite.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_python_formatter/src/statement/suite.rs b/crates/ruff_python_formatter/src/statement/suite.rs index 8658cce9582e3..df1cb3be516f3 100644 --- a/crates/ruff_python_formatter/src/statement/suite.rs +++ b/crates/ruff_python_formatter/src/statement/suite.rs @@ -746,7 +746,7 @@ impl<'a> DocstringStmt<'a> { suite_kind: SuiteKind, source_type: PySourceType, ) -> Option> { - // Notebooks don't have a concept of notebooks, never recognise them as docstrings. + // Notebooks don't have a concept of modules, therefore, don't recognise the first string as the module docstring. if source_type.is_ipynb() && suite_kind == SuiteKind::TopLevel { return None; } From bacf688668dc23038aa869f96f78667856cc895b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 12 Feb 2024 19:07:17 +0100 Subject: [PATCH 3/3] Extend test --- .../ruff/notebook_docstring.options.json | 3 ++ .../test/fixtures/ruff/notebook_docstring.py | 2 + .../format@notebook_docstring.py.snap | 45 ++++++++++++++++++- 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json index b4c9bc2c29ac2..85d47e2e42e14 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.options.json @@ -1,5 +1,8 @@ [ { "source_type": "Ipynb" + }, + { + "source_type": "Python" } ] diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py index e83aebd6adde6..f5c7a561fcd75 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_docstring.py @@ -2,3 +2,5 @@ This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. Ruff should leave it as is """; + +"another normal string" diff --git a/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap index 12d3a8528ef96..f1ec8638e3ef4 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@notebook_docstring.py.snap @@ -8,6 +8,8 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/notebook_d This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. Ruff should leave it as is """; + +"another normal string" ``` ## Outputs @@ -30,7 +32,48 @@ source_type = Ipynb """ This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. Ruff should leave it as is -"""; +""" +"another normal string" +``` + + +### Output 2 +``` +indent-style = space +line-width = 88 +indent-width = 4 +quote-style = Double +line-ending = LineFeed +magic-trailing-comma = Respect +docstring-code = Disabled +docstring-code-line-width = "dynamic" +preview = Disabled +target_version = Py38 +source_type = Python +``` + +```python +""" + This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. + Ruff should leave it as is +""" +"another normal string" +``` + + +#### Preview changes +```diff +--- Stable ++++ Preview +@@ -1,5 +1,6 @@ + """ +- This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. +- Ruff should leave it as is ++This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. ++Ruff should leave it as is + """ ++ + "another normal string" ```