Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic when using Fixer: assertion failed: (left == right) #146

Open
Vannevelj opened this issue Apr 17, 2022 · 3 comments
Open

Panic when using Fixer: assertion failed: (left == right) #146

Vannevelj opened this issue Apr 17, 2022 · 3 comments

Comments

@Vannevelj
Copy link

I'm using rslint's Fixer to make changes in source code but have found that certain order of operations cause an internal panic. I have narrowed it down to this:

use rslint_core::autofix::Fixer;
use rslint_parser::{
    ast::ParameterList, parse_with_syntax, AstNode, Syntax, SyntaxNode, SyntaxNodeExt,
};
use std::sync::Arc;

fn main() {
    let source = "
function foo(a, b, c) {
    console.log(a);
    console.log(b.field);
}";

    let syntax = Syntax::default().typescript();
    let parse = parse_with_syntax(source, 0, syntax);
    let ast = parse.syntax();
    let mut fixer = Fixer::new(Arc::from(source));

    // If parameters are changed last, it works
    insert_interface(&ast, &mut fixer);
    update_parameters(&ast, &mut fixer);
    
    println!("{}", fixer.apply());
}

fn insert_interface(ast: &SyntaxNode, fixer: &mut Fixer) {
    let start_of_file = ast.text_range();

    fixer.insert_before(
        start_of_file,
        "
interface B {
    field: any,
}
",
    );
}

fn update_parameters(ast: &SyntaxNode, fixer: &mut Fixer) {
    let parameters = ast
        .descendants()
        .find(|node| node.is::<ParameterList>())
        .map(|node| node.to::<ParameterList>())
        .unwrap();

    for parameter in parameters.parameters() {
        if parameter.text() == "b" {
            fixer.insert_after(parameter.syntax().text_range(), ": B");
        } else {
            fixer.insert_after(parameter.syntax().text_range(), ": any");
        }
    }
}

If insert_interface() is called before update_parameters() then I get the intended output:

interface B {
    field: any,
}

function foo(a: any, b: B, c: any) {
    console.log(a);
    console.log(b.field);
}

However if I swap them around and call update_parameters() first, I receive this:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `139`,
 right: `118`', C:\Users\jer_v\.cargo\registry\src\github.com-1ecc6299db9ec823\rslint_text_edit-0.1.0\src\lib.rs:74:5
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\std\src\panicking.rs:584 
   1: core::panicking::panic_fmt
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:143
   2: core::fmt::Arguments::new_v1
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\fmt\mod.rs:387  
   3: core::panicking::assert_failed_inner
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\/library\core\src\panicking.rs:225
   4: core::panicking::assert_failed<text_size::size::TextSize,text_size::size::TextSize>
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\library\core\src\panicking.rs:182
   5: rslint_text_edit::apply_indels
             at C:\Users\jer_v\.cargo\registry\src\github.com-1ecc6299db9ec823\rslint_text_edit-0.1.0\src\lib.rs:74
   6: rslint_core::autofix::Fixer::apply
             at C:\Users\jer_v\.cargo\registry\src\github.com-1ecc6299db9ec823\rslint_core-0.3.0\src\autofix\mod.rs:33
   7: fixer::main
             at .\src\main.rs:24
   8: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
             at /rustc/7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c\library\core\src\ops\function.rs:227

I also found that if I call update_parameters() first but use fixer.insert_after() for the interface (as opposed to insert_before), it also works.

@Vannevelj
Copy link
Author

I noticed that the original code from rust-analyzer has changed since you've copied it over, e.g. through https://github.com/rust-lang/rust-analyzer/pull/11573/files

Since I don't actually need most of the functionality in that Fixer, I've created my own implementation with a very similar API that worked for all the scenarios I had under test:

use rslint_parser::TextRange;

struct Change(usize, String);

pub trait TextEdit {
    fn load(text: impl ToString) -> TextEditor;
    fn insert_after(&mut self, range: TextRange, text: impl ToString);
    fn insert_before(&mut self, range: TextRange, text: impl ToString);
    fn apply(&mut self) -> String;
}

pub struct TextEditor {
    changes: Vec<Change>,
    source: String,
}

impl TextEdit for TextEditor {
    fn load(text: impl ToString) -> TextEditor {
        TextEditor {
            changes: vec![],
            source: text.to_string(),
        }
    }

    fn insert_after(&mut self, range: TextRange, text: impl ToString) {
        self.changes
            .push(Change(range.end().into(), text.to_string()));
    }

    fn insert_before(&mut self, range: TextRange, text: impl ToString) {
        self.changes
            .push(Change(range.start().into(), text.to_string()));
    }

    fn apply(&mut self) -> String {
        let new_source_length = {
            let total_insertion_length: usize =
                self.changes.iter().map(|change| change.1.len()).sum();

            self.source.len() + total_insertion_length
        };

        self.changes.sort_by(|a, b| a.0.cmp(&b.0));
        let mut buf = String::with_capacity(new_source_length);
        let mut pointer = 0usize;

        for change in &self.changes {
            let current_pointer = change.0;
            if current_pointer > pointer {
                buf.push_str(&self.source[pointer..current_pointer]);
            }

            buf.push_str(change.1.as_str());
            pointer = current_pointer
        }
        buf.push_str(&self.source[pointer..self.source.len()]);

        buf
    }
}

@Stupremee
Copy link
Member

Sorry for replying so late!

But thank you for looking into it and finding the fixed version from rust-analyser.
I will probably just copy it into rslint, and then it should be fixed.

@yetyetanotherusername
Copy link

Has this fix been merged yet? I ran into a similar error where changing the order of two replacements "fixes" it and wondering if this is some kind of regression/different issue or the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants