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.

Sign up for GitHub

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

Eval Console Autocomplete #3013

Merged
merged 6 commits into from
May 13, 2021
Merged

Eval Console Autocomplete #3013

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df1186e
Eval Autocomplete
grouma May 13, 2021
39b8049
Merge branch 'master' into eval-auto-complete
grouma May 13, 2021
510a19e
add tests
grouma May 13, 2021
2c8d42b
handle ops
grouma May 13, 2021
380e662
review feedback
grouma May 13, 2021
12d8e6e
Merge branch 'master' into eval-auto-complete
grouma May 13, 2021
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
134 changes: 67 additions & 67 deletions packages/devtools_app/lib/src/debugger/evaluate.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import '../theme.dart';
import '../ui/search.dart';
import 'debugger_controller.dart';

// TODO(devoncarew): We should insert eval result objects into the console as
// expandable objects.

class ExpressionEvalField extends StatefulWidget {
const ExpressionEvalField({
this.controller,
Expand Down Expand Up @@ -91,7 +88,7 @@ class _ExpressionEvalFieldState extends State<ExpressionEvalField>
// Only show pop-up if there's a real variable name or field.
if (parts.activeWord.isEmpty && !parts.isField) return;

final matches = await _matchesFor(parts);
final matches = await autoCompleteResultsFor(parts, widget.controller);

final normalizedMatches = matches.toSet().toList()..sort();
_autoCompleteController.searchAutoComplete.value =
Expand All @@ -104,69 +101,6 @@ class _ExpressionEvalFieldState extends State<ExpressionEvalField>
}
}

Future<List<String>> _matchesFor(EditingParts parts) async {
final result = <String>{};
if (!parts.isField) {
for (var variable in widget.controller.variables.value) {
if (variable.boundVar.name.startsWith(parts.activeWord)) {
result.add(variable.boundVar.name);
}
}
} else {
try {
var left = parts.leftSide.split(' ').last;
// Removing trailing `.`.
left = left.substring(0, left.length - 1);

// Response is either a ErrorRef, InstanceRef, or Sentinel.
final response = await widget.controller.evalAtCurrentFrame(left);

// Display the response to the user.
if (response is InstanceRef) {
final Instance instance = await widget.controller.getObject(response);
result.addAll(await _autoCompleteProperties(instance.classRef));
// TODO(grouma) - This shouldn't be necessary but package:dwds does
// not properly provide superclass information.
result.addAll(instance.fields.map((field) => field.decl.name));
result.removeWhere((prop) => !prop.startsWith(parts.activeWord));
}
} catch (_) {}
}
return result.toList();
}

Future<List<String>> _autoCompleteProperties(ClassRef classRef) async {
final result = <String>[];
if (classRef != null) {
final Class clazz = await widget.controller.getObject(classRef);
result.addAll(clazz.fields.map((field) => field.name));
result.addAll(clazz.functions
.where((funcRef) => _validFunction(funcRef, clazz))
// The VM shows setters as `<member>=`.
.map((funcRef) => funcRef.name.replaceAll('=', '')));
result.addAll(await _autoCompleteProperties(clazz.superClass));
result.removeWhere((member) => !_isAccessible(member, clazz));
}
return result;
}

bool _validFunction(FuncRef funcRef, Class clazz) {
return !funcRef.isStatic &&
!_isContructor(funcRef, clazz) &&
funcRef.name != '==';
}

bool _isContructor(FuncRef funcRef, Class clazz) =>
funcRef.name == clazz.name || funcRef.name.startsWith('${clazz.name}.');

bool _isAccessible(String member, Class clazz) {
final frame = widget.controller.selectedStackFrame.value?.frame ??
widget.controller.stackFramesWithLocation.value.first.frame;
final currentScript = frame.location.script;
return !(member.startsWith('_') &&
currentScript.id != clazz.location?.script?.id);
}

@override
Widget build(BuildContext context) {
final theme = Theme.of(context);
Expand Down Expand Up @@ -349,3 +283,69 @@ class _ExpressionEvalFieldState extends State<ExpressionEvalField>
});
}
}

Future<List<String>> autoCompleteResultsFor(
EditingParts parts,
DebuggerController controller,
) async {
final result = <String>{};
if (!parts.isField) {
result.addAll(controller.variables.value
.map((variable) => variable.boundVar.name)
.where((name) => name.startsWith(parts.activeWord)));
} else {
try {
var left = parts.leftSide.split(' ').last;
// Removing trailing `.`.
left = left.substring(0, left.length - 1);
final response = await controller.evalAtCurrentFrame(left);
if (response is InstanceRef) {
final Instance instance = await controller.getObject(response);
result.addAll(
await _autoCompleteMembersFor(
instance.classRef,
controller,
),
);
// TODO(grouma) - This shouldn't be necessary but package:dwds does
// not properly provide superclass information.
result.addAll(instance.fields.map((field) => field.decl.name));
result.removeWhere((prop) => !prop.startsWith(parts.activeWord));
}
} catch (_) {}
}
return result.toList();
}

Future<List<String>> _autoCompleteMembersFor(
ClassRef classRef, DebuggerController controller) async {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: trailing comma

final result = <String>[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make this return a Set as well as you are currently adding dupes.

if (classRef != null) {
final Class clazz = await controller.getObject(classRef);
result.addAll(clazz.fields.map((field) => field.name));
result.addAll(clazz.functions
.where((funcRef) => _validFunction(funcRef, clazz))
// The VM shows setters as `<member>=`.
.map((funcRef) => funcRef.name.replaceAll('=', '')));
result.addAll(await _autoCompleteMembersFor(clazz.superClass, controller));
result.removeWhere((member) => !_isAccessible(member, clazz, controller));
}
return result;
}

bool _validFunction(FuncRef funcRef, Class clazz) {
return !funcRef.isStatic &&
!_isContructor(funcRef, clazz) &&
funcRef.name != '==';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to detect other operators or you will suggest them as invalid autocompletes because the syntax is
foo+bar not foo.+bar

}

bool _isContructor(FuncRef funcRef, Class clazz) =>
funcRef.name == clazz.name || funcRef.name.startsWith('${clazz.name}.');

bool _isAccessible(String member, Class clazz, DebuggerController controller) {
final frame = controller.selectedStackFrame.value?.frame ??
controller.stackFramesWithLocation.value.first.frame;
final currentScript = frame.location.script;
return !(member.startsWith('_') &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think
!member.startsWith('_') || currentScript.id == clazz.location?.script?.id
is a little easier to read.

currentScript.id != clazz.location?.script?.id);
}
191 changes: 191 additions & 0 deletions packages/devtools_app/test/evaluate_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we rename this file to debugger_evalution_test.dart so it is clear that this test is testing functionality in the debugger

// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:devtools_app/src/debugger/debugger_model.dart';
import 'package:devtools_app/src/debugger/evaluate.dart';
import 'package:devtools_app/src/ui/search.dart';
import 'package:flutter/widgets.dart';
import 'package:mockito/mockito.dart';
import 'package:test/test.dart';
import 'package:vm_service/vm_service.dart';

import 'support/mocks.dart';

void main() {
MockDebuggerController debuggerController;

setUp(() async {
debuggerController = MockDebuggerController();

when(debuggerController.selectedStackFrame).thenReturn(
ValueNotifier(
StackFrameAndSourcePosition(
Frame(
index: 0,
location: SourceLocation(
script: ScriptRef(id: 'some-script-loc', uri: ''), tokenPos: 0),
),
),
),
);
when(debuggerController.variables).thenReturn(
ValueNotifier(
[
Variable.create(
BoundVariable(
name: 'foo',
value: null,
declarationTokenPos: 0,
scopeStartTokenPos: 0,
scopeEndTokenPos: 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing comma for all these BoundVariable, FieldRef, Class, FuncRef, etc. instances

),
Variable.create(
BoundVariable(
name: 'foobar',
value: null,
declarationTokenPos: 0,
scopeStartTokenPos: 0,
scopeEndTokenPos: 0),
),
Variable.create(
BoundVariable(
name: 'bar',
value: null,
declarationTokenPos: 0,
scopeStartTokenPos: 0,
scopeEndTokenPos: 0),
),
Variable.create(
BoundVariable(
name: 'baz',
value: null,
declarationTokenPos: 0,
scopeStartTokenPos: 0,
scopeEndTokenPos: 0),
),
],
),
);
when(debuggerController.evalAtCurrentFrame(any)).thenAnswer(
(_) async =>
InstanceRef(kind: '', identityHashCode: 0, classRef: null, id: ''),
);
when(debuggerController.getObject(any)).thenAnswer((inv) async {
final obj = inv.positionalArguments.first;
if (obj is ClassRef) {
return Class(
fields: [
FieldRef(
name: 'field1',
owner: null,
declaredType: null,
isConst: false,
isFinal: false,
isStatic: false,
id: ''),
FieldRef(
name: 'field2',
owner: null,
declaredType: null,
isConst: false,
isFinal: false,
isStatic: false,
id: ''),
],
functions: [
FuncRef(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a couple constructors to make sure they are filtered out.

name: 'func1',
owner: null,
isStatic: false,
isConst: false,
id: '',
),
FuncRef(
name: 'func2',
owner: null,
isStatic: false,
isConst: false,
id: '',
),
FuncRef(
name: 'funcStatic',
owner: null,
isStatic: true,
isConst: false,
id: '',
),
],
id: '',
interfaces: [],
isAbstract: null,
isConst: null,
library: null,
name: 'FooClass',
subclasses: [],
traceAllocations: null);
}
if (obj is InstanceRef) {
return Instance(
classRef: ClassRef(id: '', name: 'FooClass'),
id: '',
fields: [
BoundField(
decl: FieldRef(
name: 'fieldBound1',
owner: null,
declaredType: null,
isConst: false,
isFinal: false,
isStatic: false,
id: ''),
value: null),
],
identityHashCode: null,
kind: '');
}
return null;
});
});
test('returns scoped variables when EditingParts is not a field', () async {
expect(
await autoCompleteResultsFor(
EditingParts(
activeWord: 'foo',
leftSide: '',
rightSide: '',
),
debuggerController),
equals(['foo', 'foobar']));
expect(
await autoCompleteResultsFor(
EditingParts(
activeWord: 'b',
leftSide: '',
rightSide: '',
),
debuggerController),
equals(['bar', 'baz']));
});

test('returns filtered members when EditingParts is a field ', () async {
expect(
await autoCompleteResultsFor(
EditingParts(
activeWord: 'f',
leftSide: 'foo.',
rightSide: '',
),
debuggerController),
equals(['field1', 'field2', 'func1', 'func2', 'fieldBound1']));
expect(
await autoCompleteResultsFor(
EditingParts(
activeWord: 'fu',
leftSide: 'foo.',
rightSide: '',
),
debuggerController),
equals(['func1', 'func2']));
});
}
5 changes: 0 additions & 5 deletions packages/devtools_app/test/support/mocks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import 'package:devtools_app/src/vm_service_wrapper.dart';
import 'package:devtools_shared/devtools_shared.dart';
import 'package:devtools_testing/support/cpu_profile_test_data.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart' as flutter;
import 'package:mockito/mockito.dart';
import 'package:vm_service/vm_service.dart';

Expand Down Expand Up @@ -537,10 +536,6 @@ class MockBannerMessagesController extends Mock
implements BannerMessagesController {}

class MockLoggingController extends Mock implements LoggingController {
@override
flutter.TextEditingValue searchTextFieldValue =
const flutter.TextEditingValue();

@override
ValueListenable<LogData> get selectedLog => _selectedLog;

Expand Down