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 15 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
103 changes: 66 additions & 37 deletions src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,17 @@ 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, ArraySegment<string> remaining)> ResolveStaticMembersInStaticTypes(ArraySegment<string> parts, CancellationToken token)
{
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;

if (methodInfo == null)
return (null, null);

int typeId = -1;
for (int i = 0; i < parts.Length; i++)
for (int i = 0; i < parts.Count; i++)
{
string part = parts[i];

Expand All @@ -97,9 +96,9 @@ public async Task<JObject> GetValueFromObject(JToken objRet, CancellationToken t
JObject memberObject = await FindStaticMemberInType(classNameToFind, part, typeId);
if (memberObject != null)
{
string remaining = null;
if (i < parts.Length - 1)
remaining = string.Join('.', parts[(i + 1)..]);
ArraySegment<string> remaining = null;
if (i < parts.Count - 1)
remaining = parts[i..];

return (memberObject, remaining);
}
Expand Down Expand Up @@ -177,6 +176,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[^1] == '?')
throw new ReturnAsErrorException($"Expected expression.", "ReferenceError");

//has method calls
if (varName.Contains('('))
return null;
Expand All @@ -187,18 +190,19 @@ 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(".");
if (parts.Length == 0)
return null;
string[] parts = varName.Split(".", StringSplitOptions.TrimEntries);
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
if (parts.Length == 0 || string.IsNullOrEmpty(parts[0]))
throw new ReturnAsErrorException($"Failed to resolve expression: {varName}", "ReferenceError");

JObject retObject = await ResolveAsLocalOrThisMember(parts[0]);
bool throwOnNullReference = parts[0][^1] != '?';
if (retObject != null && parts.Length > 1)
retObject = await ResolveAsInstanceMember(string.Join('.', parts[1..]), retObject);
retObject = await ResolveAsInstanceMember(parts, retObject, throwOnNullReference);

if (retObject == null)
{
(retObject, string remaining) = await ResolveStaticMembersInStaticTypes(varName, token);
if (!string.IsNullOrEmpty(remaining))
(retObject, ArraySegment<string> remaining) = await ResolveStaticMembersInStaticTypes(parts, token);
if (remaining != null && remaining.Count != 0)
{
if (retObject.IsNullValuedObject())
{
Expand All @@ -207,7 +211,7 @@ public async Task<JObject> Resolve(string varName, CancellationToken token)
}
else
{
retObject = await ResolveAsInstanceMember(remaining, retObject);
retObject = await ResolveAsInstanceMember(remaining, retObject, throwOnNullReference);
}
}
}
Expand All @@ -217,7 +221,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 @@ -226,7 +229,11 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
localsFetched = true;
}

if (scopeCache.Locals.TryGetValue(nameTrimmed, out JObject obj))
// remove null-condition, otherwise TryGet by name fails
if (name[^1] == '?' || name[^1] == '!')
name = name.Remove(name.Length - 1);

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

if (!scopeCache.Locals.TryGetValue("this", out JObject objThis))
Expand All @@ -242,54 +249,76 @@ async Task<JObject> ResolveAsLocalOrThisMember(string name)
return null;
}

JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"].Value<string>() == nameTrimmed);
JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"].Value<string>() == name);
if (objRet != null)
return await GetValueFromObject(objRet, token);

return null;
}

async Task<JObject> ResolveAsInstanceMember(string expr, JObject baseObject)
async Task<JObject> ResolveAsInstanceMember(ArraySegment<string> parts, JObject baseObject, bool throwOnNullReference)
{
JObject resolvedObject = baseObject;
string[] parts = expr.Split('.');
for (int i = 0; i < parts.Length; i++)
// parts[0] - name of baseObject
for (int i = 1; i < parts.Count; i++)
{
string partTrimmed = parts[i].Trim();
if (partTrimmed.Length == 0)
string part = parts[i];
if (part.Length == 0)
return null;

if (part[^1] == '!')
part = part.Remove(part.Length - 1);

bool hasCurrentPartNullCondition = part[^1] == '?';
radical marked this conversation as resolved.
Show resolved Hide resolved

// current value of resolvedObject is on parts[i - 1]
if (resolvedObject.IsNullValuedObject())
{
// trying null.$member
if (throwOnNullReference)
throw new ReturnAsErrorException($"Expression threw NullReferenceException trying to access \"{part}\" on a null-valued object.", "ReferenceError");

if (i == parts.Count - 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;
}

// check if null condition is correctly applied: should we throw or return null-object
throwOnNullReference = !hasCurrentPartNullCondition;
continue;
}

if (!DotnetObjectId.TryParse(resolvedObject?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
return null;
{
if (resolvedObject["type"].Value<string>() == "string")
throw new ReturnAsErrorException($"String properties evaluation is not supported yet.", "ReferenceError"); // Issue #66823
if (!throwOnNullReference)
throw new ReturnAsErrorException($"Operation '?' not allowed on primitive type - '{parts[i - 1]}'", "ReferenceError");
throw new ReturnAsErrorException($"Cannot find member '{part}' on a primitive type", "ReferenceError");
}

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;
}

JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value<string>() == partTrimmed);
if (hasCurrentPartNullCondition)
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
part = part.Remove(part.Length - 1);
ilonatommy marked this conversation as resolved.
Show resolved Hide resolved
JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value<string>() == part);
if (objRet == null)
return null;

resolvedObject = await GetValueFromObject(objRet, token);
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;
}
throwOnNullReference = !hasCurrentPartNullCondition;
}

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 @@ -752,14 +752,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 @@ -507,13 +507,13 @@ public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAt

var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false );
Assert.Equal(
$"Method 'MyMethodWrong' not found in type 'DebuggerTests.EvaluateMethodTestsClass.ParmToTest'",
$"Method 'MyMethodWrong' not found in type 'DebuggerTests.EvaluateMethodTestsClass.ParmToTest'",
res.Error["result"]?["description"]?.Value<string>());

(_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false);
Assert.Equal(
"Unable to evaluate method 'MyMethod'. Too many arguments passed.",
res.Error["result"]?["description"]?.Value<string>());
Assert.Equal(
"Unable to evaluate method 'MyMethod'. Too many arguments passed.",
res.Error["result"]?["description"]?.Value<string>());

(_, res) = await EvaluateOnCallFrame(id, "this.CallMethodWithParm(\"1\")", expect_ok: false );
Assert.Contains("Unable to evaluate method 'this.CallMethodWithParm(\"1\")'", res.Error["message"]?.Value<string>());
Expand All @@ -523,7 +523,7 @@ public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAt

(_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false );
Assert.Contains(
"Cannot evaluate '(this.ParmToTestObjException.MyMethod()\n)'",
"Cannot evaluate '(this.ParmToTestObjException.MyMethod()\n)'",
res.Error["result"]?["description"]?.Value<string>());
});

Expand Down Expand Up @@ -1147,8 +1147,64 @@ 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)),
("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!.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)),
("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>();
await CheckEvaluateFail("list.Count.x", "Cannot find member 'x' on a primitive type");
await CheckEvaluateFail("listNull.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("listNull!.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tcNull.MemberListNull.Count", GetNullReferenceErrorOn("\"MemberListNull\""));
await CheckEvaluateFail("tc.MemberListNull.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tcNull?.MemberListNull.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("listNull?.Count.NonExistingProperty", GetNullReferenceErrorOn("\"NonExistingProperty\""));
await CheckEvaluateFail("tc?.MemberListNull! .Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tc?. MemberListNull!.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tc?.MemberListNull.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tc! .MemberListNull!.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tc!.MemberListNull. Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("tcNull?.Sibling.MemberListNull?.Count", GetNullReferenceErrorOn("\"MemberListNull?\""));
await CheckEvaluateFail("listNull?", "Expected expression.");
await CheckEvaluateFail("listNull!.Count", GetNullReferenceErrorOn("\"Count\""));
await CheckEvaluateFail("x?.p", "Operation '?' not allowed on primitive type - 'x?'");

string GetNullReferenceErrorOn(string name) => $"Expression threw NullReferenceException trying to access {name} on a null-valued object.";

async Task CheckEvaluateFail(string expr, string message)
{
(_, Result _res) = await EvaluateOnCallFrame(id, expr, expect_ok: false);
AssertEqual(message, _res.Error["result"]?["description"]?.Value<string>(), $"Expression '{expr}' - wrong error message");
}
});

[Fact]
public async Task EvaluateMethodsOnPrimitiveTypesReturningPrimitives() => await CheckInspectLocalsAtBreakpointSite(
public async Task EvaluateMethodsOnPrimitiveTypesReturningPrimitives() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.PrimitiveTypeMethods", "Evaluate", 11, "Evaluate",
"window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.PrimitiveTypeMethods:Evaluate'); })",
wait_for_event_fn: async (pause_location) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,7 @@ public static void Evaluate()
var test = new TestClass();
}
}

public static class PrimitiveTypeMethods
{
public class TestClass
Expand Down Expand Up @@ -1401,6 +1401,30 @@ 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;
public List<int> MemberList = new List<int>() {1, 2};
public TestClass Sibling { get; set; }
}
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;
string str = "str#value";
string str_null = null;
int x = 5;
int? x_null = null;
int? x_val = x;
}
}
}

namespace DebuggerTestsV2
Expand Down