unnecessary_string_new

[yoav-lavi]

What it does

Detects cases of &String::new() being passed to something expecting &str.

This is a followup on @flip1995's suggestion:

A separate lint about using an owned empty string where also a "" would be possible might be better to put time in than trying to cover all cases in this lint.

When checking godbolt.org it seems that &String::new() does an extra allocation although in benchmarks they seemed to perform similarly (which leads me to believe it may be optimized away in some cases, or that the allocation is almost insignificant in this case).

See examples of this happening here (might include cases of &String)

I also opened a question on /r/rust to make sure there aren't cases that this would be valid.

Lint Name

unnecessary_string_new

Category

style

Advantage

Shorter, possibly more performant

Drawbacks

None that I'm aware of

Example

"12345".split(&String::new());

Could be written as:

"12345".split("");

[flip1995]

Thanks for opening this issue!

Looking at the godbolt, even with -Copt-level=3 the compiler still creates the allocation. I guess a zero-size allocation is really cheap, so doing this in only a few cases probably doesn't show that much of an performance impact.

This lint might be similar to the ptr_arg lint, but with the additional step of figuring out what the actual required type is. Maybe this can be done with expr_ty_adjusted, but if not this might be a bit more difficult with figuring out the actual type by looking at the function definition.

But the ptr_arg lint should be a good starting point for figuring out if you're dealing with a type like &String, &Vec, ...

[yoav-lavi]

/u/lebensterben also suggested the following on the Reddit post (which could be a separate lint):

Yep. Also &Vec::new() should also be considered anti-pattern when it's expecting &[]

[yoav-lavi]

@flip1995 apparently the reason this performs the same is that there isn't an actual allocation,
also shown in the docs:

Given that the String is empty, this will not allocate any initial buffer.

I'm assuming this is still valid for stylistic reasons, also needs to be checked if 100% of the additional output from String::new() is optimized away

[yoav-lavi]

/u/1vader added that &String::from("abc") where you can pass "abc" would also be a good idea to avoid