From a88a1a902e523016223ddfb02a2c9209103c72fc Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Tue, 30 Apr 2024 17:47:26 +0200 Subject: [PATCH] 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 | 2 +- src/compiler/args.ml | 4 +++- src/compiler/compiler.ml | 3 ++- src/compiler/messageReporting.ml | 4 ++-- tests/display/src/BaseDisplayTestContext.hx | 2 +- tests/misc/flash/projects/Issue9805/compile.hxml | 3 ++- tests/misc/java/projects/Issue11095/compile-fail.hxml | 1 + tests/misc/projects/Issue10871/Compiler/Main.hx | 9 ++++++++- tests/misc/projects/Issue11087/Main.hx | 11 +++++++++-- .../projects/Issue11128/compile2-fail.hxml.stderr | 2 +- .../misc/projects/Issue3300/test-cwd-fail.hxml.stderr | 2 +- tests/misc/src/Main.hx | 2 +- 12 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src-json/define.json b/src-json/define.json index c3a3d03c91b..28d13c75039 100644 --- a/src-json/define.json +++ b/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"] }, { diff --git a/src/compiler/args.ml b/src/compiler/args.ml index 3f95944aa54..fcd72ecb143 100644 --- a/src/compiler/args.ml +++ b/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 diff --git a/src/compiler/compiler.ml b/src/compiler/compiler.ml index 57c89dae094..1f17c2bc5d4 100644 --- a/src/compiler/compiler.ml +++ b/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 -> diff --git a/src/compiler/messageReporting.ml b/src/compiler/messageReporting.ml index 2d9ecc80f3e..82fbe656982 100644 --- a/src/compiler/messageReporting.ml +++ b/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 diff --git a/tests/display/src/BaseDisplayTestContext.hx b/tests/display/src/BaseDisplayTestContext.hx index 803d8707d1f..6f4b799fb86 100644 --- a/tests/display/src/BaseDisplayTestContext.hx +++ b/tests/display/src/BaseDisplayTestContext.hx @@ -39,7 +39,7 @@ class BaseDisplayTestContext { } static public function runHaxe(args:Array, ?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 { diff --git a/tests/misc/flash/projects/Issue9805/compile.hxml b/tests/misc/flash/projects/Issue9805/compile.hxml index 3b8b35e70c3..89fb7200197 100644 --- a/tests/misc/flash/projects/Issue9805/compile.hxml +++ b/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 \ No newline at end of file +--swf bin/test.swf diff --git a/tests/misc/java/projects/Issue11095/compile-fail.hxml b/tests/misc/java/projects/Issue11095/compile-fail.hxml index 98aa6a185f3..efcf1e7a862 100644 --- a/tests/misc/java/projects/Issue11095/compile-fail.hxml +++ b/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 diff --git a/tests/misc/projects/Issue10871/Compiler/Main.hx b/tests/misc/projects/Issue10871/Compiler/Main.hx index 25e568aed60..d3f6fcf7e1c 100644 --- a/tests/misc/projects/Issue10871/Compiler/Main.hx +++ b/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):Array { + 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 diff --git a/tests/misc/projects/Issue11087/Main.hx b/tests/misc/projects/Issue11087/Main.hx index dcec6730897..eb92f827cf4 100644 --- a/tests/misc/projects/Issue11087/Main.hx +++ b/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):Array { + 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; } diff --git a/tests/misc/projects/Issue11128/compile2-fail.hxml.stderr b/tests/misc/projects/Issue11128/compile2-fail.hxml.stderr index 20de0526ff4..5d1d741724a 100644 --- a/tests/misc/projects/Issue11128/compile2-fail.hxml.stderr +++ b/tests/misc/projects/Issue11128/compile2-fail.hxml.stderr @@ -1 +1 @@ -Error: : --custom-target cannot use reserved name js. \ No newline at end of file +Error: --custom-target cannot use reserved name js. diff --git a/tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr b/tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr index ff010fb6f41..ffffdedad5b 100644 --- a/tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr +++ b/tests/misc/projects/Issue3300/test-cwd-fail.hxml.stderr @@ -1 +1 @@ -Error: Invalid directory: unexistant \ No newline at end of file +Error: Invalid directory: unexistant. diff --git a/tests/misc/src/Main.hx b/tests/misc/src/Main.hx index 7e05e1da364..c84f4120b81 100644 --- a/tests/misc/src/Main.hx +++ b/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++;