Pretty errors as default message reporting (#11587)

* Switch to pretty errors by default

* Avoid 'Error: : ' in args errors

* Do not eagerly report --cwd errors

* [tests] Lazy migration of misc tests to default pretty errors

* [tests] Don't use pretty errors for display tests

* [tests] More misc tests

src-json/define.json

@@ -786,7 +786,7 @@
 	{
 		"name": "MessageReporting",
 		"define": "message.reporting",
-		"doc": "Select message reporting mode for compiler output. (default: classic)",
+		"doc": "Select message reporting mode for compiler output. (default: pretty)",
 		"params": ["mode: classic | pretty | indent"]
 	},
 	{

src/compiler/args.ml

@@ -325,11 +325,13 @@ let parse_args com =
 					List.rev acc
 			in
 			let args = loop [] args in
-			Arg.parse_argv ~current (Array.of_list ("" :: args)) all_args_spec args_callback "";
+			Arg.parse_argv ~current (Array.of_list ("Haxe" :: args)) all_args_spec args_callback "";
 		with
 		| Arg.Help _ ->
 			raise (Helper.HelpMessage (usage_string all_args usage))
 		| Arg.Bad msg ->
+			(* Strip error prefix added by ocaml's arg parser *)
+			let msg = if ExtLib.String.starts_with msg "Haxe: " then (String.sub msg 6 ((String.length msg) - 6)) else msg in
 			let first_line = List.nth (Str.split (Str.regexp "\n") msg) 0 in
 			let new_msg = (Printf.sprintf "%s" first_line) in
 			let r = Str.regexp "unknown option [`']?\\([-A-Za-z]+\\)[`']?" in

src/compiler/compiler.ml

@@ -599,7 +599,8 @@ module HighLevel = struct
 				loop acc l
 			| "--cwd" :: dir :: l | "-C" :: dir :: l ->
 				(* we need to change it immediately since it will affect hxml loading *)
-				(try Unix.chdir dir with _ -> raise (Arg.Bad ("Invalid directory: " ^ dir)));
+				(* Exceptions are ignored there to let arg parsing do the error handling in expected order *)
+				(try Unix.chdir dir with _ -> ());
 				(* Push the --cwd arg so the arg processor know we did something. *)
 				loop (dir :: "--cwd" :: acc) l
 			| "--connect" :: hp :: l ->

src/compiler/messageReporting.ml

@@ -346,7 +346,7 @@ let format_messages com messages =
 	let absolute_positions = Define.defined com.defines Define.MessageAbsolutePositions in
 	let ectx = create_error_context absolute_positions in
 	ectx.max_lines <- get_max_line ectx.max_lines messages;
-	let message_formatter = get_formatter com Define.MessageReporting "classic" in
+	let message_formatter = get_formatter com Define.MessageReporting "pretty" in
 	let lines = List.rev (
 		List.fold_left (fun lines cm -> match (message_formatter ectx cm) with
 			| None -> lines
@@ -372,7 +372,7 @@ let display_messages ctx on_message = begin
 			compiler_message_string
 	in
 
-	let message_formatter = get_formatter ctx.com Define.MessageReporting "classic" in
+	let message_formatter = get_formatter ctx.com Define.MessageReporting "pretty" in
 	let log_formatter = get_formatter ctx.com Define.MessageLogFormat "indent" in
 
 	let log_messages = ref (Define.defined ctx.com.defines Define.MessageLogFile) in

tests/display/src/BaseDisplayTestContext.hx

@@ -39,7 +39,7 @@ class BaseDisplayTestContext {
 	}
 
 	static public function runHaxe(args:Array<String>, ?stdin:String) {
-		return haxeServer.rawRequest(args, stdin == null ? null : Bytes.ofString(stdin));
+		return haxeServer.rawRequest(["-D", "message.reporting=classic"].concat(args), stdin == null ? null : Bytes.ofString(stdin));
 	}
 
 	static function normalizePath(p:String):String {

tests/misc/flash/projects/Issue9805/compile.hxml

@@ -2,6 +2,7 @@
 
 --next
 
+-D message.reporting=classic
 --main Main
 --swf-lib bin/lib.swc
---swf bin/test.swf
+--swf bin/test.swf

tests/misc/java/projects/Issue11095/compile-fail.hxml

@@ -4,6 +4,7 @@ NoRudy
 
 --next
 
+-D message.reporting=classic
 --main Main
 --java-lib no-rudy.jar
 --jvm run.jar

tests/misc/projects/Issue10871/Compiler/Main.hx

@@ -13,7 +13,7 @@ class MacroClass {
 	public static function start() {
 		final config = Compiler.getConfiguration();
 
-		trace(config.args);
+		trace(filterArgs(config.args));
 		trace(config.debug);
 		trace(config.verbose);
 		trace(config.foptimize);
@@ -37,5 +37,12 @@ class MacroClass {
 			case _: "unused platform";
 		}
 	}
+
+	static function filterArgs(args:Array<String>):Array<String> {
+		if (args.length < 2) return args;
+		// We're currently prepending that to all tests while moving to pretty errors by default
+		if (args[0] == "-D" && args[1] == "message.reporting=classic") return args.slice(2);
+		return args;
+	}
 }
 #end

tests/misc/projects/Issue11087/Main.hx

@@ -1,9 +1,16 @@
 macro function test() {
-	trace(Sys.args());
+	trace(filterArgs(Sys.args()));
 	return macro null;
 }
 
 function main() {
 	test();
-	trace(Sys.args());
+	trace(filterArgs(Sys.args()));
+}
+
+function filterArgs(args:Array<String>):Array<String> {
+	if (args.length < 2) return args;
+	// We're currently prepending that to all tests while moving to pretty errors by default
+	if (args[0] == "-D" && args[1] == "message.reporting=classic") return args.slice(2);
+	return args;
 }

tests/misc/projects/Issue11128/compile2-fail.hxml.stderr

@@ -1 +1 @@
-Error: : --custom-target cannot use reserved name js.
+Error: --custom-target cannot use reserved name js.

tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr

@@ -1 +1 @@
-Error: Invalid directory: unexistant
+Error: Invalid directory: unexistant.

tests/misc/src/Main.hx

@@ -47,7 +47,7 @@ class Main {
 					var expectFailure = file.endsWith("-fail.hxml");
 					var expectStdout = if (FileSystem.exists('$file.stdout')) prepareExpectedOutput(File.getContent('$file.stdout')) else null;
 					var expectStderr = if (FileSystem.exists('$file.stderr')) prepareExpectedOutput(File.getContent('$file.stderr')) else null;
-					var result = runCommand("haxe", [file], expectFailure, expectStdout, expectStderr);
+					var result = runCommand("haxe", ["-D", "message.reporting=classic", file], expectFailure, expectStdout, expectStderr);
 					++count;
 					if (!result.success) {
 						failures++;