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.

Sign up for GitHub

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

Jump to bottom

chore: remove duplicated code from LSP #5116

Merged
merged 2 commits into from
May 28, 2024
Merged

chore: remove duplicated code from LSP #5116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e758ff
chore: remove duplicated code from LSP
TomAFrench May 25, 2024
0aa6e6c
Apply suggestions from code review
TomAFrench May 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 12 additions & 28 deletions tooling/lsp/src/requests/goto_declaration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ use crate::LspState;
use crate::{parse_diff, resolve_workspace_for_source_path};
use async_lsp::{ErrorCode, ResponseError};

use fm::PathString;
use lsp_types::request::{GotoDeclarationParams, GotoDeclarationResponse};

use nargo::insert_all_files_for_workspace_into_file_manager;
use noirc_driver::file_manager_with_stdlib;

use super::{position_to_byte_index, to_lsp_location};
use super::{position_to_location, to_lsp_location};

pub(crate) fn on_goto_declaration_request(
state: &mut LspState,
Expand All @@ -32,49 +33,32 @@ fn on_goto_definition_inner(
let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap();
let package = workspace.members.first().unwrap();

let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into();

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_diff(&workspace_file_manager, state);

let (mut context, crate_id) =
nargo::prepare_package(&workspace_file_manager, &parsed_files, package);

let interner;
if let Some(def_interner) = state.cached_definitions.get(&package_root_path) {
interner = def_interner;
let package_root_path = package.root_dir.as_os_str().to_string_lossy().into_owned();
let interner = if let Some(def_interner) = state.cached_definitions.get(&package_root_path) {
def_interner
} else {
// We ignore the warnings and errors produced by compilation while resolving the definition
let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false);
interner = &context.def_interner;
}

let files = context.file_manager.as_file_map();
let file_id = context.file_manager.name_to_id(file_path.clone()).ok_or(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not find file in file manager. File path: {:?}", file_path),
))?;
let byte_index =
position_to_byte_index(files, file_id, &params.text_document_position_params.position)
.map_err(|err| {
ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not convert position to byte index. Error: {:?}", err),
)
})?;

let search_for_location = noirc_errors::Location {
file: file_id,
span: noirc_errors::Span::single_char(byte_index as u32),
&context.def_interner
};

let files = workspace_file_manager.as_file_map();
let file_path = PathString::from(file_path);
let search_for_location =
position_to_location(files, &file_path, &params.text_document_position_params.position)?;

let goto_declaration_response =
interner.get_declaration_location_from(search_for_location).and_then(|found_location| {
let file_id = found_location.file;
let definition_position = to_lsp_location(files, file_id, found_location.span)?;
let response: GotoDeclarationResponse =
GotoDeclarationResponse::from(definition_position).to_owned();
let response = GotoDeclarationResponse::from(definition_position).to_owned();
Some(response)
});

Expand Down
40 changes: 12 additions & 28 deletions tooling/lsp/src/requests/goto_definition.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ use crate::{parse_diff, resolve_workspace_for_source_path};
use crate::{types::GotoDefinitionResult, LspState};
use async_lsp::{ErrorCode, ResponseError};

use fm::PathString;
use lsp_types::request::GotoTypeDefinitionParams;
use lsp_types::{GotoDefinitionParams, GotoDefinitionResponse};
use nargo::insert_all_files_for_workspace_into_file_manager;
use noirc_driver::file_manager_with_stdlib;

use super::{position_to_byte_index, to_lsp_location};
use super::{position_to_location, to_lsp_location};

pub(crate) fn on_goto_definition_request(
state: &mut LspState,
Expand Down Expand Up @@ -40,50 +41,33 @@ fn on_goto_definition_inner(
let workspace = resolve_workspace_for_source_path(file_path.as_path()).unwrap();
let package = workspace.members.first().unwrap();

let package_root_path: String = package.root_dir.as_os_str().to_string_lossy().into();

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_diff(&workspace_file_manager, state);

let (mut context, crate_id) =
nargo::prepare_package(&workspace_file_manager, &parsed_files, package);

let interner;
if let Some(def_interner) = state.cached_definitions.get(&package_root_path) {
interner = def_interner;
let package_root_path = package.root_dir.as_os_str().to_string_lossy().into_owned();
let interner = if let Some(def_interner) = state.cached_definitions.get(&package_root_path) {
def_interner
} else {
// We ignore the warnings and errors produced by compilation while resolving the definition
let _ = noirc_driver::check_crate(&mut context, crate_id, false, false, false);
interner = &context.def_interner;
}

let files = context.file_manager.as_file_map();
let file_id = context.file_manager.name_to_id(file_path.clone()).ok_or(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not find file in file manager. File path: {:?}", file_path),
))?;
let byte_index =
position_to_byte_index(files, file_id, &params.text_document_position_params.position)
.map_err(|err| {
ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not convert position to byte index. Error: {:?}", err),
)
})?;

let search_for_location = noirc_errors::Location {
file: file_id,
span: noirc_errors::Span::single_char(byte_index as u32),
&context.def_interner
};

let files = workspace_file_manager.as_file_map();
let file_path = PathString::from(file_path);
let search_for_location =
position_to_location(files, &file_path, &params.text_document_position_params.position)?;

let goto_definition_response = interner
.get_definition_location_from(search_for_location, return_type_location_instead)
.and_then(|found_location| {
let file_id = found_location.file;
let definition_position = to_lsp_location(files, file_id, found_location.span)?;
let response: GotoDefinitionResponse =
GotoDefinitionResponse::from(definition_position).to_owned();
let response = GotoDefinitionResponse::from(definition_position).to_owned();
Some(response)
});

Expand Down
28 changes: 26 additions & 2 deletions tooling/lsp/src/requests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::future::Future;

use crate::types::{CodeLensOptions, InitializeParams};
use async_lsp::ResponseError;
use fm::codespan_files::Error;
use async_lsp::{ErrorCode, ResponseError};
use fm::{codespan_files::Error, FileMap, PathString};
use lsp_types::{
DeclarationCapability, Location, Position, TextDocumentSyncCapability, TextDocumentSyncKind,
TypeDefinitionProviderCapability, Url,
Expand Down Expand Up @@ -171,6 +171,30 @@ where
}
}

fn position_to_location(
files: &FileMap,
file_path: &PathString,
position: &Position,
) -> Result<noirc_errors::Location, ResponseError> {
let file_id = files.get_file_id(file_path).ok_or(ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not find file in file manager. File path: {:?}", file_path),
))?;
let byte_index = position_to_byte_index(files, file_id, position).map_err(|err| {
ResponseError::new(
ErrorCode::REQUEST_FAILED,
format!("Could not convert position to byte index. Error: {:?}", err),
)
})?;

let location = noirc_errors::Location {
file: file_id,
span: noirc_errors::Span::single_char(byte_index as u32),
};

Ok(location)
}

fn character_to_line_offset(line: &str, character: u32) -> Result<usize, Error> {
let line_len = line.len();
let mut character_offset = 0;
Expand Down
Loading