Run some filters in diagnostics (#11220)

* let's see how much breaks

* [tests] enable diagnostics tests for 11177 and 11184

* [tests] Update test for 5306

* Don't cache/run filters for find reference/implementation requests (#11226)

* Only run filters and save cache on diagnostics, not usage requests

* [tests] Update test for 11184

* disable test

* add VUsedByTyper to avoid bad unused local errors

* revert @:compilerGenerated change

---------

Co-authored-by: Rudy Ges <k@klabz.org>

src/compiler/compiler.ml

@@ -302,10 +302,10 @@ let finalize_typing ctx tctx =
 	com.modules <- modules;
 	t()
 
-let filter ctx tctx =
+let filter ctx tctx before_destruction =
 	let t = Timer.timer ["filters"] in
 	DeprecationCheck.run ctx.com;
-	Filters.run ctx.com tctx ctx.com.main;
+	run_or_diagnose ctx (fun () -> Filters.run tctx ctx.com.main before_destruction);
 	t()
 
 let compile ctx actx =
@@ -334,8 +334,12 @@ let compile ctx actx =
 		end;
 		DisplayProcessing.handle_display_after_typing ctx tctx display_file_dot_path;
 		finalize_typing ctx tctx;
-		DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path;
-		filter ctx tctx;
+		if is_diagnostics com then
+			filter ctx tctx (fun () -> DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path)
+		else begin
+			DisplayProcessing.handle_display_after_finalization ctx tctx display_file_dot_path;
+			filter ctx tctx (fun () -> ());
+		end;
 		if ctx.has_error then raise Abort;
 		Generate.check_auxiliary_output com actx;
 		com.stage <- CGenerationStart;

src/context/display/diagnostics.ml

@@ -20,7 +20,7 @@ let find_unused_variables com e =
 	let vars = Hashtbl.create 0 in
 	let pmin_map = Hashtbl.create 0 in
 	let rec loop e = match e.eexpr with
-		| TVar({v_kind = VUser _} as v,eo) when v.v_name <> "_" ->
+		| TVar({v_kind = VUser _} as v,eo) when v.v_name <> "_" && not (has_var_flag v VUsedByTyper) ->
 			Hashtbl.add pmin_map e.epos.pmin v;
 			let p = match eo with
 				| None -> e.epos
@@ -179,15 +179,10 @@ let prepare com =
 	dctx.unresolved_identifiers <- com.display_information.unresolved_identifiers;
 	dctx
 
-let secure_generated_code ctx e =
-	(* This causes problems and sucks in general... need a different solution. But I forgot which problem this solved anyway. *)
-	(* mk (TMeta((Meta.Extern,[],e.epos),e)) e.etype e.epos *)
-	e
-
 let print com =
 	let dctx = prepare com in
 	Json.string_of_json (DiagnosticsPrinter.json_of_diagnostics com dctx)
 
 let run com =
 	let dctx = prepare com in
-	dctx
+	dctx

src/core/displayTypes.ml

@@ -235,7 +235,6 @@ module DisplayMode = struct
 		| DMDefault | DMDefinition | DMTypeDefinition | DMPackage | DMHover | DMSignature -> settings
 		| DMUsage _ | DMImplementation -> { settings with
 				dms_full_typing = true;
-				dms_populate_cache = !ServerConfig.populate_cache_from_display;
 				dms_force_macro_typing = true;
 				dms_display_file_policy = DFPAlso;
 				dms_exit_during_typing = false

src/core/tType.ml

@@ -442,7 +442,12 @@ let flag_tclass_field_names = [
 type flag_tvar =
 	| VCaptured
 	| VFinal
-	| VUsed (* used by the analyzer *)
+	| VAnalyzed
 	| VAssigned
 	| VCaught
 	| VStatic
+	| VUsedByTyper (* Set if the typer looked up this variable *)
+
+let flag_tvar_names = [
+	"VCaptured";"VFinal";"VAnalyzed";"VAssigned";"VCaught";"VStatic";"VUsedByTyper"
+]

src/filters/filters.ml

@@ -916,7 +916,8 @@ let save_class_state ctx t =
 			a.a_meta <- List.filter (fun (m,_,_) -> m <> Meta.ValueUsed) a.a_meta
 		)
 
-let run com tctx main =
+let run tctx main before_destruction =
+	let com = tctx.com in
 	let detail_times = Common.defined com DefineList.FilterTimes in
 	let new_types = List.filter (fun t ->
 		let cached = is_cached com t in
@@ -1025,4 +1026,5 @@ let run com tctx main =
 	let t = filter_timer detail_times ["callbacks"] in
 	com.callbacks#run com.callbacks#get_after_save; (* macros onGenerate etc. *)
 	t();
-	destruction tctx detail_times main locals
+	before_destruction();
+	destruction tctx detail_times main locals

src/optimization/analyzer.ml

@@ -661,22 +661,22 @@ module LocalDce = struct
 
 	let rec apply ctx =
 		let is_used v =
-			has_var_flag v VUsed
+			has_var_flag v VAnalyzed
 		in
 		let keep v =
 			is_used v || ((match v.v_kind with VUser _ | VInlined -> true | _ -> false) && not ctx.config.local_dce) || ExtType.has_reference_semantics v.v_type || has_var_flag v VCaptured || Meta.has Meta.This v.v_meta
 		in
 		let rec use v =
 			if not (is_used v) then begin
-				add_var_flag v VUsed;
+				add_var_flag v VAnalyzed;
 				(try expr (get_var_value ctx.graph v) with Not_found -> ());
 				begin match Ssa.get_reaching_def ctx.graph v with
 					| None ->
 						(* We don't want to fully recurse for the origin variable because we don't care about its
 						   reaching definition (issue #10972). Simply marking it as being used should be sufficient. *)
 						let v' = get_var_origin ctx.graph v in
 						if not (is_used v') then
-							add_var_flag v' VUsed
+							add_var_flag v' VAnalyzed
 					| Some v ->
 						use v;
 				end

src/optimization/inline.ml

@@ -589,7 +589,6 @@ class inline_state ctx ethis params cf f p = object(self)
 				mk (TBlock (DynArray.to_list el)) tret e.epos
 		in
 		let e = inline_metadata e cf.cf_meta in
-		let e = Diagnostics.secure_generated_code ctx e in
 		if has_params then begin
 			let mt = map_type cf.cf_type in
 			let unify_func () = unify_raise mt (TFun (tl,tret)) p in

src/typing/callUnification.ml

@@ -488,7 +488,6 @@ object(self)
 			!ethis_f();
 			raise exc
 		in
-		let e = Diagnostics.secure_generated_code ctx e in
 		ctx.com.located_error <- old;
 		!ethis_f();
 		e

src/typing/typer.ml

@@ -350,6 +350,7 @@ let rec type_ident_raise ctx i p mode with_type =
 	| _ ->
 	try
 		let v = PMap.find i ctx.locals in
+		add_var_flag v VUsedByTyper;
 		(match v.v_extra with
 		| Some ve ->
 			let (params,e) = (ve.v_params,ve.v_expr) in
@@ -1130,7 +1131,7 @@ and type_new ctx path el with_type force_inline p =
 		typing_error (s_type (print_context()) t ^ " cannot be constructed") p
 	end with Error(No_constructor _ as err,p,depth) when ctx.com.display.dms_kind <> DMNone ->
 		located_display_error ~depth ctx.com (error_msg p err);
-		Diagnostics.secure_generated_code ctx (mk (TConst TNull) t p)
+		mk (TConst TNull) t p
 
 and type_try ctx e1 catches with_type p =
 	let e1 = type_expr ctx (Expr.ensure_block e1) with_type in
@@ -1796,7 +1797,6 @@ and type_call_builtin ctx e el mode with_type p =
 		| _ ->
 			let e = type_expr ctx e WithType.value in
 			warning ctx WInfo (s_type (print_context()) e.etype) e.epos;
-			let e = Diagnostics.secure_generated_code ctx e in
 			e
 		end
 	| (EField(e,"match",efk_todo),p), [epat] ->

src/typing/typerBase.ml

@@ -155,9 +155,11 @@ let get_this ctx p =
 	| FunMemberClassLocal | FunMemberAbstractLocal ->
 		let v = match ctx.vthis with
 			| None ->
-				let v = if ctx.curfun = FunMemberAbstractLocal then
-					PMap.find "this" ctx.locals
-				else
+				let v = if ctx.curfun = FunMemberAbstractLocal then begin
+					let v = PMap.find "this" ctx.locals in
+					add_var_flag v VUsedByTyper;
+					v
+				end else
 					add_local ctx VGenerated "`this" ctx.tthis p
 				in
 				ctx.vthis <- Some v;
@@ -347,4 +349,4 @@ let get_abstract_froms ctx a pl =
 				acc)
 		| _ ->
 			acc
-	) l a.a_from_field
+	) l a.a_from_field

tests/display/src/cases/Issue5306.hx

@@ -7,8 +7,8 @@ class Issue5306 extends DisplayTestCase {
 		class Main {
 			static function main() {
 				var ib:Array<Int>;
-				ib[0] = 0; ib[1] = 1; ib[2]
-				{-5-}trace{-6-}("test");
+				{-5-}ib{-6-}[0] = 0; ib[1] = 1; ib[2]
+				{-7-}trace{-8-}("test");
 			}
 		}
 	**/
@@ -22,7 +22,7 @@ class Issue5306 extends DisplayTestCase {
 			// },
 			{
 				kind: DKParserError,
-				range: diagnosticsRange(pos(5), pos(6)),
+				range: diagnosticsRange(pos(7), pos(8)),
 				severity: Error,
 				relatedInformation: [],
 				args: "Missing ;"
@@ -33,6 +33,14 @@ class Issue5306 extends DisplayTestCase {
 				severity: Error,
 				relatedInformation: [],
 				args: "Type not found : InvalidType"
+			},
+			{
+				kind: DKCompilerError,
+				range: diagnosticsRange(pos(5), pos(6)),
+				severity: Error,
+				code: null,
+				relatedInformation: [],
+				args: "Local variable ib used without being initialized"
 			}
 		];
 		arrayEq(expected, diagnostics());

tests/server/src/cases/issues/Issue11177.hx

@@ -1,33 +1,32 @@
 package cases.issues;
 
 class Issue11177 extends TestCase {
-	// Disabled for now until #11177 is actually fixed, likely by #11220
-	// function test(_) {
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx"));
-	// 	vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx"));
-	// 	vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx"));
-	// 	var args = ["-main", "Main", "--interp"];
-	// 	runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> {
-	// 		Assert.equals(0, res.length);
-	// 	});
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx"));
-	// 	runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
-	// 	runHaxe(args);
-	// 	runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> {
-	// 		Assert.equals(0, res.length);
-	// 	});
-	// }
+	function test(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx"));
+		vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx"));
+		vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx"));
+		var args = ["-main", "Main", "--interp"];
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> {
+			Assert.equals(0, res.length);
+		});
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx"));
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Buttons.hx")}, res -> {
+			Assert.equals(0, res.length);
+		});
+	}
 
-	// function testLegacyDiagnostics(_) {
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx"));
-	// 	vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx"));
-	// 	vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx"));
-	// 	var args = ["-main", "Main", "--interp"];
-	// 	runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"]));
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx"));
-	// 	runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
-	// 	runHaxe(args);
-	// 	runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"]));
-	// 	Assert.isTrue(lastResult.stderr.length == 2);
-	// }
+	function testLegacyDiagnostics(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main.hx"));
+		vfs.putContent("Buttons.hx", getTemplate("issues/Issue11177/Buttons.hx"));
+		vfs.putContent("KeyCode.hx", getTemplate("issues/Issue11177/KeyCode.hx"));
+		var args = ["-main", "Main", "--interp"];
+		runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"]));
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11177/Main2.hx"));
+		runHaxeJson([], ServerMethods.Invalidate, {file: new FsPath("Main.hx")});
+		runHaxe(args);
+		runHaxe(args.concat(["--display", "Buttons.hx@0@diagnostics"]));
+		Assert.isTrue(lastResult.stderr.length == 2);
+	}
 }

tests/server/src/cases/issues/Issue11184.hx

@@ -1,32 +1,31 @@
 package cases.issues;
 
 class Issue11184 extends TestCase {
-	// Disabled for now until #11184 is actually fixed, likely by #11220
-	// function testDiagnostics(_) {
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx"));
-	// 	var args = ["-main", "Main", "-js", "bin/test.js"];
+	function testDiagnostics(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx"));
+		var args = ["-main", "Main", "-js", "bin/test.js"];
 
-	// 	runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Main.hx")}, res -> {
-	// 		Assert.equals(1, res.length);
-	// 		Assert.equals(1, res[0].diagnostics.length);
-	// 		Assert.equals(res[0].diagnostics[0].args, "Cannot use Void as value");
-	// 	});
+		runHaxeJsonCb(args, DisplayMethods.Diagnostics, {file: new FsPath("Main.hx")}, res -> {
+			Assert.equals(1, res.length);
+			Assert.equals(1, res[0].diagnostics.length);
+			Assert.equals(res[0].diagnostics[0].args, "Cannot use Void as value");
+		});
 
-	// 	runHaxe(args);
-	// 	Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
-	// 	runHaxe(args);
-	// 	Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
-	// }
+		runHaxe(args);
+		Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
+		runHaxe(args);
+		Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
+	}
 
-	// function testLegacyDiagnostics(_) {
-	// 	vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx"));
-	// 	var args = ["-main", "Main", "-js", "bin/test.js"];
-	// 	runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"]));
-	// 	final diagnostics = haxe.Json.parse(lastResult.stderr)[0].diagnostics;
-	// 	Assert.equals(diagnostics[0].args, "Cannot use Void as value");
-	// 	runHaxe(args);
-	// 	Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
-	// 	runHaxe(args);
-	// 	Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
-	// }
+	function testLegacyDiagnostics(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11184/Main.hx"));
+		var args = ["-main", "Main", "-js", "bin/test.js"];
+		runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"]));
+		final diagnostics = haxe.Json.parse(lastResult.stderr)[0].diagnostics;
+		Assert.equals(diagnostics[0].args, "Cannot use Void as value");
+		runHaxe(args);
+		Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
+		runHaxe(args);
+		Assert.isTrue(hasErrorMessage("Cannot use Void as value"));
+	}
 }

tests/server/src/cases/issues/Issue11203.hx

@@ -0,0 +1,23 @@
+package cases.issues;
+
+class Issue11203 extends TestCase {
+	function testClass(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11203/MainClass.hx"));
+		var args = ["Main", "--interp"];
+		runHaxe(args);
+		runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"]));
+
+		var diag = parseDiagnostics();
+		Assert.isTrue(diag.length == 0);
+	}
+
+	function testAbstract(_) {
+		vfs.putContent("Main.hx", getTemplate("issues/Issue11203/MainAbstract.hx"));
+		var args = ["Main", "--interp"];
+		runHaxe(args);
+		runHaxe(args.concat(["--display", "Main.hx@0@diagnostics"]));
+
+		var diag = parseDiagnostics();
+		Assert.isTrue(diag.length == 0);
+	}
+}

tests/server/test/templates/issues/Issue11203/MainAbstract.hx

@@ -0,0 +1,16 @@
+class Main {
+	static function main() {
+		var future = new Future();
+		future.eager();
+	}
+}
+
+abstract Future({}) from {} {
+	public function new()
+		this = {};
+
+	public inline function eager():Future {
+		trace("much side effect!");
+		return this;
+	}
+}

tests/server/test/templates/issues/Issue11203/MainClass.hx

@@ -0,0 +1,15 @@
+class Main {
+	static function main() {
+		var future = new Future();
+		future.eager();
+	}
+}
+
+class Future {
+	public function new() {}
+
+	public inline function eager():Future {
+		trace("much side effect!");
+		return this;
+	}
+}