Skip to content

Commit 6e38342

Browse files
Update design for PR rust-lang#16093: feat(format_push_string): give a (possibly incomplete) suggestion
1 parent 4d6fa3c commit 6e38342

File tree

4 files changed

+79
-5
lines changed

4 files changed

+79
-5
lines changed

.exp/design-workflow-1-cargo-clippy.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ sequenceDiagram
6161
LS->>CL: Register declared_lints::LINTS
6262
CL-->>LS: Lint metadata registered
6363
LS->>CL: clippy_lints::register_lint_passes(store, conf)
64-
CL-->>LS: LintPass instances registered (filtered by conf)
64+
Note right of CL: internally: initialize shared resources (e.g., FormatArgsStorage::default())
65+
CL-->>LS: LintPass instances registered (filtered by conf, some stateful with cloned resources)
6566
Note over RI: Compilation pipeline executes:<br/>- Parse to AST<br/>- Expand/lower to HIR<br/>- MIR construction<br/>- At each phase, applicable lints run via visitors/checkers
6667
alt --fix mode
6768
Note over CL,RI: Lints emit rustfix suggestions

.exp/design-workflow-2-clippy-driver.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ sequenceDiagram
4949
D->>D: Add --cfg clippy, CLIPPY_ARGS
5050
D->>RD: run_compiler(args, ClippyCallbacks)
5151
RD->>C: config() - load conf, set opts (mir_opt_level=0)
52-
C->>LS: register_lint_passes() - add early/late lints
52+
C->>LS: register_lint_passes() - init shared resources (e.g., FormatArgsStorage), add early/late lints (some with state)
5353
RD->>C: psess_created() - track deps (Cargo.toml, etc.)
5454
RD->>CP: Run phases (parse, expand, typeck, MIR, ...)
5555
loop During phases

.exp/design-workflow-5-lint-development.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ sequenceDiagram
5757
User->>CargoClippy: run cargo clippy on project
5858
CargoClippy->>Driver: invoke driver as rustc wrapper
5959
Driver->>LintsCrate: load and call register_lint_passes on store
60-
LintsCrate->>Store: extend early and late lint passes with lint impls
60+
LintsCrate->>LintsCrate: initialize shared resources (e.g., FormatArgsStorage::default())
61+
LintsCrate->>Store: extend early and late lint passes with lint impls (passing cloned resources to stateful lints)
6162
Driver->>Store: create LintListBuilder, insert declared_lints LINTS, register lints and groups
6263
Driver->>Compiler: run compilation with registered lints
6364
Store->>Compiler: execute lint passes during compilation phases
@@ -83,14 +84,16 @@ The \`update_lints\` tool parses all \`declare_clippy_lint!\` macros across \`cl
8384

8485
This ensures new lints are automatically included when compiling \`clippy_lints\`.
8586

86-
### Lint Implementation Details
87+
### Lint Implementation Details (updated)
8788

8889
- **Declaration**: \`declare_clippy_lint!\` expands to \`declare_tool_lint!\` (from rustc) plus \`LintInfo\` static with category, explanation, etc.
89-
- **Passes**: Early passes (AST/HIR pre-analysis) vs. late (after typeck, access to MIR/ty).
90+
- **Passes**: Early passes (AST/HIR pre-analysis) vs. late (after typeck, access to MIR/ty). Updated.
9091
- **Hooks**: Lints impl methods like \`check_expr\`, \`check_item\` using visitor patterns or queries via \`cx\`.
9192
- **Groups and Levels**: Lints assigned to categories (correctness, style, etc.) auto-grouped by \`LintListBuilder\`; levels (Allow, Warn, Deny).
9293
- **Fixes**: Use \`rustfix::diagnostics::Diagnostic::fix` for auto-fixes via \`--fix\`.
9394

95+
- **Stateful Passes**: Some lints require state for efficient analysis, such as \`FormatArgsStorage\` from \`clippy_utils::macros\` for parsing \`format!\` and other formatting macros. This storage is created once in \`clippy_lints/src/lib.rs::register_lint_passes\` and cloned into the constructors of relevant lint passes (e.g., lints in \`format_args\` group). This pattern allows sharing parsed data across invocations. The \`format_push_string\` lint, enhanced in [PR #16093](https://github.com/rust-lang/rust-clippy/pull/16093), uses this to detect \`format!\` calls even in nested contexts and generate code suggestions replacing them with \`write!\`.
96+
9497
### Edge Cases and Extensibility
9598

9699
- **Submodule Placement**: Specify via \`--type <submod>\` (e.g., cargo, methods); affects file path and lib.rs mods.

pr-analysis-16093.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# PR #16093: Workflow Design Impact Analysis
2+
3+
## Affected Workflows
4+
- **cargo-clippy** (Workflow 1): The PR changes code in `clippy_lints/src/` (lint implementation and registration), which is executed during Cargo project analysis via the clippy-driver wrapper. This impacts the diagnostics produced in the workflow's output.
5+
6+
- **clippy-driver** (Workflow 2): Direct changes to lint passes registered by the driver, affecting individual file or non-Cargo linting.
7+
8+
- **testing** (Workflow 4): Updates to multiple UI test files in `tests/ui/` for the `format_push_string` lint, including new tests for suggestion behavior under various configurations (no_std, unfixable cases) and updated expected outputs (`.stderr`, `.fixed`).
9+
10+
- **lint-development** (Workflow 5): The PR is a direct instance of lint development, modifying an existing lint's logic to add suggestion support, updating manual registration in `lib.rs` to use stateful pass construction, and maintaining corresponding UI tests.
11+
12+
## cargo-clippy Analysis
13+
### Summary of design changes
14+
Specific aspects affected: The registration and execution of lint passes in the compilation pipeline. The PR updates the constructor for `FormatPushString` pass to include `FormatArgsStorage`, enabling advanced detection of `format!` macro calls for generating targeted suggestions. This is implemented within `register_lint_passes` by cloning the shared storage into the pass. Potential benefits: Improved lint accuracy and user-friendly fixes, reducing unnecessary String allocations in formatted string concatenation. Implications: Enhanced diagnostic quality in project-wide linting runs.
15+
16+
The design diagram's registration step has been updated to note internal resource initialization and stateful pass creation.
17+
18+
```mermaid
19+
sequenceDiagram
20+
participant LS as "Lint Store"
21+
participant CL as "Clippy Lints"
22+
Note over LS,CL: Diff visualization
23+
Note over CL: Addition (green rect)
24+
CL->>CL: initialize FormatArgsStorage
25+
Note over LS: Change (yellow rect)
26+
LS->>CL: register_lint_passes
27+
CL-->>LS: register stateful passes (e.g., format_push_string with storage)
28+
```
29+
(Green: new internal step; Yellow: modified pass registration to support state.)
30+
31+
## clippy-driver Analysis
32+
### Summary of design changes
33+
Similar to cargo-clippy, the PR affects the lint pass registration in the driver's compiler callbacks. The updated pass can now provide suggestions during direct compilations. Implemented by the same code changes in clippy_lints. Benefits: Consistent improvements across invocation methods. The design diagram updated accordingly.
34+
35+
```mermaid
36+
sequenceDiagram
37+
participant C as Compiler Config & Callbacks
38+
participant LS as Lint Store
39+
Note over C,LS: Existing
40+
C->>LS: register_lint_passes()
41+
Note over C,LS: After PR
42+
Note over LS: Change (yellow): adds stateful lints
43+
C->>LS: register_lint_passes() - with shared resources init
44+
```
45+
(Yellow: updated description to include resource handling.)
46+
47+
## testing Analysis
48+
### Summary of design changes
49+
The PR updates 11 test files to match the new diagnostic outputs, including addition of `.fixed` files for testing auto-fix applicability and new test cases for edge cases (e.g., no_std_unfixable). This validates the lint's new suggestion logic without changing the UI test execution flow or components. Implemented by updating expected `.stderr` with new messages, labels, notes, and fix results. Benefits: Ensures regression-free enhancements and full coverage of new features. No mermaid diagram updates needed, as the sequence remains the same.
50+
51+
## lint-development Analysis
52+
### Summary of design changes
53+
The PR modifies the documented design by exemplifying and prompting documentation of stateful lint passes using shared `FormatArgsStorage` for macro analysis. Specific aspects: Lint implementation now includes state in pass struct for efficiency, registration manually passes cloned storage. New detection logic spans nested expressions via recursive search. Suggestions use `span_suggestion_verbose` with `Applicability::MaybeIncorrect` and dynamic paths via `std_or_core`. The design doc has been updated with a new bullet in "Lint Implementation Details" and enhanced integration diagram to show resource preparation and stateful extension. Benefits: Enables complex analysis patterns for lints, improves suggestion completeness for `format_push_string`. Implications: Developers can adopt this pattern for other macro-heavy lints.
54+
55+
Updated mermaid diagram in doc shows additions/changes; diff visualization:
56+
57+
```mermaid
58+
sequenceDiagram
59+
participant Driver
60+
participant LintsCrate
61+
participant Store
62+
Driver->>LintsCrate: call register_lint_passes on store
63+
Note over LintsCrate: Addition (green)
64+
LintsCrate->>LintsCrate: initialize FormatArgsStorage
65+
Note over Store: Change (yellow)
66+
LintsCrate->>Store: extend passes (with storage for some lints like format_push_string)
67+
```
68+
(Green: new resource init step; Yellow: modified extension to include state passing.)
69+
70+
The PR does not affect other workflows' designs significantly.

0 commit comments

Comments
 (0)