Skip to content

Commit 998b100

Browse files
vicbvicb
committed
[WIP] feat: Removing implicit map dereference in expressions
Closes dart-archive#915 Before this PR a.b would have match either a.b or a['b']. The reason is that the context was a Map (before dart-archive#914) and we don't want to write ctrl['foo'] but ctrl.foo. It was also not possible to access Map properties, ie map.length would have returned map['length']. Now "a.b" return a.b and "a['b']" return a['b']
1 parent e3d137f commit 998b100

16 files changed

+276
-222
lines changed

‎lib/change_detection/dirty_checking_change_detector.dart

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -460,22 +460,17 @@ class DirtyCheckingRecord<H> implements WatchRecord<H> {
460460
return;
461461
}
462462

463-
if (object is Map) {
464-
_mode = _MODE_MAP_FIELD_;
465-
_getter = null;
466-
} else {
467-
while (object is ContextLocals) {
468-
var ctx = object as ContextLocals;
469-
if (ctx.hasProperty(field)) {
470-
_mode = _MODE_MAP_FIELD_;
471-
_getter = null;
472-
return;
473-
}
474-
object = ctx.parentContext;
463+
while (object is ContextLocals) {
464+
var ctx = object as ContextLocals;
465+
if (ctx.hasProperty(field)) {
466+
_mode = _MODE_MAP_FIELD_;
467+
_getter = null;
468+
return;
475469
}
476-
_mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
477-
_getter = _fieldGetterFactory.getter(object, field);
470+
object = ctx.parentContext;
478471
}
472+
_mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
473+
_getter = _fieldGetterFactory.getter(object, field);
479474
}
480475

481476
bool check() {

‎lib/core/parser/eval_access.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ abstract class AccessFast {
4747

4848
dynamic _eval(holder) {
4949
if (holder == null) return null;
50-
if (holder is Map) return holder[name];
5150
return getter(holder);
5251
}
5352

@@ -56,7 +55,6 @@ abstract class AccessFast {
5655
_assignToNonExisting(scope, value);
5756
return value;
5857
} else {
59-
if (holder is Map) return holder[name] = value;
6058
return setter(holder, value);
6159
}
6260
}

‎lib/core/parser/parser_dynamic.dart

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,14 @@ class DynamicClosureMap implements ClosureMap {
88
final Map<String, Symbol> symbols = {};
99
Getter lookupGetter(String name) {
1010
var symbol = new Symbol(name);
11-
return (o) {
12-
if (o is Map) {
13-
return o[name];
14-
} else {
15-
return reflect(o).getField(symbol).reflectee;
16-
}
17-
};
11+
return (o) => reflect(o).getField(symbol).reflectee;
1812
}
1913

2014
Setter lookupSetter(String name) {
2115
var symbol = new Symbol(name);
2216
return (o, value) {
23-
if (o is Map) {
24-
return o[name] = value;
25-
} else {
26-
reflect(o).setField(symbol, value);
27-
return value;
28-
}
17+
reflect(o).setField(symbol, value);
18+
return value;
2919
};
3020
}
3121

@@ -37,19 +27,10 @@ class DynamicClosureMap implements ClosureMap {
3727
var symbol = symbols.putIfAbsent(name, () => new Symbol(name));
3828
sNamedArgs[symbol] = value;
3929
});
40-
if (o is Map) {
41-
var fn = o[name];
42-
if (fn is Function) {
43-
return Function.apply(fn, posArgs, sNamedArgs);
44-
} else {
45-
throw "Property '$name' is not of type function.";
46-
}
47-
} else {
48-
try {
49-
return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
50-
} on NoSuchMethodError catch (e) {
51-
throw 'Undefined function $name';
52-
}
30+
try {
31+
return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
32+
} on NoSuchMethodError catch (e) {
33+
throw 'Undefined function $name';
5334
}
5435
};
5536
}

‎lib/core/parser/parser_static.dart

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,7 @@ class StaticClosureMap extends ClosureMap {
2727
return (o, posArgs, namedArgs) {
2828
var sNamedArgs = {};
2929
namedArgs.forEach((name, value) => sNamedArgs[symbols[name]] = value);
30-
if (o is Map) {
31-
var fn = o[name];
32-
if (fn is Function) {
33-
return Function.apply(fn, posArgs, sNamedArgs);
34-
} else {
35-
throw "Property '$name' is not of type function.";
36-
}
37-
} else {
38-
return Function.apply(fn(o), posArgs, sNamedArgs);
39-
}
30+
return Function.apply(fn(o), posArgs, sNamedArgs);
4031
};
4132
}
4233

‎lib/mock/module.dart

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import 'dart:async' as dart_async;
1717
import 'dart:collection' show ListBase;
1818
import 'dart:html';
1919
import 'dart:js' as js;
20+
import 'dart:mirrors';
2021

2122
import 'package:angular/angular.dart';
2223
import 'package:angular/core/module_internal.dart';
@@ -65,5 +66,33 @@ class AngularMockModule extends Module {
6566
..onError = (e, s, LongStackTrace ls) => dump('EXCEPTION: $e\n$s\n$ls');
6667
});
6768
bind(Window, toImplementation: MockWindow);
69+
bind(Object, toImplementation: TestContext);
70+
}
71+
}
72+
73+
class TestContext {
74+
Map _locals = {};
75+
76+
void operator []=(k, v) {
77+
_locals[k] = v;
78+
}
79+
80+
dynamic operator[](k) => _locals[k];
81+
82+
noSuchMethod(Invocation invocation) {
83+
var pArgs = invocation.positionalArguments;
84+
var field = MirrorSystem.getName(invocation.memberName);
85+
if (invocation.isGetter) {
86+
return _locals[field];
87+
}
88+
if (invocation.isSetter) {
89+
field = field.substring(0, field.length - 1);
90+
_locals[field] = pArgs[0];
91+
return;
92+
}
93+
if (invocation.isMethod) {
94+
return Function.apply(_locals[field], pArgs, invocation.namedArguments);
95+
}
96+
throw new UnimplementedError(field);
6897
}
6998
}

‎test/change_detection/dirty_checking_change_detector_spec.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
7676
});
7777

7878
it('should treat map field dereference as []', () {
79-
var obj = {'name':'misko'};
79+
var obj = new TestContext();
80+
obj.name = 'misko';
8081
detector.watch(obj, 'name', null);
8182
detector.collectChanges(); // throw away first set
8283

@@ -90,7 +91,7 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
9091

9192
describe('insertions / removals', () {
9293
it('should insert at the end of list', () {
93-
var obj = {};
94+
var obj = new TestContext();
9495
var a = detector.watch(obj, 'a', 'a');
9596
var b = detector.watch(obj, 'b', 'b');
9697

@@ -116,7 +117,7 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
116117
});
117118

118119
it('should remove all watches in group and group\'s children', () {
119-
var obj = {};
120+
var obj = new TestContext();
120121
detector.watch(obj, 'a', '0a');
121122
var child1a = detector.newGroup();
122123
var child1b = detector.newGroup();
@@ -138,11 +139,10 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
138139
});
139140

140141
it('should add watches within its own group', () {
141-
var obj = {};
142+
var obj = new TestContext();
142143
var ra = detector.watch(obj, 'a', 'a');
143144
var child = detector.newGroup();
144145
var cb = child.watch(obj,'b', 'b');
145-
var iterotar;
146146

147147
obj['a'] = obj['b'] = 1;
148148
expect(detector.collectChanges(), toEqualChanges(['a', 'b']));
@@ -723,6 +723,12 @@ void main() {
723723
"toString": (o) => o.toString,
724724
"isUnderAge": (o) => o.isUnderAge,
725725
"isUnderAgeAsVariable": (o) => o.isUnderAgeAsVariable,
726+
"a": (o) => o.a,
727+
"b": (o) => o.b,
728+
"name": (o) => o.name,
729+
"someField": (o) => o.someField,
730+
"f1": (o) => o.f1,
731+
"f2": (o) => o.f2,
726732
}));
727733
}
728734

‎test/change_detection/watch_group_spec.dart

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ void main() {
2323
ExpressionVisitor visitor;
2424

2525
beforeEach(inject((Logger _logger, Parser _parser) {
26-
context = {};
26+
context = new TestContext();
2727
var getterFactory = new DynamicFieldGetterFactory();
2828
changeDetector = new DirtyCheckingChangeDetector(getterFactory);
2929
watchGrp = new RootWatchGroup(getterFactory, changeDetector, context);
@@ -217,8 +217,8 @@ void main() {
217217
// should fire on initial adding
218218
expect(watchGrp.fieldCost).toEqual(0);
219219
expect(changeDetector.count).toEqual(0);
220-
var watch = watchGrp.watch(parse('a.b'), (v, p) => logger(v));
221-
expect(watch.expression).toEqual('a.b');
220+
var watch = watchGrp.watch(parse('a["b"]'), (v, p) => logger(v));
221+
expect(watch.expression).toEqual('[](a, "b")');
222222
expect(watchGrp.fieldCost).toEqual(2);
223223
expect(changeDetector.count).toEqual(2);
224224
watchGrp.detectChanges();
@@ -260,8 +260,8 @@ void main() {
260260
// should fire on initial adding
261261
expect(watchGrp.fieldCost).toEqual(0);
262262
var watch = watchGrp.watch(parse('user'), (v, p) => logger(v));
263-
var watchFirst = watchGrp.watch(parse('user.first'), (v, p) => logger(v));
264-
var watchLast = watchGrp.watch(parse('user.last'), (v, p) => logger(v));
263+
var watchFirst = watchGrp.watch(parse('user["first"]'), (v, p) => logger(v));
264+
var watchLast = watchGrp.watch(parse('user["last"]'), (v, p) => logger(v));
265265
expect(watchGrp.fieldCost).toEqual(3);
266266

267267
watchGrp.detectChanges();
@@ -290,7 +290,7 @@ void main() {
290290

291291
FunctionApply fn = new LoggingFunctionApply(logger);
292292
var watch = watchGrp.watch(
293-
new PureFunctionAST('add', fn, [parse('a.val')]),
293+
new PureFunctionAST('add', fn, [parse('a["val"]')]),
294294
(v, p) => logger(v)
295295
);
296296

@@ -316,7 +316,7 @@ void main() {
316316
var watch = watchGrp.watch(
317317
new PureFunctionAST('add',
318318
(a, b) { logger('+'); return a+b; },
319-
[parse('a.val'), parse('b.val')]
319+
[parse('a["val"]'), parse('b["val"]')]
320320
),
321321
(v, p) => logger(v)
322322
);
@@ -361,7 +361,7 @@ void main() {
361361
var watch = watchGrp.watch(
362362
new ClosureAST('sum',
363363
(a, b) { logger('+'); return innerState+a+b; },
364-
[parse('a.val'), parse('b.val')]
364+
[parse('a["val"]'), parse('b["val"]')]
365365
),
366366
(v, p) => logger(v)
367367
);
@@ -414,11 +414,11 @@ void main() {
414414

415415
var a_plus_b = new PureFunctionAST('add1',
416416
(a, b) { logger('$a+$b'); return a + b; },
417-
[parse('a.val'), parse('b.val')]);
417+
[parse('a["val"]'), parse('b["val"]')]);
418418

419419
var a_plus_b_plus_c = new PureFunctionAST('add2',
420420
(b, c) { logger('$b+$c'); return b + c; },
421-
[a_plus_b, parse('c.val')]);
421+
[a_plus_b, parse('c["val"]')]);
422422

423423
var watch = watchGrp.watch(a_plus_b_plus_c, (v, p) => logger(v));
424424

@@ -762,8 +762,8 @@ void main() {
762762
});
763763

764764
it('should support custom context', () {
765-
expect(eval('x', {'x': 42})).toBe(42);
766-
expect(eval('x', {'x': 87})).toBe(87);
765+
expect(eval('x', new Context(42))).toBe(42);
766+
expect(eval('x', new Context(87))).toBe(87);
767767
});
768768

769769
it('should support named arguments for scope calls', () {
@@ -857,10 +857,10 @@ void main() {
857857
watchGrp.watch(countMethod, (v, p) => logger('0a'));
858858
expectOrder(['0a']);
859859

860-
var child1a = watchGrp.newGroup({'my': myClass});
861-
var child1b = watchGrp.newGroup({'my': myClass});
862-
var child2 = child1a.newGroup({'my': myClass});
863-
var child3 = child2.newGroup({'my': myClass});
860+
var child1a = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
861+
var child1b = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
862+
var child2 = child1a.newGroup(new ContextLocals(context, {'my': myClass}));
863+
var child3 = child2.newGroup(new ContextLocals(context, {'my': myClass}));
864864
child1a.watch(countMethod, (v, p) => logger('1a'));
865865
expectOrder(['0a', '1a']);
866866
child1b.watch(countMethod, (v, p) => logger('1b'));
@@ -888,7 +888,7 @@ void main() {
888888
context['my'] = myClass;
889889
AST countMethod = new MethodAST(parse('my'), 'count', []);
890890
var ra = watchGrp.watch(countMethod, (v, p) => logger('a'));
891-
var child = watchGrp.newGroup({'my': myClass});
891+
var child = watchGrp.newGroup(new ContextLocals(context, {'my': myClass}));
892892
var cb = child.watch(countMethod, (v, p) => logger('b'));
893893

894894
expectOrder(['a', 'b']);
@@ -975,3 +975,8 @@ class LoggingFunctionApply extends FunctionApply {
975975
LoggingFunctionApply(this.logger);
976976
apply(List args) => logger(args);
977977
}
978+
979+
class Context {
980+
var x;
981+
Context(this.x);
982+
}

0 commit comments

Comments
 (0)