From 1133c2bcaf1d05309dcfc44133944c999903220a Mon Sep 17 00:00:00 2001 From: LucasLvy Date: Thu, 7 Mar 2024 10:31:52 +0100 Subject: [PATCH 1/2] feat(lsp): code action for unused variable --- .../cairo-lang-diagnostics/src/error_code.rs | 19 +++--- crates/cairo-lang-diagnostics/src/lib.rs | 2 +- crates/cairo-lang-language-server/src/lib.rs | 58 ++++++++++++++++++- crates/cairo-lang-semantic/src/diagnostic.rs | 4 +- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/crates/cairo-lang-diagnostics/src/error_code.rs b/crates/cairo-lang-diagnostics/src/error_code.rs index e583b928ec0..09eaa35f278 100644 --- a/crates/cairo-lang-diagnostics/src/error_code.rs +++ b/crates/cairo-lang-diagnostics/src/error_code.rs @@ -22,8 +22,8 @@ impl ErrorCode { /// Format this error code in a way that is suitable for display in error message. /// /// ``` - /// # use cairo_lang_diagnostics::error_code; - /// assert_eq!(error_code!(E0001).display_bracketed(), "[E0001]"); + /// # use cairo_lang_diagnostics::{codes, error_code}; + /// assert_eq!(error_code!(codes::UNUSED_VARIABLE).display_bracketed(), "[E0001]"); /// ``` pub fn display_bracketed(self) -> String { format!("[{}]", self) @@ -39,8 +39,8 @@ impl fmt::Display for ErrorCode { /// Constructs an [`ErrorCode`]. /// /// ``` -/// # use cairo_lang_diagnostics::{error_code, ErrorCode}; -/// let code: ErrorCode = error_code!(E0001); +/// # use cairo_lang_diagnostics::{codes, error_code, ErrorCode}; +/// let code: ErrorCode = error_code!(codes::UNUSED_VARIABLE); /// # assert_eq!(format!("{code}"), "E0001"); /// ``` #[macro_export] @@ -48,7 +48,7 @@ macro_rules! error_code { ($code:expr) => {{ use $crate::ErrorCode; // NOTE: This is a magic trick to trigger the validation in compile time. - const ENSURE_CONST: ErrorCode = ErrorCode::new(stringify!($code)); + const ENSURE_CONST: ErrorCode = ErrorCode::new($code); ENSURE_CONST }}; } @@ -62,11 +62,16 @@ impl OptionErrorCodeExt for Option { /// Format this error code in a way that is suitable for display in error message. /// /// ``` - /// # use cairo_lang_diagnostics::{error_code, OptionErrorCodeExt}; - /// assert_eq!(Some(error_code!(E0001)).display_bracketed(), "[E0001]"); + /// # use cairo_lang_diagnostics::{codes, error_code, OptionErrorCodeExt}; + /// assert_eq!(Some(error_code!(codes::UNUSED_VARIABLE)).display_bracketed(), "[E0001]"); /// assert_eq!(None.display_bracketed(), ""); /// ``` fn display_bracketed(self) -> String { self.map(ErrorCode::display_bracketed).unwrap_or_default() } } + +pub mod codes { + /// Unused variable + pub const UNUSED_VARIABLE: &str = "E0001"; +} diff --git a/crates/cairo-lang-diagnostics/src/lib.rs b/crates/cairo-lang-diagnostics/src/lib.rs index 1ecfc5ef9b5..645549193e5 100644 --- a/crates/cairo-lang-diagnostics/src/lib.rs +++ b/crates/cairo-lang-diagnostics/src/lib.rs @@ -6,7 +6,7 @@ pub use diagnostics::{ DiagnosticNote, Diagnostics, DiagnosticsBuilder, FormattedDiagnosticEntry, Maybe, Severity, ToMaybe, ToOption, }; -pub use error_code::{ErrorCode, OptionErrorCodeExt}; +pub use error_code::{codes, ErrorCode, OptionErrorCodeExt}; pub use location_marks::get_location_marks; mod diagnostics; diff --git a/crates/cairo-lang-language-server/src/lib.rs b/crates/cairo-lang-language-server/src/lib.rs index 06d9e1fcd9c..dbb40041f5e 100644 --- a/crates/cairo-lang-language-server/src/lib.rs +++ b/crates/cairo-lang-language-server/src/lib.rs @@ -21,7 +21,7 @@ use cairo_lang_defs::ids::{ TraitItemId, TraitLongId, UseLongId, }; use cairo_lang_diagnostics::{ - DiagnosticEntry, DiagnosticLocation, Diagnostics, Severity, ToOption, + codes, DiagnosticEntry, DiagnosticLocation, Diagnostics, Severity, ToOption, }; use cairo_lang_filesystem::cfg::{Cfg, CfgSet}; use cairo_lang_filesystem::db::{ @@ -619,6 +619,7 @@ impl LanguageServer for Backend { document_formatting_provider: Some(OneOf::Left(true)), hover_provider: Some(HoverProviderCapability::Simple(true)), definition_provider: Some(OneOf::Left(true)), + code_action_provider: Some(CodeActionProviderCapability::Simple(true)), ..ServerCapabilities::default() }, }) @@ -932,6 +933,60 @@ impl LanguageServer for Backend { }) .await } + + #[tracing::instrument(level = "debug", skip_all, fields(uri = %params.text_document.uri))] + async fn code_action(&self, params: CodeActionParams) -> LSPResult> { + eprintln!("Code action"); + self.with_db(|db| { + let mut actions = Vec::with_capacity(params.context.diagnostics.len()); + let file_id = file(db, params.text_document.uri.clone()); + let (node, _lookup_items) = get_node_and_lookup_items(db, file_id, params.range.start)?; + for diagnostic in params.context.diagnostics.iter() { + let action = if let Some(NumberOrString::String(code)) = &diagnostic.code { + match code.as_str() { + codes::UNUSED_VARIABLE => unused_variable( + db, + &node, + diagnostic.clone(), + params.text_document.uri.clone(), + ), + _ => CodeAction::default(), + } + } else { + CodeAction::default() + }; + actions.push(CodeActionOrCommand::from(action)); + } + Some(actions) + }) + .await + } +} + +/// Create a code action that prefixes an unused variable with an `_`. +#[tracing::instrument(level = "trace", skip_all)] +fn unused_variable( + db: &dyn SemanticGroup, + node: &SyntaxNode, + diagnostic: Diagnostic, + uri: Url, +) -> CodeAction { + CodeAction { + title: format!("Rename to `_{}`", node.get_text(db.upcast())), + edit: Some(WorkspaceEdit { + changes: Some(HashMap::from_iter([( + uri, + // The diagnostic range is just the first char of the variable name so we can just + // pass an underscore as the new text it won't replace the current variable name + // and it will prefix it with `_` + vec![TextEdit { range: diagnostic.range, new_text: "_".to_owned() }], + )])), + document_changes: None, + change_annotations: None, + }), + diagnostics: Some(vec![diagnostic]), + ..Default::default() + } } #[tracing::instrument(level = "trace", skip_all)] @@ -1601,6 +1656,7 @@ fn get_diagnostics( Severity::Error => DiagnosticSeverity::ERROR, Severity::Warning => DiagnosticSeverity::WARNING, }), + code: diagnostic.error_code().map(|code| NumberOrString::String(code.to_string())), ..Diagnostic::default() }); } diff --git a/crates/cairo-lang-semantic/src/diagnostic.rs b/crates/cairo-lang-semantic/src/diagnostic.rs index b0178164155..33eb9db2c82 100644 --- a/crates/cairo-lang-semantic/src/diagnostic.rs +++ b/crates/cairo-lang-semantic/src/diagnostic.rs @@ -8,7 +8,7 @@ use cairo_lang_defs::ids::{ }; use cairo_lang_defs::plugin::PluginDiagnostic; use cairo_lang_diagnostics::{ - error_code, DiagnosticAdded, DiagnosticEntry, DiagnosticLocation, Diagnostics, + codes, error_code, DiagnosticAdded, DiagnosticEntry, DiagnosticLocation, Diagnostics, DiagnosticsBuilder, ErrorCode, Severity, }; use cairo_lang_filesystem::ids::FileId; @@ -1099,7 +1099,7 @@ pub enum SemanticDiagnosticKind { impl SemanticDiagnosticKind { pub fn error_code(&self) -> Option { Some(match &self { - Self::UnusedVariable => error_code!(E0001), + Self::UnusedVariable => error_code!(codes::UNUSED_VARIABLE), _ => return None, }) } From 135f3241dfd99964e99812e43317117574840dce Mon Sep 17 00:00:00 2001 From: LucasLvy Date: Thu, 7 Mar 2024 16:32:18 +0100 Subject: [PATCH 2/2] chore: impl suggestion --- crates/cairo-lang-diagnostics/src/error_code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cairo-lang-diagnostics/src/error_code.rs b/crates/cairo-lang-diagnostics/src/error_code.rs index 09eaa35f278..28a971068df 100644 --- a/crates/cairo-lang-diagnostics/src/error_code.rs +++ b/crates/cairo-lang-diagnostics/src/error_code.rs @@ -72,6 +72,6 @@ impl OptionErrorCodeExt for Option { } pub mod codes { - /// Unused variable + /// Unused variable. pub const UNUSED_VARIABLE: &str = "E0001"; }