[diagnostics] fix multi display files (#11722)

* [diagnostics] fix json rpc diagnostics display config

* [tests] Server tests: do not fail silently when runHaxeJsonCb errors

* [tests] add more diagnostics tests

* [display] rework multiple display files handling

* clean up a bit...

* [diagnostics] handle a.b.c.hx case, even if pointless

* [diagnostics] do not skip errors during DisplayProcessing.process_display_file

src/compiler/compiler.ml

@@ -346,7 +346,9 @@ let compile ctx actx callbacks =
 	let com = ctx.com in
 	(* Set up display configuration *)
 	DisplayProcessing.process_display_configuration ctx;
+	let restore = disable_report_mode com in
 	let display_file_dot_path = DisplayProcessing.process_display_file com actx in
+	restore ();
 	let mctx = match com.platform with
 		| CustomTarget name ->
 			begin try

src/compiler/displayProcessing.ml

@@ -156,36 +156,41 @@ let process_display_file com actx =
 				actx.classes <- [];
 				com.main.main_class <- None;
 			end;
-			let real = Path.get_real_path (DisplayPosition.display_position#get).pfile in
-			let path = match get_module_path_from_file_path com real with
-			| Some path ->
-				if com.display.dms_kind = DMPackage then DisplayException.raise_package (fst path);
-				let path = match ExtString.String.nsplit (snd path) "." with
-					| [name;"macro"] ->
-						(* If we have a .macro.hx path, don't add the file to classes because the compiler won't find it.
-						   This can happen if we're completing in such a file. *)
-						DPKMacro (fst path,name)
-					| [name] ->
-						actx.classes <- path :: actx.classes;
-						DPKNormal path
-					| [name;target] ->
-						let path = fst path, name in
-						actx.classes <- path :: actx.classes;
-						DPKNormal path
-					| e ->
-						die "" __LOC__
+			let dpk = List.map (fun file_key ->
+				let real = Path.get_real_path (Path.UniqueKey.to_string file_key) in
+				let dpk = match get_module_path_from_file_path com real with
+				| Some path ->
+					if com.display.dms_kind = DMPackage then DisplayException.raise_package (fst path);
+					let dpk = match ExtString.String.nsplit (snd path) "." with
+						| [name;"macro"] ->
+							(* If we have a .macro.hx path, don't add the file to classes because the compiler won't find it.
+								 This can happen if we're completing in such a file. *)
+							DPKMacro (fst path,name)
+						| [name] ->
+							actx.classes <- path :: actx.classes;
+							DPKNormal path
+						| [name;target] ->
+							let path = fst path, name in
+							actx.classes <- path :: actx.classes;
+							DPKNormal path
+						| _ ->
+							failwith ("Invalid display file '" ^ real ^ "'")
+					in
+					dpk
+				| None ->
+					if not (Sys.file_exists real) then failwith "Display file does not exist";
+					(match List.rev (ExtString.String.nsplit real Path.path_sep) with
+					| file :: _ when file.[0] >= 'a' && file.[0] <= 'z' -> failwith ("Display file '" ^ file ^ "' should not start with a lowercase letter")
+					| _ -> ());
+					DPKDirect real
 				in
-				path
-			| None ->
-				if not (Sys.file_exists real) then failwith "Display file does not exist";
-				(match List.rev (ExtString.String.nsplit real Path.path_sep) with
-				| file :: _ when file.[0] >= 'a' && file.[0] <= 'z' -> failwith ("Display file '" ^ file ^ "' should not start with a lowercase letter")
-				| _ -> ());
-				DPKDirect real
-			in
-			Common.log com ("Display file : " ^ real);
+				Common.log com ("Display file : " ^ real);
+				dpk
+			) DisplayPosition.display_position#get_files in
 			Common.log com ("Classes found : ["  ^ (String.concat "," (List.map s_type_path actx.classes)) ^ "]");
-			path
+			match dpk with
+				| [dfile] -> dfile
+				| _ -> DPKNone
 
 (* 3. Loaders for display file that might be called *)
 

src/context/display/displayJson.ml

@@ -99,7 +99,7 @@ class display_handler (jsonrpc : jsonrpc_handler) com (cs : CompilationCache.t)
 			com.file_contents <- file_contents;
 
 			match files with
-			| [] | [_] -> DisplayPosition.display_position#set { pfile = file; pmin = pos; pmax = pos; };
+			| [] -> DisplayPosition.display_position#set { pfile = file; pmin = pos; pmax = pos; };
 			| _ -> DisplayPosition.display_position#set_files files;
 		end
 end
@@ -210,12 +210,8 @@ let handler =
 		"display/diagnostics", (fun hctx ->
 			hctx.display#set_display_file false false;
 			hctx.display#enable_display DMNone;
+			hctx.com.display <- { hctx.com.display with dms_display_file_policy = DFPAlso; dms_per_file = true; dms_populate_cache = true };
 			hctx.com.report_mode <- RMDiagnostics (List.map (fun (f,_) -> f) hctx.com.file_contents);
-
-			(match hctx.com.file_contents with
-			| [file, None] ->
-				hctx.com.display <- { hctx.com.display with dms_display_file_policy = DFPAlso; dms_per_file = true; dms_populate_cache = true };
-			| _ -> ());
 		);
 		"display/implementation", (fun hctx ->
 			hctx.display#set_display_file false true;

src/core/display/displayPosition.ml

@@ -23,11 +23,15 @@ class display_position_container =
 		method set p =
 			pos <- p;
 			last_pos <- p;
-			file_key <- None
+			file_key <- None;
+			file_keys <- if p.pfile = DisplayProcessingGlobals.file_input_marker then [] else [Path.UniqueKey.create p.pfile]
 
 		method set_files files =
 			file_keys <- files
 
+		method get_files =
+			file_keys
+
 		(**
 			Get current display position
 		*)

tests/server/src/TestCase.hx

@@ -119,7 +119,7 @@ class TestCase implements ITest implements ITestCase {
 	}
 
 	function runHaxeJsonCb<TParams, TResponse>(args:Array<String>, method:HaxeRequestMethod<TParams, Response<TResponse>>, methodArgs:TParams,
-			callback:TResponse->Void, done:() -> Void) {
+			callback:TResponse->Void, done:() -> Void, ?pos:PosInfos) {
 		var methodArgs = {method: method, id: 1, params: methodArgs};
 		args = args.concat(['--display', Json.stringify(methodArgs)]);
 		messages = [];
@@ -127,10 +127,11 @@ class TestCase implements ITest implements ITestCase {
 		server.rawRequest(args, null, function(result) {
 			handleResult(result);
 			var json = try Json.parse(result.stderr) catch(e) {result: null, error: e.message};
+
 			if (json.result != null) {
-				callback(json.result.result);
+				callback(json.result?.result);
 			} else {
-				sendErrorMessage('Error: ' + json.error);
+				Assert.fail('Error: ' + json.error, pos);
 			}
 			done();
 		}, function(msg) {

tests/server/src/cases/ServerTests.hx

@@ -251,6 +251,22 @@ class ServerTests extends TestCase {
 		assertReuse("HelloWorld");
 	}
 
+	function testDiagnosticsRecache1() {
+		vfs.putContent("HelloWorld.hx", getTemplate("HelloWorld.hx"));
+		var args = ["--main", "HelloWorld", "--interp"];
+		runHaxe(args);
+		runHaxe(args);
+		assertReuse("HelloWorld");
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("HelloWorld.hx")});
+		runHaxe(args);
+		assertSkipping("HelloWorld", Tainted("server/invalidate"));
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [{file: new FsPath("HelloWorld.hx")}]}, res -> {
+			Assert.equals(0, res.length);
+		});
+		runHaxe(args);
+		assertReuse("HelloWorld");
+	}
+
 	function testDiagnosticsRecache2() {
 		vfs.putContent("HelloWorld.hx", getTemplate("HelloWorld.hx"));
 		var args = ["--main", "HelloWorld", "--interp"];
@@ -284,22 +300,21 @@ class ServerTests extends TestCase {
 		var args = ["--main", "Main", "--interp"];
 		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [
 			{file: new FsPath("Main.hx")},
-			{file: new FsPath("File1.hx")},
-			{file: new FsPath("File2.hx")},
+			{file: new FsPath("File1.hx")}
 		]}, res -> {
-			Assert.equals(3, res.length); // Asked diagnostics for 3 files
+			Assert.equals(2, res.length); // Asked diagnostics for 2 files
 
 			for (fileDiagnostics in res) {
 				final path = ~/[\/|\\]/g.split(fileDiagnostics.file.toString()).pop();
 
 				switch (path) {
 					case "Main.hx":
-						Assert.equals(3, fileDiagnostics.diagnostics.length);
+						Assert.equals(2, fileDiagnostics.diagnostics.length);
 						for (diag in fileDiagnostics.diagnostics) {
 							Assert.equals(diag.kind, DKUnusedImport);
 						}
 
-					case "File1.hx" | "File2.hx":
+					case "File1.hx":
 						Assert.equals(1, fileDiagnostics.diagnostics.length);
 						var diag:Diagnostic<ReplaceableCodeDiagnostics> = fileDiagnostics.diagnostics[0];
 						Assert.equals(diag.kind, ReplaceableCode);
@@ -310,11 +325,37 @@ class ServerTests extends TestCase {
 			}
 		});
 
-		// Check that File3 was reached
+		// Check that File2 was reached
 		var context = null;
 		runHaxeJsonCb(args, ServerMethods.Contexts, null, res -> context = res.find(ctx -> ctx.desc == "after_init_macros"));
-		runHaxeJsonCb(args, ServerMethods.Type, {signature: context.signature, modulePath: "File3", typeName: "File3"}, res -> Assert.equals(res.pos.file, "File3.hx"));
-		assertSuccess();
+		runHaxeJsonCb(args, ServerMethods.Type, {signature: context.signature, modulePath: "File2", typeName: "File2"}, res -> Assert.equals(res.pos.file, "File2.hx"));
+
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {fileContents: [
+			{file: new FsPath("Main.hx")},
+			{file: new FsPath("File3.hx")}, // Not reached by normal compilation
+		]}, res -> {
+			Assert.equals(2, res.length); // Asked diagnostics for 2 files
+
+			for (fileDiagnostics in res) {
+				final path = ~/[\/|\\]/g.split(fileDiagnostics.file.toString()).pop();
+
+				switch (path) {
+					case "Main.hx":
+						Assert.equals(2, fileDiagnostics.diagnostics.length);
+						for (diag in fileDiagnostics.diagnostics) {
+							Assert.equals(diag.kind, DKUnusedImport);
+						}
+
+					case "File3.hx":
+						Assert.equals(1, fileDiagnostics.diagnostics.length);
+						var diag:Diagnostic<ReplaceableCodeDiagnostics> = fileDiagnostics.diagnostics[0];
+						Assert.equals(diag.kind, ReplaceableCode);
+						Assert.equals(diag.args.description, "Unused variable");
+
+					case _: throw 'Did not expect diagnostics for $path';
+				}
+			}
+		});
 	}
 
 	function testSyntaxCache() {

tests/server/test/templates/diagnostics/multi-files/Main.hx

@@ -1,5 +1,4 @@
 import File1;
 import File2;
-import File3;
 
 function main() {}