From 7206360baa700b36108716793f9823e974984e45 Mon Sep 17 00:00:00 2001 From: rvcas Date: Tue, 24 Jan 2023 10:22:03 -0500 Subject: [PATCH] feat(when): single when clause now emits warning --- crates/aiken-lang/src/tipo/error.rs | 121 ++++++++++++++++------------ crates/aiken-lang/src/tipo/expr.rs | 15 ++++ 2 files changed, 83 insertions(+), 53 deletions(-) diff --git a/crates/aiken-lang/src/tipo/error.rs b/crates/aiken-lang/src/tipo/error.rs index edef1333..9068ad6f 100644 --- a/crates/aiken-lang/src/tipo/error.rs +++ b/crates/aiken-lang/src/tipo/error.rs @@ -1,7 +1,7 @@ use super::Type; use crate::{ ast::{Annotation, BinOp, CallArg, Span, TodoKind, UntypedPattern}, - expr, + expr::{self, UntypedExpr}, format::Formatter, levenshtein, pretty::Documentable, @@ -172,20 +172,7 @@ You can use '{discard}' and numbers to distinguish between similar names. If you really meant to return that last expression, try to replace it with the following: {sample}"# - , sample = Formatter::new() - .expr(expr) - .to_pretty_string(70) - .lines() - .enumerate() - .map(|(ix, line)| { - if ix == 0 { - format!("╰─▶ {}", line.yellow()) - } else { - format!(" {line}").yellow().to_string() - } - }) - .collect::>() - .join("") + , sample = format_suggestion(expr) ))] LastExpressionIsAssignment { #[label("let-binding as last expression")] @@ -1034,14 +1021,12 @@ fn suggest_import_constructor() -> String { #[derive(Debug, PartialEq, Clone, thiserror::Error, Diagnostic)] pub enum Warning { - #[error("I found a todo left in the code.\n")] - #[diagnostic(help("You probably want to replace that one with real code... eventually."))] - #[diagnostic(code("todo"))] - Todo { - kind: TodoKind, + #[error("I found a record update using all fields; thus redundant.\n")] + #[diagnostic(url("https://aiken-lang.org/language-tour/custom-types#record-updates"))] + #[diagnostic(code("record_update::all_fields"))] + AllFieldsRecordUpdate { #[label] location: Span, - tipo: Arc, }, #[error( @@ -1056,9 +1041,10 @@ pub enum Warning { location: Span, }, - #[error("I found a literal that is unused.\n")] - #[diagnostic(code("unused::literal"))] - UnusedLiteral { + #[error("I found a record update with no fields; effectively updating nothing.\n")] + #[diagnostic(url("https://aiken-lang.org/language-tour/custom-types#record-updates"))] + #[diagnostic(code("record_update::no_fields"))] + NoFieldsRecordUpdate { #[label] location: Span, }, @@ -1070,29 +1056,25 @@ pub enum Warning { location: Span, }, - #[error("I found a record update with no fields; effectively updating nothing.\n")] - #[diagnostic(url("https://aiken-lang.org/language-tour/custom-types#record-updates"))] - #[diagnostic(code("record_update::no_fields"))] - NoFieldsRecordUpdate { + #[error("I found a when expression with a single clause.")] + #[diagnostic( + code("single_when_clause"), + help("Prefer using a {} binding like so...\n\n{}", "let".purple(), format_suggestion(sample)) + )] + SingleWhenClause { #[label] location: Span, + sample: UntypedExpr, }, - #[error("I found a record update using all fields; thus redundant.\n")] - #[diagnostic(url("https://aiken-lang.org/language-tour/custom-types#record-updates"))] - #[diagnostic(code("record_update::all_fields"))] - AllFieldsRecordUpdate { + #[error("I found a todo left in the code.\n")] + #[diagnostic(help("You probably want to replace that one with real code... eventually."))] + #[diagnostic(code("todo"))] + Todo { + kind: TodoKind, #[label] location: Span, - }, - - #[error("I discovered an unused type: '{}'.\n", name.purple())] - #[diagnostic(code("unused::type"))] - UnusedType { - #[label] - location: Span, - imported: bool, - name: String, + tipo: Arc, }, #[error("I discovered an unused constructor: '{}'.\n", name.purple())] @@ -1107,6 +1089,17 @@ pub enum Warning { name: String, }, + #[error("I discovered an unused imported module: '{}'.\n", name.purple())] + #[diagnostic(help( + "No big deal, but you might want to remove it to get rid of that warning." + ))] + #[diagnostic(code("unused::import::module"))] + UnusedImportedModule { + #[label] + location: Span, + name: String, + }, + #[error("I discovered an unused imported value: '{}'.\n", name.purple())] #[diagnostic(help( "No big deal, but you might want to remove it to get rid of that warning." @@ -1118,12 +1111,21 @@ pub enum Warning { name: String, }, - #[error("I discovered an unused imported module: '{}'.\n", name.purple())] + #[error("I found a literal that is unused.\n")] + #[diagnostic(code("unused::literal"))] + UnusedLiteral { + #[label] + location: Span, + }, + + #[error("I found an unused private function: '{}'.\n", name.purple())] #[diagnostic(help( - "No big deal, but you might want to remove it to get rid of that warning." + "Perhaps your forgot to make it public using the '{keyword_pub}' keyword?\n\ + Otherwise, you might want to get rid of it altogether." + , keyword_pub = "pub".bright_blue() ))] - #[diagnostic(code("unused::import::module"))] - UnusedImportedModule { + #[diagnostic(code("unused::function"))] + UnusedPrivateFunction { #[label] location: Span, name: String, @@ -1142,16 +1144,12 @@ pub enum Warning { name: String, }, - #[error("I found an unused private function: '{}'.\n", name.purple())] - #[diagnostic(help( - "Perhaps your forgot to make it public using the '{keyword_pub}' keyword?\n\ - Otherwise, you might want to get rid of it altogether." - , keyword_pub = "pub".bright_blue() - ))] - #[diagnostic(code("unused::function"))] - UnusedPrivateFunction { + #[error("I discovered an unused type: '{}'.\n", name.purple())] + #[diagnostic(code("unused::type"))] + UnusedType { #[label] location: Span, + imported: bool, name: String, }, @@ -1187,3 +1185,20 @@ pub enum UnknownRecordFieldSituation { /// This unknown record field is being called as a function. i.e. `record.field()` FunctionCall, } + +fn format_suggestion<'a>(sample: &'a UntypedExpr) -> String { + Formatter::new() + .expr(sample) + .to_pretty_string(70) + .lines() + .enumerate() + .map(|(ix, line)| { + if ix == 0 { + format!("╰─▶ {}", line.yellow()) + } else { + format!(" {line}").yellow().to_string() + } + }) + .collect::>() + .join("") +} diff --git a/crates/aiken-lang/src/tipo/expr.rs b/crates/aiken-lang/src/tipo/expr.rs index 788178cf..cf9775ab 100644 --- a/crates/aiken-lang/src/tipo/expr.rs +++ b/crates/aiken-lang/src/tipo/expr.rs @@ -1930,6 +1930,21 @@ impl<'a, 'b> ExprTyper<'a, 'b> { clauses: Vec, location: Span, ) -> Result { + // if there is only one clause we want to present a warning + // that suggests that a `let` binding should be used instead. + if clauses.len() == 1 { + self.environment.warnings.push(Warning::SingleWhenClause { + location: clauses[0].location, + sample: UntypedExpr::Assignment { + location: Span::empty(), + value: Box::new(subjects[0].clone()), + pattern: clauses[0].pattern[0].clone(), + kind: AssignmentKind::Let, + annotation: None, + }, + }); + } + let subjects_count = subjects.len(); let mut typed_subjects = Vec::with_capacity(subjects_count);