Skip to content

Commit

Permalink
Merge 893aa0b into 5bf7352
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored Sep 23, 2024
2 parents 5bf7352 + 893aa0b commit d05688b
Show file tree
Hide file tree
Showing 8 changed files with 379 additions and 26 deletions.
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,9 +330,13 @@ impl Display for UseTree {

match &self.kind {
UseTreeKind::Path(name, alias) => {
if !(self.prefix.segments.is_empty() && self.prefix.kind == PathKind::Plain) {
write!(f, "::")?;
}

write!(f, "{name}")?;

while let Some(alias) = alias {
if let Some(alias) = alias {
write!(f, " as {alias}")?;
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ pub trait Visitor {
true
}

fn visit_import(&mut self, _: &UseTree, _visibility: ItemVisibility) -> bool {
fn visit_import(&mut self, _: &UseTree, _: Span, _visibility: ItemVisibility) -> bool {
true
}

Expand Down Expand Up @@ -506,7 +506,7 @@ impl Item {
}
ItemKind::Trait(noir_trait) => noir_trait.accept(self.span, visitor),
ItemKind::Import(use_tree, visibility) => {
if visitor.visit_import(use_tree, *visibility) {
if visitor.visit_import(use_tree, self.span, *visibility) {
use_tree.accept(visitor);
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ impl DefCollector {
crate_id: CrateId,
errors: &mut Vec<(CompilationError, FileId)>,
) {
let unused_imports = context.def_interner.usage_tracker.unused_items().iter();
let unused_imports = context.def_interner.unused_items().iter();
let unused_imports = unused_imports.filter(|(module_id, _)| module_id.krate == crate_id);

errors.extend(unused_imports.flat_map(|(module_id, usage_tracker)| {
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::hir::type_check::generics::TraitGenerics;
use crate::hir_def::traits::NamedType;
use crate::macros_api::ModuleDefId;
use crate::macros_api::UnaryOp;
use crate::usage_tracker::UnusedItem;
use crate::usage_tracker::UsageTracker;
use crate::QuotedType;

Expand Down Expand Up @@ -2249,6 +2250,12 @@ impl NodeInterner {
pub fn doc_comments(&self, id: ReferenceId) -> Option<&Vec<String>> {
self.doc_comments.get(&id)
}

pub fn unused_items(
&self,
) -> &std::collections::HashMap<ModuleId, std::collections::HashMap<Ident, UnusedItem>> {
self.usage_tracker.unused_items()
}
}

impl Methods {
Expand Down
63 changes: 42 additions & 21 deletions tooling/lsp/src/requests/code_action.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{
collections::{BTreeMap, HashMap},
future::{self, Future},
ops::Range,
};

use async_lsp::ResponseError;
Expand All @@ -11,7 +12,7 @@ use lsp_types::{
};
use noirc_errors::Span;
use noirc_frontend::{
ast::{ConstructorExpression, NoirTraitImpl, Path, Visitor},
ast::{ConstructorExpression, ItemVisibility, NoirTraitImpl, Path, UseTree, Visitor},
graph::CrateId,
hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
macros_api::NodeInterner,
Expand All @@ -28,7 +29,7 @@ use super::{process_request, to_lsp_location};
mod fill_struct_fields;
mod implement_missing_members;
mod import_or_qualify;
#[cfg(test)]
mod remove_unused_import;
mod tests;

pub(crate) fn on_code_action_request(
Expand All @@ -43,7 +44,7 @@ pub(crate) fn on_code_action_request(
let result = process_request(state, text_document_position_params, |args| {
let path = PathString::from_path(uri.to_file_path().unwrap());
args.files.get_file_id(&path).and_then(|file_id| {
utils::position_to_byte_index(args.files, file_id, &position).and_then(|byte_index| {
utils::range_to_byte_span(args.files, file_id, &params.range).and_then(|byte_range| {
let file = args.files.get_file(file_id).unwrap();
let source = file.source();
let (parsed_module, _errors) = noirc_frontend::parse_program(source);
Expand All @@ -53,7 +54,7 @@ pub(crate) fn on_code_action_request(
args.files,
file_id,
source,
byte_index,
byte_range,
args.crate_id,
args.def_maps,
args.interner,
Expand All @@ -71,7 +72,7 @@ struct CodeActionFinder<'a> {
file: FileId,
source: &'a str,
lines: Vec<&'a str>,
byte_index: usize,
byte_range: Range<usize>,
/// The module ID in scope. This might change as we traverse the AST
/// if we are analyzing something inside an inline module declaration.
module_id: ModuleId,
Expand All @@ -81,7 +82,9 @@ struct CodeActionFinder<'a> {
nesting: usize,
/// The line where an auto_import must be inserted
auto_import_line: usize,
code_actions: Vec<CodeActionOrCommand>,
/// Text edits for the "Remove all unused imports" code action
unused_imports_text_edits: Vec<TextEdit>,
code_actions: Vec<CodeAction>,
}

impl<'a> CodeActionFinder<'a> {
Expand All @@ -91,7 +94,7 @@ impl<'a> CodeActionFinder<'a> {
files: &'a FileMap,
file: FileId,
source: &'a str,
byte_index: usize,
byte_range: Range<usize>,
krate: CrateId,
def_maps: &'a BTreeMap<CrateId, CrateDefMap>,
interner: &'a NodeInterner,
Expand All @@ -112,12 +115,13 @@ impl<'a> CodeActionFinder<'a> {
file,
source,
lines: source.lines().collect(),
byte_index,
byte_range,
module_id,
def_maps,
interner,
nesting: 0,
auto_import_line: 0,
unused_imports_text_edits: vec![],
code_actions: vec![],
}
}
Expand All @@ -129,28 +133,38 @@ impl<'a> CodeActionFinder<'a> {
return None;
}

// We also suggest a single "Remove all the unused imports" code action that combines all of the
// "Remove unused imports" (similar to Rust Analyzer)
if self.unused_imports_text_edits.len() > 1 {
let text_edits = std::mem::take(&mut self.unused_imports_text_edits);
let code_action = self.new_quick_fix_multiple_edits(
"Remove all the unused imports".to_string(),
text_edits,
);
self.code_actions.push(code_action);
}

let mut code_actions = std::mem::take(&mut self.code_actions);
code_actions.sort_by_key(|code_action| {
let CodeActionOrCommand::CodeAction(code_action) = code_action else {
panic!("We only gather code actions, never commands");
};
code_action.title.clone()
});

Some(code_actions)
code_actions.sort_by_key(|code_action| code_action.title.clone());

Some(code_actions.into_iter().map(CodeActionOrCommand::CodeAction).collect())
}

fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeActionOrCommand {
fn new_quick_fix(&self, title: String, text_edit: TextEdit) -> CodeAction {
self.new_quick_fix_multiple_edits(title, vec![text_edit])
}

fn new_quick_fix_multiple_edits(&self, title: String, text_edits: Vec<TextEdit>) -> CodeAction {
let mut changes = HashMap::new();
changes.insert(self.uri.clone(), vec![text_edit]);
changes.insert(self.uri.clone(), text_edits);

let workspace_edit = WorkspaceEdit {
changes: Some(changes),
document_changes: None,
change_annotations: None,
};

CodeActionOrCommand::CodeAction(CodeAction {
CodeAction {
title,
kind: Some(CodeActionKind::QUICKFIX),
diagnostics: None,
Expand All @@ -159,11 +173,12 @@ impl<'a> CodeActionFinder<'a> {
is_preferred: None,
disabled: None,
data: None,
})
}
}

fn includes_span(&self, span: Span) -> bool {
span.start() as usize <= self.byte_index && self.byte_index <= span.end() as usize
let byte_range_span = Span::from(self.byte_range.start as u32..self.byte_range.end as u32);
span.intersects(&byte_range_span)
}
}

Expand Down Expand Up @@ -207,6 +222,12 @@ impl<'a> Visitor for CodeActionFinder<'a> {
false
}

fn visit_import(&mut self, use_tree: &UseTree, span: Span, visibility: ItemVisibility) -> bool {
self.remove_unused_import(use_tree, visibility, span);

true
}

fn visit_path(&mut self, path: &Path) {
self.import_or_qualify(path);
}
Expand Down
Loading

0 comments on commit d05688b

Please sign in to comment.