Skip to content

Commit 69e111b

Browse files
Update design for PR rust-lang#16197: Fix tuple_array_conversions FP when binded vars are used before conversion
1 parent 4d6fa3c commit 69e111b

File tree

1 file changed

+47
-0
lines changed

1 file changed

+47
-0
lines changed

.exp/pr-analysis-16197.md

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
# PR #16197: Workflow Design Impact Analysis
2+
3+
## Affected Workflows
4+
- **lint-development**: The PR directly modifies the `tuple_array_conversions` lint implementation in `clippy_lints/src/tuple_array_conversions.rs` by adding a new check and helper function to fix a false positive when bound variables are used before the conversion. It also adds a corresponding UI test case in `tests/ui/tuple_array_conversions.rs`. These files are central to the lint-development workflow as described in workflows.json (relevant_files: ["clippy_dev/src/new_lint.rs", "clippy_dev/src/update_lints.rs", "clippy_lints/src/", "declare_clippy_lint/src/lib.rs", "tests/ui/"]).
5+
6+
- **testing**: The PR updates the UI tests by adding a new test case `issue16192` to verify the fixed behavior, impacting the UI test execution and validation in the testing workflow (relevant_files: ["tests/", "ui/", "tests/ui-*/", "clippy_dev/src/dogfood.rs", "clippy_dev/src/main.rs"]).
7+
8+
Other workflows like `cargo-clippy` and `clippy-driver` indirectly involve `clippy_lints/src/` but are not primarily affected in terms of design changes from this PR.
9+
10+
## lint-development Analysis
11+
12+
### Summary of design changes
13+
The PR fixes a false positive in the `tuple_array_conversions` lint. Specifically, it adds a condition in `all_bindings_are_for_conv` to check that none of the locals from array destructuring are used until the tuple construction expression, using a new helper `local_used_until_expr`. This function traverses the enclosing block with `for_each_expr` to detect any use of the local before the target expression.
14+
15+
This change refines the internal logic of an existing late lint pass but does not affect the overall workflow design:
16+
- No new steps in scaffolding (e.g., `cargo dev new_lint` or `update_lints`).
17+
- No modifications to components like `declare_clippy_lint!` macro, `LintListBuilder`, or registration in `declared_lints.rs`.
18+
- The integration sequence (loading lints in driver, registering passes) remains unchanged.
19+
- It leverages existing utilities from `clippy_utils::visitors` (e.g., `for_each_expr`, `get_enclosing_block`), aligning with documented practices for lint implementations using visitor patterns and HIR traversal.
20+
21+
**Potential benefits**: Improves lint precision, reducing user frustration from incorrect suggestions and enhancing Clippy's reliability for array-to-tuple conversions involving destructuring.
22+
**Implications**: Developers modifying similar lints may now consider inter-expression variable usage checks, but no workflow process changes.
23+
24+
### Mermaid diagrams that need to be updated
25+
None. The existing scaffolding and integration sequence diagrams do not require updates, as the change is an internal implementation detail within a single lint module, not altering high-level flows or components.
26+
27+
## testing Analysis
28+
29+
### Summary of design changes
30+
The PR adds a new function `issue16192` in `tests/ui/tuple_array_conversions.rs` demonstrating a case where array destructuring binds variables `a` and `b`, one is used (in a loop and later assignment), and then a tuple is formed. Previously, this triggered a false positive; now, with the fix, it should not lint, verified by the expected `.stderr`.
31+
32+
This is a standard addition of a UI test case to cover the fixed edge case:
33+
- No changes to test orchestration in `tests/compile-test.rs` or `ui_test` framework.
34+
- Follows UI test patterns: input `.rs` with code triggering (or not) the lint, matched against `.stderr`.
35+
- No impact on dogfooding, unit tests, or other categories (ui-internal, ui-toml, ui-cargo).
36+
- May require `cargo test` or `cargo uibless` during development to update expectations.
37+
38+
**Potential benefits**: Increases test coverage for variable usage patterns post-destructuring, preventing future regressions in this lint.
39+
**Implications**: Reinforces the importance of comprehensive UI tests for lint fixes, but the testing workflow design remains intact.
40+
41+
### Mermaid diagrams that need to be updated
42+
None. The UI tests sequence diagram accurately depicts the process of spawning `clippy-driver` on test files, emitting diagnostics, and comparing outputs—unchanged by adding a specific test case. Similarly, dogfooding diagram unaffected.
43+
44+
## Overall Impact
45+
This PR is a targeted bug fix in a single lint and its tests, exemplifying the lint-development and testing workflows without altering their documented designs. No updates to `.exp` design documents or Mermaid diagrams are necessary. The fix addresses [issue #16192](https://github.com/rust-lang/rust-clippy/issues/16192), improving lint accuracy.
46+
47+
To validate, no Mermaid diagrams were modified, so no `mmdc` validation needed beyond existing files (assumed valid).

0 commit comments

Comments
 (0)