Skip to content

Commit

Permalink
Disable top-level docstring formatting for notebooks (#9957)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Feb 12, 2024
1 parent ab2253d commit edfe842
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[
{
"source_type": "Ipynb"
},
{
"source_type": "Python"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +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
""";

"another normal string"
6 changes: 3 additions & 3 deletions crates/ruff_python_formatter/src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
32 changes: 26 additions & 6 deletions crates/ruff_python_formatter/src/statement/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,19 @@ impl FormatRule<Suite, PyFormatContext<'_>> 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)
}
}

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()
Expand Down Expand Up @@ -143,7 +147,9 @@ impl FormatRule<Suite, PyFormatContext<'_>> 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)
Expand Down Expand Up @@ -184,7 +190,8 @@ impl FormatRule<Suite, PyFormatContext<'_>> 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.
Expand Down Expand Up @@ -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<DocstringStmt<'a>> {
fn try_from_statement(
stmt: &'a Stmt,
suite_kind: SuiteKind,
source_type: PySourceType,
) -> Option<DocstringStmt<'a>> {
// 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;
}

let Stmt::Expr(ast::StmtExpr { value, .. }) = stmt else {
return None;
};
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
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
""";
"another normal string"
```

## 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
"""
"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"
```



0 comments on commit edfe842

Please sign in to comment.