From c790e030b52693c6e5904b956b79ad077c2a40ed Mon Sep 17 00:00:00 2001 From: augustelalande Date: Tue, 10 Sep 2024 16:25:30 -0400 Subject: [PATCH] correctly extract section_name from sphinx docstring --- .../test/fixtures/pydocstyle/sections.py | 13 ++++ crates/ruff_linter/src/docstrings/sections.rs | 60 ++++++++++++------- .../src/rules/pydocstyle/rules/sections.rs | 4 +- ...__pydocstyle__tests__D410_sections.py.snap | 41 ++++++++++++- ...__pydocstyle__tests__D411_sections.py.snap | 41 ++++++++++++- ...__pydocstyle__tests__D413_sections.py.snap | 21 ++++++- crates/ruff_python_ast/src/docstrings.rs | 10 +++- 7 files changed, 162 insertions(+), 28 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py index d04c38a74bff9..4c59e8a25eec9 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/sections.py @@ -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 diff --git a/crates/ruff_linter/src/docstrings/sections.rs b/crates/ruff_linter/src/docstrings/sections.rs index 1a890fe3b4dab..b8fd73d0aa4e4 100644 --- a/crates/ruff_linter/src/docstrings/sections.rs +++ b/crates/ruff_linter/src/docstrings/sections.rs @@ -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; @@ -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); @@ -447,13 +471,7 @@ impl Debug for SectionContext<'_> { } fn suspected_as_section(line: &str, style: SectionStyle) -> Option { - 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); } diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs index 54272e43f8342..f11119dd913f9 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/sections.rs @@ -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(), @@ -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(); diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D410_sections.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D410_sections.py.snap index e8c466e1387ca..e9b243968b0d9 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D410_sections.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D410_sections.py.snap @@ -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 diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D411_sections.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D411_sections.py.snap index b516028f32fe3..7eb0852217a65 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D411_sections.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D411_sections.py.snap @@ -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 diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_sections.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_sections.py.snap index 223c7c38218af..988ff3db450f1 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_sections.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D413_sections.py.snap @@ -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 diff --git a/crates/ruff_python_ast/src/docstrings.rs b/crates/ruff_python_ast/src/docstrings.rs index 6b9ca7ba3b0ce..fc4df3c57f6eb 100644 --- a/crates/ruff_python_ast/src/docstrings.rs +++ b/crates/ruff_python_ast/src/docstrings.rs @@ -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])