From 4f67cf221ab4328637025c2b39e43d68a30c6813 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Mon, 12 Sep 2022 01:44:17 +0300 Subject: feat(tvix/eval): implement initial fancy display for warnings This implements an initial fancy display for warnings emitted by the tvix compiler, using the codemap_diagnostic crate. Each warning variant has an associated message, and optionally an associated annotation for the span displayed to the user. In theory we could get a lot more fancy with the display for specific variants if needed (e.g. re-parse the AST and actually add multiple semantic spans based on context), but this is already a good start. Example: tvix-repl> let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} warning[W004]: declared variable 'toString' shadows a built-in global! --> [tvix-repl]:1:5 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^^^^^^^ variable declared here warning[W001]: URL literal syntax is deprecated, use a quoted string instead --> [tvix-repl]:1:16 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^^^^^^^^^^^^^^ warning[W002]: inherited variable already exists with the same value --> [tvix-repl]:1:40 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^^^^^^^^^^^^^^^^ warning[W999]: feature not yet implemented in tvix: recursive attribute sets --> [tvix-repl]:1:70 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^^^^^ warning[W999]: feature not yet implemented in tvix: closed formals --> [tvix-repl]:1:62 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^ warning[W003]: variable 'toString' is declared, but never used: --> [tvix-repl]:1:5 | 1 | let toString = https://tvl.fyi; in let inherit toString; in ({}: 42) rec {} | ^^^^^^^^ variable declared here => 42 :: int These are coloured when output to a terminal. Change-Id: If315648a07e333895db4ae1d0915ee2013806585 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6532 Autosubmit: tazjin Reviewed-by: grfn Tested-by: BuildkiteCI --- tvix/eval/src/compiler/attrs.rs | 2 +- tvix/eval/src/eval.rs | 11 ++--- tvix/eval/src/warnings.rs | 90 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 9 deletions(-) (limited to 'tvix/eval/src') diff --git a/tvix/eval/src/compiler/attrs.rs b/tvix/eval/src/compiler/attrs.rs index 864cc2cb3d..91039d6dbf 100644 --- a/tvix/eval/src/compiler/attrs.rs +++ b/tvix/eval/src/compiler/attrs.rs @@ -33,7 +33,7 @@ impl Compiler<'_, '_> { let span = self.span_for(&node); self.emit_warning( span, - WarningKind::NotImplemented("recursive attribute sets are not yet implemented"), + WarningKind::NotImplemented("recursive attribute sets"), ); } diff --git a/tvix/eval/src/eval.rs b/tvix/eval/src/eval.rs index 585463dde2..4ed97b329d 100644 --- a/tvix/eval/src/eval.rs +++ b/tvix/eval/src/eval.rs @@ -13,7 +13,7 @@ pub fn interpret(code: &str, location: Option) -> EvalResult { location .as_ref() .map(|p| p.to_string_lossy().to_string()) - .unwrap_or_else(|| "".into()), + .unwrap_or_else(|| "[tvix-repl]".into()), code.into(), ); let codemap = Rc::new(codemap); @@ -47,7 +47,7 @@ pub fn interpret(code: &str, location: Option) -> EvalResult { location, &file, global_builtins(), - &mut DisassemblingObserver::new(codemap, std::io::stderr()), + &mut DisassemblingObserver::new(codemap.clone(), std::io::stderr()), ) } else { crate::compiler::compile( @@ -60,12 +60,7 @@ pub fn interpret(code: &str, location: Option) -> EvalResult { }?; for warning in result.warnings { - eprintln!( - "warning: {:?} at `{}`[line {}]", - warning.kind, - file.source_slice(warning.span), - file.find_line(warning.span.low()) + 1 - ) + warning.fancy_format_stderr(&codemap); } for error in &result.errors { diff --git a/tvix/eval/src/warnings.rs b/tvix/eval/src/warnings.rs index b37d0fc918..a09c5746d3 100644 --- a/tvix/eval/src/warnings.rs +++ b/tvix/eval/src/warnings.rs @@ -1,6 +1,9 @@ //! Implements warnings that are emitted in cases where code passed to //! Tvix exhibits problems that the user could address. +use codemap::CodeMap; +use codemap_diagnostic::{ColorConfig, Diagnostic, Emitter, Level, SpanLabel, SpanStyle}; + #[derive(Debug)] pub enum WarningKind { DeprecatedLiteralURL, @@ -18,3 +21,90 @@ pub struct EvalWarning { pub kind: WarningKind, pub span: codemap::Span, } + +impl EvalWarning { + /// Render a fancy, human-readable output of this warning and + /// return it as a String. Note that this version of the output + /// does not include any colours or font styles. + pub fn fancy_format_str(&self, codemap: &CodeMap) -> String { + let mut out = vec![]; + Emitter::vec(&mut out, Some(codemap)).emit(&[self.diagnostic(codemap)]); + String::from_utf8_lossy(&out).to_string() + } + + /// Render a fancy, human-readable output of this warning and + /// print it to stderr. If rendered in a terminal that supports + /// colours and font styles, the output will include those. + pub fn fancy_format_stderr(&self, codemap: &CodeMap) { + Emitter::stderr(ColorConfig::Auto, Some(codemap)).emit(&[self.diagnostic(codemap)]); + } + + /// Create the optional span label displayed as an annotation on + /// the underlined span of the warning. + fn span_label(&self) -> Option { + match self.kind { + WarningKind::UnusedBinding | WarningKind::ShadowedGlobal(_) => { + Some("variable declared here".into()) + } + _ => None, + } + } + + /// Create the primary warning message displayed to users for a + /// warning. + fn message(&self, codemap: &CodeMap) -> String { + match self.kind { + WarningKind::DeprecatedLiteralURL => { + format!("URL literal syntax is deprecated, use a quoted string instead") + } + + WarningKind::UselessInherit => { + format!("inherited variable already exists with the same value") + } + + WarningKind::UnusedBinding => { + let file = codemap.find_file(self.span.low()); + + format!( + "variable '{}' is declared, but never used:", + file.source_slice(self.span) + ) + } + + WarningKind::ShadowedGlobal(name) => { + format!("declared variable '{}' shadows a built-in global!", name) + } + + WarningKind::NotImplemented(what) => { + format!("feature not yet implemented in tvix: {}", what) + } + } + } + + /// Return the unique warning code for this variant which can be + /// used to refer users to documentation. + fn code(&self) -> &'static str { + match self.kind { + WarningKind::DeprecatedLiteralURL => "W001", + WarningKind::UselessInherit => "W002", + WarningKind::UnusedBinding => "W003", + WarningKind::ShadowedGlobal(_) => "W004", + WarningKind::NotImplemented(_) => "W999", + } + } + + fn diagnostic(&self, codemap: &CodeMap) -> Diagnostic { + let span_label = SpanLabel { + label: self.span_label(), + span: self.span, + style: SpanStyle::Primary, + }; + + Diagnostic { + level: Level::Warning, + message: self.message(codemap), + spans: vec![span_label], + code: Some(self.code().into()), + } + } +} -- cgit 1.4.1