Extract into function; Option::unwrap()

[zhiburt]

Hi there

I was doing Extract Function in vs code and then this happened. It does happen continuously.

Just in case: The code is valid and can be compiled. cargo clean din't help.

Panic context:
> 
version: db2a7087b 2021-12-13 stable
request: codeAction/resolve CodeAction {
    title: "Extract into function",
    group: None,
    kind: Some(
        CodeActionKind(
            "refactor.extract",
        ),
    ),
    edit: None,
    is_preferred: None,
    data: Some(
        CodeActionData {
            code_action_params: CodeActionParams {
                text_document: TextDocumentIdentifier {
                    uri: Url {
                        scheme: "file",
                        cannot_be_a_base: false,
                        username: "",
                        password: None,
                        host: None,
                        port: None,
                        path: "/home/maxim/projects/tabled/papergrid/src/lib.rs",
                        query: None,
                        fragment: None,
                    },
                },
                range: Range {
                    start: Position {
                        line: 2389,
                        character: 4,
                    },
                    end: Position {
                        line: 2453,
                        character: 5,
                    },
                },
                context: CodeActionContext {
                    diagnostics: [],
                    only: None,
                },
                work_done_progress_params: WorkDoneProgressParams {
                    work_done_token: None,
                },
                partial_result_params: PartialResultParams {
                    partial_result_token: None,
                },
            },
            id: "extract_function:RefactorExtract:0",
        },
    ),
}

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', crates/syntax/src/ted.rs:137:41
stack backtrace:
   0: rust_begin_unwind
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/panicking.rs:100:14
   2: core::panicking::panic
             at /rustc/f1edd0429582dd29cccacaf50fd134b05593bd9c/library/core/src/panicking.rs:50:5
   3: syntax::ted::replace_all
   4: syntax::ted::replace_with_many
   5: ide_assists::handlers::extract_function::rewrite_body_segment
   6: <alloc::vec::Vec<T> as alloc::vec::spec_from_iter::SpecFromIter<T,I>>::from_iter
   7: ide_assists::handlers::extract_function::format_function
   8: ide_assists::assist_context::Assists::add::{{closure}}
   9: ide_assists::assist_context::Assists::add_impl
  10: ide_assists::handlers::extract_function::extract_function
  11: ide_assists::assists
  12: ide::Analysis::assists_with_fixes::{{closure}}
  13: std::panicking::try
  14: rust_analyzer::handlers::handle_code_action_resolve
  15: std::panicking::try
  16: <F as threadpool::FnBox>::call_box
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

rust-analyzer version: (eg. output of "Rust Analyzer: Show RA Version" command)

rust-analyzer version: db2a7087b 2021-12-13 stable

rustc version: (eg. output of rustc -V)

rustc 1.59.0 (9d1b2106e 2022-02-23)

Take care

[Veykril]

Are you able to share the snippet of code you wanted to extract?

[Veykril]

Probably hit one of these two unwraps, though that would imply the code that was being extracted wasn't well formed which I kind of doubt here (nevertheless those unwraps shouldn't be there)

https://github.com/rust-lang/rust-analyzer/blob/c61bb6be8c3f45c15a24a26413a665c6c8f63f5c/crates/ide_assists/src/handlers/extract_function.rs#L1619-L1628

Edit: doesn't seem to be the cause here

[zhiburt]

@Veykril sure

These lines in that branch

https://github.com/zhiburt/tabled/blob/14c0629cc74f43c676a4c83be8f5010f57a53ec1/papergrid/src/lib.rs#L2390-L2454

[Alastair-smith2]

It seems specifically the section from line 2422 to 2437 that causes the error. Refactoring that block of code alone you get the error and if that section is removed before refactoring the other blocks, the other blocks refactor successfully

For reference, it's the following code that when refactored causes a panic -

if grid.margin.top.size > 0 {
    container = Container::new(
        container.width,
        container.height + grid.margin.top.size,
        ContainerKind::Rows(vec![
            Container::new(
                container.width,
                grid.margin.top.size,
                ContainerKind::Split {
                    c: grid.margin.top.fill,
                },
            ),
            container,
        ]),
    );
}

[Alastair-smith2]

If it's of interest, it seems in the above example by replacing container.width with a declared variable (e.g. let w = container.width; and then using w the refactor works as intended.

The following won't panic (which is very similar to the final if block with subtle differences):

if grid.margin.top.size > 0 {
    let w = container.width;
    container = Container::new(
        w,
        container.height + grid.margin.top.size,
        ContainerKind::Rows(vec![
            Container::new(
                w,
                grid.margin.top.size,
                ContainerKind::Split {
                    c: grid.margin.top.fill,
                },
            ),
            container,
        ]),
    );
}

while the minimal example (using container.width directly) will.

The final if block in the highlighted code will also fail to refactor if the variable w is replaced with direct container.width usage.

[Alastair-smith2]

@Veykril is* there a chance that somehow the following was entered into? https://github.com/rust-lang/rust-analyzer/blob/c61bb6be8c3f45c15a24a26413a665c6c8f63f5c/crates/ide_assists/src/handlers/extract_function.rs#L1663-L1688 From a glance that'd be the only other place I can see at the moment where the flow ends up with replace_all being called if the earlier snippet you shared wasn't the problem.

[Veykril]

Oh right, the panic happened in replace_all actually, I missed that. Ye so the problem here is that we call replace on something that has no parent which causes the unwrap in replace_all to trigger. So this should only happen if we try to replace the SourceFile ast node, which I don't think we do here. Or if we try to replace the top-node of a macro expansion file, which is what I assume might be happening here given the vec! calls.

This makes me again suspicious of the previous linked function https://github.com/rust-lang/rust-analyzer/blob/c61bb6be8c3f45c15a24a26413a665c6c8f63f5c/crates/ide_assists/src/handlers/extract_function.rs#L1590-L1638

Since we a usage search here and the nodes we get out of that could be in the macro expansion. Now I am still not sure why that would cause us to ever try to replace the entire macro expansion though... This might require some more debugging the concrete case.

[Alastair-smith2]

The stack trace for this I had was the following when in debug:

unwrap<rowan::api::SyntaxNode<syntax::syntax_node::RustLanguage>> (@core::option::Option$LT$T$GT$::unwrap::h87f4fa70037a9ae6:20)
replace_all (/<path>/rust-analyzer/crates/syntax/src/ted.rs:137)
replace_with_many<&rowan::api::SyntaxNode<syntax::syntax_node::RustLanguage>> (/<path>/rust-analyzer/crates/syntax/src/ted.rs:132)
replace<&rowan::api::SyntaxNode<syntax::syntax_node::RustLanguage>, &rowan::api::SyntaxNode<syntax::syntax_node::RustLanguage>> (/<path>/rust-analyzer/crates/syntax/src/ted.rs:128)
fix_param_usages (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:1631)
rewrite_body_segment (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:1585)
make_body (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:1435)
format_function (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:1310)
{closure#0} (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:124)
{closure#0}<&str, ide_assists::handlers::extract_function::extract_function::{closure_env#0}> (/<path>/rust-analyzer/crates/ide_assists/src/assist_context.rs:169)
add_impl (/<path>/rust-analyzer/crates/ide_assists/src/assist_context.rs:199)
add<&str, ide_assists::handlers::extract_function::extract_function::{closure_env#0}> (/<path>/rust-analyzer/crates/ide_assists/src/assist_context.rs:169)
extract_function (/<path>/rust-analyzer/crates/ide_assists/src/handlers/extract_function.rs:94)

[jinohkang-theori]

Minimal reproducer:

pub fn rust_analyzer_12065_reproducer(value: String) -> String {

    // Extract the following statement into a function
    vec![&value == &value];

    value
}