Validate the location of crate in paths

[djrenren]

This solution does not fully handle use statements. See below

This pull requests implements simple validation of usages of the crate keyword in Paths. Specifically it validates that:

• If a PathSegment is starts with the crate keyword, it is also the first segment of the Path
• All other usages of crate in Paths are considered errors.

This aligns with rustc's rules. Unlike rustc this implementation does not issue a special error message in the case of ::crate but it does catch the error.

Furthermore, this change does not cover all error cases. Specifically the following is not caught:

use foo::{crate}

This is because this check is context sensitive. From an AST perspective, crate is the root of the Path. Only by inspecting the full UseItem do we see that it is not in fact the root. This problem becomes worse because UseTrees are allowed to be arbitrarily nested:

use {crate, {{crate, foo::{crate}}}

So this is a hard problem to solve without essentially a breadth-first search. In a traditional compiler, I'd say this error is most easily found during the AST -> HIR conversion pass but within rust-analyzer I'm not sure where it belongs.

Under the implementation in this PR, such errors are ignored so we're more correct just not entirely correct.

[djrenren]

There's an alternate implementation that checks that the PathSegment is exactly the crate keyword:

    let segment = path.segment();

    let first_element_as_token = segment.as_ref().and_then(|it| it.syntax().first_child_or_token()?.into_token());
    let last_element_as_token = segment.as_ref().and_then(|it| it.syntax().last_child_or_token()?.into_token());

    let start_of_path = path.qualifier().is_none();

    // `crate` is allowed to be the start of the path, so we can't error if this is true
    if start_of_path && first_element_as_token == last_element_as_token {
        return
    }

    let child_tokens = segment
        .iter()
        .flat_map(|seg| seg.syntax().children_with_tokens())
        .flat_map(|it| it.into_token());


    // Beyond the first position, `crate` is never allowed to appear
    for child in child_tokens {
        if child.kind() == CRATE_KW {
            errors.push(SyntaxError::new("`crate` keyword is only allowed as the first segment of a path", child.text_range()))
        }
    }

but I wasn't sure what was preferable.

[djrenren]

Was thinking about this more last night, and this code is mainly weird because there's no accessor on PathSegment that would return the crate keyword. Maybe it would make more sense to add one? I imagine there's a similar situation for the super keyword.

[matklad]

Maybe it would make more sense to add one? I imagine there's a similar situation for the super keyword.

Definitelly, this should work by adding T![crate] here: https://github.com/rust-analyzer/rust-analyzer/blob/549ce9a9cf25efa3eba6549f96b2e43bc640faa9/xtask/src/ast_src.rs#L598

but within rust-analyzer I'm not sure where it belongs.

Not that I know for sure :D

My gut feeling would probably try to push all diagnostics as early as possible, and, in this case, this means syntax tree validation.

[djrenren]

Okay looks like it should handle everything! And the code looks so much cleaner.

[matklad]

bors r+

The next step would be to check self and super as well

[bors]

Build succeeded:

• Rust (macos-latest)
• Rust (ubuntu-latest)
• Rust (windows-latest)
• TypeScript

[djrenren]

Went to work on self/super and found a bug! I've submitted a fix #4227

Sorry about that.

[djrenren]

Next step: #4246