Repository: casey/just
Pull request #473 (2019-09-21 20:59:01)

Add context to deprecation warnings
Previously, warnings upon encountering a deprecated use `=` in assignments, exports, and alaises would print an 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.


Patch:
diff --git a/src/common.rs b/src/common.rs
index 75abad85..842d97cf 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<T, CompilationError<'a>>;
diff --git a/src/compilation_error.rs b/src/compilation_error.rs
index 40252b19..b203c778 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 9a9dee5e..5221bbf0 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<Warning<'a>>,
 }
 
 impl<'a> Justfile<'a> where {
diff --git a/src/lib.rs b/src/lib.rs
index 7fd341cd..d2089a0e 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 60ac5299..d9f388f7 100644
--- a/src/misc.rs
+++ b/src/misc.rs
@@ -55,8 +55,9 @@ pub(crate) fn conjoin<T: Display>(
   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 eb1b5f87..bf14e1f2 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<Warning<'a>>,
 }
 
 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 61324c61..a0b3c091 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 1495cc73..cef8a92c 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 00000000..32d8c385
--- /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(())
+  }
+}


Test patch:
diff --git a/tests/integration.rs b/tests/integration.rs
index 5cd5df30..6437bfe9 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,