# Rust should warn on impl Trait returning unit **URL:** \ **Category:** compiler\ **Created:** [December 23, 2024, 10:50pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068 "2024-12-23T22:50:24Z")\ **Posts on this page:** 15\ **Page:** 1
**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\ **Post date:** [December 23, 2024, 10:50pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/1 "2024-12-23T22:50:24Z")
Here is an example bug from the codebase I'm working on atm and which was live in codebase for a couple of months: ```rust pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { self.items.iter().map(|x| x.id_string.clone()).collect::>(); } ``` The intent behind this implementation was "We don't want to disclosure our internal Id type, the only contract we provide is that you can use it as a key, but that's it". Originally it was more on the line of `self.some_key` but then our new developer changed it according to new requirements and made a typo. As you can imagine, the original intent was to write: ```rust pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { self.items.iter().map(|x| x.id_string.clone()).collect::>() } ``` This is a small and barely noticeable difference but it has huge implications on everything downsteam. Since this code falls into "Formally correct but almost certainly wrong" I have a strong opinion that we need a compiler warning for such cases. It's highly unlikely that user wrote `impl SomeTrait` and wanted a `()` as a result. This warning can be suppressed for cases where it's really intended (if they exist, I'm trying really hard to think of them but if it doesn't come from trait implementation I can't imagine why anyone will want to write impl Trait instead of just `()`), but in majority of cases it just points out that there is probably a type and you added a semicolon where you didn't actually intend to. ---
**Author:** ![steffahn](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/steffahn/32/13288_2.png) [@steffahn](https://internals.rust-lang.org/u/steffahn)\ **Post date:** [December 23, 2024, 11:10pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/2 "2024-12-23T23:10:33Z")
I fully agree that this is a reasonable thing to lint against; the rare cases where this is deliberate should be encouraged to write ```rust pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { self.items.iter().map(|x| x.id_string.clone()).collect::>(); () } ``` That being said, your _concrete_ code example should have also triggered `unused_must_use`, right? ```plaintext warning: unused return value of `collect` that must be used --> src/lib.rs:11:9 | 11 | self.items.iter().map(|x| x.id_string.clone()).collect::>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: if you really need to exhaust the iterator, consider `.for_each(drop)` instead = note: `#[warn(unused_must_use)]` on by default help: use `let _ = ...` to ignore the resulting value | 11 | let _ = self.items.iter().map(|x| x.id_string.clone()).collect::>(); | +++++++ warning: `playground` (lib) generated 1 warning ``` * * * The specifics of such a lint are a bit non-trivial unfortunately. E.g. one could question - whether returning `()` _from a function_ that produces `()` should be allowed - this is relevant because certain API designs might do in-place mutation with a function name that is ambiguous about whether it operating in-place or returning a new result; and realistically, such API design might rely on the fact that users expecting the wrong behavior would get a type error\[1\] (or an `unused_must_use`\[2\]). - what’s the intended interaction with control-flow expressions that have optional `;` anyways - e.g. ```rs pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { if condition { foo; } else { bar; } } ``` would probably clearly be problematic, too; but what if only _one_ of `foo` or `bar` is without the `;` (and evaluates to `()`, too)? So for example ```rs if condition { foo; } else { bar; () } ``` looking at which… I don’t think that’s in any way problematic anymore. - there could also be differing levels of enforcement between rustc and clippy, e.g. clippy could still lint the last example above for bad style, and always require an explicit `; ()`/`()` at the end ```rs if condition { foo; } else { bar; } () ``` - what if the `()` already made it into a local variable? E.g. ```rust pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { let calculated_id = { let excluded = "special"; self.items .iter() .filter(|x| x.id_string == excluded) .map(|x| x.id_string.clone()) .collect::>(); }; self.other.do_something_else(); calculated_id } ``` - should one possibly even aim to handle cases where the `()` is nested into something else, e.g. ```rust pub fn unique_id(&self) -> (impl Hash + PartialEq + Eq, Kind) { let calculated_id = { let excluded = "special"; self.items .iter() .filter(|x| x.id_string == excluded) .map(|x| x.id_string.clone()) .collect::>(); }; (calculated_id, Kind::StructuralId) } ``` here, we still at least have the `impl Hash + PartialEq + Eq` being instantiated by `()`, but it could also become part of another thing, e.g.: ```rust pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { self.items .iter() .map(|x| { self.item_watcher.log_visited_item(&x.id_string); x.id_string.clone(); }) .collect::>() } ``` producing a `Vec<()>` accidentally due to a semicolon, so the `impl Hash + PartialEq + Eq` itself isn’t `()` _directly_. * * * 1. if they expected a value and called an in-place operation 2. if they expected an in-place operation but it returns a value instead ---
**Author:** ![elidupree](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/elidupree/32/4304_2.png) [@elidupree](https://internals.rust-lang.org/u/elidupree)\ **Post date:** [December 24, 2024, 12:01am UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/3 "2024-12-24T00:01:05Z")
> [@steffahn](#): > > - what if the `()` already made it into a local variable? E.g. tbh I'd think we should also warn-by-default in _all_ cases where someone explicitly assigns to a local variable and it's implicitly given the type `()`! (Maybe even "implicitly making a `Vec<()>`" too, although that one's more of a stretch.) ---
**Author:** ![Pzixel](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/pzixel/32/4349_2.png) [@Pzixel](https://internals.rust-lang.org/u/Pzixel)\ **Post date:** [December 24, 2024, 12:06am UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/4 "2024-12-24T00:06:47Z")
> [@steffahn](#): > > That being said, your _concrete_ code example should have also triggered `unused_must_use`, right? Interesting. I've checked out codebase and I found one difference, our developer wrote this instead: ```rust impl Foo { pub fn unique_id(&self) -> impl Hash + PartialEq + Eq { self.items.iter().map(|x| x.id_string.clone()).collect_vec(); } } ``` And if you check it it won't warn this case: [Rust Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=20f5f7e7f6b22dfc4f243c50f8e0ff18) . I will create an issue/PR in the itertools repo, but I think this more highlight the issue rather than _being_ the issue I agree that such a lint need to thought through before implementing. ---
**Author:** ![cuviper](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cuviper/32/1897_2.png) [@cuviper](https://internals.rust-lang.org/u/cuviper)\ **Post date:** [December 24, 2024, 1:33am UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/5 "2024-12-24T01:33:02Z")
[`clippy::unit_return_expecting_ord`](https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord) is similar, but specific to `Ord`. ---
**Author:** ![kpreid](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kpreid/32/8484_2.png) [@kpreid](https://internals.rust-lang.org/u/kpreid)\ **Post date:** [December 24, 2024, 4:16am UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/6 "2024-12-24T04:16:07Z")
> [@elidupree](#): > > tbh I'd think we should also warn-by-default in _all_ cases where someone explicitly assigns to a local variable and it's implicitly given the type `()`! `clippy` already has a default lint for that: [`let_unit_value`](https://rust-lang.github.io/rust-clippy/master/#let_unit_value). ---
**Author:** ![kpreid](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kpreid/32/8484_2.png) [@kpreid](https://internals.rust-lang.org/u/kpreid)\ **Post date:** [December 24, 2024, 4:19am UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/7 "2024-12-24T04:19:37Z")
> [@steffahn](#): > > The specifics of such a lint are a bit non-trivial unfortunately. How about this definition? Lint when: - the declared return type of a function is `impl SomeTrait`, - the concrete return type is `()`, and - the `()` was _not_ produced by an explicit `()` expression at the end of the function. This way, it doesn't need any control flow analysis. I think this is reasonable because if you _do_ want to intentionally return `()`, the clearest way to do that is at the end; anything else is arguably obfuscating the fact that you're returning `()`. ---
**Author:** ![CAD97](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cad97/32/3460_2.png) [@CAD97](https://internals.rust-lang.org/u/CAD97)\ **Post date:** [December 24, 2024, 7:15pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/8 "2024-12-24T19:15:21Z")
> [@steffahn](#): > > whether returning `()` _from a function_ that produces `()` should be allowed Clippy would lean towards answering _no_ here, e.g. given `f: fn()` clippy doesn't like `Ok(f())` and wants `f(); Ok(())` instead. I agree this is generally\[1\] good style. * * * 1. My known exception to prove the rule is `f: fn() -> Result<()>` where sometimes I _do_ want to write `Ok(f()?)` because I'm logically still returning the result of the `f()` computation, not doing `f()` and indicating that control flow completed successfully. And `Ok(_?)` is a much better indicator of that than `_.map_err(Into::into)` or even `_?; Ok(())` imho. (`try` functions fix this) ---
**Author:** ![SUPERCILEX](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/supercilex/32/8541_2.png) [@SUPERCILEX](https://internals.rust-lang.org/u/SUPERCILEX)\ **Post date:** [December 27, 2024, 8:14pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/9 "2024-12-27T20:14:24Z")
One idea could be to ban semicolons at the end of expressions whose return type is not `()` (including generics). Of you did want to return the empty tuple, you'd have to write it explicitly. ---
**Author:** ![kpreid](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/kpreid/32/8484_2.png) [@kpreid](https://internals.rust-lang.org/u/kpreid)\ **Post date:** [December 30, 2024, 4:44pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/10 "2024-12-30T16:44:07Z")
I wrote a Clippy issue for this: > > > \### What it does > > If \`Trait\` is implemented for \`()\`, then a function returning …\`impl Trait\` can accidentally return \`()\` as that implementor simply by a misplaced semicolon. Clippy could have a lint against > > \* any return position \`impl Trait\` > \* whose concrete type is \`()\`, > \* but where that \`()\` value did not come from a literal \`()\` expression in tail position or \`return\`. > > This lint would fit into the \`suspicious\` category. > > \### Advantage > > Users will be able to find such bugs quicker, without needing to first test the behavior of the returned opaquely-typed value and find that it is wrong. > > See also \[prior discussion on IRLO\](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/). > > > \### Drawbacks > > An author might be intentionally passing through \`()\` by calling another function that currently returns \`()\` but will change in the future, and not want to satisfy the lint by adding \`; ()\` because that would disconnect the relationship between the two. > > \### Example > > The lint should fire on this code, because it returns \`()\` even though it looks like it returns something more meaningful: > > \`\`\`rust > fn key() -\> impl Eq { > \[1, 10, 2, 0\].sort() > } > \`\`\` > > The lint should not fire on this code: > > \`\`\`rust > fn key() -\> impl Eq { > \[1, 10, 2, 0\].sort(); // pretend I wrote something more useful here > () > } > \`\`\` > > It should also lint \*any part\* of the return value that is \`impl Trait\`; for example, the \`impl Trait\` might be wrapped in \`Result\` (or a tuple, or a \`Vec\`, or many other things): > > \`\`\`rust > fn try\_get\_key() -\> Result\ { > Ok(\[1, 10, 2, 0\].sort()) // should fire > } > > fn is\_on\_purpose\_really() -\> Result\ { > Ok(()) // should not fire > } > \`\`\` > > It is debatable whether it should fire on this code: > > \`\`\`rust > fn hmm() -\> impl Eq { > let x = (); > x > } > \`\`\` > > but the \`let\_unit\_value\` lint fires in that case anyway. ---
**Author:** ![zackw](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/zackw/32/2071_2.png) [@zackw](https://internals.rust-lang.org/u/zackw)\ **Post date:** [January 3, 2025, 2:57pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/11 "2025-01-03T14:57:37Z")
It's not a realistic change from the current language, but if I'd been involved at the beginning of Rust I would have made `{ a; b; }` return the value of `b`. The semicolon after `b` would have been _required_, and you would have had to write `{ a; b; (); }` if you wanted the block to return unit. ... I kinda wonder if we could lint our way there over the course of a few editions. ---
**Author:** ![FZs](https://avatars.discourse-cdn.com/v4/letter/f/85f322/32.png) [@FZs](https://internals.rust-lang.org/u/FZs)\ **Post date:** [January 3, 2025, 5:35pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/12 "2025-01-03T17:35:29Z")
That would be annoying in cases where two branches of a control flow structure have tail expressions of different types, but you don't want to return them. ```rust fn do_something() -> i32; fn do_something_else() -> &'static str; fn main() { if condition { do_something(); } else { do_something_else(); }; (); } ``` would not compile. * * * I also like the fact that tail expressions have a nice marker that they are different (the lack of a semicolon). As a beginner ~~coming from JavaScript and having used `eval()`~~, one could reasonably expect the following to return `0` if the condition is not met\[1\]: ```rust { 0; if condition { 1; } } ``` ...while currently you can't write this: ```rust { 0 if condition { 1 } } ``` which makes it clear that Rust returns the value of the syntactically last expression, not the last expression that was actually evaluated, which makes reasoning about easier. That was my experience anyway, to me the status quo of semicolon-lacking tail-returns feel very natural. But I can see that this reason might be more subjective than objective. * * * 1. I remembered that wrong, JS `eval` doesn't return `0`, it returns `undefined` there ---
**Author:** ![zackw](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/zackw/32/2071_2.png) [@zackw](https://internals.rust-lang.org/u/zackw)\ **Post date:** [January 5, 2025, 7:12pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/13 "2025-01-05T19:12:31Z")
> [@FZs](#): > > That would be annoying in cases where two branches of a control flow structure have tail expressions of different types, but you don't want to return them. > > ```rust > fn do_something() -> i32; > fn do_something_else() -> &'static str; > > fn main() { > if condition { > do_something(); > } else { > do_something_else(); > }; > (); > } > > ``` > > would not compile. Yeah, this hypothetical altered language would need some additional sugar surrounding type unification -- specifically, if the value of any sort of conditional expression is not used then the arms of that expression would not be required to have the same type. Probably other such cases. I'd also still allow omission of the semicolon after any expression that ends with a block, when that expression is not part of a larger expression. So in this case you would write ```rust fn main() { if condition { do_something(); } else { do_something_else(); } (); } ``` and that _would_ compile. > [@FZs](#): > > ```rust > { > 0; > if condition { > 1; > } > } > > ``` I see where someone might get confused here but I think their confusion would be cleared up as soon as they saw the error message. (E0317: `if` may be missing an `else` clause - same as you'd get now for leaving the semicolon off the `1` but _not_ off the `0`). ---
**Author:** ![CAD97](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/cad97/32/3460_2.png) [@CAD97](https://internals.rust-lang.org/u/CAD97)\ **Post date:** [January 5, 2025, 7:54pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/14 "2025-01-05T19:54:14Z")
Such a design probably also wants to differentiate between `fn f() {` (produces "nothing") and `fn f() -> () {` (produces `()`) — while producing a useful value is common, especially in more functional/pure style API design, subroutines run slowly for their side effect aren't _uncommon_, and writing `();` to indicate that yes, this function doesn't return anything would get a bit annoying for functions that don't return anything (as opposed to those which return `()` deliberately). While it doesn't generalize to compound types which contain `()`, this does suggest a fairly simple style rule for clippy to push users toward: functions which are declared _without_ `-> $ReturnType` should end with an empty expression (or expression-with-block which itself ends with such an expression) (i.e. with a semicolon), and functions _with_ a syntactical return type shouldn't be allowed to end with an implicit `()` (i.e. an empty expression as prior or a function call which is declared without `-> $ReturnType`). This will, as specified, make a lint-only distinction between the functions which produce "nothing" versus producing `()` deliberately as a value, based on syntax, while still keeping most semantic benefit of unifying the two. ---
**Author:** ![system](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/system/32/14092_2.png) [@system](https://internals.rust-lang.org/u/system)\ **Post date:** [June 29, 2026, 7:55pm UTC](https://internals.rust-lang.org/t/rust-should-warn-on-impl-trait-returning-unit/22068/15 "2026-06-29T19:55:10Z")
This topic was automatically closed 540 days after the last reply. New replies are no longer allowed.