From 1230af138c424c13bf9315fbac935752588501ed Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 21 Sep 2019 18:53:30 -0700 Subject: [PATCH] Add context to deprecation warnings (#473) Previously, warnings upon encountering a deprecated use `=` in assignments, exports, and aliases would print a message without any indication of where the offending `=` was. This diff adds a proper `Warning` enum, and uses it to report context, as is done with compilation and runtime errors. --- src/common.rs | 2 +- src/compilation_error.rs | 5 ++-- src/justfile.rs | 2 +- src/lib.rs | 1 + src/misc.rs | 8 +++--- src/parser.rs | 24 ++++++++++------- src/run.rs | 22 +++++----------- src/runtime_error.rs | 5 ++-- src/warning.rs | 56 ++++++++++++++++++++++++++++++++++++++++ tests/integration.rs | 9 +++++++ 10 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 src/warning.rs diff --git a/src/common.rs b/src/common.rs index 75abad85c7..842d97cf13 100644 --- a/src/common.rs +++ b/src/common.rs @@ -46,7 +46,7 @@ pub(crate) use crate::{ recipe_context::RecipeContext, recipe_resolver::RecipeResolver, runtime_error::RuntimeError, search_error::SearchError, shebang::Shebang, state::State, string_literal::StringLiteral, token::Token, token_kind::TokenKind, use_color::UseColor, variables::Variables, - verbosity::Verbosity, + verbosity::Verbosity, warning::Warning, }; pub(crate) type CompilationResult<'a, T> = Result>; diff --git a/src/compilation_error.rs b/src/compilation_error.rs index 40252b192e..b203c77862 100644 --- a/src/compilation_error.rs +++ b/src/compilation_error.rs @@ -1,6 +1,6 @@ use crate::common::*; -use crate::misc::{maybe_s, show_whitespace, write_error_context, Or}; +use crate::misc::{maybe_s, show_whitespace, write_message_context, Or}; #[derive(Debug, PartialEq)] pub(crate) struct CompilationError<'a> { @@ -211,8 +211,9 @@ impl<'a> Display for CompilationError<'a> { write!(f, "{}", message.suffix())?; - write_error_context( + write_message_context( f, + Color::fmt(f).error(), self.text, self.offset, self.line, diff --git a/src/justfile.rs b/src/justfile.rs index 9a9dee5e01..5221bbf0ff 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -6,7 +6,7 @@ pub(crate) struct Justfile<'a> { pub(crate) assignments: BTreeMap<&'a str, Expression<'a>>, pub(crate) exports: BTreeSet<&'a str>, pub(crate) aliases: BTreeMap<&'a str, Alias<'a>>, - pub(crate) deprecated_equals: bool, + pub(crate) warnings: Vec>, } impl<'a> Justfile<'a> where { diff --git a/src/lib.rs b/src/lib.rs index 7fd341cdad..d2089a0ef5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,6 +56,7 @@ mod token_kind; mod use_color; mod variables; mod verbosity; +mod warning; pub use crate::run::run; diff --git a/src/misc.rs b/src/misc.rs index 60ac529967..d9f388f759 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -55,8 +55,9 @@ pub(crate) fn conjoin( Ok(()) } -pub(crate) fn write_error_context( +pub(crate) fn write_message_context( f: &mut Formatter, + color: Color, text: &str, offset: usize, line: usize, @@ -66,7 +67,6 @@ pub(crate) fn write_error_context( let width = if width == 0 { 1 } else { width }; let line_number = line.ordinal(); - let red = Color::fmt(f).error(); match text.lines().nth(line) { Some(line) => { let mut i = 0; @@ -102,10 +102,10 @@ pub(crate) fn write_error_context( " {0:1$}{2}{3:^<4$}{5}", "", space_column, - red.prefix(), + color.prefix(), "", space_width, - red.suffix() + color.suffix() )?; } None => { diff --git a/src/parser.rs b/src/parser.rs index eb1b5f87ed..bf14e1f237 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,7 +12,7 @@ pub(crate) struct Parser<'a> { exports: BTreeSet<&'a str>, aliases: BTreeMap<&'a str, Alias<'a>>, alias_tokens: BTreeMap<&'a str, Token<'a>>, - deprecated_equals: bool, + warnings: Vec>, } impl<'a> Parser<'a> { @@ -32,7 +32,7 @@ impl<'a> Parser<'a> { exports: empty(), aliases: empty(), alias_tokens: empty(), - deprecated_equals: false, + warnings: Vec::new(), text, } } @@ -421,8 +421,10 @@ impl<'a> Parser<'a> { Name => { if token.lexeme() == "export" { let next = self.tokens.next().unwrap(); - if next.kind == Name && self.accepted(Equals) { - self.deprecated_equals = true; + if next.kind == Name && self.peek(Equals) { + self.warnings.push(Warning::DeprecatedEquals { + equals: self.tokens.next().unwrap(), + }); self.assignment(next, true)?; doc = None; } else if next.kind == Name && self.accepted(ColonEquals) { @@ -435,8 +437,10 @@ impl<'a> Parser<'a> { } } else if token.lexeme() == "alias" { let next = self.tokens.next().unwrap(); - if next.kind == Name && self.accepted(Equals) { - self.deprecated_equals = true; + if next.kind == Name && self.peek(Equals) { + self.warnings.push(Warning::DeprecatedEquals { + equals: self.tokens.next().unwrap(), + }); self.alias(next)?; doc = None; } else if next.kind == Name && self.accepted(ColonEquals) { @@ -447,8 +451,10 @@ impl<'a> Parser<'a> { self.recipe(&token, doc, false)?; doc = None; } - } else if self.accepted(Equals) { - self.deprecated_equals = true; + } else if self.peek(Equals) { + self.warnings.push(Warning::DeprecatedEquals { + equals: self.tokens.next().unwrap(), + }); self.assignment(token, false)?; doc = None; } else if self.accepted(ColonEquals) { @@ -515,7 +521,7 @@ impl<'a> Parser<'a> { assignments: self.assignments, exports: self.exports, aliases: self.aliases, - deprecated_equals: self.deprecated_equals, + warnings: self.warnings, }) } } diff --git a/src/run.rs b/src/run.rs index 61324c619d..a0b3c0912e 100644 --- a/src/run.rs +++ b/src/run.rs @@ -318,22 +318,12 @@ pub fn run() { } }); - if justfile.deprecated_equals { - let warning = color.warning().stderr(); - let message = color.message().stderr(); - - eprintln!( - "{}", - warning.paint( - "warning: `=` in assignments, exports, and aliases is being phased out on favor of `:=`" - ) - ); - - eprintln!( - "{}", - message - .paint("Please see this issue for more details: https://github.com/casey/just/issues/379") - ); + for warning in &justfile.warnings { + if color.stderr().active() { + eprintln!("{:#}", warning); + } else { + eprintln!("{}", warning); + } } if matches.is_present("SUMMARY") { diff --git a/src/runtime_error.rs b/src/runtime_error.rs index 1495cc7357..cef8a92cda 100644 --- a/src/runtime_error.rs +++ b/src/runtime_error.rs @@ -1,6 +1,6 @@ use crate::common::*; -use crate::misc::{maybe_s, ticks, write_error_context, And, Or, Tick}; +use crate::misc::{maybe_s, ticks, write_message_context, And, Or, Tick}; #[derive(Debug)] pub(crate) enum RuntimeError<'a> { @@ -387,8 +387,9 @@ impl<'a> Display for RuntimeError<'a> { write!(f, "{}", message.suffix())?; if let Some(token) = error_token { - write_error_context( + write_message_context( f, + Color::fmt(f).error(), token.text, token.offset, token.line, diff --git a/src/warning.rs b/src/warning.rs new file mode 100644 index 0000000000..32d8c385e8 --- /dev/null +++ b/src/warning.rs @@ -0,0 +1,56 @@ +use crate::common::*; +use crate::misc::write_message_context; + +use Warning::*; + +#[derive(Debug)] +pub(crate) enum Warning<'a> { + DeprecatedEquals { equals: Token<'a> }, +} + +impl Warning<'_> { + fn context(&self) -> Option<&Token> { + match self { + DeprecatedEquals { equals } => Some(equals), + } + } +} + +impl Display for Warning<'_> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + let warning = Color::fmt(f).warning(); + let message = Color::fmt(f).message(); + + write!(f, "{} {}", warning.paint("warning:"), message.prefix())?; + + match self { + DeprecatedEquals { .. } => { + writeln!( + f, + "`=` in assignments, exports, and aliases is being phased out on favor of `:=`" + )?; + write!( + f, + "Please see this issue for more details: https://github.com/casey/just/issues/379" + )?; + } + } + + write!(f, "{}", message.suffix())?; + + if let Some(token) = self.context() { + writeln!(f)?; + write_message_context( + f, + Color::fmt(f).warning(), + token.text, + token.offset, + token.line, + token.column, + token.lexeme().len(), + )?; + } + + Ok(()) + } +} diff --git a/tests/integration.rs b/tests/integration.rs index 5cd5df30bf..6437bfe9b4 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -2414,6 +2414,9 @@ default: stdout: "bar\n", stderr: "warning: `=` in assignments, exports, and aliases is being phased out on favor of `:=` Please see this issue for more details: https://github.com/casey/just/issues/379 + | +3 | foo = 'bar' + | ^ echo bar ", status: EXIT_SUCCESS, @@ -2434,6 +2437,9 @@ default: stdout: "bar\n", stderr: "warning: `=` in assignments, exports, and aliases is being phased out on favor of `:=` Please see this issue for more details: https://github.com/casey/just/issues/379 + | +3 | export FOO = 'bar' + | ^ echo $FOO ", status: EXIT_SUCCESS, @@ -2454,6 +2460,9 @@ default: stdout: "default\n", stderr: "warning: `=` in assignments, exports, and aliases is being phased out on favor of `:=` Please see this issue for more details: https://github.com/casey/just/issues/379 + | +3 | alias foo = default + | ^ echo default ", status: EXIT_SUCCESS,