Use section name range for all section-related docstring diagnostics #10740
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexWaygood
reviewed
Apr 2, 2024
There was a problem hiding this comment.
Nice, overall this is a big improvement!
I left some comments where I feel like the ranges aren't quite right -- but I don't know how hard it would be to get them perfect. If it would be really hard, for some of them I feel like the existing approach of highlighting the whole range might be better.
.../src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D215_sections.py.snap
Outdated
Show resolved
Hide resolved
Comment on lines
+8
to
+10
| 32 | Returns: | ||
| | ^^^^^^^ D406 | ||
| 33 | ------- |
There was a problem hiding this comment.
The range here could possibly be just the range of the invalid suffix (in this case, the colon at the end of the header), since the fix is just to delete the suffix
There was a problem hiding this comment.
I still feel like for this one, the range is somewhat confusing, because we're specifically complaining about the colon at the end of "Returns:", which is actually excluded from the range here? But no strong opinion :)
There was a problem hiding this comment.
Honestly, I could go either way. I left it as-is for now but I'm open to changing it.
.../src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D408_sections.py.snap
Outdated
Show resolved
Hide resolved
.../src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D409_sections.py.snap
Outdated
Show resolved
Hide resolved
| 216 | """Toggle the gizmo. | ||
| 217 | | ||
| 218 | Short summary | ||
| | ^^^^^^^^^^^^^ D412 |
There was a problem hiding this comment.
Here the range is possibly two lines too high? It should probably be highlighting the blank line below the underline
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| D413 | 3006 | 1503 | 1503 | 0 | 0 |
| D407 | 1940 | 970 | 970 | 0 | 0 |
| D406 | 318 | 159 | 159 | 0 | 0 |
| D412 | 290 | 145 | 145 | 0 | 0 |
| D416 | 16 | 8 | 8 | 0 | 0 |
| D411 | 12 | 6 | 6 | 0 | 0 |
| D405 | 12 | 6 | 6 | 0 | 0 |
| D214 | 6 | 3 | 3 | 0 | 0 |
| D410 | 4 | 2 | 2 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+2802 -2802 violations, +0 -0 fixes in 6 projects; 38 projects unchanged)
RasaHQ/rasa (+2 -2 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
- tests/core/test_tracker_stores.py:812:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + tests/core/test_tracker_stores.py:818:5: D412 [*] No blank lines allowed between a section header and its content ("Example") - tests/shared/nlu/training_data/test_features.py:179:5: D411 [*] Missing blank line before section ("Args") + tests/shared/nlu/training_data/test_features.py:184:5: D411 [*] Missing blank line before section ("Args")
apache/airflow (+45 -45 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- airflow/decorators/base.py:130:5: D407 [*] Missing dashed underline after section ("Example") - airflow/decorators/base.py:130:5: D413 [*] Missing blank line after last section ("Example") + airflow/decorators/base.py:136:5: D407 [*] Missing dashed underline after section ("Example") + airflow/decorators/base.py:136:5: D413 [*] Missing blank line after last section ("Example") - airflow/hooks/filesystem.py:27:5: D405 [*] Section name should be properly capitalized ("example") - airflow/hooks/filesystem.py:27:5: D407 [*] Missing dashed underline after section ("example") - airflow/hooks/filesystem.py:27:5: D413 [*] Missing blank line after last section ("example") + airflow/hooks/filesystem.py:32:5: D405 [*] Section name should be properly capitalized ("example") + airflow/hooks/filesystem.py:32:5: D407 [*] Missing dashed underline after section ("example") + airflow/hooks/filesystem.py:32:5: D413 [*] Missing blank line after last section ("example") ... 80 additional changes omitted for project
bokeh/bokeh (+1262 -1262 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- src/bokeh/__init__.py:63:5: D406 [*] Section name should end with a newline ("Returns") - src/bokeh/__init__.py:63:5: D407 [*] Missing dashed underline after section ("Returns") + src/bokeh/__init__.py:65:5: D406 [*] Section name should end with a newline ("Returns") + src/bokeh/__init__.py:65:5: D407 [*] Missing dashed underline after section ("Returns") - src/bokeh/application/application.py:155:9: D407 [*] Missing dashed underline after section ("Args") + src/bokeh/application/application.py:157:9: D407 [*] Missing dashed underline after section ("Args") - src/bokeh/application/application.py:250:9: D407 [*] Missing dashed underline after section ("Args") - src/bokeh/application/application.py:250:9: D407 [*] Missing dashed underline after section ("Returns") ... 1879 additional changes omitted for rule D407 - src/bokeh/application/application.py:250:9: D413 [*] Missing blank line after last section ("Returns") + src/bokeh/application/application.py:256:9: D413 [*] Missing blank line after last section ("Returns") - src/bokeh/application/handlers/code_runner.py:169:9: D406 [*] Section name should end with a newline ("Returns") + src/bokeh/application/handlers/code_runner.py:171:9: D406 [*] Section name should end with a newline ("Returns") - src/bokeh/application/handlers/directory.py:293:9: D413 [*] Missing blank line after last section ("Returns") + src/bokeh/application/handlers/directory.py:299:9: D413 [*] Missing blank line after last section ("Returns") - src/bokeh/application/handlers/handler.py:199:9: D413 [*] Missing blank line after last section ("Returns") + src/bokeh/application/handlers/handler.py:205:9: D413 [*] Missing blank line after last section ("Returns") ... 53 additional changes omitted for rule D413 - src/bokeh/client/connection.py:168:9: D406 [*] Section name should end with a newline ("Returns") + src/bokeh/client/connection.py:174:9: D406 [*] Section name should end with a newline ("Returns") ... 275 additional changes omitted for rule D406 + src/bokeh/command/subcommand.py:101:5: D412 [*] No blank lines allowed between a section header and its content ("Example") - src/bokeh/command/subcommand.py:79:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + src/bokeh/command/subcommands/file_output.py:100:9: D412 [*] No blank lines allowed between a section header and its content ("Example") - src/bokeh/command/subcommands/file_output.py:65:9: D412 [*] No blank lines allowed between a section header and its content ("Example") ... 2502 additional changes omitted for project
latchbio/latch (+8 -8 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
- latch/resources/tasks.py:317:5: D411 [*] Missing blank line before section ("Args") + latch/resources/tasks.py:322:5: D411 [*] Missing blank line before section ("Args") + latch/types/metadata.py:100:5: D412 [*] No blank lines allowed between a section header and its content ("Example") - latch/types/metadata.py:461:5: D412 [*] No blank lines allowed between a section header and its content ("Example") + latch/types/metadata.py:463:5: D412 [*] No blank lines allowed between a section header and its content ("Example") - latch/types/metadata.py:98:5: D412 [*] No blank lines allowed between a section header and its content ("Example") - latch_cli/auth/oauth2.py:157:9: D410 [*] Missing blank line after section ("Args") - latch_cli/auth/oauth2.py:157:9: D411 [*] Missing blank line before section ("Returns") + latch_cli/auth/oauth2.py:159:9: D410 [*] Missing blank line after section ("Args") + latch_cli/auth/oauth2.py:161:9: D411 [*] Missing blank line before section ("Returns") ... 6 additional changes omitted for project
reflex-dev/reflex (+1445 -1445 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview
- benchmarks/conftest.py:12:5: D413 [*] Missing blank line after last section ("Returns") + benchmarks/conftest.py:17:5: D413 [*] Missing blank line after last section ("Returns") - benchmarks/test_benchmark_compile_components.py:152:5: D413 [*] Missing blank line after last section ("Yields") + benchmarks/test_benchmark_compile_components.py:157:5: D413 [*] Missing blank line after last section ("Yields") - benchmarks/test_benchmark_compile_components.py:174:5: D413 [*] Missing blank line after last section ("Yields") + benchmarks/test_benchmark_compile_components.py:179:5: D413 [*] Missing blank line after last section ("Yields") - benchmarks/test_benchmark_compile_components.py:196:5: D413 [*] Missing blank line after last section ("Yields") - benchmarks/test_benchmark_compile_components.py:19:5: D413 [*] Missing blank line after last section ("Returns") + benchmarks/test_benchmark_compile_components.py:201:5: D413 [*] Missing blank line after last section ("Yields") - benchmarks/test_benchmark_compile_components.py:222:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:224:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:24:5: D413 [*] Missing blank line after last section ("Returns") - benchmarks/test_benchmark_compile_components.py:250:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:252:5: D413 [*] Missing blank line after last section ("Args") - benchmarks/test_benchmark_compile_components.py:275:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:277:5: D413 [*] Missing blank line after last section ("Args") - benchmarks/test_benchmark_compile_components.py:303:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:305:5: D413 [*] Missing blank line after last section ("Args") - benchmarks/test_benchmark_compile_components.py:328:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:330:5: D413 [*] Missing blank line after last section ("Args") - benchmarks/test_benchmark_compile_components.py:356:5: D413 [*] Missing blank line after last section ("Args") + benchmarks/test_benchmark_compile_components.py:358:5: D413 [*] Missing blank line after last section ("Args") - benchmarks/test_benchmark_compile_pages.py:202:5: D413 [*] Missing blank line after last section ("Yields") + benchmarks/test_benchmark_compile_pages.py:207:5: D413 [*] Missing blank line after last section ("Yields") - benchmarks/test_benchmark_compile_pages.py:219:5: D413 [*] Missing blank line after last section ("Yields") ... 2865 additional changes omitted for project
zulip/zulip (+40 -40 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- zerver/data_import/gitter.py:141:5: D406 [*] Section name should end with a newline ("Returns") - zerver/data_import/gitter.py:141:5: D407 [*] Missing dashed underline after section ("Returns") - zerver/data_import/gitter.py:141:5: D413 [*] Missing blank line after last section ("Returns") + zerver/data_import/gitter.py:142:5: D406 [*] Section name should end with a newline ("Returns") + zerver/data_import/gitter.py:142:5: D407 [*] Missing dashed underline after section ("Returns") + zerver/data_import/gitter.py:142:5: D413 [*] Missing blank line after last section ("Returns") - zerver/data_import/gitter.py:41:5: D406 [*] Section name should end with a newline ("Returns") - zerver/data_import/gitter.py:41:5: D407 [*] Missing dashed underline after section ("Returns") - zerver/data_import/gitter.py:41:5: D413 [*] Missing blank line after last section ("Returns") + zerver/data_import/gitter.py:42:5: D406 [*] Section name should end with a newline ("Returns") ... 70 additional changes omitted for project
Changes by rule (9 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| D413 | 3006 | 1503 | 1503 | 0 | 0 |
| D407 | 1940 | 970 | 970 | 0 | 0 |
| D406 | 318 | 159 | 159 | 0 | 0 |
| D412 | 290 | 145 | 145 | 0 | 0 |
| D416 | 16 | 8 | 8 | 0 | 0 |
| D411 | 12 | 6 | 6 | 0 | 0 |
| D405 | 12 | 6 | 6 | 0 | 0 |
| D214 | 6 | 3 | 3 | 0 | 0 |
| D410 | 4 | 2 | 2 | 0 | 0 |
|
I'd like to change the "underline" rules to highlight the underline. But I'll probably leave the "blank line" rules as-is. The message indicates that we're talking about a blank line under the section, I think it's ok to highlight the section name. |
|
Makes sense. |
charliermarsh
force-pushed
the
charlie/range
branch
from
4cab597 to
e21c2df
Compare
charliermarsh
force-pushed
the
charlie/range
branch
from
e21c2df to
0328d86
Compare
AlexWaygood
approved these changes
Apr 2, 2024
Comment on lines
+8
to
+10
| 32 | Returns: | ||
| | ^^^^^^^ D406 | ||
| 33 | ------- |
There was a problem hiding this comment.
I still feel like for this one, the range is somewhat confusing, because we're specifically complaining about the colon at the end of "Returns:", which is actually excluded from the range here? But no strong opinion :)
Glyphack
pushed a commit
to Glyphack/ruff
that referenced
this pull request
Apr 12, 2024
…stral-sh#10740) ## Summary We may not have had access to this in the past, but in short, if the diagnostic is related to a specific section of a docstring, it seems better to highlight the section (via the header) than the _entire_ docstring. This should be completely compatible with existing `# noqa` since it's always inside of a multi-line string anyway, and in such cases the `# noqa` is always placed at the end of the multiline string. Closes astral-sh#10736.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
We may not have had access to this in the past, but in short, if the diagnostic is related to a specific section of a docstring, it seems better to highlight the section (via the header) than the entire docstring.
This should be completely compatible with existing
# noqasince it's always inside of a multi-line string anyway, and in such cases the# noqais always placed at the end of the multiline string.Closes #10736.