Vec extend to append

[Valentine-Mario]

This PR adds a check to suggest changes of vector from

vec.extend(other_vec.drain(..))

could be written as

vec![].append(&mut vec![]);

changelog: Add vec_extend_to_append lint
issue: #7209

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[flip1995]

r? @xFrednet This seems to be a good PR to review.

[Valentine-Mario]

r? @xFrednet This seems to be a good PR to review.

The test keeps failing. Not sure what I'm doing wrong. I'm pretty new to the repo 😪

[flip1995]

You can update the tests with cargo dev bless. Also see CI:

Not all lints defined properly. Please run `cargo dev update_lints` to make sure all lints are defined properly.

To get started with working on Clippy, make sure to read our documentation in doc/. Especially adding_lints.md and basics.md should help you out.

[xFrednet]

Hey @Valentine-Mario, you've come to the right place to ask :). I've added some comments to your code with explanations, how it can be improved.

The test keeps failing. Not sure what I'm doing wrong. I'm pretty new to the repo 😪

You have two tests which are failing:

1. UI tests: these tests run Clippy on the .rs files inside tests/ui/ directory and then compare the output with the .stderr file with the same name. You've correctly written the tests/ui/vec_extend_to_append.rs file but the corresponding tests/ui/vec_extend_to_append.stderr is missing. Our dev tool can copy it to the correct location with cargo dev bless.
2. Test update_lints: This test verifies that cargo dev update_lints produces no changes. Your merge from master most likely altered something that results in a new output. You can simply rerun cargo dev update_lints.
3. Test fmt: This test didn't run, but it's likely that it would fail. You can simply fix this by running cargo dev fmt (Note: The dev is required because we're using a slightly different version than cargo fmt)

It's usually good to run a few commands before every commit. This is roughly my procedure:

# Formatting the code base and tests
cargo dev fmt
# Running Clippy's tests
cargo test
# Updating UI test outputs if anything failed
cargo dev bless
# I usually only run this if `Test update_lints` fails but you can also run it every time as it's quite fast
cargo dev update_lints

# git add -A
# git commit -m "Your awesome commit message"
# git push

---

All of this might be a lot to take in, but your effort is appreciated, everyone started out like this. Feel free to ask if you're stuck or something is unclear 🙃

[xFrednet]

Hey, I've added a comment with guidance for the next steps.

In regard to commits. You've now overridden your previous commits with a force push. This can be useful to clean up at the end but right now complicates the review and destroys links from previous comments. Could you please add the next changes in a new commit and also describe a bit better what you're doing in the message itself 🙃

[xFrednet]

Hey @Valentine-Mario, I've reviewed your recent changes and added some suggestions. Don't let them overwhelm you, the implementation is getting better each time.

[Valentine-Mario]

Hey @Valentine-Mario, I've reviewed your recent changes and added some suggestions. Don't let them overwhelm you, the implementation is getting better each time.

Added the updates. Sorry for the delay. It was weekend

[xFrednet]

Hey, thank you for the changes. You've also added the automatic applicability of the suggestion correctly. 👍

Here are some last annotations. You can fix the red pipeline by adjusting the lint description. I'll do a final review when you've addressed these comments.

Added the updates. Sorry for the delay. It was weekend

That's totally okay, you can take your time with these changes 🙃

[giraffate]

Can you do a rebase, not merge? We follow a no-merge commit policy: https://github.com/rust-lang/rust-clippy/blob/master/doc/basics.md#pr

[flip1995]

The rebase gone wrong: You have to force push after a rebase, instead of merging the rebased branch. You should be able to fix this by running:

$ git fetch upstream # assuming upstream is the remote of this repo
$ git rebase upstream/master
$ git diff origin/vec_extend_to_append  # < This should be empty
$ git push --force-with-lease

[flip1995]

Thanks! Please squash your commit and this should be ready to go.

[Valentine-Mario]

Thanks! Please squash your commit and this should be ready to go.

Done

[flip1995]

There are some dogfood errors and you have to run cargo dev update_lints

[Valentine-Mario]

There are some dogfood errors and you have to run cargo dev update_lints

done

[flip1995]

There are still 2 dogfood errors.

[bors]

☔ The latest upstream changes (presumably #7315) made this pull request unmergeable. Please resolve the merge conflicts.

[Valentine-Mario]

There are still 2 dogfood errors.

Done

[flip1995]

@bors r+

Thanks!

[bors]

📌 Commit 44608b1 has been approved by flip1995

[bors]

⌛ Testing commit 44608b1 with merge a36a7c8...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing a36a7c8 to master...

[Valentine-Mario]

closes #7209

[xFrednet]

It has to be in the PR description before the merge. It's setup that way to allow the maintainers to double-check it beforehand. You can add it to future PRs :)