Skip to content

Commit

Permalink
Remove empty line before raw dostrings
Browse files Browse the repository at this point in the history
**Summary** This fixes a deviation with black where black would remove
empty lines before raw docstrings for some reason.
  • Loading branch information
konstin committed Oct 17, 2023
1 parent dc6b4ad commit 9d6354c
Show file tree
Hide file tree
Showing 12 changed files with 375 additions and 105 deletions.
1 change: 1 addition & 0 deletions crates/ruff_python_ast/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::path::Path;

pub use expression::*;
pub use int::*;
pub use node::AnyNodeRef;
pub use nodes::*;

pub mod all;
Expand Down
66 changes: 63 additions & 3 deletions crates/ruff_python_ast/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4817,7 +4817,7 @@ pub enum AnyNodeRef<'a> {
ElifElseClause(&'a ast::ElifElseClause),
}

impl AnyNodeRef<'_> {
impl<'a> AnyNodeRef<'a> {
pub fn as_ptr(&self) -> NonNull<()> {
match self {
AnyNodeRef::ModModule(node) => NonNull::from(*node).cast(),
Expand Down Expand Up @@ -5456,9 +5456,9 @@ impl AnyNodeRef<'_> {
)
}

pub fn visit_preorder<'a, V>(&'a self, visitor: &mut V)
pub fn visit_preorder<'b, V>(&'b self, visitor: &mut V)
where
V: PreorderVisitor<'a> + ?Sized,
V: PreorderVisitor<'b> + ?Sized,
{
match self {
AnyNodeRef::ModModule(node) => node.visit_preorder(visitor),
Expand Down Expand Up @@ -5544,6 +5544,66 @@ impl AnyNodeRef<'_> {
AnyNodeRef::ElifElseClause(node) => node.visit_preorder(visitor),
}
}

/// The last child of the last branch, if the node has multiple branches.
pub fn last_child_in_body(&self) -> Option<AnyNodeRef<'a>> {
let body = match self {
AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. })
| AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. })
| AnyNodeRef::StmtWith(ast::StmtWith { body, .. })
| AnyNodeRef::MatchCase(MatchCase { body, .. })
| AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler {
body,
..
})
| AnyNodeRef::ElifElseClause(ast::ElifElseClause { body, .. }) => body,
AnyNodeRef::StmtIf(ast::StmtIf {
body,
elif_else_clauses,
..
}) => elif_else_clauses.last().map_or(body, |clause| &clause.body),

AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. })
| AnyNodeRef::StmtWhile(ast::StmtWhile { body, orelse, .. }) => {
if orelse.is_empty() {
body
} else {
orelse
}
}

AnyNodeRef::StmtMatch(ast::StmtMatch { cases, .. }) => {
return cases.last().map(AnyNodeRef::from);
}

AnyNodeRef::StmtTry(ast::StmtTry {
body,
handlers,
orelse,
finalbody,
..
}) => {
if finalbody.is_empty() {
if orelse.is_empty() {
if handlers.is_empty() {
body
} else {
return handlers.last().map(AnyNodeRef::from);
}
} else {
orelse
}
} else {
finalbody
}
}

// Not a node that contains an indented child node.
_ => return None,
};

body.last().map(AnyNodeRef::from)
}
}

impl<'a> From<&'a ast::ModModule> for AnyNodeRef<'a> {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
###
# Blank lines around functions
###
import sys

x = 1

Expand Down Expand Up @@ -159,3 +160,79 @@ def f():
# comment

x = 1


def f():
if True:

def double(s):
return s + s
print("below function")
if True:

class A:
x = 1
print("below class")
if True:

def double(s):
return s + s
#
print("below comment function")
if True:

class A:
x = 1
#
print("below comment class")
if True:

def double(s):
return s + s
#
def outer():
def inner():
pass
print("below nested functions")

if True:

def double(s):
return s + s
print("below function")
if True:

class A:
x = 1
print("below class")
def outer():
def inner():
pass
print("below nested functions")


class Path:
if sys.version_info >= (3, 11):
def joinpath(self): ...

# The .open method comes from pathlib.pyi and should be kept in sync.
@overload
def open(self): ...




def fakehttp():

class FakeHTTPConnection:
if mock_close:
def close(self):
pass
FakeHTTPConnection.fakedata = fakedata




if True:
def nested_trailing_function():
pass
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ class Test:
x = 1


class EmptyLineBeforeRawDocstring:

r"""Character and line based layer over a BufferedIOBase object, buffer."""


class C(): # comment
pass

Expand Down
67 changes: 4 additions & 63 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,9 @@ fn handle_end_of_line_comment_around_body<'a>(
// ```
// The first earlier branch filters out ambiguities e.g. around try-except-finally.
if let Some(preceding) = comment.preceding_node() {
if let Some(last_child) = last_child_in_body(preceding) {
if let Some(last_child) = preceding.last_child_in_body() {
let innermost_child =
std::iter::successors(Some(last_child), |parent| last_child_in_body(*parent))
std::iter::successors(Some(last_child), |parent| parent.last_child_in_body())
.last()
.unwrap_or(last_child);
return CommentPlacement::trailing(innermost_child, comment);
Expand Down Expand Up @@ -670,7 +670,7 @@ fn handle_own_line_comment_after_branch<'a>(
preceding: AnyNodeRef<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
let Some(last_child) = last_child_in_body(preceding) else {
let Some(last_child) = preceding.last_child_in_body() else {
return CommentPlacement::Default(comment);
};

Expand Down Expand Up @@ -734,7 +734,7 @@ fn handle_own_line_comment_after_branch<'a>(
return CommentPlacement::trailing(last_child_in_parent, comment);
}
Ordering::Greater => {
if let Some(nested_child) = last_child_in_body(last_child_in_parent) {
if let Some(nested_child) = last_child_in_parent.last_child_in_body() {
// The comment belongs to the inner block.
parent = Some(last_child_in_parent);
last_child_in_parent = nested_child;
Expand Down Expand Up @@ -2176,65 +2176,6 @@ where
right.is_some_and(|right| left.ptr_eq(right.into()))
}

/// The last child of the last branch, if the node has multiple branches.
fn last_child_in_body(node: AnyNodeRef) -> Option<AnyNodeRef> {
let body = match node {
AnyNodeRef::StmtFunctionDef(ast::StmtFunctionDef { body, .. })
| AnyNodeRef::StmtClassDef(ast::StmtClassDef { body, .. })
| AnyNodeRef::StmtWith(ast::StmtWith { body, .. })
| AnyNodeRef::MatchCase(MatchCase { body, .. })
| AnyNodeRef::ExceptHandlerExceptHandler(ast::ExceptHandlerExceptHandler {
body, ..
})
| AnyNodeRef::ElifElseClause(ast::ElifElseClause { body, .. }) => body,
AnyNodeRef::StmtIf(ast::StmtIf {
body,
elif_else_clauses,
..
}) => elif_else_clauses.last().map_or(body, |clause| &clause.body),

AnyNodeRef::StmtFor(ast::StmtFor { body, orelse, .. })
| AnyNodeRef::StmtWhile(ast::StmtWhile { body, orelse, .. }) => {
if orelse.is_empty() {
body
} else {
orelse
}
}

AnyNodeRef::StmtMatch(ast::StmtMatch { cases, .. }) => {
return cases.last().map(AnyNodeRef::from);
}

AnyNodeRef::StmtTry(ast::StmtTry {
body,
handlers,
orelse,
finalbody,
..
}) => {
if finalbody.is_empty() {
if orelse.is_empty() {
if handlers.is_empty() {
body
} else {
return handlers.last().map(AnyNodeRef::from);
}
} else {
orelse
}
} else {
finalbody
}
}

// Not a node that contains an indented child node.
_ => return None,
};

body.last().map(AnyNodeRef::from)
}

/// Returns `true` if `statement` is the first statement in an alternate `body` (e.g. the else of an if statement)
fn is_first_statement_in_alternate_body(statement: AnyNodeRef, has_body: AnyNodeRef) -> bool {
match has_body {
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_python_formatter/src/expression/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ impl Format<PyFormatContext<'_>> for NormalizedString<'_> {

bitflags! {
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(super) struct StringPrefix: u8 {
pub(crate) struct StringPrefix: u8 {
const UNICODE = 0b0000_0001;
/// `r"test"`
const RAW = 0b0000_0010;
Expand All @@ -508,7 +508,7 @@ bitflags! {
}

impl StringPrefix {
pub(super) fn parse(input: &str) -> StringPrefix {
pub(crate) fn parse(input: &str) -> StringPrefix {
let chars = input.chars();
let mut prefix = StringPrefix::empty();

Expand All @@ -533,15 +533,15 @@ impl StringPrefix {
prefix
}

pub(super) const fn text_len(self) -> TextSize {
pub(crate) const fn text_len(self) -> TextSize {
TextSize::new(self.bits().count_ones())
}

pub(super) const fn is_raw_string(self) -> bool {
pub(crate) const fn is_raw_string(self) -> bool {
self.contains(StringPrefix::RAW) || self.contains(StringPrefix::RAW_UPPER)
}

pub(super) const fn is_fstring(self) -> bool {
pub(crate) const fn is_fstring(self) -> bool {
self.contains(StringPrefix::F_STRING)
}
}
Expand Down
Loading

0 comments on commit 9d6354c

Please sign in to comment.