Skip to content

Commit 767ceef

Browse files
Update design for PR rust-lang#16201: Fix println_empty_string suggestion caused error when there is a , after arg
1 parent 4d6fa3c commit 767ceef

File tree

1 file changed

+35
-0
lines changed

1 file changed

+35
-0
lines changed

pr-analysis-16201.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# PR #16201: Workflow Design Impact Analysis
2+
3+
## Affected Workflows
4+
- **lint-development**: This workflow is impacted because the PR modifies an existing lint implementation in `clippy_lints/src/write/empty_string.rs` (part of `clippy_lints/src/`) and updates UI tests in `tests/ui/` (listed in `relevant_files` for lint-development in `.exp/workflows.json`). The change fixes a bug in suggestion span calculation for the `println_empty_string` lint when a trailing comma follows the empty string argument.
5+
- **testing**: This workflow is affected due to direct updates to UI test files (`tests/ui/println_empty_string.rs`, `.fixed`, `.stderr`), which are core to verifying lint diagnostics and fixes. New test cases are added for edge cases like trailing commas, `eprintln!`, and nested contexts.
6+
7+
## lint-development Analysis
8+
### Summary of design changes
9+
The PR enhances the `check` function in `empty_string.rs` by adding logic to extend the `format_args.span` if the next character is a comma (`forward_span.check_source_text(cx, |s| s.ends_with(','))`), using `SpanRangeExt` from `clippy_utils::source`. This ensures the diagnostic span includes the trailing comma, allowing the suggestion to correctly remove both the empty string and the comma, fixing a compilation error in the suggested code.
10+
11+
This is a targeted improvement within the existing lint pass implementation (a `LateLintPass` checking `FormatArgs` in macros like `println!`). It leverages compiler spans and source text utilities but does not alter components, sequences, or flows in the workflow design. The scaffolding sequence (using `cargo dev new_lint` etc.) and integration sequence (registering passes and emitting diagnostics) remain unchanged.
12+
13+
**Potential benefits**: More accurate auto-fixes, reducing user friction and preventing invalid suggestions.
14+
**Implications**: Demonstrates the extensibility of lint implementations for handling syntax variations; may require similar checks in other lints using macro args.
15+
16+
No Mermaid diagrams require updates, as the high-level design in `.exp/design-workflow-5-lint-development.md` is unaffected.
17+
18+
## testing Analysis
19+
### Summary of design changes
20+
The PR expands the `println_empty_string` UI test suite by adding test cases for:
21+
- Trailing comma after empty string: `println!("",);`
22+
- Equivalent for `eprintln!`
23+
- Multi-line empty string literals followed by comma
24+
- These in nested contexts (e.g., match arms)
25+
26+
Updates `.stderr` to reflect adjusted diagnostic spans and help messages, and `.fixed` to show correct removal (e.g., `println!();` without comma).
27+
28+
This follows the standard UI testing process: when lint behavior changes, update input files and expected outputs to validate new diagnostics and fixes. It aligns with the UI Tests Sequence Diagram, where `compile-test` spawns `clippy-driver`, compares outputs, and (in dev) blesses updates.
29+
30+
No changes to testing infrastructure, components, or sequences; just content refinement for better coverage.
31+
32+
**Potential benefits**: Increased test robustness against syntax edge cases, catching regressions early.
33+
**Implications**: Highlights the importance of comprehensive UI tests for lint suggestions; manual updates here mimic `cargo bless` in practice.
34+
35+
No Mermaid diagrams require updates, as the design in `.exp/design-workflow-4-testing.md` remains accurate.

0 commit comments

Comments
 (0)