Parser fixes 2020 (#9148)

* [parser] rework some of the resuming logic

see #7948

* [tests] add tests

closes #7944
closes #7948

src/syntax/grammar.mly

@@ -114,21 +114,18 @@ let rec	parse_file s =
 	| [< '(Kwd Package,_); pack = parse_package; s >] ->
 		begin match s with parser
 		| [< '(Const(Ident _),p) when pack = [] >] -> error (Custom "Package name must start with a lowercase character") p
-		| [< psem = semicolon; l = parse_type_decls TCAfterImport psem.pmax pack []; '(Eof,_) >] -> pack , l
+		| [< psem = semicolon; l = parse_type_decls TCAfterImport psem.pmax pack [] >] -> pack , l
 		end
-	| [< l = parse_type_decls TCBeforePackage (-1) [] []; '(Eof,_) >] -> [] , l
+	| [< l = parse_type_decls TCBeforePackage (-1) [] [] >] -> [] , l
 
 and parse_type_decls mode pmax pack acc s =
-	try
-		check_type_decl_completion mode pmax s;
-		match s with parser
-		| [< (v,p) = parse_type_decl mode >] ->
-			let mode = match v with
-				| EImport _ | EUsing _ -> TCAfterImport
-				| _ -> TCAfterType
-			in
-			parse_type_decls mode p.pmax pack ((v,p) :: acc) s
-		| [< >] -> List.rev acc
+	check_type_decl_completion mode pmax s;
+	let result = try
+		begin match s with parser
+		| [< cff = parse_type_decl mode >] -> Success cff
+		| [< '(Eof,p) >] -> End p
+		| [< >] -> Error ""
+		end
 	with
 	| TypePath ([],Some (name,false),b,p) ->
 		(* resolve imports *)
@@ -142,6 +139,18 @@ and parse_type_decls mode pmax pack acc s =
 		) acc;
 		raise (TypePath (pack,Some(name,true),b,p))
 	| Stream.Error msg when !in_display_file ->
+		Error msg
+	in
+	match result with
+	| Success (td,p) ->
+		let mode = match td with
+			| EImport _ | EUsing _ -> TCAfterImport
+			| _ -> TCAfterType
+		in
+		parse_type_decls mode p.pmax pack ((td,p) :: acc) s
+	| End _ ->
+		List.rev acc
+	| Error msg ->
 		handle_stream_error msg s;
 		ignore(resume false false s);
 		parse_type_decls mode (last_pos s).pmax pack acc s
@@ -150,7 +159,7 @@ and parse_abstract doc meta flags = parser
 	| [< '(Kwd Abstract,p1); name = type_name; tl = parse_constraint_params; st = parse_abstract_subtype; sl = plist parse_abstract_relations; s >] ->
 		let fl,p2 = match s with parser
 			| [< '(BrOpen,_); fl, p2 = parse_class_fields false p1 >] -> fl,p2
-			| [< >] -> syntax_error (Expected ["{"]) s ([],last_pos s)
+			| [< >] -> syntax_error (Expected ["{";"to";"from"]) s ([],last_pos s)
 		in
 		let flags = List.map decl_flag_to_abstract_flag flags in
 		let flags = (match st with None -> flags | Some t -> AbOver t :: flags) in
@@ -345,16 +354,6 @@ and parse_abstract_subtype s =
 
 and parse_package s = psep Dot lower_ident_or_macro s
 
-and parse_class_fields tdecl p1 s =
-	let l = parse_class_field_resume tdecl s in
-	let p2 = (match s with parser
-		| [< '(BrClose,p2) >] -> p2
-		| [< >] ->
-			(* We don't want to register this as a syntax error because it's part of the logic in display mode *)
-			if !in_display then (pos (last_token s)) else error (Expected ["}"]) (next_pos s)
-	) in
-	l, p2
-
 and resume tdecl fdecl s =
 	(* look for next variable/function or next type declaration *)
 	let rec junk k =
@@ -417,18 +416,38 @@ and resume tdecl fdecl s =
 	in
 	loop 1
 
-and parse_class_field_resume tdecl s =
-	if not (!in_display_file) then
-		plist (parse_class_field tdecl) s
-	else try
-		let c = parse_class_field tdecl s in
-		c :: parse_class_field_resume tdecl s
-	with
-	| Stream.Error msg ->
+and parse_class_field_resume acc tdecl s =
+	let result = try
+		begin match s with parser
+		| [< cff = parse_class_field tdecl >] -> Success cff
+		| [< '(BrClose,p) >] -> End p
+		| [< >] -> Error ""
+		end
+	with Stream.Error msg ->
+		Error msg
+	in
+	match result with
+	| Success cff ->
+		parse_class_field_resume (cff :: acc) tdecl s
+	| End p ->
+		List.rev acc,p
+	| Error msg ->
 		handle_stream_error msg s;
-		if resume tdecl true s then parse_class_field_resume tdecl s else []
-	| Stream.Failure ->
-		if resume tdecl true s then parse_class_field_resume tdecl s else []
+		if resume tdecl true s then
+			parse_class_field_resume acc tdecl s
+		else
+			acc,last_pos s
+
+and parse_class_fields tdecl p1 s =
+	if not (!in_display_file) then begin
+		let acc = plist (parse_class_field tdecl) s in
+		let p2 = (match s with parser
+			| [< '(BrClose,p2) >] -> p2
+			| [< >] -> error (Expected ["}"]) (next_pos s)
+		) in
+		acc,p2
+	end else
+		parse_class_field_resume [] tdecl s
 
 and parse_common_flags = parser
 	| [< '(Kwd Private,p); l = parse_common_flags >] -> (DPrivate,p) :: l

src/syntax/parser.ml

@@ -51,6 +51,11 @@ type syntax_completion =
 	| SCTypeDecl of type_decl_completion_mode
 	| SCAfterTypeFlag of decl_flag list
 
+type 'a sequence_parsing_result =
+	| Success of 'a
+	| End of pos
+	| Error of string
+
 exception Error of error_msg * pos
 exception TypePath of string list * (string * bool) option * bool (* in import *) * pos
 exception SyntaxCompletion of syntax_completion * DisplayTypes.completion_subject

tests/display/src/cases/Issue7114.hx

@@ -9,53 +9,51 @@ class Issue7114 extends DisplayTestCase {
 		using haxe.macro.ExprTools;
 
 		class Macro {
-		  public static macro function example_macro(e:Expr):Expr
-		  {
-		trace('Block before macro:\n${ e.toString() }');
+		public static macro function example_macro(e:Expr):Expr {
+			trace('Block before macro:\n${e.toString()}');
 
-		function injectCasts(ident:String, ct:ComplexType, expr:Expr):Expr
-		{
-		  if (expr == null) return null;
-		  switch (expr.expr) {
-			case EConst(CIdent(id)) if (id==ident):
-				var rtn = macro ((cast $i{ ident }):$ct);
-				rtn.pos = expr.pos;
-				return rtn;
-			default:
-		  }
+			function injectCasts(ident:String, ct:ComplexType, expr:Expr):Expr {
+				if (expr == null)
+					return null;
+				switch (expr.expr) {
+					case EConst(CIdent(id)) if (id == ident):
+						var rtn = macro((cast $i{ident}) : $ct);
+						rtn.pos = expr.pos;
+						return rtn;
+					default:
+				}
 
-		  return ExprTools.map(expr, function(e:Expr) return injectCasts(ident, ct, e) );
-		}
+				return ExprTools.map(expr, function(e:Expr) return injectCasts(ident, ct, e));
+			}
 
-		var blk = injectCasts('macro_override_cast', (macro :String), e);
+			var blk = injectCasts('macro_override_cast', (macro:String), e);
 
-		var rtn = macro {
-		  var macro_injected:String = "injected";
-		  var macro_override_var:String = cast macro_override_var;
-		  $blk;
-		};
+			var rtn = macro {
+				var macro_injected:String = "injected";
+				var macro_override_var:String = cast macro_override_var;
+				$blk;
+			};
 
-		trace('Block after macro:\n${ rtn.toString() }');
-		return rtn;
-		  }
+			trace('Block after macro:\n${rtn.toString()}');
+			return rtn;
+		}
+		}
 
 		class Test {
-		  static function main() {
+		static function main() {
+			var macro_override_var:SomeDynamic = "barbar";
+			var macro_override_cast:SomeDynamic = "foofoo";
 
-		var macro_override_var:SomeDynamic = "barbar";
-		var macro_override_cast:SomeDynamic = "foofoo";
-
-		Macro.example_macro({
-		  macro_injected.{-1-}
-		  macro_override_var.{-2-}
-		  macro_override_cast.{-3-}
-		});
-		  }
+			Macro.example_macro({
+				macro_injected.{-1-}
+				macro_override_var.{-2-}
+				macro_override_cast.{-3-}
+			});
+		}
 		}
 
 		abstract SomeDynamic(Dynamic) from Dynamic {
-		  public function oh_no__wrong_completions() {}
-		}
+		public function oh_no__wrong_completions() {}
 		}
 	**/
 	function test() {

tests/display/src/cases/Issue7944.hx

@@ -0,0 +1,21 @@
+package cases;
+
+class Issue7944 extends DisplayTestCase {
+	/**
+		class Main {
+		static function main() {}
+
+		{-1-}fun{-2-} f() {}
+		}
+	**/
+	function test() {
+		arrayEq([
+			{
+				kind: DKParserError,
+				severity: Error,
+				range: diagnosticsRange(pos(1), pos(2)),
+				args: "Unexpected fun"
+			}
+		], diagnostics());
+	}
+}

tests/display/src/cases/Issue7945.hx

@@ -10,7 +10,7 @@ class Issue7945 extends DisplayTestCase {
 				kind: DKParserError,
 				severity: Error,
 				range: diagnosticsRange(pos(1), pos(2)),
-				args: "Unexpected too"
+				args: "Expected { or to or from"
 			}
 		], diagnostics());
 	}

tests/display/src/cases/Issue7948.hx

@@ -0,0 +1,39 @@
+package cases;
+
+class Issue7948 extends DisplayTestCase {
+	/**
+		class Main {
+			{-1-}class{-2-} Moin {
+
+			}
+		}
+	**/
+	function test() {
+		arrayEq([
+			{
+				kind: DKParserError,
+				severity: Error,
+				range: diagnosticsRange(pos(1), pos(2)),
+				args: "Unexpected class"
+			}
+		], diagnostics());
+	}
+
+	/**
+		class Main {
+			static function main()
+				trace("Test");
+			}
+		{-1-}}{-2-}
+	**/
+	function test2() {
+		arrayEq([
+			{
+				kind: DKParserError,
+				severity: Error,
+				range: diagnosticsRange(pos(1), pos(2)),
+				args: "Unexpected }"
+			}
+		], diagnostics());
+	}
+}