fix(parser): Select the correct offside location inside parentheses

Parentheses do not insert an extra block inside them which the `in` token insertion code assumed which caused us to select the location of the `Parens` context instead of the `Let` (or `Type`) context which triggered the offside rule in the wrong way.
gluon-lang · Dec 2, 2017 · 4060aad · 4060aad
1 parent 57a87cd
commit 4060aad
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 36 deletions.
diff --git a/parser/src/layout.rs b/parser/src/layout.rs
@@ -50,6 +50,7 @@ enum Context {
     Lambda,
 }
 
+#[derive(Debug)]
 struct Contexts {
     stack: Vec<Offside>,
 }
@@ -188,18 +189,18 @@ where
             debug!("--------\n{:?}\n{:?}", token, offside);
 
             match (&token.value, offside.context) {
-                (&Token::Comma, Context::Brace)
-                | (&Token::Comma, Context::Paren)
-                | (&Token::Comma, Context::Bracket) => return Ok(token),
+                (&Token::Comma, Context::Brace) |
+                (&Token::Comma, Context::Paren) |
+                (&Token::Comma, Context::Bracket) => return Ok(token),
 
                 // If it is closing token we remove contexts until a context for that token is found
-                (&Token::In, _)
-                | (&Token::CloseBlock, _)
-                | (&Token::Else, _)
-                | (&Token::RBrace, _)
-                | (&Token::RBracket, _)
-                | (&Token::RParen, _)
-                | (&Token::Comma, _) => {
+                (&Token::In, _) |
+                (&Token::CloseBlock, _) |
+                (&Token::Else, _) |
+                (&Token::RBrace, _) |
+                (&Token::RBracket, _) |
+                (&Token::RParen, _) |
+                (&Token::Comma, _) => {
                     self.indent_levels.pop();
 
                     // If none of the contexts would be closed by this token then this is likely a
@@ -328,8 +329,9 @@ where
                     if token.value != Token::And && token.value != Token::RBrace =>
                 {
                     // Insert an `in` token
-                    self.indent_levels.pop();
-                    let location = {
+
+                    let let_location = self.indent_levels.pop().unwrap().location;
+                    {
                         let offside = self.indent_levels
                             .last_mut()
                             .expect("No top level block found");
@@ -341,8 +343,7 @@ where
                         {
                             *emit_semi = false;
                         }
-                        offside.location
-                    };
+                    }
                     let span = token.span;
                     let result = Ok(self.layout_token(token, Token::In));
 
@@ -353,7 +354,7 @@ where
                     // b
                     // ```
                     // `let x = 1 in {{ a; b }}` and not `{{ (let x = 1 in a) ; b }}`
-                    let offside = Offside::new(location, Context::Block { emit_semi: false });
+                    let offside = Offside::new(let_location, Context::Block { emit_semi: false });
                     self.indent_levels.push(offside)?;
                     self.unprocessed_tokens
                         .push(pos::spanned(span, Token::OpenBlock));
@@ -365,8 +366,7 @@ where
 
             // Some tokens directly insert a new context when emitted
             let push_context = match token.value {
-                Token::Let => Some(Context::Let),
-                Token::Do => Some(Context::Let),
+                Token::Let | Token::Do => Some(Context::Let),
                 Token::If => Some(Context::If),
                 Token::Type => Some(Context::Type),
                 Token::Match => Some(Context::Expr),
@@ -390,12 +390,10 @@ where
                     }
                 }
 
-                (&Token::Equals, Context::Let)
-                | (&Token::RArrow, Context::Lambda)
-                | (&Token::RArrow, Context::MatchClause)
-                | (&Token::Then, _) => {
-                    self.scan_for_next_block(Context::Block { emit_semi: false })?
-                }
+                (&Token::Equals, Context::Let) |
+                (&Token::RArrow, Context::Lambda) |
+                (&Token::RArrow, Context::MatchClause) |
+                (&Token::Then, _) => self.scan_for_next_block(Context::Block { emit_semi: false })?,
                 (&Token::With, _) => self.scan_for_next_block(Context::MatchClause)?,
 
                 (&Token::Else, _) => {
@@ -439,14 +437,14 @@ where
 
 fn token_closes_context(token: &Token, context: Context) -> bool {
     match (token, context) {
-        (&Token::Else, Context::If)
-        | (&Token::RBrace, Context::Brace)
-        | (&Token::RBracket, Context::Bracket)
-        | (&Token::RParen, Context::Paren)
-        | (&Token::CloseBlock, Context::Block { .. })
-        | (&Token::In, Context::Let)
-        | (&Token::In, Context::Type)
-        | (_, Context::Block { .. }) => true,
+        (&Token::Else, Context::If) |
+        (&Token::RBrace, Context::Brace) |
+        (&Token::RBracket, Context::Bracket) |
+        (&Token::RParen, Context::Paren) |
+        (&Token::CloseBlock, Context::Block { .. }) |
+        (&Token::In, Context::Let) |
+        (&Token::In, Context::Type) |
+        (_, Context::Block { .. }) => true,
         (_, _) => false,
     }
 }

diff --git a/parser/tests/basic.rs b/parser/tests/basic.rs
@@ -755,3 +755,16 @@ fn shebang_at_top_is_ignored() {
         record_a(vec![("Test".into(), None)], vec![("x".into(), None)])
     )
 }
+
+
+#[test]
+fn do_in_parens() {
+    let _ = ::env_logger::init();
+    let text = r"
+        scope_state (
+            do _ = add_args
+            eval_exprs
+        )
+    ";
+    parse_clear_span!(text);
+}
diff --git a/parser/tests/support/mod.rs b/parser/tests/support/mod.rs
@@ -1,9 +1,9 @@
 #![allow(unused)]
 
-use base::ast::{Alternative, Array, AstType, DisplayEnv, Expr, ExprField, IdentEnv, Lambda,
-                Literal, Pattern, SpannedExpr, SpannedPattern, MutVisitor, walk_mut_expr,
-                walk_mut_pattern, walk_mut_ast_type, TypeBinding, TypedIdent,
-                SpannedIdent, SpannedAlias, SpannedAstType, ValueBinding};
+use base::ast::{walk_mut_ast_type, walk_mut_expr, walk_mut_pattern, Alternative, Array, AstType,
+                DisplayEnv, Expr, ExprField, IdentEnv, Lambda, Literal, MutVisitor, Pattern,
+                SpannedAlias, SpannedAstType, SpannedExpr, SpannedIdent, SpannedPattern,
+                TypeBinding, TypedIdent, ValueBinding};
 use base::error::Errors;
 use base::pos::{self, BytePos, Span, Spanned};
 use base::kind::Kind;
@@ -88,7 +88,7 @@ macro_rules! parse_new {
     ($input:expr) => {{
         // Replace windows line endings so that byte positions match up on multiline expressions
         let input = $input.replace("\r\n", "\n");
-        parse(&input).unwrap_or_else(|(_, err)| panic!("{}", err))
+        parse(&input).unwrap_or_else(|(_, err)| panic!("{}", ::base::error::InFile::new("test", &input, err)))
     }}
 }