diff --git a/CHANGELOG.md b/CHANGELOG.md index 35221e9b..f9fbc613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## v1.1.15 - UNRELEASED + +### Added + +- **aiken-lsp**: an additional code action to use constructors or identifiers from qualified imports is now offered on missing constructor or identifier. @KtorZ + +### Changed + +- **aiken-lang**: fixed `UnknownTypeConstructor` wrongly reported as `UnknownVariable` (then messing up with LSP quickfix suggestions). @KtorZ + ## v1.1.14 - 2025-02-21 ### Changed diff --git a/crates/aiken-lang/src/tipo.rs b/crates/aiken-lang/src/tipo.rs index 5b09da53..3952d84b 100644 --- a/crates/aiken-lang/src/tipo.rs +++ b/crates/aiken-lang/src/tipo.rs @@ -1357,6 +1357,10 @@ impl TypeConstructor { public: true, } } + + pub fn might_be(name: &str) -> bool { + name.chars().next().unwrap().is_uppercase() + } } #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index e2a414d4..21c22220 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -898,7 +898,7 @@ Perhaps, try the following: #[diagnostic(code("unknown::type_constructor"))] #[diagnostic(help( "{}", - suggest_neighbor(name, constructors.iter(), "Did you forget to import it?") + suggest_neighbor(name, constructors.iter(), &suggest_import_constructor()) ))] UnknownTypeConstructor { #[label("unknown constructor")] @@ -914,11 +914,7 @@ Perhaps, try the following: suggest_neighbor( name, variables.iter(), - &if name.chars().next().unwrap().is_uppercase() { - suggest_import_constructor() - } else { - "Did you forget to import it?".to_string() - } + "Did you forget to import it?", ) ))] UnknownVariable { @@ -1527,29 +1523,23 @@ fn suggest_import_constructor() -> String { Data-type constructors are not automatically imported, even if their type is imported. So, if a module 'aiken/pet' defines the following type: - ┍━ aiken/pet.ak ━━━━━━━━ - │ {keyword_pub} {keyword_type} {type_Pet} {{ - │ {variant_Cat} - │ {variant_Dog} - │ }} + ┍━ aiken/pet.ak ━ ==> ┍━ foo.ak ━━━━━━━━━━━━━━━━ + │ {keyword_pub} {keyword_type} {type_Pet} {{ │ {keyword_use} aiken/pet.{{{type_Pet}, {variant_Dog}}} + │ {variant_Cat} │ + │ {variant_Dog} │ {keyword_fn} foo(pet : {type_Pet}) {{ + │ }} │ {keyword_when} pet {keyword_is} {{ + │ pet.{variant_Cat} -> // ... + │ {variant_Dog} -> // ... + │ }} + │ }} You must import its constructors explicitly to use them, or prefix them with the module's name. - - ┍━ foo.ak ━━━━━━━━ - │ {keyword_use} aiken/pet.{{{type_Pet}, {variant_Dog}}} - │ - │ {keyword_fn} foo(pet : {type_Pet}) {{ - │ {keyword_when} pet {keyword_is} {{ - │ pet.{variant_Cat} -> // ... - │ {variant_Dog} -> // ... - │ }} - │ }} "# , keyword_fn = "fn".if_supports_color(Stdout, |s| s.yellow()) , keyword_is = "is".if_supports_color(Stdout, |s| s.yellow()) - , keyword_pub = "pub".if_supports_color(Stdout, |s| s.bright_blue()) - , keyword_type = "type".if_supports_color(Stdout, |s| s.bright_blue()) - , keyword_use = "use".if_supports_color(Stdout, |s| s.bright_blue()) + , keyword_pub = "pub".if_supports_color(Stdout, |s| s.bright_purple()) + , keyword_type = "type".if_supports_color(Stdout, |s| s.purple()) + , keyword_use = "use".if_supports_color(Stdout, |s| s.bright_purple()) , keyword_when = "when".if_supports_color(Stdout, |s| s.yellow()) , type_Pet = "Pet" .if_supports_color(Stdout, |s| s.bright_blue()) diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index f04689f3..5064da62 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -21,7 +21,9 @@ use crate::{ expr::{FnStyle, TypedExpr, UntypedExpr}, format, parser::token::Base, - tipo::{fields::FieldMap, DefaultFunction, ModuleKind, PatternConstructor, TypeVar}, + tipo::{ + fields::FieldMap, DefaultFunction, ModuleKind, PatternConstructor, TypeConstructor, TypeVar, + }, IdGenerator, }; use std::{ @@ -2481,10 +2483,30 @@ impl<'a, 'b> ExprTyper<'a, 'b> { self.environment .get_variable(name) .cloned() - .ok_or_else(|| Error::UnknownVariable { - location: *location, - name: name.to_string(), - variables: self.environment.local_value_names(), + .ok_or_else(|| { + if TypeConstructor::might_be(name) { + Error::UnknownTypeConstructor { + location: *location, + name: name.to_string(), + constructors: self + .environment + .local_value_names() + .into_iter() + .filter(|s| TypeConstructor::might_be(s)) + .collect::>(), + } + } else { + Error::UnknownVariable { + location: *location, + name: name.to_string(), + variables: self + .environment + .local_value_names() + .into_iter() + .filter(|s| !TypeConstructor::might_be(s)) + .collect::>(), + } + } })?; if let ValueConstructorVariant::ModuleFn { name: fn_name, .. } = diff --git a/crates/aiken-lsp/src/edits.rs b/crates/aiken-lsp/src/edits.rs index 0b035309..90d2cb74 100644 --- a/crates/aiken-lsp/src/edits.rs +++ b/crates/aiken-lsp/src/edits.rs @@ -14,7 +14,10 @@ pub struct ParsedDocument { source_code: String, } -pub type AnnotatedEdit = (String, lsp_types::TextEdit); +pub enum AnnotatedEdit { + SimpleEdit(String, lsp_types::TextEdit), + CombinedEdits(String, Vec), +} /// Parse the target document as an 'UntypedModule' alongside its line numbers. This is useful in /// case we need to manipulate the AST for a quickfix. @@ -166,20 +169,36 @@ impl ParsedDocument { let new_text = String::new(); - ( + AnnotatedEdit::SimpleEdit( "Remove redundant import".to_string(), lsp_types::TextEdit { range, new_text }, ) } + pub fn use_qualified( + module: &str, + unqualified: &str, + range: &lsp_types::Range, + ) -> Option { + let title = format!("Use qualified from {}", module); + let suffix = module.split("/").last()?; + Some(AnnotatedEdit::SimpleEdit( + title, + lsp_types::TextEdit { + range: *range, + new_text: format!("{suffix}.{unqualified}"), + }, + )) + } + fn insert_qualified_before( &self, import: &CheckedModule, unqualified: &str, location: Span, ) -> AnnotatedEdit { - let title = format!("Use '{}' from {}", unqualified, import.name); - ( + let title = format!("Import '{}' from {}", unqualified, import.name); + AnnotatedEdit::SimpleEdit( title, insert_text( location.start, @@ -195,8 +214,8 @@ impl ParsedDocument { unqualified: &str, location: Span, ) -> AnnotatedEdit { - let title = format!("Use '{}' from {}", unqualified, import.name); - ( + let title = format!("Import '{}' from {}", unqualified, import.name); + AnnotatedEdit::SimpleEdit( title, insert_text( location.end, @@ -212,8 +231,8 @@ impl ParsedDocument { unqualified: &str, location: Span, ) -> AnnotatedEdit { - let title = format!("Use '{}' from {}", unqualified, import.name); - ( + let title = format!("Import '{}' from {}", unqualified, import.name); + AnnotatedEdit::SimpleEdit( title, insert_text( location.end, @@ -238,9 +257,9 @@ impl ParsedDocument { } ); - let title = format!("Add new import line: {import_line}"); + let title = format!("Add line: '{import_line}'"); - ( + AnnotatedEdit::SimpleEdit( title, match location { None => insert_text(0, &self.line_numbers, format!("{import_line}\n")), diff --git a/crates/aiken-lsp/src/lib.rs b/crates/aiken-lsp/src/lib.rs index d7b8d9d4..640585e6 100644 --- a/crates/aiken-lsp/src/lib.rs +++ b/crates/aiken-lsp/src/lib.rs @@ -9,7 +9,7 @@ mod edits; pub mod error; mod quickfix; pub mod server; -mod utils; +pub mod utils; #[allow(clippy::result_large_err)] pub fn start() -> Result<(), Error> { diff --git a/crates/aiken-lsp/src/quickfix.rs b/crates/aiken-lsp/src/quickfix.rs index ab457608..1347e0bf 100644 --- a/crates/aiken-lsp/src/quickfix.rs +++ b/crates/aiken-lsp/src/quickfix.rs @@ -2,6 +2,7 @@ use crate::{ edits::{self, AnnotatedEdit, ParsedDocument}, server::lsp_project::LspProject, }; +use aiken_project::module::CheckedModule; use std::{collections::HashMap, str::FromStr}; const UNKNOWN_VARIABLE: &str = "aiken::check::unknown::variable"; @@ -94,7 +95,12 @@ pub fn quickfix( &mut actions, text_document, diagnostic, - unknown_identifier(compiler, parsed_document, diagnostic.data.as_ref()), + unknown_identifier( + compiler, + parsed_document, + &diagnostic.range, + diagnostic.data.as_ref(), + ), ); } Quickfix::UnknownModule(diagnostic) => each_as_distinct_action( @@ -107,7 +113,12 @@ pub fn quickfix( &mut actions, text_document, diagnostic, - unknown_constructor(compiler, parsed_document, diagnostic.data.as_ref()), + unknown_constructor( + compiler, + parsed_document, + &diagnostic.range, + diagnostic.data.as_ref(), + ), ), Quickfix::UnusedImports(diagnostics) => as_single_action( &mut actions, @@ -152,10 +163,19 @@ fn each_as_distinct_action( diagnostic: &lsp_types::Diagnostic, edits: Vec, ) { - for (title, edit) in edits.into_iter() { + for edit in edits.into_iter() { let mut changes = HashMap::new(); - changes.insert(text_document.uri.clone(), vec![edit]); + let title = match edit { + AnnotatedEdit::SimpleEdit(title, one) => { + changes.insert(text_document.uri.clone(), vec![one]); + title + } + AnnotatedEdit::CombinedEdits(title, many) => { + changes.insert(text_document.uri.clone(), many); + title + } + }; actions.push(lsp_types::CodeAction { title, @@ -185,7 +205,13 @@ fn as_single_action( changes.insert( text_document.uri.clone(), - edits.into_iter().map(|(_, b)| b).collect(), + edits + .into_iter() + .flat_map(|edit| match edit { + AnnotatedEdit::SimpleEdit(_, one) => vec![one], + AnnotatedEdit::CombinedEdits(_, many) => many, + }) + .collect(), ); actions.push(lsp_types::CodeAction { @@ -207,6 +233,7 @@ fn as_single_action( fn unknown_identifier( compiler: &LspProject, parsed_document: &ParsedDocument, + range: &lsp_types::Range, data: Option<&serde_json::Value>, ) -> Vec { let mut edits = Vec::new(); @@ -217,6 +244,10 @@ fn unknown_identifier( if let Some(edit) = parsed_document.import(&module, Some(var_name)) { edits.push(edit) } + + if let Some(edit) = suggest_qualified(parsed_document, &module, var_name, range) { + edits.push(edit) + } } } } @@ -227,6 +258,7 @@ fn unknown_identifier( fn unknown_constructor( compiler: &LspProject, parsed_document: &ParsedDocument, + range: &lsp_types::Range, data: Option<&serde_json::Value>, ) -> Vec { let mut edits = Vec::new(); @@ -237,6 +269,12 @@ fn unknown_constructor( if let Some(edit) = parsed_document.import(&module, Some(constructor_name)) { edits.push(edit) } + + if let Some(edit) = + suggest_qualified(parsed_document, &module, constructor_name, range) + { + edits.push(edit) + } } } } @@ -244,6 +282,33 @@ fn unknown_constructor( edits } +fn suggest_qualified( + parsed_document: &ParsedDocument, + module: &CheckedModule, + identifier: &str, + range: &lsp_types::Range, +) -> Option { + if let Some(AnnotatedEdit::SimpleEdit(use_qualified_title, use_qualified)) = + ParsedDocument::use_qualified(&module.name, identifier, range) + { + if let Some(AnnotatedEdit::SimpleEdit(_, add_new_line)) = + parsed_document.import(module, None) + { + return Some(AnnotatedEdit::CombinedEdits( + use_qualified_title, + vec![add_new_line, use_qualified], + )); + } else { + return Some(AnnotatedEdit::SimpleEdit( + use_qualified_title, + use_qualified, + )); + } + } + + None +} + fn unknown_module( compiler: &LspProject, parsed_document: &ParsedDocument, @@ -298,7 +363,7 @@ fn utf8_byte_array_is_hex_string(diagnostic: &lsp_types::Diagnostic) -> Vec Vec Vec { - vec![( + vec![AnnotatedEdit::SimpleEdit( "Use 'let' instead of 'expect'".to_string(), lsp_types::TextEdit { range: diagnostic.range, @@ -324,7 +389,7 @@ fn unused_record_fields(diagnostic: &lsp_types::Diagnostic) -> Vec TextEdit { TextEdit { range: lsp_types::Range {