Conversation
schneems
changed the title
16cbb5c to
f2e6fd5
Compare
c83a3a3 to
ceeac02
Compare
The `join_absolute_paths` lint is essentially the same logic, but for `join` instead of `push`. This change updates `path_buf_push_overwrite` to match `join_absolute_paths`, which is out of nursery and has received more recent updates:
- Copy the tests from `join_absolute_paths`.
- Add a no-lint case for `sh`.
- Adapt the detection logic from `join_absolute_paths`.
- Align the public docs and lint description.
- Add a comment to both lints as a hint that changes to one should be propagated to the other.
- Fix a typo in the `join_absolute_paths` docs: `Use instead;` => `Use instead:`.
- Add a blank line after doc headers in `path_buf_push_overwrite`. I didn't update `join_absolute_paths`.
- Move `path_buf_push_overwrite` from `nursery` to `suspicious`.
The main difference between the two is the "if this is intentional" suggestion. `Path::join` returns a new path, so its suggestion replaces only the call site:
```
LL - path.join("/sh");
LL + PathBuf::from("/sh");
```
`PathBuf::push` mutates in place, so its suggestion assigns to the receiver instead. When the receiver auto-derefs to the `PathBuf`, the suggestion adds one `*` per deref, the same approach `swap_with_temporary` uses:
```
LL - path.push("/sh");
LL + *path = PathBuf::from("/sh");
```
The `multiple_deref()` test (and some of the logic) is adapted from `swap_with_temporary` lint.
ceeac02 to
e26c7e3
Compare
rustbot
added
S-waiting-on-review
Could you go into more details what you mean here? Context |
|
Good question. I wasn't totally sure how to concisely represent what I did. I read the policy before posting (I think it's good to talk more openly about this stuff, also, thanks for your work around the repo). I missed the "originally created" description at the bottom. I didn't always start with human code. So, I should have gone through the full "experiment" flow and pre-arranged a reviewer. Sorry, I didn't do that. Specifically, I had an LLM write code that I then edited. And often threw away. Repeatedly, not always in that order. I'm aiming for higher rigor (like https://rfd.shared.oxide.computer/rfd/0576). This output is mine, as in I own it fully. I stand behind it and cannot improve it without further assistance, but would like to. I'll take more care in the future, and I'll look for a reviewer in the morning. |
CommanderStorm
added
the
llm-assisted
|
After a conversation on zulip it sounds like it will be helpful for me to rewrite this by hand. The project seems to be drowning in these LLM tagged PRs and few maintainers (especially compared to mainline rust). It is worth it to increase the signal to noise ratio to close and redo by hand. That's my plan. |
rustbot
removed
S-waiting-on-community-reviews
3 participants
LLM disclosure: I iterated on this with an LLM for code review, research, and experimenting. All prose in the PR here, code comments, and the commit message is mine. I've manually reviewed all changes locally using
gitxvisual diff and again on GitHub's UI before moving it out of "draft". I (human) fully own this being up to my standards and ready for review.What
The
path_buf_push_overwriteandjoin_absolute_pathslints are basically the same thing. However,path_buf_push_overwritewas proposed first (#3954) and is still in the nursery (due to #4012), whilejoin_absolute_pathscame later (2026 #11453) and is currently in the suspicious group. This PR alignspath_buf_push_overwriteto matchjoin_absolute_pathslogic and moves it to thesuspiciousgroup.This PR updates
path_buf_push_overwriteto matchjoin_absolute_paths(tests, docs, behavior). It delivers different suggestions that handle deref similar toswap_with_temporary. I also propose we move it from the nursery tosuspiciousto matchjoin_absolute_paths(which this PR also does).This came up from a discussion on Zulip https://rust-lang.zulipchat.com/#narrow/channel/219381-t-libs/topic/Path.20redesign/near/627828098.