Skip to content

Commit

Permalink
Protect against panic retrieving form_ast
Browse files Browse the repository at this point in the history
Summary:
We are seeing panics when retrieving the ast syntax node from a form_id, triggered when the function of interest is defined in a different module.

Make this process safe by explicitly checking that the `form_id` and the assist context source both come from the same file.

Reviewed By: jcpetruzza

Differential Revision: D67764074

fbshipit-source-id: 855edd91b9be7846b841aaf14073e679e323d7dd
  • Loading branch information
alanz authored and facebook-github-bot committed Jan 6, 2025
1 parent 81ea858 commit b3f2f7e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
8 changes: 6 additions & 2 deletions crates/ide_assists/src/assist_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,15 @@ impl<'a> AssistContext<'a> {
SymbolClass::classify(&self.sema, InFile::new(self.file_id(), token))
}

pub(crate) fn form_ast<T>(&self, form_id: FormId<T>) -> T
pub(crate) fn form_ast<T>(&self, form_id: InFile<FormId<T>>) -> Option<T>
where
T: AstNode,
{
form_id.get(&self.source_file)
if self.file_id() == form_id.file_id {
Some(form_id.value.get(&self.source_file))
} else {
None
}
}

pub(crate) fn ast_ptr_get<T>(&self, ptr: InFileAstPtr<T>) -> Option<T>
Expand Down
7 changes: 4 additions & 3 deletions crates/ide_assists/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use hir::ExprId;
use hir::FormList;
use hir::FunctionClauseBody;
use hir::FunctionDef;
use hir::InFile;
use hir::InFileAstPtr;
use hir::InFunctionClauseBody;
use hir::NameArity;
Expand Down Expand Up @@ -385,9 +386,9 @@ pub(crate) fn ranges_for_delete_function(
})
.collect();

let spec_range = function_def.spec.map(|spec| {
let ast_spec = ctx.form_ast(spec.spec.form_id);
extend_form_range_for_delete(ast_spec.syntax())
let spec_range = function_def.spec.and_then(|spec| {
let ast_spec = ctx.form_ast(InFile::new(spec.file.file_id, spec.spec.form_id));
ast_spec.map(|ast_spec| extend_form_range_for_delete(ast_spec.syntax()))
});

Some(FunctionRanges {
Expand Down

0 comments on commit b3f2f7e

Please sign in to comment.