PropertyPath in observe v0.11.0 still doesn't support Map indexers

[DartBot]

Issue by jolleekin Originally opened as dart-lang/sdk#20294

---

1. PropertyPath doesn't support Map indexers of type string

    import 'package:observe/observe.dart';

    main() {       var p = new PropertyPath('resources["commands"]["+add"]');

      // Output: resources.commands["+add"]       // Expect: resources["commands"]["+add"]       print(p);     }

"commands" is actually converted into a symbol while "+add" is left as is. Ideally, indexers should be left as is whether they are Map indexers or List indexers.

1. PropertyPath doesn't support Map indexers of type int

The following code only checks for integer List indexers and forgets about integer Map indexers.

    // path_observe.dart     _getObjectProperty(object, property) {       if (object == null) return null;

      if (property is int) {         if (object is List && property >= 0 && property < object.length) {           return object[property];         }       } else if (property is String) {         return object[property];       } else if (property is Symbol) {         ...       }       ...     }

What version of the product are you using? observe 0.11.0

On what operating system?

What browser (if applicable)?

Please provide any additional information below.

[DartBot]

Comment by zoechi

---

is this related to https://code.google.com/p/dart/issues/detail?id=20286 ?

[DartBot]

Comment by jolleekin

---

I don't think so. Polymer doesn't use PropertyPath.

[DartBot]

Comment by sigmundch

---

Thanks for the detailed bug description. This makes sense now.

Re (1): that is also how the JS implementation is written. We have to follow up with them to make sure we are consistent.

---

Removed Priority-Unassigned label. Added Priority-Medium, Pkg-Observe, Area-Pkg, PolymerMilestone-Next, Triaged labels.

[DartBot]

Comment by jolleekin

---

I opened a new issue for the JS implementation.

https://github.com/Polymer/observe-js/issues/64

[DartBot]

Comment by jmesserly

---

i think this is fixed, https://github.com/dart-lang/observe/blob/master/lib/src/path_observer.dart#L116

if not it's now possible to pass in a list to PropertyPath

['foo', 'bar', '+baz']

---

Added AssumedStale label.

[DartBot]

Comment by jolleekin

---

It's not fixed though. Take a look at [_setObjectProperty]. This method doesn't check for strings. If you pass a list that contains strings and Symbols, [_setObjectProperty] will fail.

  bool _setObjectProperty(object, property, value) {     if (object == null) return false;

    if (property is int) {       if (object is List && property >= 0 && property < object.length) {         object[property] = value;         return true;       }     } else if (property is Symbol) {       // Support indexer if available, e.g. Maps or polymer_expressions Scope.       if (object is Indexable || object is Map && !_MAP_PROPERTIES.contains(property)) {         object[smoke.symbolToName(property)] = value;         return true;       }       try {         smoke.write(object, property, value);         return true;       } on NoSuchMethodError catch (e, s) {         if (!smoke.hasNoSuchMethod(object.runtimeType)) rethrow;       }     }

    if (_logger.isLoggable(Level.FINER)) {       _logger.finer("can't set $property in $object");     }     return false;   }

To fix the problem, [_setObjectProperty] needs to handle strings as follows.

  bool _setObjectProperty(object, property, value) {     if (object == null) return false;

    if (property is int) {       if (object is List && property >= 0 && property < object.length) {         object[property] = value;         return true;       }     } else if (property is String) {       if (object is Indexable || object is Map && !_MAP_PROPERTIES.contains(property)) {         object[property] = value;         return true;       }     } else if (property is Symbol) {       try {         smoke.write(object, property, value);         return true;       } on NoSuchMethodError catch (e, s) {         if (!smoke.hasNoSuchMethod(object.runtimeType)) rethrow;       }     }

    if (_logger.isLoggable(Level.FINER)) {       _logger.finer("can't set $property in $object");     }     return false;   }