Skip to content

Commit

Permalink
correctly extract section_name from sphinx docstring
Browse files Browse the repository at this point in the history
  • Loading branch information
augustelalande committed Sep 10, 2024
1 parent 52db1b0 commit c790e03
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 28 deletions.
13 changes: 13 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,3 +619,16 @@ def another_valid_google_style_docstring(a: str) -> str:
"""
return a


def foo(dag_id: str, keep_records_in_log: bool = True) -> int:
"""
Delete a DAG by a dag_id.
:param dag_id: the dag_id of the DAG to delete
:param keep_records_in_log: whether keep records of the given dag_id
in the Log table in the backend database (for reasons like auditing).
The default value is True.
:return count of deleted dags
"""
return 0
60 changes: 39 additions & 21 deletions crates/ruff_linter/src/docstrings/sections.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use std::fmt::{Debug, Formatter};
use std::iter::FusedIterator;

use ruff_python_ast::docstrings::{leading_space, leading_words, sphinx_title};
use ruff_python_ast::docstrings::{
leading_space, leading_space_and_colon, leading_words, sphinx_section_name,
};
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use strum_macros::EnumIter;

Expand Down Expand Up @@ -189,25 +191,47 @@ impl<'a> SectionContexts<'a> {
let mut previous_line = lines.next();

while let Some(line) = lines.next() {
if let Some(section_kind) = suspected_as_section(&line, style) {
if matches!(style, SectionStyle::Sphinx) {
let section_name = sphinx_section_name(&line);
if let Some(kind) = SectionKind::from_str(section_name) {
if style.sections().contains(&kind) {
let indent = leading_space_and_colon(&line);
let indent_size = indent.text_len();
let section_name_size = section_name.text_len();

if let Some(mut last) = last.take() {
last.range = TextRange::new(last.start(), line.start());
contexts.push(last);
}

last = Some(SectionContextData {
kind,
indent_size: indent.text_len(),
name_range: TextRange::at(
line.start() + indent_size,
section_name_size,
),
range: TextRange::empty(line.start()),
summary_full_end: line.full_end(),
});
}
}
} else if let Some(section_kind) = suspected_as_section(&line, style) {
let indent = leading_space(&line);
let indent_size = indent.text_len();

let section_name = leading_words(&line);
let section_name_size = section_name.text_len();

// Suspected sphinx matches are always correct
if matches!(style, SectionStyle::Sphinx)
|| is_docstring_section(
&line,
indent_size,
section_name_size,
section_kind,
last.as_ref(),
previous_line.as_ref(),
lines.peek(),
)
{
if is_docstring_section(
&line,
indent_size,
section_name_size,
section_kind,
last.as_ref(),
previous_line.as_ref(),
lines.peek(),
) {
if let Some(mut last) = last.take() {
last.range = TextRange::new(last.start(), line.start());
contexts.push(last);
Expand Down Expand Up @@ -447,13 +471,7 @@ impl Debug for SectionContext<'_> {
}

fn suspected_as_section(line: &str, style: SectionStyle) -> Option<SectionKind> {
if matches!(style, SectionStyle::Sphinx) {
if let Some(kind) = SectionKind::from_str(sphinx_title(line)) {
if style.sections().contains(&kind) {
return Some(kind);
}
}
} else if let Some(kind) = SectionKind::from_str(leading_words(line)) {
if let Some(kind) = SectionKind::from_str(leading_words(line)) {
if style.sections().contains(&kind) {
return Some(kind);
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1633,7 +1633,7 @@ fn blanks_and_section_underline(

checker.diagnostics.push(diagnostic);
}
if checker.enabled(Rule::EmptyDocstringSection) {
if !style.is_sphinx() && checker.enabled(Rule::EmptyDocstringSection) {
checker.diagnostics.push(Diagnostic::new(
EmptyDocstringSection {
name: context.section_name().to_string(),
Expand All @@ -1651,7 +1651,7 @@ fn common_section(
next: Option<&SectionContext>,
style: SectionStyle,
) {
if checker.enabled(Rule::CapitalizeSectionName) {
if !style.is_sphinx() && checker.enabled(Rule::CapitalizeSectionName) {
let capitalized_section_name = context.kind().as_str();
if context.section_name() != capitalized_section_name {
let section_range = context.section_name_range();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,43 @@ sections.py:224:5: D410 [*] Missing blank line after section ("Returns")
227 |+
227 228 | Raises:
228 229 | My attention.
229 230 |
229 230 |

sections.py:628:6: D410 [*] Missing blank line after section ("param")
|
626 | Delete a DAG by a dag_id.
627 |
628 | :param dag_id: the dag_id of the DAG to delete
| ^^^^^ D410
629 | :param keep_records_in_log: whether keep records of the given dag_id
630 | in the Log table in the backend database (for reasons like auditing).
|
= help: Add blank line after "param"

Safe fix
626 626 | Delete a DAG by a dag_id.
627 627 |
628 628 | :param dag_id: the dag_id of the DAG to delete
629 |+
629 630 | :param keep_records_in_log: whether keep records of the given dag_id
630 631 | in the Log table in the backend database (for reasons like auditing).
631 632 | The default value is True.

sections.py:629:6: D410 [*] Missing blank line after section ("param")
|
628 | :param dag_id: the dag_id of the DAG to delete
629 | :param keep_records_in_log: whether keep records of the given dag_id
| ^^^^^ D410
630 | in the Log table in the backend database (for reasons like auditing).
631 | The default value is True.
|
= help: Add blank line after "param"

Safe fix
629 629 | :param keep_records_in_log: whether keep records of the given dag_id
630 630 | in the Log table in the backend database (for reasons like auditing).
631 631 | The default value is True.
632 |+
632 633 | :return count of deleted dags
633 634 | """
634 635 | return 0
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,43 @@ sections.py:227:5: D411 [*] Missing blank line before section ("Raises")
227 |+
227 228 | Raises:
228 229 | My attention.
229 230 |
229 230 |

sections.py:629:6: D411 [*] Missing blank line before section ("param")
|
628 | :param dag_id: the dag_id of the DAG to delete
629 | :param keep_records_in_log: whether keep records of the given dag_id
| ^^^^^ D411
630 | in the Log table in the backend database (for reasons like auditing).
631 | The default value is True.
|
= help: Add blank line before "param"

ℹ Safe fix
626 626 | Delete a DAG by a dag_id.
627 627 |
628 628 | :param dag_id: the dag_id of the DAG to delete
629 |+
629 630 | :param keep_records_in_log: whether keep records of the given dag_id
630 631 | in the Log table in the backend database (for reasons like auditing).
631 632 | The default value is True.

sections.py:632:6: D411 [*] Missing blank line before section ("return")
|
630 | in the Log table in the backend database (for reasons like auditing).
631 | The default value is True.
632 | :return count of deleted dags
| ^^^^^^ D411
633 | """
634 | return 0
|
= help: Add blank line before "return"

ℹ Safe fix
629 629 | :param keep_records_in_log: whether keep records of the given dag_id
630 630 | in the Log table in the backend database (for reasons like auditing).
631 631 | The default value is True.
632 |+
632 633 | :return count of deleted dags
633 634 | """
634 635 | return 0
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,23 @@ sections.py:590:5: D413 [*] Missing blank line after last section ("Parameters")
596 |+
596 597 | """
597 598 |
598 599 |
598 599 |

sections.py:632:6: D413 [*] Missing blank line after last section ("return")
|
630 | in the Log table in the backend database (for reasons like auditing).
631 | The default value is True.
632 | :return count of deleted dags
| ^^^^^^ D413
633 | """
634 | return 0
|
= help: Add blank line after "return"

Safe fix
630 630 | in the Log table in the backend database (for reasons like auditing).
631 631 | The default value is True.
632 632 | :return count of deleted dags
633 |+
633 634 | """
634 635 | return 0
10 changes: 8 additions & 2 deletions crates/ruff_python_ast/src/docstrings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,14 @@ pub fn clean_space(indentation: &str) -> String {
.collect()
}

/// Sphinx section title.
pub fn sphinx_title(line: &str) -> &str {
/// Extract the leading whitespace and colon from a line of text within a Python docstring.
pub fn leading_space_and_colon(line: &str) -> &str {
line.find(|char: char| !char.is_whitespace() && char != ':')
.map_or(line, |index| &line[..index])
}

/// Sphinx section section name.
pub fn sphinx_section_name(line: &str) -> &str {
let line = line.trim_start_matches([' ', ':']);
line.find(|char: char| !char.is_alphanumeric())
.map_or(line, |index| &line[..index])
Expand Down

0 comments on commit c790e03

Please sign in to comment.