Add manual_empty_string_creations lint

[Guilherme-Vasconcelos]

Closes #2972

• Followed [lint naming conventions][lint_naming]
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt

changelog: [manual_empty_string_creations]: Add lint for empty String not being created with String::new()

[rust-highfive]

r? @dswij

(rust-highfive has picked a reviewer for you, use r? to override)

[bors]

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

[ghost]

Why not make this MachineApplicable?

[Guilherme-Vasconcelos]

I was a little bit unsure if this new lint can have false positives. Do you think it seems safe to make it MachineApplicable?

It worked fine in clippy's codebase, but since I'm still unexperienced with these lints I'm not sure.

[ghost]

IMO, machine applicable suggestions won't make the false positives any more annoying but it will allow people to use Cargo fix.

[dswij]

I don't see how the suggestions can go really wrong with this lint. Making it machine applicable seems fine, the lint will still trigger in FP cases either way.

[Guilherme-Vasconcelos]

Thanks for the suggestions! Fixed in 4e9aebfbd2b9b94b08d27f0d61ea47d8e9e80633

[ghost]

It looks like this lint needs a macro check. Here is a false positive from clap.

---> clap-3.2.17/src/util/id.rs:24:31                                 
warning: empty String is being created manually                       
  --> src/util/id.rs:24:31
   |
24 |                           name: $name.into(),
   |                                 ^^^^^^^^^^^^ help: consider using: `String::new()`
...
34 | / precomputed_hashes! {
35 | |     empty_hash,   0x1C9D_3ADB_639F_298E, "";
36 | |     help_hash,    0x5963_6393_CFFB_FE5F, "help";
37 | |     version_hash, 0x30FF_0B7C_4D07_9478, "version";
38 | | }
   | |_- in this macro invocation
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_empty_string_creation
   = note: this warning originates in the macro `precomputed_hashes` (in Nightly builds, run with -Z macro-backtrace for more info)

[Guilherme-Vasconcelos]

It looks like this lint needs a macro check. Here is a false positive from clap.

---> clap-3.2.17/src/util/id.rs:24:31                                 
warning: empty String is being created manually                       
  --> src/util/id.rs:24:31
   |
24 |                           name: $name.into(),
   |                                 ^^^^^^^^^^^^ help: consider using: `String::new()`
...
34 | / precomputed_hashes! {
35 | |     empty_hash,   0x1C9D_3ADB_639F_298E, "";
36 | |     help_hash,    0x5963_6393_CFFB_FE5F, "help";
37 | |     version_hash, 0x30FF_0B7C_4D07_9478, "version";
38 | | }
   | |_- in this macro invocation
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_empty_string_creation
   = note: this warning originates in the macro `precomputed_hashes` (in Nightly builds, run with -Z macro-backtrace for more info)

Thanks for catching this! I thought checking for external macros would be enough, but it seems that is not the case. Fixed in 4e9aebf

[dswij]

Thanks for the PR! The lint itself looks great.

1. Clippy prefers plural lint names. Can you help to rename the lint with plural form? i.e. manual_empty_string_creations.
2. Can we add a test case covering the macro scenario?

[dswij]

@Guilherme-Vasconcelos Huge thanks for this! This lint is certainly nice to have.

@bors r+

[bors]

📌 Commit 1bf8841 has been approved by dswij

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1bf8841 with merge 868dba9...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 868dba9 to master...