From a89694ed75765981fa18bec846c71bf1007fc83d Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 22 Feb 2025 17:52:21 +0100 Subject: [PATCH 1/5] Use less vertical space for type-constructor hint; Also, show it actually for UnknownTypeConstructor, and not UnknownVariable. There's currently an error that wrongly assign an 'UnknownVariable' in place where it should be an 'UnknownTypeConstructor'. Fix coming in the next commit. Signed-off-by: KtorZ --- crates/aiken-lang/src/tipo/error.rs | 32 ++++++++++++----------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index e2a414d4..7b5cda49 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")] @@ -1527,29 +1527,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()) From 9f24a5c577a3ef10985e84776807184dd740e780 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 22 Feb 2025 17:53:58 +0100 Subject: [PATCH 2/5] Fix wrong use of 'UnknownVariable' instead of 'UnknownTypeConstructor' While the feedback for human users is mostly the same, it does in fact matter for the LSP since the quickfix will be different depending on whether we look for a top-level identifier or if we look for a constructor. The typical case we see in the stdlib is the `VerificationKey` and `Script` constructors for `Credential`, often being mixed with their types counterparts in aiken/crypto! Signed-off-by: KtorZ --- CHANGELOG.md | 6 ++++++ crates/aiken-lang/src/tipo.rs | 4 ++++ crates/aiken-lang/src/tipo/error.rs | 6 +----- crates/aiken-lang/src/tipo/expr.rs | 32 ++++++++++++++++++++++++----- 4 files changed, 38 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35221e9b..af839752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## v1.1.15 - UNRELEASED + +### 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 7b5cda49..21c22220 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -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 { 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, .. } = From 1e64dc9aebc121eb82c91adc7f05425d3c807754 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 22 Feb 2025 18:05:13 +0100 Subject: [PATCH 3/5] Tweak progress line title in LSP Has somewhat always been bothering me that it says 'compiling Aiken', which means kind of nothing in this context. Signed-off-by: KtorZ --- crates/aiken-lsp/src/server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/aiken-lsp/src/server.rs b/crates/aiken-lsp/src/server.rs index f519304d..c1486d7e 100644 --- a/crates/aiken-lsp/src/server.rs +++ b/crates/aiken-lsp/src/server.rs @@ -638,7 +638,7 @@ impl Server { self.send_work_done_notification( connection, lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin { - title: "Compiling Aiken".into(), + title: "Compiling Aiken project".into(), cancellable: Some(false), message: None, percentage: None, From c3f571334cb9b8bce1b1e5a6c31bf5876179c586 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 22 Feb 2025 19:00:40 +0100 Subject: [PATCH 4/5] Add utility to print trace line in LSP server output. Signed-off-by: KtorZ --- crates/aiken-lsp/src/lib.rs | 2 +- crates/aiken-lsp/src/utils.rs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) 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/utils.rs b/crates/aiken-lsp/src/utils.rs index d78dc45c..8849c750 100644 --- a/crates/aiken-lsp/src/utils.rs +++ b/crates/aiken-lsp/src/utils.rs @@ -1,13 +1,26 @@ use crate::error::Error; use aiken_lang::{ast::Span, line_numbers::LineNumbers}; use itertools::Itertools; -use lsp_types::TextEdit; +use lsp_types::{notification::Notification, TextEdit}; use std::path::{Path, PathBuf}; use urlencoding::decode; pub const COMPILING_PROGRESS_TOKEN: &str = "compiling-aiken"; pub const CREATE_COMPILING_PROGRESS_TOKEN: &str = "create-compiling-progress-token"; +/// Trace some information from the server. +pub fn debug(connection: &lsp_server::Connection, message: impl serde::ser::Serialize) { + connection + .sender + .send(lsp_server::Message::Notification( + lsp_server::Notification { + method: lsp_types::notification::LogTrace::METHOD.to_string(), + params: serde_json::json! {{ "message": message }}, + }, + )) + .expect("failed to send notification"); +} + pub fn text_edit_replace(new_text: String) -> TextEdit { TextEdit { range: lsp_types::Range { From 3c8bc7ebb65ce810a4b45c6f321f66401908b7e4 Mon Sep 17 00:00:00 2001 From: KtorZ Date: Sat, 22 Feb 2025 19:04:34 +0100 Subject: [PATCH 5/5] Also suggest qualified imports as quickfix when relevant. Signed-off-by: KtorZ --- CHANGELOG.md | 4 ++ crates/aiken-lsp/src/edits.rs | 39 +++++++++++---- crates/aiken-lsp/src/quickfix.rs | 81 ++++++++++++++++++++++++++++---- 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index af839752..f9fbc613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## 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 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/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