Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-quotes] Fix Autofix Error (Q000, Q002) #10199

Merged
merged 14 commits into from Mar 18, 2024
@@ -0,0 +1,9 @@
class SingleLineDocstrings():
""'Start with empty string' ' and lint docstring safely'
""" Not a docstring """

def foo(self, bar="""not a docstring"""):
""'Start with empty string' ' and lint docstring safely'
pass

class Nested(foo()[:]): ""'Start with empty string' ' and lint docstring safely'; pass
@@ -0,0 +1,9 @@
class SingleLineDocstrings():
"Do not"' start with empty string' ' and lint docstring safely'
""" Not a docstring """

def foo(self, bar="""not a docstring"""):
"Do not"' start with empty string' ' and lint docstring safely'
pass

class Nested(foo()[:]): "Do not"' start with empty string' ' and lint docstring safely'; pass
@@ -0,0 +1,5 @@
""'Start with empty string' ' and lint docstring safely'

def foo():
pass
""" this is not a docstring """
@@ -0,0 +1,5 @@
"Do not"' start with empty string' ' and lint docstring safely'

def foo():
pass
""" this is not a docstring """
@@ -0,0 +1,9 @@
class SingleLineDocstrings():
''"Start with empty string" ' and lint docstring safely'
''' Not a docstring '''

def foo(self, bar='''not a docstring'''):
''"Start with empty string" ' and lint docstring safely'
pass

class Nested(foo()[:]): ''"Start with empty string" ' and lint docstring safely'; pass
@@ -0,0 +1,9 @@
class SingleLineDocstrings():
'Do not'" start with empty string" ' and lint docstring safely'
''' Not a docstring '''

def foo(self, bar='''not a docstring'''):
'Do not'" start with empty string" ' and lint docstring safely'
pass

class Nested(foo()[:]): 'Do not'" start with empty string" ' and lint docstring safely'; pass
@@ -0,0 +1,5 @@
''"Start with empty string" ' and lint docstring safely'

def foo():
pass
""" this is not a docstring """
@@ -0,0 +1,5 @@
'Do not'" start with empty string" ' and lint docstring safely'

def foo():
pass
""" this is not a docstring """
@@ -0,0 +1,2 @@
s = ""'Start with empty string' ' and lint docstring safely'
s = "Do not"' start with empty string' ' and lint docstring safely'
@@ -0,0 +1,2 @@
s = ''"Start with empty string" ' and lint docstring safely'
s = 'Do not'" start with empty string" ' and lint docstring safely'
10 changes: 10 additions & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/mod.rs
Expand Up @@ -24,6 +24,7 @@ mod tests {
#[test_case(Path::new("doubles_multiline_string.py"))]
#[test_case(Path::new("doubles_noqa.py"))]
#[test_case(Path::new("doubles_wrapped.py"))]
#[test_case(Path::new("doubles_would_be_triple_quotes.py"))]
fn require_singles(path: &Path) -> Result<()> {
let snapshot = format!("require_singles_over_{}", path.to_string_lossy());
let diagnostics = test_path(
Expand Down Expand Up @@ -93,6 +94,7 @@ mod tests {
#[test_case(Path::new("singles_multiline_string.py"))]
#[test_case(Path::new("singles_noqa.py"))]
#[test_case(Path::new("singles_wrapped.py"))]
#[test_case(Path::new("singles_would_be_triple_quotes.py"))]
fn require_doubles(path: &Path) -> Result<()> {
let snapshot = format!("require_doubles_over_{}", path.to_string_lossy());
let diagnostics = test_path(
Expand Down Expand Up @@ -127,6 +129,10 @@ mod tests {
#[test_case(Path::new("docstring_singles_module_singleline.py"))]
#[test_case(Path::new("docstring_singles_class.py"))]
#[test_case(Path::new("docstring_singles_function.py"))]
#[test_case(Path::new("docstring_singles_mixed_quotes_module_singleline_var_1.py"))]
#[test_case(Path::new("docstring_singles_mixed_quotes_module_singleline_var_2.py"))]
#[test_case(Path::new("docstring_singles_mixed_quotes_class_var_1.py"))]
#[test_case(Path::new("docstring_singles_mixed_quotes_class_var_2.py"))]
fn require_docstring_doubles(path: &Path) -> Result<()> {
let snapshot = format!("require_docstring_doubles_over_{}", path.to_string_lossy());
let diagnostics = test_path(
Expand Down Expand Up @@ -161,6 +167,10 @@ mod tests {
#[test_case(Path::new("docstring_singles_module_singleline.py"))]
#[test_case(Path::new("docstring_singles_class.py"))]
#[test_case(Path::new("docstring_singles_function.py"))]
#[test_case(Path::new("docstring_doubles_mixed_quotes_module_singleline_var_1.py"))]
#[test_case(Path::new("docstring_doubles_mixed_quotes_module_singleline_var_2.py"))]
#[test_case(Path::new("docstring_doubles_mixed_quotes_class_var_1.py"))]
#[test_case(Path::new("docstring_doubles_mixed_quotes_class_var_2.py"))]
fn require_docstring_singles(path: &Path) -> Result<()> {
let snapshot = format!("require_docstring_singles_over_{}", path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Expand Up @@ -2,7 +2,7 @@ use ruff_python_parser::lexer::LexResult;
use ruff_python_parser::Tok;
use ruff_text_size::{TextRange, TextSize};

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_source_file::Locator;

Expand Down Expand Up @@ -44,7 +44,9 @@ pub struct BadQuotesInlineString {
preferred_quote: Quote,
}

impl AlwaysFixableViolation for BadQuotesInlineString {
impl Violation for BadQuotesInlineString {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let BadQuotesInlineString { preferred_quote } = self;
Expand All @@ -54,11 +56,11 @@ impl AlwaysFixableViolation for BadQuotesInlineString {
}
}

fn fix_title(&self) -> String {
fn fix_title(&self) -> Option<String> {
let BadQuotesInlineString { preferred_quote } = self;
match preferred_quote {
Quote::Double => "Replace single quotes with double quotes".to_string(),
Quote::Single => "Replace double quotes with single quotes".to_string(),
Quote::Double => Some("Replace single quotes with double quotes".to_string()),
Quote::Single => Some("Replace double quotes with single quotes".to_string()),
}
}
}
Expand Down Expand Up @@ -155,7 +157,9 @@ pub struct BadQuotesDocstring {
preferred_quote: Quote,
}

impl AlwaysFixableViolation for BadQuotesDocstring {
impl Violation for BadQuotesDocstring {
const FIX_AVAILABILITY: FixAvailability = FixAvailability::Sometimes;

#[derive_message_formats]
fn message(&self) -> String {
let BadQuotesDocstring { preferred_quote } = self;
Expand All @@ -165,11 +169,11 @@ impl AlwaysFixableViolation for BadQuotesDocstring {
}
}

fn fix_title(&self) -> String {
fn fix_title(&self) -> Option<String> {
let BadQuotesDocstring { preferred_quote } = self;
match preferred_quote {
Quote::Double => "Replace single quotes docstring with double quotes".to_string(),
Quote::Single => "Replace double quotes docstring with single quotes".to_string(),
Quote::Double => Some("Replace single quotes docstring with double quotes".to_string()),
Quote::Single => Some("Replace double quotes docstring with single quotes".to_string()),
}
}
}
Expand All @@ -188,10 +192,10 @@ const fn good_multiline_ending(quote: Quote) -> &'static str {
}
}

const fn good_docstring(quote: Quote) -> &'static str {
const fn good_docstring(quote: Quote) -> char {
match quote {
Quote::Double => "\"",
Quote::Single => "'",
Quote::Double => '"',
Quote::Single => '\'',
}
}

Expand All @@ -203,6 +207,12 @@ struct Trivia<'a> {
is_multiline: bool,
}

impl Trivia<'_> {
fn has_empty_text(&self) -> bool {
self.raw_text == "\"\"" || self.raw_text == "''"
}
}

impl<'a> From<&'a str> for Trivia<'a> {
fn from(value: &'a str) -> Self {
// Remove any prefixes (e.g., remove `u` from `u"foo"`).
Expand Down Expand Up @@ -231,12 +241,38 @@ impl<'a> From<&'a str> for Trivia<'a> {
}
}

/// Returns `true` if the [`TextRange`] is preceded by two consecutive quotes.
fn text_starts_at_consecutive_quote(locator: &Locator, range: TextRange, quote: Quote) -> bool {
let mut previous_two_chars = locator.up_to(range.start()).chars().rev();
previous_two_chars.next() == Some(good_docstring(quote))
&& previous_two_chars.next() == Some(good_docstring(quote))
}

/// Returns `true` if the [`TextRange`] ends at a quote character.
fn text_ends_at_quote(locator: &Locator, range: TextRange, quote: Quote) -> bool {
locator
.after(range.end())
.starts_with(good_docstring(quote))
}

/// Q002
fn docstring(locator: &Locator, range: TextRange, settings: &LinterSettings) -> Option<Diagnostic> {
let quotes_settings = &settings.flake8_quotes;

let text = locator.slice(range);
let trivia: Trivia = text.into();
if trivia.has_empty_text()
&& text_ends_at_quote(locator, range, settings.flake8_quotes.docstring_quotes)
{
// Fixing this would result in a one-sided multi-line docstring, which would
// introduce a syntax error.
return Some(Diagnostic::new(
BadQuotesDocstring {
preferred_quote: quotes_settings.docstring_quotes,
},
range,
));
}

if trivia
.raw_text
Expand All @@ -253,7 +289,9 @@ fn docstring(locator: &Locator, range: TextRange, settings: &LinterSettings) ->
);
let quote_count = if trivia.is_multiline { 3 } else { 1 };
let string_contents = &trivia.raw_text[quote_count..trivia.raw_text.len() - quote_count];
let quote = good_docstring(quotes_settings.docstring_quotes).repeat(quote_count);
let quote = good_docstring(quotes_settings.docstring_quotes)
.to_string()
.repeat(quote_count);
let mut fixed_contents =
String::with_capacity(trivia.prefix.len() + string_contents.len() + quote.len() * 2);
fixed_contents.push_str(trivia.prefix);
Expand Down Expand Up @@ -344,6 +382,42 @@ fn strings(
// If we're not using the preferred type, only allow use to avoid escapes.
&& !relax_quote
{
if trivia.has_empty_text()
&& text_ends_at_quote(locator, *range, settings.flake8_quotes.inline_quotes)
{
// Fixing this would introduce a syntax error. For example, changing the initial
// single quotes to double quotes would result in a syntax error:
// ```python
// ''"assert" ' SAM macro definitions '''
// ```
diagnostics.push(Diagnostic::new(
BadQuotesInlineString {
preferred_quote: quotes_settings.inline_quotes,
},
*range,
));
continue;
}

if text_starts_at_consecutive_quote(
locator,
*range,
settings.flake8_quotes.inline_quotes,
) {
// Fixing this would introduce a syntax error. For example, changing the double
// doubles to single quotes would result in a syntax error:
// ```python
// ''"assert" ' SAM macro definitions '''
// ```
diagnostics.push(Diagnostic::new(
BadQuotesInlineString {
preferred_quote: quotes_settings.inline_quotes,
},
*range,
));
continue;
}

let mut diagnostic = Diagnostic::new(
BadQuotesInlineString {
preferred_quote: quotes_settings.inline_quotes,
Expand Down
@@ -0,0 +1,56 @@
---
source: crates/ruff_linter/src/rules/flake8_quotes/mod.rs
---
docstring_singles_mixed_quotes_class_var_1.py:2:5: Q002 Single quote docstring found but double quotes preferred
|
1 | class SingleLineDocstrings():
2 | ''"Start with empty string" ' and lint docstring safely'
| ^^ Q002
3 | ''' Not a docstring '''
|
= help: Replace single quotes docstring with double quotes

docstring_singles_mixed_quotes_class_var_1.py:2:7: Q000 Double quotes found but single quotes preferred
|
1 | class SingleLineDocstrings():
2 | ''"Start with empty string" ' and lint docstring safely'
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Q000
3 | ''' Not a docstring '''
|
= help: Replace double quotes with single quotes

docstring_singles_mixed_quotes_class_var_1.py:6:9: Q002 Single quote docstring found but double quotes preferred
|
5 | def foo(self, bar='''not a docstring'''):
6 | ''"Start with empty string" ' and lint docstring safely'
| ^^ Q002
7 | pass
|
= help: Replace single quotes docstring with double quotes

docstring_singles_mixed_quotes_class_var_1.py:6:11: Q000 Double quotes found but single quotes preferred
|
5 | def foo(self, bar='''not a docstring'''):
6 | ''"Start with empty string" ' and lint docstring safely'
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Q000
7 | pass
|
= help: Replace double quotes with single quotes

docstring_singles_mixed_quotes_class_var_1.py:9:29: Q002 Single quote docstring found but double quotes preferred
|
7 | pass
8 |
9 | class Nested(foo()[:]): ''"Start with empty string" ' and lint docstring safely'; pass
| ^^ Q002
|
= help: Replace single quotes docstring with double quotes

docstring_singles_mixed_quotes_class_var_1.py:9:31: Q000 Double quotes found but single quotes preferred
|
7 | pass
8 |
9 | class Nested(foo()[:]): ''"Start with empty string" ' and lint docstring safely'; pass
| ^^^^^^^^^^^^^^^^^^^^^^^^^ Q000
|
= help: Replace double quotes with single quotes