Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new lint: vec_resize_to_zero #5637

Merged
merged 1 commit into from
May 31, 2020

Conversation

tnielens
Copy link
Contributor

implements #5444

changelog: new lint vec_resize_to_zero

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 23, 2020
@bors
Copy link
Contributor

bors commented May 26, 2020

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

@yaahc
Copy link
Member

yaahc commented May 27, 2020

This looks good btw, just needs to be rebased and have a CI run that doesn't fall over.

cc @flip1995

if let ExprKind::Lit(Spanned { node: LitKind::Int(0, _), .. }) = args[1].kind;
if let ExprKind::Lit(Spanned { node: LitKind::Int(..), .. }) = args[2].kind;
then {
span_lint(cx, VEC_RESIZE_TO_ZERO, expr.span, "this empties the vector. It could be an argument inversion mistake. If not, call `clear()` instead.");
Copy link
Member

@flip1995 flip1995 May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to split up the message and suggestion:

Suggested change
span_lint(cx, VEC_RESIZE_TO_ZERO, expr.span, "this empties the vector. It could be an argument inversion mistake. If not, call `clear()` instead.");
span_lint_and_then(
cx,
VEC_RESIZE_TO_ZERO,
expr.span,
"emptying a vector with `resize`",
|db| {
db.help("the arguments may be inverted...");
db.span_suggestion(
resize_span,
"...or you can use `clear` to empty the vector",
"clear()",
Applicability::MaybeIncorrect,
);
},
);

Note, that you also have to get the span of the method call as resize_span for the suggestion to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion added

@yaahc yaahc added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 29, 2020
@tnielens tnielens force-pushed the feature/vec_resize_to_zero branch from fb6e6db to 5faab87 Compare May 30, 2020 15:52
@tnielens
Copy link
Contributor Author

Should this lint move to the methods.rs module ?

@flip1995
Copy link
Member

I think having this in its own module is fine. The methods module is more about methods (chaining) in general, while this is more about arguments to a specific method.

@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 31, 2020
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting on rustup

@flip1995
Copy link
Member

@bors r=yaahc,flip1995

@bors
Copy link
Contributor

bors commented May 31, 2020

📌 Commit 5faab87 has been approved by yaahc,flip1995

bors added a commit that referenced this pull request May 31, 2020
Rollup of 3 pull requests

Successful merges:

 - #5637 (new lint: vec_resize_to_zero)
 - #5656 (len_zero: skip ranges if feature `range_is_empty` is not enabled)
 - #5663 (add testcase that no longer ICEs)

Failed merges:

r? @ghost

changelog: rollup
@bors
Copy link
Contributor

bors commented May 31, 2020

⌛ Testing commit 5faab87 with merge 961db3d...

bors added a commit that referenced this pull request May 31, 2020
…ip1995

new lint: vec_resize_to_zero

implements #5444

changelog: new lint vec_resize_to_zero
@flip1995
Copy link
Member

@bors retry (yeeting to rollup)

bors added a commit that referenced this pull request May 31, 2020
Rollup of 3 pull requests

Successful merges:

 - #5637 (new lint: vec_resize_to_zero)
 - #5656 (len_zero: skip ranges if feature `range_is_empty` is not enabled)
 - #5663 (add testcase that no longer ICEs)

Failed merges:

r? @ghost

changelog: rollup
@bors
Copy link
Contributor

bors commented May 31, 2020

⌛ Testing commit 5faab87 with merge 8b191b5...

@bors bors merged commit 0fff522 into rust-lang:master May 31, 2020
@CBSpeir CBSpeir mentioned this pull request May 1, 2024
bors added a commit that referenced this pull request May 2, 2024
Remove `dead_code` paths

The following paths are `dead_code` and can be removed:

### `clippy_utils::paths::VEC_RESIZE`
* Introduced when `vec_resize_to_zero` lint added in PR #5637
* No longer used after commit 8acc4d2
### `clippy_utils::paths::SLICE_GET`
* Introduced when `get_first` lint added in PR #8882
* No longer used after commit a8d80d5
### `clippy_utils::paths::STR_BYTES`
* Introduced when `bytes_count_to_len` lint added in PR #8711
* No longer used after commit ba6a459

When the lints were moved into the `Methods` lint pass, they switched from using paths to diagnostic items.  However, the paths were never removed.  This occurred in PR #8957.

This relates to issue #5393

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants