Skip to content

Commit 1af5f83

Browse files
gauravagerwalagauravagerwala
committed
Update design for PR rust-lang#16095: Add lint clones_into_boxed_slices 
1 parent 4d6fa3c commit 1af5f83

File tree

2 files changed

+108
-73
lines changed

2 files changed

+108
-73
lines changed

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

Lines changed: 24 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ From analysis of source code (e.g., \`clippy_dev/src/new_lint.rs\`, \`clippy_dev
1111
- **clippy_dev**: CLI tools (\`cargo dev\`) including \`new_lint\` for scaffolding and \`update_lints\` for generating registration code from parsed lint declarations.
1212
- **clippy_lints**: Core library housing all lint implementations as structs implementing \`rustc_lint::EarlyLintPass\` or \`LateLintPass\`. Each lint file (\`src/<lint>.rs\` or in submodules like \`methods/\`) contains a \`declare_clippy_lint!\` invocation defining metadata (name, level, category, description, version, location).
1313
- **declare_clippy_lint**: Crate providing the macro for lint declaration (generates \`Lint\` static and \`LintInfo\`) and \`LintListBuilder\` for bulk registration of individual lints and groups (e.g., \`clippy::all\`, \`clippy::correctness\`).
14-
- **clippy_utils**: Shared utilities for common lint tasks (e.g., type queries, def path matching, expression analysis).
14+
- **clippy_utils**: Shared utilities for common lint tasks (e.g., type queries, def path matching, expression analysis, symbol interning via `sym.rs` for method and identifier matching, MSRV tracking via `msrvs.rs` for version-gated lints).
1515
- **tests/ui/**: UI test infrastructure where each lint has a subdirectory with input \`.rs\` files and expected \`.stderr\` outputs (and \`.fixed\` for fixes) from Clippy runs to verify diagnostics.
1616
- **clippy-driver** (in root \`src/driver.rs\`): Custom compiler driver that loads \`clippy_lints\`, uses generated \`declared_lints::LINTS\` to register lints via \`LintListBuilder\`, calls \`register_lint_passes\` to add passes, and hooks into rustc's pipeline.
1717
- **lintcheck**: Separate tool (\`lintcheck/src/main.rs\`) for running lints on external crates listed in \`.toml\` files to detect regressions or false positives/negatives.
@@ -21,81 +21,32 @@ Other aspects: Lints support configuration options, MSRV restrictions via attrib
2121

2222
## Scaffolding Sequence Diagram
2323

24-
\`\`\`mermaid
24+
```mermaid
2525
sequenceDiagram
2626
participant Developer
2727
participant "cargo dev new_lint" as NewLintTool
28-
participant "clippy_lints/src/lint.rs" as LintImpl
29-
participant "clippy_lints/src/lib.rs" as LibRs
30-
participant "tests/ui/lint/" as UITests
28+
participant LintImpl as "clippy_lints/src/lint.rs (or submod)"
29+
participant "clippy_utils/src/" as Utils
30+
participant UITests as "tests/ui/lint/"
3131
participant "update_lints" as UpdateTool
3232
participant "declared_lints.rs" as DeclaredLints
33-
Developer->>NewLintTool: cargo dev new_lint name category options
34-
NewLintTool->>LintImpl: Generate file with declare clippy lint macro and LintPass skeleton
35-
NewLintTool->>UITests: Create test directory with rs and stderr files
36-
opt no submodule
37-
NewLintTool->>LibRs: Add mod lint declaration
33+
participant ModRs as "submod/mod.rs"
34+
Developer->>NewLintTool: cargo dev new_lint name category [options]
35+
NewLintTool->>LintImpl: Generate file with declare_clippy_lint! and pass skeleton
36+
NewLintTool->>UITests: Create initial test dir
37+
alt submodule case
38+
NewLintTool->>LintImpl: Place in submod/lint.rs
39+
NewLintTool->>ModRs: Add pub mod lint; in submod/mod.rs
3840
end
39-
NewLintTool->>UpdateTool: Invoke update_lints
40-
UpdateTool->>LintImpl: Parse all declare clippy lint invocations
41-
UpdateTool->>DeclaredLints: Update LINTS array including new lint
42-
UpdateTool->>LibRs: Update mod declarations between comments
43-
UpdateTool->>README: Update lint counts and links
44-
Note over UpdateTool: Handles deprecated and renamed lints too
45-
\`\`\`
46-
47-
## Integration and Execution Sequence Diagram
48-
49-
\`\`\`mermaid
50-
sequenceDiagram
51-
participant User
52-
participant "cargo clippy" as CargoClippy
53-
participant "clippy-driver" as Driver
54-
participant "clippy_lints" as LintsCrate
55-
participant "LintStore" as Store
56-
participant Compiler as Rustc
57-
User->>CargoClippy: run cargo clippy on project
58-
CargoClippy->>Driver: invoke driver as rustc wrapper
59-
Driver->>LintsCrate: load and call register_lint_passes on store
60-
LintsCrate->>Store: extend early and late lint passes with lint impls
61-
Driver->>Store: create LintListBuilder, insert declared_lints LINTS, register lints and groups
62-
Driver->>Compiler: run compilation with registered lints
63-
Store->>Compiler: execute lint passes during compilation phases
64-
Compiler->>User: output diagnostics from lints
65-
\`\`\`
66-
67-
## Additional High-Level Design Aspects
68-
69-
### Testing Workflow
70-
71-
- **Unit/UI Tests**: \`cargo test\` runs tests including UI tests in \`tests/ui/\` which invoke the driver on sample code and verify exact diagnostic output.
72-
- **Dogfooding**: \`cargo dev dogfood\` lints Clippy's source to catch self-violations.
73-
- **External Validation**: \`cargo lintcheck\` applies lints to curated external crates, generating reports on triggers.
74-
- **Blessing**: \`cargo bless\` updates expected test outputs after changes.
75-
76-
### Update and Registration Automation
77-
78-
The \`update_lints\` tool parses all \`declare_clippy_lint!\` macros across \`clippy_lints/src/\` (using a custom parser in \`clippy_dev/src/parse.rs\`) to:
79-
- Generate \`declared_lints.rs\` with \`pub static LINTS: &[&LintInfo]\` listing all lints' metadata.
80-
- Update \`lib.rs\` mod list between special comments.
81-
- Refresh docs (README.md, book/, CHANGELOG.md) with lint counts (rounded to 50) and markdown links.
82-
- Manage deprecated/renamed lints in \`deprecated_lints.rs\` and dedicated UI tests.
83-
84-
This ensures new lints are automatically included when compiling \`clippy_lints\`.
85-
86-
### Lint Implementation Details
87-
88-
- **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-
- **Hooks**: Lints impl methods like \`check_expr\`, \`check_item\` using visitor patterns or queries via \`cx\`.
91-
- **Groups and Levels**: Lints assigned to categories (correctness, style, etc.) auto-grouped by \`LintListBuilder\`; levels (Allow, Warn, Deny).
92-
- **Fixes**: Use \`rustfix::diagnostics::Diagnostic::fix` for auto-fixes via \`--fix\`.
93-
94-
### Edge Cases and Extensibility
95-
96-
- **Submodule Placement**: Specify via \`--type <submod>\` (e.g., cargo, methods); affects file path and lib.rs mods.
97-
- **Custom Categories**: Cargo, nursery, pedantic, restriction handled specially in builder.
98-
- **Dependencies**: Lints may depend on other crates like \`clippy_utils\`; build.rs handles rustc version syncing.
99-
- **Community Contribution**: Tools ease adding lints; PRs update workflows via git hooks or CI.
100-
101-
This design balances automation with flexibility, supporting Clippy's 700+ lints while maintaining compile-time efficiency and correctness.
41+
NewLintTool->>UpdateTool: Invoke initial update_lints
42+
UpdateTool->>LintImpl: Parse invocations
43+
UpdateTool->>DeclaredLints: Update LINTS incl. new
44+
UpdateTool->> "lib.rs or mod.rs": Update mod lists
45+
Developer->>LintImpl: Implement logic
46+
Developer->>Utils: Extend sym, msrvs etc. if needed
47+
Developer->>UITests: Expand tests
48+
Developer->>UpdateTool: Re-run update_lints
49+
UpdateTool->>DeclaredLints: Final update
50+
Note over UpdateTool: Deprec/rename handling
51+
Note over Utils: Addition highlighted (green in analysis)
52+
```

‎pr-analysis-16095.md‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# PR #16095: Workflow Design Impact Analysis
2+
3+
## Affected Workflows
4+
- **Workflow 5: lint-development**: The PR adds a new lint implementation in `clippy_lints/src/methods/clones_into_boxed_slices.rs`, updates `methods/mod.rs` and generated `declared_lints.rs`, extends `clippy_utils` with new entries in `sym.rs` and `msrvs.rs`, and adds UI tests. This directly utilizes and slightly extends the lint-development workflow as described in workflows.json and design doc.
5+
6+
- **Workflow 4: testing**: New UI test files in `tests/ui/clones_into_boxed_slices.{rs,stderr,fixed}` and updates to existing tests (`crashes/ice-3969.rs`, `unnecessary_box_returns.rs`). Impacts the UI testing component.
7+
8+
- **Workflow 1: cargo-clippy**: Changes to `clippy_lints/src/` and `clippy_utils/src/` integrate the new lint into the analysis performed during `cargo clippy` runs.
9+
10+
- **Workflow 2: clippy-driver**: Similar integration of new lint and utils into the compiler driver pipeline.
11+
12+
- **Workflow 8: release-process**: Update to `CHANGELOG.md` adding the new lint entry, consistent with release note preparation.
13+
14+
## Workflow 5 Analysis (lint-development)
15+
### Summary of design changes
16+
The PR implements the `clones_into_boxed_slices` lint, which detects patterns like `vec.clone().into_boxed_slice()` and suggests `Box::from(vec)` for efficiency. It follows the scaffolding process but highlights the need for manual extensions to shared utilities:
17+
18+
- **Addition to clippy_utils component**: New symbols (`into_boxed_slice`, `into_boxed_str`, etc.) in `sym.rs` for matching specific methods, and MSRV entry in `msrvs.rs` setting min version 1.7.0. This enables symbol-based checks and version gating without breaking older Rust versions.
19+
20+
- **Standard integration**: Lint declared with `declare_clippy_lint!`, implemented as LateLintPass checking method chains, using utils for type analysis and suggestions. Tests added for verification.
21+
22+
- **Implications**: Enhances lint expressiveness for std lib methods; design now better documents utility extensions as part of implementation step. Updated design doc text and diagram to reflect this (additions in green below for illustration).
23+
24+
The updated design incorporates these extensions explicitly in the scaffolding sequence, ensuring future developers account for them.
25+
26+
```mermaid
27+
sequenceDiagram
28+
participant Developer
29+
participant "cargo dev new_lint" as NewLintTool
30+
participant "clippy_lints/src/methods/clones_into_boxed_slices.rs" as LintImpl
31+
participant "clippy_utils/src/" as Utils
32+
participant "tests/ui/clones_into_boxed_slices/" as UITests
33+
participant "cargo dev update_lints" as UpdateTool
34+
participant "declared_lints.rs" as DeclaredLints
35+
Developer->>NewLintTool: cargo dev new_lint clones_into_boxed_slices correctness --type methods
36+
NewLintTool->>LintImpl: Generate skeleton with declare_clippy_lint! and LatePass
37+
NewLintTool->>UITests: Initial test files
38+
NewLintTool->> "methods/mod.rs": Add pub mod clones_into_boxed_slices;
39+
NewLintTool->>UpdateTool: Initial update_lints
40+
UpdateTool->>DeclaredLints: Add lint metadata to LINTS
41+
Developer->>LintImpl: Implement check for clone + into_boxed_*
42+
Developer->>Utils: Add into_boxed_* symbols to sym.rs & MSRV to msrvs.rs
43+
Developer->>UITests: Add test cases & .fixed
44+
Developer->>UpdateTool: Final update_lints
45+
UpdateTool->>DeclaredLints: Finalize with full info
46+
Note over Utils: Green highlight for PR addition
47+
```
48+
49+
(Note: The above is an instance-specific sequence for this PR, showing green-highlighted addition in Utils. Original design sequence updated in .exp/design-workflow-5-lint-development.md to generalize this step.)
50+
51+
## Workflow 4 Analysis (testing)
52+
### Summary of design changes
53+
New lint requires new UI tests to validate diagnostics and fixes, added as standard .rs/.stderr/.fixed files. Updates to existing tests ensure no regressions or unwanted triggers from new lint logic.
54+
55+
- Specific: New dir for lint-specific tests; fixes in crashes and other to bless updated outputs.
56+
57+
- No changes to compile-test execution, ui_test config, or dogfooding (though new lint will be dogfooded automatically).
58+
59+
- Benefits: Verifies lint accuracy, fix applicability, and stability.
60+
61+
No mermaid diagrams need updating; uses existing UI tests flow.
62+
63+
## Workflow 1 Analysis (cargo-clippy)
64+
### Summary of design changes
65+
New lint added to `clippy_lints` crate, registered via `declared_lints::LINTS` in driver, executed during compilation phases. Utils extension supports lint but doesn't alter wrapper logic, env setup, or Cargo invocation.
66+
67+
- How implemented: Standard lint registration in driver callbacks.
68+
69+
- Implications: `cargo clippy` now includes this check for all targets, improving coverage for boxed slice patterns.
70+
71+
No changes to sequence diagram; additional lint pass is implicit in Store->>Compiler execute lints.
72+
73+
## Workflow 2 Analysis (clippy-driver)
74+
### Summary of design changes
75+
Similar to workflow 1; new LateLintPass added to late passes extension in LintStore during driver init. Changes transparent to direct driver invocations.
76+
77+
No design alterations.
78+
79+
## Workflow 8 Analysis (release-process)
80+
### Summary of design changes
81+
Added changelog entry: "new lint: [`clones_into_boxed_slices`]". This is a routine update during lint addition, ahead of release.
82+
83+
No changes to release tool or process; manual but conventional.
84+

0 commit comments

Comments
 (0)