Cleanup some warnings from Clippy #2282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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. |
| @@ -116,7 +116,7 @@ fn execute(flags: Flags, config: &Config) -> CliResult<Option<()>> { | |||
| // help message. | |||
| "" | "help" if flags.arg_args.is_empty() => { | |||
| config.shell().set_verbose(true); | |||
| let args = &["cargo".to_string(), "-h".to_string()]; | |||
There was a problem hiding this comment.
Currently in Cargo it's more idiomatic to just remember that .to_string() always works as opposed to switching between one of .to_owned(), .cloned(), or .to_string() as appropriate, so could these be backed out for now?
Fix all Clippy’s needless_lifetimes warnings.
Fix most of Clippy’s needless_return warnings. Remaining cases are false positives.
Fix all Clippy’s len_without_is_empty warnings.
Fix all of Clippy’s len_zero warnings.
Fix all Clippy’s redundant_closure warnings.
Fix all Clippy’s single_match warnings.
Fix all of Clippy’s unnecessary_mut_passed warnings.
Fix some Clippy’s string_to_string warnings.
Fix most of Clippy’s map_clone warnings.
|
@alexcrichton: done. Is there anything else you’d like to change? |
bors
added a commit
that referenced
this pull request
Jan 19, 2016
For information, here is the [log before and after](https://gist.github.com/mcarton/684c030321c4c60d6bc9) with Clippy. Remaining warnings from Clippy are mostly false positive or `str` to `string` conversion (too many of them for this PR). ``` warnings before: cmp_owned : 2 cyclomatic_complexity : 1 deprecated : 11 explicit_iter_loop : 57 identity_op : 3 len_without_is_empty : 3 len_zero : 20 let_and_return : 3 map_clone : 14 needless_lifetimes : 4 needless_return : 24 ok_expect : 2 option_map_unwrap_or : 10 private_in_public : 28 redundant_closure : 2 should_implement_trait : 2 single_match : 3 str_to_string : 112 string_to_string : 12 type_complexity : 3 unnecessary_mut_passed : 2 unneeded_field_pattern : 3 unused_lifetimes : 1 unused_variables : 2 while_let_loop : 2 warnings after: cmp_owned : 2 cyclomatic_complexity : 1 identity_op : 1 let_and_return : 3 map_clone : 1 needless_return : 3 ok_expect : 2 option_map_unwrap_or : 1 private_in_public : 28 should_implement_trait : 2 str_to_string : 80 type_complexity : 3 while_let_loop : 2 ```
|
☀️ Test successful - cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 |
bors
added a commit
that referenced
this pull request
Apr 27, 2018
Profile Overrides (RFC #2282 Part 1) Profile Overrides (RFC #2282 Part 1) WIP: Putting this up before I dig into writing tests, but should be mostly complete. I also have a variety of questions below. This implements the ability to override profiles for dependencies and build scripts. This includes a general rework of how profiles work internally. Closes #5298. Profile overrides are available with `profile-overrides` set in `cargo-features` in the manifest. Part 2 is to implement profiles in config files (to be in a separate PR). General overview of changes: - `Profiles` moved to `core/profiles.rs`. All profile selection is centralized there. - Removed Profile flags `test`, `doc`, `run_custom_build`, and `check`. - Removed `Profile` from `Unit` and replaced it with two enums: `CompileMode` and `ProfileFor`. This is the minimum information needed to compute profiles at a later stage. - Also removed `rustc_args`/`rustdoc_args` from `Profile` and place them in `Context`. This is currently not very elegant because it is a special case, but it works. An alternate solution I considered was to leave them in the `Profile` and add a special uber-override layer. Let me know if you think it should change. - Did some general cleanup in `generate_targets`. ## Misc Fixes - `cargo check` now honors the `--release` flag. Fixes #5218. - `cargo build --test` will set `panic` correctly for dependences. Fixes #5369. - `cargo check --tests` will no longer include bins twice (once as a normal check, once as a `--test` check). It only does `--test` check now. - Similarly, `cargo check --test name` no longer implicitly checks bins. - Examples are no longer considered a "test". (See #5397). Consequences: - `cargo test` will continue to build examples as a regular build (no change). - `cargo test --tests` will no longer build examples at all. - `cargo test --all-targets` will no longer build examples as tests, but instead build them as a regular build (now matches `cargo test` behavior). - `cargo check --all-targets` will no longer check examples twice (once as normal, once as `--test`). It now only checks it once as a normal target. ## Questions - Thumbs up/down on the general approach? - The method to detect if a package is a member of a workspace should probably be redone. I'm uncertain of the best approach. Maybe `Workspace.members` could be a set? - `Hash` and `PartialEq` are implemented manually for `Profile` only to avoid matching on the `name` field. The `name` field is only there for debug purposes. Is it worth it to keep `name`? Maybe useful for future use (like #4140)? - I'm unhappy with the `Finished` line summary that displays `[unoptimized + debuginfo]`. It doesn't actually show what was compiled. Currently it just picks the base "dev" or "release" profile. I'm not sure what a good solution is (to be accurate it would need to potentially display a list of different options). Is it ok? (See also #4140 for the wrong profile name being printed.) - Build-dependencies use different profiles based on whether or not `--release` flag is given. This means that if you want build-dependencies to always use a specific set of settings, you have to specify both `[profile.dev.build_override]` and `[profile.release.build_override]`. Is that reasonable (for now)? I've noticed some issues (like #1774, #2234, #2424) discussing having more control over how build-dependencies are handled. - `build --bench xxx` or `--benches` builds dependencies with dev profile, which may be surprising. `--release` does the correct thing. Perhaps print a warning when using `cargo build` that builds benchmark deps in dev mode? - Should it warn/error if you have an override for a package that does not exist? - Should it warn/error if you attempt to set `panic` on the `test` or `bench` profile? ## TODO - I have a long list of tests to add. - Address a few "TODO" comments left behind.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For information, here is the log before and after with Clippy.
Remaining warnings from Clippy are mostly false positive or
strtostringconversion (too many of them for this PR).