Strange `async use ...;` compiler suggestion

[gamozolabs]

Really minor issue, but when building the following, I get a strange fix suggestion which seems incorrect:

#[tokio::main]
async fn main() {
    Command::new("git");
}

[package]
name = "replicate"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
tokio = { version = "1", features = ["process", "rt-multi-thread", "macros"] }

   Compiling replicate v0.1.0 (/home/pleb/replicate)
error[E0433]: failed to resolve: use of undeclared type `Command`
 --> src/main.rs:3:5
  |
3 |     Command::new("git");
  |     ^^^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
2 | async use std::process::Command;
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^
2 | async use tokio::process::Command;
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0433`.
error: could not compile `replicate` due to previous error

pleb@gamey ~/replicate $ rustc --version
rustc 1.56.0-nightly (492723897 2021-07-29)
pleb@gamey ~/replicate $ rustup default
nightly-x86_64-unknown-linux-gnu (default)

The async use seems to be a bit strange for the suggestion. Nevertheless, have a wonderful day <3

[booleancoercion]

This also appears to happen on the latest stable version (1.54). Here's a playground link.
It should be noted that expanding the proc macro manually produces a correct suggestion.

[hkratz]

Rustc suggests inserting the use directive between async and fn main(). Essentially the tokio proc macro removes the async from the main fn declaration so that it is no longer part of the fn for rustc. In absence of other use directives rustc suggests inserting the import directive directly before the fn main() leading to this behavior.

[tmandry]

It looks like the first item in the module is used. We already check if the item span is from a macro expansion. The span for main ends up in the root context somehow.

[tmandry]

There was some discussion in Zulip about this. It's probably a bug in rustc but could indicate a bug in Tokio (if the proc macro is producing root context spans for everything).

[pnkfelix]

This is probably not a bug in Tokio.

This is, at least partially, an artifact of how we generate the suggestion here.

Consider this code (playground):

/* hey
whazzup */ fn main() {
    Command::new("git outta here");
}

it generates the following suggestion:

help: consider importing one of these items
  |
2 | whazzup */ use std::process::Command;
  |            ++++++++++++++++++++++++++
2 | whazzup */ use tokio::process::Command;
  |            ++++++++++++++++++++++++++++

That is, it seems like our system for generating the use suggestion is trying to construct a source line by looking at the span of the fn main, taking the starting point for that span, and then spitting out a suggestion that includes everything that preceded that fn main on that line.

(My current guess is that this was intended to be a way to generate a reasonable looking level of indentation for the use item, as well as a correct line number for it.)

((Update: Arguably its also trying to make an inference that anywhere you could insert an fn-item, you can likewise insert a use-item with the appropriate scope. I.e. that it actually is doing the "right thing" for my comment, if you assume that one wants to inject the use in between the comment and the fn-item. I don't know why one would actually assume that, but it is internally coherent, if the span of the fn-item had included the qualifiers like async...))

---

Update: okay, I should have been a little more thorough in my investigation before. The use-injection will correctly account for qualifiers like async/const/pub when you apply them to my playground example. So there is probably some interaction with the #[tokio::main] attribute, which has been alluded to above, that I did not account for in my earlier description.

Update 2: Oddly enough, using pub async will produce the correct output, even with #[tokio::main]...

[pnkfelix]

Okay, so I think the heart of the problem is this:

1. Our suggestion system relies on the spans of items accurately reflecting where items fall. For example, in this case, the generation of the use-item suggestion is built upon the span of the given fn-item for `main. So if the span is misleading, then you get misleading output.
2. The span of an item like a fn is built from its component pieces. We don't store an explicit span representing the whole item with the item itself. Instead, we compute it on-the-fly from the tokens that make up the item. So, if you want to know where an fn starts, you'd look at the low end of the span associated with the first token on that fn item.
3. The very first thing that the macro underlying #[tokio::main] does, after first calling main -> entry -> parse_knobs, is remove the async token from the input:

fn parse_knobs(mut input: syn::ItemFn, is_test: bool, config: FinalConfig) -> TokenStream {
    input.sig.asyncness = None;
    ...

(This explains why my experiments with adding things like pub made the problem go away here: killing off the async token won't mess up the overall span if its preceded by another token that is the end supplier for the low-end of the overall span.)

4. So, an idea I had was this: Before we delete the async token, can we somehow save its span and keep it with the fn-item? Unfortunately, as already mentioned, there is no separate representation for the overall span of the fn-item; it comes solely from the tokens themselves.
5. So then I came up with this hack:

--- a/tokio-macros/src/entry.rs                                                                                           
+++ b/tokio-macros/src/entry.rs                                                                                           
@@ -293,6 +293,11 @@ fn build_config(                                                                                     
 }                                                                                                                        
                                                                                                                          
 fn parse_knobs(mut input: syn::ItemFn, is_test: bool, config: FinalConfig) -> TokenStream {                              
+    if let Some(asyncness) = input.sig.asyncness {                                                                       
+        if let Some(joined_span) = asyncness.span.join(input.sig.fn_token.span) {                                        
+            input.sig.fn_token.span = joined_span;                                                                       
+        }                                                                                                                
+    }                                                                                                                    
     input.sig.asyncness = None;                                                                                          
                                                                                                                          
     // If type mismatch occurs, the current rustc points to the last statement.                                                                                                 

This is a hack for a number of reasons:

a. It sounds pretty goofy to associate a span of unbounded length with the token for a keyword like fn.

b. The join method has warnings on it that it only returns Some on nightly... on other hosts, it will return None, which means this code will have no effect there anyway.

Having said that, the hack does seem to work (on nightly alone, for the reason given above). Given this source code:

#[tokio::main]
/* what the                                                                                                               
hey */ async fn main() {
    Command::new("git");
}

here are the results we see:

% cargo +nightly build
   Compiling issue_87613 v0.1.0 (/home/ubuntu/Dev/Rust/rust-87613/objdir-dbg/issue_87613)
error[E0433]: failed to resolve: use of undeclared type `Command`
 --> src/main.rs:4:5
  |
4 |     Command::new("git");
  |     ^^^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
3 | hey */ use std::process::Command;
  |        ++++++++++++++++++++++++++
3 | hey */ use tokio::process::Command;
  |        ++++++++++++++++++++++++++++

For more information about this error, try `rustc --explain E0433`.
error: could not compile `issue_87613` due to previous error
%

For comparison, here is the (same as before) result atop stable:

% cargo +stable build
   Compiling issue_87613 v0.1.0 (/home/ubuntu/Dev/Rust/rust-87613/objdir-dbg/issue_87613)
error[E0433]: failed to resolve: use of undeclared type `Command`
 --> src/main.rs:4:5
  |
4 |     Command::new("git");
  |     ^^^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
3 | hey */ async use std::process::Command;
  |              ++++++++++++++++++++++++++
3 | hey */ async use tokio::process::Command;
  |              ++++++++++++++++++++++++++++

For more information about this error, try `rustc --explain E0433`.
error: could not compile `issue_87613` due to previous error

[pnkfelix]

Having said all that, I personally think that the real bug here is how the use-item placement is being inferred.

That is, I don't think it was a good idea to use a span taken from item.span.lo for the first non-use item in the module.

I think we'll have better luck if we put it after the item.span.hi for the item preceding the first non-use item in the module. (Preferably on the next line, if there is a line between item.span.hi and the next item's span.lo.)

After all, the current approach is suggesting adding a use based on where that first non-use item occurs. That means its allowing arbitrary amounts of other stuff (i.e. comments) to separate the use-items at the start of the module from the place it is suggesting. That doesn't sound like the Rust coding style we typically promote.

Update: I misread the existing code. It does use the span of an existing use, if it can find one before a non-use, non-extern crate item. The code path it is falling into now, is because it is finding the fn main and trying to use that as the basis for its suggestion.

So, the question now is whether we should try to make do with the span that the #[tokio::main] expansion is handing us. We are not going to be able to leverage other use-items, because there are no such items in the example here.