Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[diagnostics] fix multi display files #11722

Merged
merged 7 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 36 additions & 29 deletions src/compiler/displayProcessing.ml
Original file line number Diff line number Diff line change
Expand Up @@ -156,36 +156,43 @@ 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
| e ->
die "" __LOC__
kLabz marked this conversation as resolved.
Show resolved Hide resolved
in
Common.log com ("Display file : " ^ real);
Common.log com ("Classes found : [" ^ (String.concat "," (List.map s_type_path actx.classes)) ^ "]");
kLabz marked this conversation as resolved.
Show resolved Hide resolved
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 ("Classes found : [" ^ (String.concat "," (List.map s_type_path actx.classes)) ^ "]");
path
Common.log com ("Display file : " ^ real);
Common.log com ("Classes found : [" ^ (String.concat "," (List.map s_type_path actx.classes)) ^ "]");
dpk
) DisplayPosition.display_position#get_files in
match dpk with
| [dfile] -> dfile
| _ -> DPKNone

(* 3. Loaders for display file that might be called *)

Expand Down
8 changes: 2 additions & 6 deletions src/context/display/displayJson.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/core/display/displayPosition.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
*)
Expand Down
7 changes: 4 additions & 3 deletions tests/server/src/TestCase.hx
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,19 @@ 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 = [];
errorMessages = [];
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) {
Expand Down
57 changes: 49 additions & 8 deletions tests/server/src/cases/ServerTests.hx
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
Expand Down Expand Up @@ -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);
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import File1;
import File2;
import File3;

function main() {}
Loading