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

[wasm][debugger] Implement support for null-conditional operators in simple expressions #69307

Merged
merged 16 commits into from
Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ public async Task<JObject> GetValueFromObject(JToken objRet, CancellationToken t
return null;
}

public async Task<(JObject containerObject, string remaining)> ResolveStaticMembersInStaticTypes(string varName, CancellationToken token)
public async Task<(JObject containerObject, string remaining)> ResolveStaticMembersInStaticTypes(string[] parts, CancellationToken token)
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
{
string classNameToFind = "";
string[] parts = varName.Split(".", StringSplitOptions.TrimEntries);
var store = await proxy.LoadStore(sessionId, token);
var methodInfo = context.CallStack.FirstOrDefault(s => s.Id == scopeId)?.Method?.Info;

Expand Down Expand Up @@ -175,6 +174,10 @@ async Task<int> FindStaticTypeId(string typeName)
// Checks Locals, followed by `this`
public async Task<JObject> Resolve(string varName, CancellationToken token)
{
// question mark at the end of expression is invalid
if (varName.LastOrDefault() == '?')
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
throw new ReturnAsErrorException($"Expected expression.", "ReferenceError");

//has method calls
if (varName.Contains('('))
return null;
Expand All @@ -185,17 +188,25 @@ public async Task<JObject> Resolve(string varName, CancellationToken token)
if (scopeCache.ObjectFields.TryGetValue(varName, out JObject valueRet))
return await GetValueFromObject(valueRet, token);

string[] parts = varName.Split(".");
string[] parts = varName.Split(".", StringSplitOptions.TrimEntries);
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
if (parts.Length == 0)
return null;

JObject retObject = await ResolveAsLocalOrThisMember(parts[0]);
bool isRootNullSafe = parts[0].LastOrDefault() == '?';
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
if (retObject != null && parts.Length > 1)
retObject = await ResolveAsInstanceMember(string.Join('.', parts[1..]), retObject);
{
// cannot resolve instance member on a primitive type,
// try compiling to check if it's not a method on primitive
var typeName = retObject["className"]?.Value<string>();
if (MonoSDBHelper.IsPrimitiveType(typeName))
return null;
retObject = await ResolveAsInstanceMember(string.Join('.', parts[1..]), retObject, isRootNullSafe);
}

if (retObject == null)
{
(retObject, string remaining) = await ResolveStaticMembersInStaticTypes(varName, token);
(retObject, string remaining) = await ResolveStaticMembersInStaticTypes(parts, token);
if (!string.IsNullOrEmpty(remaining))
{
if (retObject.IsNullValuedObject())
Expand All @@ -205,7 +216,7 @@ public async Task<JObject> Resolve(string varName, CancellationToken token)
}
else
{
retObject = await ResolveAsInstanceMember(remaining, retObject);
retObject = await ResolveAsInstanceMember(remaining, retObject, isRootNullSafe);
}
}
}
Expand All @@ -215,7 +226,6 @@ public async Task<JObject> Resolve(string varName, CancellationToken token)

async Task<JObject> ResolveAsLocalOrThisMember(string name)
{
var nameTrimmed = name.Trim();
if (scopeCache.Locals.Count == 0 && !localsFetched)
{
Result scope_res = await proxy.GetScopeProperties(sessionId, scopeId, token);
Expand All @@ -224,6 +234,11 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
localsFetched = true;
}

var nameTrimmed = name.Trim();
// remove null-safety, otherwise TryGet by name fails
if (nameTrimmed.LastOrDefault() == '?')
nameTrimmed = nameTrimmed.Remove(nameTrimmed.Length - 1);

if (scopeCache.Locals.TryGetValue(nameTrimmed, out JObject obj))
return obj["value"]?.Value<JObject>();

Expand All @@ -247,8 +262,11 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
return null;
}

async Task<JObject> ResolveAsInstanceMember(string expr, JObject baseObject)
async Task<JObject> ResolveAsInstanceMember(string expr, JObject baseObject, bool isRootNullSafe)
{
// if the prevPart has "?" at the end: true
bool isResolvedObjNullSafe = isRootNullSafe;
bool isPrevPartNull = false;
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
JObject resolvedObject = baseObject;
string[] parts = expr.Split('.');
for (int i = 0; i < parts.Length; i++)
Expand All @@ -257,16 +275,42 @@ async Task<JObject> ResolveAsInstanceMember(string expr, JObject baseObject)
if (partTrimmed.Length == 0)
return null;

// current value of resolvedObject is on parts[i - 1]
if (isPrevPartNull || resolvedObject.IsNullValuedObject())
{
// trying null.$member
if (!isResolvedObjNullSafe)
throw new ReturnAsErrorException($"Cannot access member \"{partTrimmed}\" of a null-valued object.", "ReferenceError");
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved

if (i == parts.Length - 1)
{
// this is not ideal, it returns the last object
// that had objectId and was null-valued,
// so the class/description of object are not of the last part
return resolvedObject;
}
// else: trying null?.$member... - allowed operation
isPrevPartNull = true;
}

bool isCurrentPartNullSafe = partTrimmed.Last() == '?';
isResolvedObjNullSafe = isCurrentPartNullSafe;

// cannot resolve the members but can check if nullSafety is
// correctly applied and if we should throw or return null-object
if (!DotnetObjectId.TryParse(resolvedObject?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
return null;
continue;
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved

ValueOrError<GetMembersResult> valueOrError = await proxy.RuntimeGetObjectMembers(sessionId, objectId, null, token);
var args = JObject.FromObject(new { forDebuggerDisplayAttribute = true });
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
ValueOrError<GetMembersResult> valueOrError = await proxy.RuntimeGetObjectMembers(sessionId, objectId, args, token);
if (valueOrError.IsError)
{
logger.LogDebug($"ResolveAsInstanceMember failed with : {valueOrError.Error}");
return null;
}

if (isCurrentPartNullSafe)
partTrimmed = partTrimmed.Remove(partTrimmed.Length - 1);
JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value<string>() == partTrimmed);
if (objRet == null)
return null;
Expand All @@ -275,19 +319,7 @@ async Task<JObject> ResolveAsInstanceMember(string expr, JObject baseObject)
if (resolvedObject == null)
return null;

if (resolvedObject.IsNullValuedObject())
{
if (i < parts.Length - 1)
{
// there is some parts remaining, and can't
// do null.$remaining
return null;
}

return resolvedObject;
}
}

return resolvedObject;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -751,14 +751,14 @@ internal async Task<ValueOrError<GetMembersResult>> RuntimeGetObjectMembers(Sess
GetObjectCommandOptions getObjectOptions = GetObjectCommandOptions.WithProperties;
if (args != null)
{
if (args["accessorPropertiesOnly"] != null && args["accessorPropertiesOnly"].Value<bool>())
{
if (args["accessorPropertiesOnly"]?.Value<bool>() == true)
getObjectOptions |= GetObjectCommandOptions.AccessorPropertiesOnly;
}
if (args["ownProperties"] != null && args["ownProperties"].Value<bool>())
{

if (args["ownProperties"]?.Value<bool>() == true)
getObjectOptions |= GetObjectCommandOptions.OwnProperties;
}

if (args["forDebuggerDisplayAttribute"]?.Value<bool>() == true)
getObjectOptions |= GetObjectCommandOptions.ForDebuggerDisplayAttribute;
}
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1148,6 +1148,54 @@ await EvaluateOnCallFrameAndCheck(id,
);
});

[ConditionalFact(nameof(RunningOnChrome))]
public async Task EvaluateNullObjectPropertiesPositive() => await CheckInspectLocalsAtBreakpointSite(
$"DebuggerTests.EvaluateNullableProperties", "Evaluate", 6, "Evaluate",
$"window.setTimeout(function() {{ invoke_static_method ('[debugger-test] DebuggerTests.EvaluateNullableProperties:Evaluate'); 1 }})",
wait_for_event_fn: async (pause_location) =>
{
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();

// we have no way of returning int? for null values,
// so we return the last non-null class name
await EvaluateOnCallFrameAndCheck(id,
("list.Count", TNumber(1)),
("list?.Count", TNumber(1)),
("listNull", TObject("System.Collections.Generic.List<int>", is_null: true)),
("listNull?.Count", TObject("System.Collections.Generic.List<int>", is_null: true)),
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
("tc?.memberList?.Count", TNumber(2)),
("tc?.memberListNull?.Count", TObject("System.Collections.Generic.List<int>", is_null: true)),
("tc.memberListNull?.Count", TObject("System.Collections.Generic.List<int>", is_null: true)),
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
("tcNull?.memberListNull?.Count", TObject("DebuggerTests.EvaluateNullableProperties.TestClass", is_null: true)));
});

[ConditionalFact(nameof(RunningOnChrome))]
public async Task EvaluateNullObjectPropertiesNegative() => await CheckInspectLocalsAtBreakpointSite(
$"DebuggerTests.EvaluateNullableProperties", "Evaluate", 6, "Evaluate",
$"window.setTimeout(function() {{ invoke_static_method ('[debugger-test] DebuggerTests.EvaluateNullableProperties:Evaluate'); 1 }})",
wait_for_event_fn: async (pause_location) =>
{
var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
var (_, res) = await EvaluateOnCallFrame(id, "listNull.Count", expect_ok: false);
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
AssertEqual("Cannot access member \"Count\" of a null-valued object.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
(_, res) = await EvaluateOnCallFrame(id, "tcNull.memberListNull.Count", expect_ok: false);
AssertEqual("Cannot access member \"memberListNull\" of a null-valued object.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
(_, res) = await EvaluateOnCallFrame(id, "tc.memberListNull.Count", expect_ok: false);
AssertEqual("Cannot access member \"Count\" of a null-valued object.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
(_, res) = await EvaluateOnCallFrame(id, "tcNull?.memberListNull.Count", expect_ok: false);
AssertEqual("Cannot access member \"Count\" of a null-valued object.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
(_, res) = await EvaluateOnCallFrame(id, "listNull?.Count.NonExistingProperty", expect_ok: false);
AssertEqual("Cannot access member \"NonExistingProperty\" of a null-valued object.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
(_, res) = await EvaluateOnCallFrame(id, "listNull?", expect_ok: false);
AssertEqual("Expected expression.",
res.Error["result"]?["description"]?.Value<string>(), "wrong error message");
});

[Fact]
public async Task EvaluateMethodsOnPrimitiveTypesReturningPrimitives() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.PrimitiveTypeMethods", "Evaluate", 11, "Evaluate",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,24 @@ public static void Evaluate()
string localString = "S*T*R";
}
}

public static class EvaluateNullableProperties
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
{
class TestClass
{
public List<int> memberListNull = null;
List<int> memberList = new List<int>() {1, 2};
}
static void Evaluate()
{
#nullable enable
List<int>? listNull = null;
#nullable disable
List<int> list = new List<int>() {1};
TestClass tc = new TestClass();
TestClass tcNull = null;
}
}
}

namespace DebuggerTestsV2
Expand Down