Skip to content

Commit

Permalink
Always prefer double quotes for docstrings and triple-quoted srings (#…
Browse files Browse the repository at this point in the history
…7680)

## Summary

At present, `quote-style` is used universally. However, [PEP
8](https://peps.python.org/pep-0008/) and [PEP
257](https://peps.python.org/pep-0257/) suggest that while either single
or double quotes are acceptable in general (as long as they're
consistent), docstrings and triple-quoted strings should always use
double quotes. In our research, the vast majority of Ruff users that
enable the `flake8-quotes` rules only enable them for inline strings
(i.e., non-triple-quoted strings).

Additionally, many Black forks (like Blue and Pyink) use double quotes
for docstrings and triple-quoted strings.

Our decision for now is to always prefer double quotes for triple-quoted
strings (which should include docstrings). Based on feedback, we may
consider adding additional options (e.g., a `"preserve"` mode, to avoid
changing quotes; or a `"multiline-quote-style"` to override this).

Closes #7615.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Sep 28, 2023
1 parent f62b4c8 commit 695dbbc
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 68 deletions.
78 changes: 44 additions & 34 deletions crates/ruff_python_formatter/src/expression/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,32 +323,40 @@ impl StringPart {
self,
quoting: Quoting,
locator: &'a Locator,
quote_style: QuoteStyle,
configured_style: QuoteStyle,
) -> NormalizedString<'a> {
// Per PEP 8 and PEP 257, always prefer double quotes for docstrings and triple-quoted
// strings. (We assume docstrings are always triple-quoted.)
let preferred_style = if self.quotes.triple {
QuoteStyle::Double
} else {
configured_style
};

let raw_content = locator.slice(self.content_range);

let preferred_quotes = match quoting {
let quotes = match quoting {
Quoting::Preserve => self.quotes,
Quoting::CanChange => {
if self.prefix.is_raw_string() {
preferred_quotes_raw(raw_content, self.quotes, quote_style)
choose_quotes_raw(raw_content, self.quotes, preferred_style)
} else {
preferred_quotes(raw_content, self.quotes, quote_style)
choose_quotes(raw_content, self.quotes, preferred_style)
}
}
};

let normalized = normalize_string(
locator.slice(self.content_range),
preferred_quotes,
quotes,
self.prefix.is_raw_string(),
);

NormalizedString {
prefix: self.prefix,
content_range: self.content_range,
text: normalized,
quotes: preferred_quotes,
quotes,
}
}
}
Expand Down Expand Up @@ -460,16 +468,17 @@ impl Format<PyFormatContext<'_>> for StringPrefix {
}
}

/// Detects the preferred quotes for raw string `input`.
/// The configured quote style is preferred unless `input` contains unescaped quotes of the
/// configured style. For example, `r"foo"` is preferred over `r'foo'` if the configured
/// quote style is double quotes.
fn preferred_quotes_raw(
/// Choose the appropriate quote style for a raw string.
///
/// The preferred quote style is chosen unless the string contains unescaped quotes of the
/// preferred style. For example, `r"foo"` is chosen over `r'foo'` if the preferred quote
/// style is double quotes.
fn choose_quotes_raw(
input: &str,
quotes: StringQuotes,
configured_style: QuoteStyle,
preferred_style: QuoteStyle,
) -> StringQuotes {
let configured_quote_char = configured_style.as_char();
let preferred_quote_char = preferred_style.as_char();
let mut chars = input.chars().peekable();
let contains_unescaped_configured_quotes = loop {
match chars.next() {
Expand All @@ -478,7 +487,7 @@ fn preferred_quotes_raw(
chars.next();
}
// `"` or `'`
Some(c) if c == configured_quote_char => {
Some(c) if c == preferred_quote_char => {
if !quotes.triple {
break true;
}
Expand All @@ -487,13 +496,13 @@ fn preferred_quotes_raw(
// We can't turn `r'''\""'''` into `r"""\"""""`, this would confuse the parser
// about where the closing triple quotes start
None => break true,
Some(next) if *next == configured_quote_char => {
Some(next) if *next == preferred_quote_char => {
// `""` or `''`
chars.next();

// We can't turn `r'''""'''` into `r""""""""`, nor can we have
// `"""` or `'''` respectively inside the string
if chars.peek().is_none() || chars.peek() == Some(&configured_quote_char) {
if chars.peek().is_none() || chars.peek() == Some(&preferred_quote_char) {
break true;
}
}
Expand All @@ -510,41 +519,42 @@ fn preferred_quotes_raw(
style: if contains_unescaped_configured_quotes {
quotes.style
} else {
configured_style
preferred_style
},
}
}

/// Detects the preferred quotes for `input`.
/// * single quoted strings: The preferred quote style is the one that requires less escape sequences.
/// * triple quoted strings: Use double quotes except the string contains a sequence of `"""`.
fn preferred_quotes(
input: &str,
quotes: StringQuotes,
configured_style: QuoteStyle,
) -> StringQuotes {
let preferred_style = if quotes.triple {
/// Choose the appropriate quote style for a string.
///
/// For single quoted strings, the preferred quote style is used, unless the alternative quote style
/// would require fewer escapes.
///
/// For triple quoted strings, the preferred quote style is always used, unless the string contains
/// a triplet of the quote character (e.g., if double quotes are preferred, double quotes will be
/// used unless the string contains `"""`).
fn choose_quotes(input: &str, quotes: StringQuotes, preferred_style: QuoteStyle) -> StringQuotes {
let style = if quotes.triple {
// True if the string contains a triple quote sequence of the configured quote style.
let mut uses_triple_quotes = false;
let mut chars = input.chars().peekable();

while let Some(c) = chars.next() {
let configured_quote_char = configured_style.as_char();
let preferred_quote_char = preferred_style.as_char();
match c {
'\\' => {
if matches!(chars.peek(), Some('"' | '\\')) {
chars.next();
}
}
// `"` or `'`
c if c == configured_quote_char => {
c if c == preferred_quote_char => {
match chars.peek().copied() {
Some(c) if c == configured_quote_char => {
Some(c) if c == preferred_quote_char => {
// `""` or `''`
chars.next();

match chars.peek().copied() {
Some(c) if c == configured_quote_char => {
Some(c) if c == preferred_quote_char => {
// `"""` or `'''`
chars.next();
uses_triple_quotes = true;
Expand Down Expand Up @@ -579,7 +589,7 @@ fn preferred_quotes(
// Keep the existing quote style.
quotes.style
} else {
configured_style
preferred_style
}
} else {
let mut single_quotes = 0u32;
Expand All @@ -599,7 +609,7 @@ fn preferred_quotes(
}
}

match configured_style {
match preferred_style {
QuoteStyle::Single => {
if single_quotes > double_quotes {
QuoteStyle::Double
Expand All @@ -619,7 +629,7 @@ fn preferred_quotes(

StringQuotes {
triple: quotes.triple,
style: preferred_style,
style,
}
}

Expand Down Expand Up @@ -668,7 +678,7 @@ impl Format<PyFormatContext<'_>> for StringQuotes {
}

/// Adds the necessary quote escapes and removes unnecessary escape sequences when quoting `input`
/// with the provided `style`.
/// with the provided [`StringQuotes`] style.
///
/// Returns the normalized string and whether it contains new lines.
fn normalize_string(input: &str, quotes: StringQuotes, is_raw: bool) -> Cow<str> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,17 +320,17 @@ if True:
b'This string will not include \
backslashes or newline characters.'
b'''Multiline
b"""Multiline
String \"
'''
"""
b'''Multiline
b"""Multiline
String \'
'''
"""
b'''Multiline
b"""Multiline
String ""
'''
"""
b'''Multiline
String """
Expand All @@ -346,9 +346,9 @@ String '''
b"""Multiline
String '"""
b'''Multiline
b"""Multiline
String \"\"\"
'''
"""
# String continuation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,17 +365,17 @@ if True:
'This string will not include \
backslashes or newline characters.'
'''Multiline
"""Multiline
String \"
'''
"""
'''Multiline
"""Multiline
String \'
'''
"""
'''Multiline
"""Multiline
String ""
'''
"""
'''Multiline
String """
Expand All @@ -391,9 +391,9 @@ String '''
"""Multiline
String '"""
'''Multiline
"""Multiline
String \"\"\"
'''
"""
# String continuation
Expand Down Expand Up @@ -471,16 +471,16 @@ test_particular = [
# Regression test for https://github.com/astral-sh/ruff/issues/5893
x = (
'''aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'''
'''bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'''
"""aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"""
"""bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"""
)
x = (
f'''aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'''
f'''bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'''
f"""aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"""
f"""bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"""
)
x = (
b'''aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'''
b'''bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'''
b"""aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"""
b"""bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"""
)
# https://github.com/astral-sh/ruff/issues/7460
Expand Down
28 changes: 17 additions & 11 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2461,42 +2461,48 @@ pub struct FormatOptions {
default = "false",
value_type = "bool",
example = r#"
# Enable preview style formatting
# Enable preview style formatting.
preview = true
"#
)]
pub preview: Option<bool>,

/// Whether to use 4 spaces or hard tabs for indenting code.
///
/// Defaults to 4 spaces. We care about accessibility; if you do not need tabs for accessibility, we do not recommend you use them.
/// Defaults to 4 spaces. We care about accessibility; if you do not need tabs for
/// accessibility, we do not recommend you use them.
#[option(
default = "space",
value_type = r#""space" | "tab""#,
example = r#"
# Use tabs instead of 4 space indentation
# Use tabs instead of 4 space indentation.
indent-style = "tab"
"#
)]
pub indent_style: Option<IndentStyle>,

/// Whether to prefer single `'` or double `"` quotes for strings and docstrings.
/// Whether to prefer single `'` or double `"` quotes for strings. Defaults to double quotes.
///
/// Ruff may deviate from this option if using the configured quotes would require more escaped quotes:
/// In compliance with [PEP 8](https://peps.python.org/pep-0008/) and [PEP 257](https://peps.python.org/pep-0257/),
/// Ruff prefers double quotes for multiline strings and docstrings, regardless of the
/// configured quote style.
///
/// Ruff may also deviate from this option if using the configured quotes would require
/// escaping quote characters within the string. For example, given:
///
/// ```python
/// a = "It's monday morning"
/// b = "a string without any quotes"
/// a = "a string without any quotes"
/// b = "It's monday morning"
/// ```
///
/// Ruff leaves `a` unchanged when using `quote-style = "single"` because it is otherwise
/// necessary to escape the `'` which leads to less readable code: `'It\'s monday morning'`.
/// Ruff changes the quotes of `b` to use single quotes.
/// Ruff will change `a` to use single quotes when using `quote-style = "single"`. However,
/// `a` will be unchanged, as converting to single quotes would require the inner `'` to be
/// escaped, which leads to less readable code: `'It\'s monday morning'`.
#[option(
default = r#"double"#,
value_type = r#""double" | "single""#,
example = r#"
# Prefer single quotes over double quotes
# Prefer single quotes over double quotes.
quote-style = "single"
"#
)]
Expand Down
2 changes: 1 addition & 1 deletion ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 695dbbc

Please sign in to comment.