Skip to content

Commit

Permalink
Turn = deprecation warning into a hard error (#780)
Browse files Browse the repository at this point in the history
It's been around two and a half years, and many versions, since this
warning was first introduced, so it feels reasonable to finally turn it
into a hard error. It will remain a special-cased error for a little
while.
  • Loading branch information
casey authored Mar 29, 2021
1 parent 4e2e101 commit 10282bd
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 113 deletions.
18 changes: 9 additions & 9 deletions src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,8 @@ mod tests {

analysis_error! {
name: duplicate_alias,
input: "alias foo = bar\nalias foo = baz",
offset: 22,
input: "alias foo := bar\nalias foo := baz",
offset: 23,
line: 1,
column: 6,
width: 3,
Expand All @@ -212,7 +212,7 @@ mod tests {

analysis_error! {
name: unknown_alias_target,
input: "alias foo = bar\n",
input: "alias foo := bar\n",
offset: 6,
line: 0,
column: 6,
Expand All @@ -222,7 +222,7 @@ mod tests {

analysis_error! {
name: alias_shadows_recipe_before,
input: "bar: \n echo bar\nalias foo = bar\nfoo:\n echo foo",
input: "bar: \n echo bar\nalias foo := bar\nfoo:\n echo foo",
offset: 23,
line: 2,
column: 6,
Expand All @@ -232,7 +232,7 @@ mod tests {

analysis_error! {
name: alias_shadows_recipe_after,
input: "foo:\n echo foo\nalias foo = bar\nbar:\n echo bar",
input: "foo:\n echo foo\nalias foo := bar\nbar:\n echo bar",
offset: 22,
line: 2,
column: 6,
Expand Down Expand Up @@ -272,8 +272,8 @@ mod tests {

analysis_error! {
name: parameter_shadows_varible,
input: "foo = \"h\"\na foo:",
offset: 12,
input: "foo := \"h\"\na foo:",
offset: 13,
line: 1,
column: 2,
width: 3,
Expand All @@ -292,8 +292,8 @@ mod tests {

analysis_error! {
name: duplicate_variable,
input: "a = \"0\"\na = \"0\"",
offset: 8,
input: "a := \"0\"\na := \"0\"",
offset: 9,
line: 1,
column: 0,
width: 1,
Expand Down
12 changes: 6 additions & 6 deletions src/assignment_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ mod tests {

analysis_error! {
name: circular_variable_dependency,
input: "a = b\nb = a",
input: "a := b\nb := a",
offset: 0,
line: 0,
column: 0,
Expand All @@ -121,8 +121,8 @@ mod tests {

analysis_error! {
name: self_variable_dependency,
input: "a = a",
offset: 0,
input: "a := a",
offset: 0,
line: 0,
column: 0,
width: 1,
Expand All @@ -131,10 +131,10 @@ mod tests {

analysis_error! {
name: unknown_expression_variable,
input: "x = yy",
offset: 4,
input: "x := yy",
offset: 5,
line: 0,
column: 4,
column: 5,
width: 2,
kind: UndefinedVariable{variable: "yy"},
}
Expand Down
10 changes: 10 additions & 0 deletions src/compilation_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@ impl Display for CompilationError<'_> {
};
writeln!(f, "`\\{}` is not a valid escape sequence", representation)?;
},
DeprecatedEquals => {
writeln!(
f,
"`=` in assignments, exports, and aliases has been phased out on favor of `:=`"
)?;
writeln!(
f,
"Please see this issue for more details: https://github.com/casey/just/issues/379"
)?;
},
DuplicateParameter { recipe, parameter } => {
writeln!(
f,
Expand Down
1 change: 1 addition & 0 deletions src/compilation_error_kind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub(crate) enum CompilationErrorKind<'src> {
min: usize,
max: usize,
},
DeprecatedEquals,
DuplicateAlias {
alias: &'src str,
first: usize,
Expand Down
4 changes: 2 additions & 2 deletions src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ mod tests {
run_error! {
name: export_assignment_backtick,
src: r#"
export exported_variable = "A"
b = `echo $exported_variable`
export exported_variable := "A"
b := `echo $exported_variable`
recipe:
echo {{b}}
Expand Down
10 changes: 5 additions & 5 deletions src/justfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub(crate) struct Justfile<'src> {
pub(crate) assignments: Table<'src, Assignment<'src>>,
pub(crate) aliases: Table<'src, Alias<'src>>,
pub(crate) settings: Settings<'src>,
pub(crate) warnings: Vec<Warning<'src>>,
pub(crate) warnings: Vec<Warning>,
}

impl<'src> Justfile<'src> {
Expand Down Expand Up @@ -557,10 +557,10 @@ mod tests {
run_error! {
name: export_failure,
src: r#"
export foo = "a"
baz = "c"
export bar = "b"
export abc = foo + bar + baz
export foo := "a"
baz := "c"
export bar := "b"
export abc := foo + bar + baz
wut:
echo $foo $bar $baz
Expand Down
2 changes: 1 addition & 1 deletion src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ pub(crate) struct Module<'src> {
/// Items in the justfile
pub(crate) items: Vec<Item<'src>>,
/// Non-fatal warnings encountered during parsing
pub(crate) warnings: Vec<Warning<'src>>,
pub(crate) warnings: Vec<Warning>,
}
6 changes: 2 additions & 4 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,8 @@ impl<'src> Node<'src> for Set<'src> {
}
}

impl<'src> Node<'src> for Warning<'src> {
impl<'src> Node<'src> for Warning {
fn tree(&self) -> Tree<'src> {
match self {
Warning::DeprecatedEquals { .. } => Tree::atom("warning").push("deprecated_equals"),
}
unreachable!()
}
}
71 changes: 30 additions & 41 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
/// Parse a justfile, consumes self
fn parse_justfile(mut self) -> CompilationResult<'src, Module<'src>> {
let mut items = Vec::new();
let mut warnings = Vec::new();

let mut doc = None;

Expand All @@ -325,22 +324,15 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
match Keyword::from_lexeme(next.lexeme()) {
Some(Keyword::Alias) =>
if self.next_are(&[Identifier, Identifier, Equals]) {
warnings.push(Warning::DeprecatedEquals {
equals: self.get(2)?,
});
items.push(Item::Alias(self.parse_alias()?));
return Err(self.get(2)?.error(CompilationErrorKind::DeprecatedEquals));
} else if self.next_are(&[Identifier, Identifier, ColonEquals]) {
items.push(Item::Alias(self.parse_alias()?));
} else {
items.push(Item::Recipe(self.parse_recipe(doc, false)?));
},
Some(Keyword::Export) =>
if self.next_are(&[Identifier, Identifier, Equals]) {
warnings.push(Warning::DeprecatedEquals {
equals: self.get(2)?,
});
self.presume_keyword(Keyword::Export)?;
items.push(Item::Assignment(self.parse_assignment(true)?));
return Err(self.get(2)?.error(CompilationErrorKind::DeprecatedEquals));
} else if self.next_are(&[Identifier, Identifier, ColonEquals]) {
self.presume_keyword(Keyword::Export)?;
items.push(Item::Assignment(self.parse_assignment(true)?));
Expand All @@ -358,10 +350,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
},
_ =>
if self.next_are(&[Identifier, Equals]) {
warnings.push(Warning::DeprecatedEquals {
equals: self.get(1)?,
});
items.push(Item::Assignment(self.parse_assignment(false)?));
return Err(self.get(1)?.error(CompilationErrorKind::DeprecatedEquals));
} else if self.next_are(&[Identifier, ColonEquals]) {
items.push(Item::Assignment(self.parse_assignment(false)?));
} else {
Expand All @@ -385,7 +374,10 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
self.tokens.len() - self.next,
))?)
} else {
Ok(Module { items, warnings })
Ok(Module {
warnings: Vec::new(),
items,
})
}
}

Expand Down Expand Up @@ -882,10 +874,9 @@ mod tests {

test! {
name: alias_equals,
text: "alias t = test",
text: "alias t := test",
tree: (justfile
(alias t test)
(warning deprecated_equals)
),
}

Expand All @@ -897,10 +888,9 @@ mod tests {

test! {
name: export_equals,
text: r#"export x = "hello""#,
text: r#"export x := "hello""#,
tree: (justfile
(assignment #export x "hello")
(warning deprecated_equals)
),
}

Expand All @@ -912,10 +902,9 @@ mod tests {

test! {
name: assignment_equals,
text: r#"x = "hello""#,
text: r#"x := "hello""#,
tree: (justfile
(assignment x "hello")
(warning deprecated_equals)
),
}

Expand Down Expand Up @@ -1691,23 +1680,23 @@ mod tests {
}

error! {
name: alias_syntax_multiple_rhs,
input: "alias foo = bar baz",
offset: 16,
line: 0,
column: 16,
width: 3,
kind: UnexpectedToken { expected: vec![Comment, Eof, Eol], found: Identifier },
name: alias_syntax_multiple_rhs,
input: "alias foo := bar baz",
offset: 17,
line: 0,
column: 17,
width: 3,
kind: UnexpectedToken { expected: vec![Comment, Eof, Eol], found: Identifier },
}

error! {
name: alias_syntax_no_rhs,
input: "alias foo = \n",
offset: 12,
line: 0,
column: 12,
width: 1,
kind: UnexpectedToken {expected: vec![Identifier], found:Eol},
name: alias_syntax_no_rhs,
input: "alias foo := \n",
offset: 13,
line: 0,
column: 13,
width: 1,
kind: UnexpectedToken {expected: vec![Identifier], found:Eol},
}

error! {
Expand Down Expand Up @@ -1774,10 +1763,10 @@ mod tests {

error! {
name: unclosed_parenthesis_in_expression,
input: "x = foo(",
offset: 8,
input: "x := foo(",
offset: 9,
line: 0,
column: 8,
column: 9,
width: 0,
kind: UnexpectedToken{
expected: vec![Backtick, Identifier, ParenL, ParenR, StringCooked, StringRaw],
Expand Down Expand Up @@ -1952,10 +1941,10 @@ mod tests {

error! {
name: unknown_function,
input: "a = foo()",
offset: 4,
input: "a := foo()",
offset: 5,
line: 0,
column: 4,
column: 5,
width: 3,
kind: UnknownFunction{function: "foo"},
}
Expand Down
4 changes: 2 additions & 2 deletions src/recipe_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ mod tests {

analysis_error! {
name: unknown_second_interpolation_variable,
input: "wtf=\"x\"\nx:\n echo\n foo {{wtf}} {{ lol }}",
offset: 33,
input: "wtf:=\"x\"\nx:\n echo\n foo {{wtf}} {{ lol }}",
offset: 34,
line: 3,
column: 16,
width: 3,
Expand Down
30 changes: 6 additions & 24 deletions src/warning.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,22 @@
use crate::common::*;

use Warning::*;

#[derive(Debug, PartialEq)]
pub(crate) enum Warning<'src> {
DeprecatedEquals { equals: Token<'src> },
}
pub(crate) enum Warning {}

impl<'src> Warning<'src> {
fn context(&self) -> Option<&Token<'src>> {
match self {
DeprecatedEquals { equals } => Some(equals),
}
impl Warning {
fn context(&self) -> Option<&Token> {
#![allow(clippy::unused_self)]
unreachable!()
}
}

impl Display for Warning<'_> {
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() {
Expand Down
Loading

0 comments on commit 10282bd

Please sign in to comment.