Why is js-sys::Promise must_use?

While using web-sys, js-sys, and wasm-bindgen together to access some Web Javascript APIs from wasm-compiled Rust, I ran into a Web API that returned a Promise yielding a result I wanted to use. I noticed that Promise::then returns another Promise, which is labeled #[must_use], but I'm not sure what to do with it. Am I required to convert it to a Rust Future using wasm-bindgen-futures, or can I drop the Promise and still expect both the Web API to complete its work and for my callback passed to Promise::then be called? I couldn't find this documented anywhere.

In JS you don't have to do anything after then to make the Promise do its thing; then is for chaining Promises, so eventually you just put a semicolon instead of another call to then and stop the chain. The idea is that the rest of the execution context will be evaluated before it calls the callbacks that you passed to then. I don't know how to translate that to Rust, but I found this interesting from those docs:

other languages have mechanisms for lazy evaluation and deferring a computation, which they also call "promises", e.g. Scheme. Promises in JavaScript represent processes that are already happening, which can be chained with callback functions.

I think the answer is that you drop the final Promise when you're done chaining then calls. I just never thought about it that way because JS is so different... the semicolon doesn't even do anything!

Also, I think it is appropriate to use #[allow(unused_must_use)] to suppress some instances of #[must_use] types. One example is using write to send error messages. What if that write returns an error? You don't want an infinite loop of write returning more errors that can't be written.

Ah ok, that's what I suspected from looking at the MDN docs (and having used JS Promises a tiny bit in the past).. The #[must_use] attribute can document why its present, which shows up in compiler warnings, so I might submit a PR with js-sys to add that.

Most codebases in the wild that I've seen tend to use _ to explicitly ignore a result.

let _ = write!(...);

Another popular pattern is ok() to turn the Result into an Option. This has the nice side-effect that you are kinda saying "it's okay for this to fail".

write!(...).ok();

In general, I consider #[allow] to be a code smell that people use to mask warnings instead of dealing with them properly.

Why is #[allow] not considered a "proper" solution here though? It's just as explicit (if not more so IMO). And #[must_use] is ultimately a lint, so why is it not appropriate to turn off a lint with an #[allow]? Seems really arbitrary to me.

I would say it's because you are disabling the lint instead of dealing with the underlying problem it has identified (i.e. something with the result, even if you are just throwing it away).

In my experience on both these forums and in the wild, the vast majority of times people use #[allow] is because of sloppy programming and the author is just trying to make the warnings go away.

Ah, it looks like I made an incorrect assumption. I thought you could use #[allow(unused_must_use)] on individual statements. If that were true, then I don't see why it wouldn't be a perfectly valid option.

But it looks like that's not true. The #[allow] must apply to the surrounding scope, in which case I agree that using it is a code smell.

That being said, would there be any way to allow attributes to apply to statements like that? It seems like that could be a valid use case in some instances.

2 Likes

In JS, promises can be rejected (this is basically divergence), and it's best practice to always handle the rejection case so errors aren't silently ignored. Here's a JS lint to that effect: typescript-eslint/no-floating-promises.md at main · typescript-eslint/typescript-eslint · GitHub

The big difference from Rust+js_sys is that the TypeScript lint is satisfied when you end a promise chain with .catch(), however Rust's must_use is not satisfied with a catch call, because it treats all Promise values the same, no matter how they're created.

I think the best solution would be for Rust to ignore js_sys::Promise's must_use only when it comes from a call to Promise.catch. That way end users don't have to silence any lints as long as they're doing things right. This could be implemented with some theoretical #[must_use_allow_unused] attribute on the definition of js_sys::Promise::catch.

An inner attribute like #![cgf(test)] affects its own scope. An outer attribute like #[cfg(test)] affects the block scope that follows it. That can be any block, including a module or function.

I think disabling the lint makes it clear that you're ignoring the warning and that you know that. It's declarative as well so you can make it clear that "this function either works or fails silently". The _ seems like it would bury the warning in imperative code.

Yes, but it applies to the whole block, not just one statement. So unless you're going to add a dedicated block, just for that one statement (as you did in your example), then there is a chance the #[allow] could accidentally or eventually cover up a #[must_use] that you (or someone editing your code) genuinely forgot to handle.

In general, I think you should disable lints for the smallest possible scope, to prevent exactly those kinds of problems. In that case, I think I have to agree that #[allow] is a sub-optimal solution here. But only because it can't apply to individual statements. I think it would be really nice if that were possible, though, because I would probably prefer

#[allow(unused_must_use)]
returns_result();

over

let _ = returns_result();

I think the former is louder and more explicit.

then there is a chance the #[allow] could accidentally or eventually cover up a #[must_use] that you (or someone editing your code) genuinely forgot to handle

The same could be said of unsafe but that is exactly how the standard library uses unsafe blocks. It’s also how the standard library suppresses lint warnings and uses conditional compilation. Attributes are explicit and searchable, which is consistent with the coding style that Rust encourages.

This might be my ignorance talking, but why not do this?

impl<E> Promise<(), Err> {
    /// Handle errors produced at the end of a promise chain.
    /// Use this function instead of `catch` to silence Rust's `#[must_use]` warnings.
    pub fn caught<F: FnOnce(E)>(f: F) {
        // Return value Promise<(), ()> carries no useful information.
        // Silence the must-use warning.
        let _ = self.then(|()| (), f);
    }
}

The problem with that API is 1) Promise is not generic -- the success and error types are dynamic -- and 2) something like Promise::then can be called after Promise::catch.

That's actually a slick idea. If I ever have to work on web again I'll drop something like that into the project. But I don't think it belongs in the bindings and I still think the underlying issue should be solved.

This would be true if warnings were used sparingly, with as few false positives as possible. But since that is no longer true (there has been a decline over the past few years), warnings are on a slippery slope nowadays towards losing their utility. Every Rust version update in our codebases trigger chores for “adapting to the new normal”, with rarely ever a change that fixes an actual issue.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.