Skip to content

Commit

Permalink
Avoid panics for implicitly-concatenated docstrings
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 17, 2023
1 parent 1dd3cbd commit 3ce7b04
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 21 deletions.
9 changes: 9 additions & 0 deletions crates/ruff/resources/test/fixtures/pydocstyle/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,3 +504,12 @@ def test_incorrect_indent(self, x=1, y=2): # noqa: D207, D213, D407
x: Test argument.
"""


def implicit_string_concatenation():
"""Toggle the gizmo.
Returns
A value of some sort.
""""Extra content"
14 changes: 13 additions & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use crate::checkers::ast::deferred::Deferred;
use crate::docstrings::definition::{
transition_scope, Definition, DefinitionKind, Docstring, Documentable,
};
use crate::fs::relativize_path;
use crate::registry::{AsRule, Rule};
use crate::rules::{
flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap,
Expand All @@ -50,7 +51,7 @@ use crate::rules::{
};
use crate::settings::types::PythonVersion;
use crate::settings::{flags, Settings};
use crate::{autofix, docstrings, noqa};
use crate::{autofix, docstrings, noqa, warn_user};

mod deferred;

Expand Down Expand Up @@ -5133,6 +5134,7 @@ impl<'a> Checker<'a> {
}
overloaded_name = flake8_annotations::helpers::overloaded_name(self, &definition);
}

if self.is_stub {
if self.settings.rules.enabled(Rule::DocstringInStub) {
flake8_pyi::rules::docstring_in_stubs(self, definition.docstring);
Expand Down Expand Up @@ -5162,6 +5164,16 @@ impl<'a> Checker<'a> {
Location::new(expr.location.row(), expr.location.column()),
));

if pydocstyle::helpers::should_ignore_docstring(contents) {
warn_user!(
"Docstring at {}:{}:{} contains implicit string concatenation; ignoring...",
relativize_path(self.path),
expr.location.row(),
expr.location.column() + 1
);
continue;
}

let body = str::raw_contents(contents);
let docstring = Docstring {
kind: definition.kind,
Expand Down
5 changes: 4 additions & 1 deletion crates/ruff/src/docstrings/extraction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind};

use crate::docstrings::definition::{Definition, DefinitionKind, Documentable};
use ruff_python_ast::visibility::{Modifier, VisibleScope};

use crate::docstrings::definition::{Definition, DefinitionKind, Documentable};

/// Extract a docstring from a function or class body.
pub fn docstring_from(suite: &[Stmt]) -> Option<&Expr> {
let stmt = suite.first()?;
// Require the docstring to be a standalone expression.
let StmtKind::Expr { value } = &stmt.node else {
return None;
};
// Only match strings.
if !matches!(
&value.node,
ExprKind::Constant {
Expand Down
9 changes: 9 additions & 0 deletions crates/ruff/src/rules/pydocstyle/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::collections::BTreeSet;
use ruff_python_ast::cast;
use ruff_python_ast::helpers::{map_callable, to_call_path};
use ruff_python_ast::newlines::StrExt;
use ruff_python_ast::str::is_implicit_concatenation;

use crate::checkers::ast::Checker;
use crate::docstrings::definition::{Definition, DefinitionKind};
Expand Down Expand Up @@ -60,3 +61,11 @@ pub fn should_ignore_definition(
}
false
}

/// Check if a docstring should be ignored.
pub fn should_ignore_docstring(contents: &str) -> bool {
// Avoid analyzing docstrings that contain implicit string concatenations.
// Python does consider these docstrings, but they're almost certainly a
// user error, and supporting them "properly" is extremely difficult.
is_implicit_concatenation(contents)
}
12 changes: 3 additions & 9 deletions crates/ruff/src/rules/pyupgrade/rules/f_strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ use rustpython_common::format::{
FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate,
};
use rustpython_parser::ast::{Constant, Expr, ExprKind, KeywordData};
use rustpython_parser::{lexer, Mode, Tok};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::{leading_quote, trailing_quote};
use ruff_python_ast::str::{is_implicit_concatenation, leading_quote, trailing_quote};
use ruff_python_ast::types::Range;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -127,13 +126,8 @@ fn try_convert_to_f_string(checker: &Checker, expr: &Expr) -> Option<String> {

let contents = checker.locator.slice(value);

// Tokenize: we need to avoid trying to fix implicit string concatenations.
if lexer::lex(contents, Mode::Module)
.flatten()
.filter(|(_, tok, _)| matches!(tok, Tok::String { .. }))
.count()
> 1
{
// Skip implicit string concatenations.
if is_implicit_concatenation(contents) {
return None;
}

Expand Down
13 changes: 3 additions & 10 deletions crates/ruff/src/rules/pyupgrade/rules/native_literals.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use std::fmt;

use rustpython_parser::ast::{Constant, Expr, ExprKind, Keyword};
use rustpython_parser::{lexer, Mode, Tok};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::str::is_implicit_concatenation;
use ruff_python_ast::types::Range;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -112,16 +112,9 @@ pub fn native_literals(
return;
}

// rust-python merges adjacent string/bytes literals into one node, but we can't
// safely remove the outer call in this situation. We're following pyupgrade
// here and skip.
// Skip implicit string concatenations.
let arg_code = checker.locator.slice(arg);
if lexer::lex_located(arg_code, Mode::Module, arg.location)
.flatten()
.filter(|(_, tok, _)| matches!(tok, Tok::String { .. }))
.count()
> 1
{
if is_implicit_concatenation(arg_code) {
return;
}

Expand Down
11 changes: 11 additions & 0 deletions crates/ruff_python_ast/src/str.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use rustpython_parser::{lexer, Mode, Tok};

/// See: <https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals>
const TRIPLE_QUOTE_STR_PREFIXES: &[&str] = &[
"u\"\"\"", "u'''", "r\"\"\"", "r'''", "U\"\"\"", "U'''", "R\"\"\"", "R'''", "\"\"\"", "'''",
Expand Down Expand Up @@ -67,6 +69,15 @@ pub fn is_triple_quote(content: &str) -> bool {
TRIPLE_QUOTE_STR_PREFIXES.contains(&content) || TRIPLE_QUOTE_BYTE_PREFIXES.contains(&content)
}

/// Return `true` if the string expression is an implicit concatenation.
pub fn is_implicit_concatenation(content: &str) -> bool {
lexer::lex(content, Mode::Module)
.flatten()
.filter(|(_, tok, _)| matches!(tok, Tok::String { .. }))
.nth(1)
.is_some()
}

#[cfg(test)]
mod tests {
use super::{
Expand Down

0 comments on commit 3ce7b04

Please sign in to comment.