fix #27631, get/set methods no longer shadow index operator

pkg/dev_compiler/lib/src/compiler/code_generator.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:collection' show HashMap, HashSet;
 import 'dart:math' show min, max;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/ast.dart';
@@ skipping 4463 matching lines @@
 
     if (op.lexeme == '++' || op.lexeme == '--') {
       // Increment or decrement requires expansion.
       // Desugar `++x` as `x = x + 1`, ensuring that if `x` has subexpressions
       // (for example, x is IndexExpression) we evaluate those once.
       var one = AstBuilder.integerLiteral(1)..staticType = types.intType;
       return _emitOpAssign(expr, one, op.lexeme[0], node.staticElement,
           context: expr);
     }
 
-    return _emitSend(expr, op.lexeme[0], []);
+    var operatorName = op.lexeme;
+    if (operatorName == '-') operatorName = 'unary-';

[Bob Nystrom, 2016/10/20 21:01:13]

Is "unary-" just used as a magic string for _emitMemberName()? Can you just use
"_negate" here instead?

[Jennifer Messerly, 2016/10/20 22:45:04]

it's not. It's defined by the Dart language spec and used by Analyzer.

+    return _emitSend(expr, operatorName, []);
   }
 
   // Cascades can contain [IndexExpression], [MethodInvocation] and
   // [PropertyAccess]. The code generation for those is handled in their
   // respective visit methods.
   @override
   JS.Node visitCascadeExpression(CascadeExpression node) {
     var savedCascadeTemp = _cascadeTarget;
 
     var vars = <JS.MetaLetVariable, JS.Expression>{};
@@ skipping 212 matching lines @@
   }
 
   /// Emits a generic send, like an operator method.
   ///
   /// **Please note** this function does not support method invocation syntax
   /// `obj.name(args)` because that could be a getter followed by a call.
   /// See [visitMethodInvocation].
   JS.Expression _emitSend(
       Expression target, String name, List<Expression> args) {
     var type = getStaticType(target);
-    var memberName = _emitMemberName(name, unary: args.isEmpty, type: type);
+    var memberName = _emitMemberName(name, type: type);
     if (isDynamicInvoke(target)) {
       if (_inWhitelistCode(target)) {
         var vars = <JS.MetaLetVariable, JS.Expression>{};
         var l = _visit(_bindValue(vars, 'l', target));
         return new JS.MetaLet(vars, [
           js.call('(#[(#[dart._extensionType]) ? dartx[#] : #]).call(#, #)',
               [l, l, memberName, memberName, l, _visitList(args)])
         ]);
       }
       // dynamic dispatch
@@ skipping 491 matching lines @@
   /// directly rather than computing the relevant options for [_emitMemberName].
   JS.Expression _elementMemberName(ExecutableElement e, {bool useExtension}) {
     String name;
     if (e is PropertyAccessorElement) {
       name = e.variable.name;
     } else {
       name = e.name;
     }
     return _emitMemberName(name,
         type: (e.enclosingElement as ClassElement).type,
-        unary: e.parameters.isEmpty,
         isStatic: e.isStatic,
         useExtension: useExtension);
   }
 
   /// This handles member renaming for private names and operators.
   ///
   /// Private names are generated using ES6 symbols:
   ///
   ///     // At the top of the module:
   ///     let _x = Symbol('_x');
@@ skipping 19 matching lines @@
   ///
   /// There are three exceptions: [], []= and unary -.
   /// The indexing operators we use `get` and `set` instead:
   ///
   ///     x.get('hi')
   ///     x.set('hi', 123)
   ///
   /// This follows the same pattern as ECMAScript 6 Map:
   /// <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map>
   ///
-  /// Unary minus looks like: `x['unary-']()`. Note that [unary] must be passed
+  /// Unary minus looks like: `x._negate()`. Note that [unary] must be passed

[Bob Nystrom, 2016/10/20 21:01:13]

Can you update the part of the doc around "[unary]" since that parameter is gone
now?

[Jennifer Messerly, 2016/10/20 22:45:04]

Done.

   /// for this transformation to happen, otherwise binary minus is assumed.
   ///
   /// Equality is a bit special, it is generated via the Dart `equals` runtime
   /// helper, that checks for null. The user defined method is called '=='.
   ///
   JS.Expression _emitMemberName(String name,
-      {DartType type,
-      bool unary: false,
-      bool isStatic: false,
-      bool useExtension}) {
+      {DartType type, bool isStatic: false, bool useExtension}) {
     // Static members skip the rename steps.
     if (isStatic) return _propertyName(name);
 
     if (name.startsWith('_')) {
       return _emitPrivateNameSymbol(currentLibrary, name);
     }
 
+    // When generating synthetic names, we use _ as the prefix, since Dart names
+    // won't have this (eliminated above), nor will static names reach here.
     if (name == '[]') {

[kevmoo, 2016/10/20 20:06:03]

better as a switch, no?

[Jennifer Messerly, 2016/10/20 20:12:14]

I tend to find Dart's C-style switch clunky, e.g. if any condition needs to
change we'd have to edit it all back. Avoiding churn is a worthwhile goal as
well IMO. I don't think the Dart style team has much advice on this either way.

Personally, I hope we get pattern matching syntax in Dart.

Anyway happy to let Bob be the tiebreaker :)

[Bob Nystrom, 2016/10/20 21:01:13]

If it was my code, I'd probably use switch () to highlight that each condition
is just doing a strict comparison with a string literal. But I also care less
about minimizing local code churn—I prefer a codebase to be an ideal
implementation of what it currently does and less so a speculation of what we
may want it to do in the future.

But I don't have a strong opinion either way, so I'll let you do what you like.

-      name = 'get';
+      name = '_get';
     } else if (name == '[]=') {
-      name = 'set';
-    } else if (name == '-' && unary) {
-      name = 'unary-';
+      name = '_set';
+    } else if (name == 'unary-') {
+      name = '_negate';
     } else if (name == 'constructor' || name == 'prototype') {
-      // This uses an illegal (in Dart) character for a member, avoiding the
-      // conflict. We could use practically any character for this.
-      name = '+$name';
+      name = '_$name';
     }
 
     var result = _propertyName(name);
 
     if (useExtension == null) {
       // Dart "extension" methods. Used for JS Array, Boolean, Number, String.
       var baseType = type;
       while (baseType is TypeParameterType) {
         baseType = (baseType.element as TypeParameterElement).bound;
       }
@@ skipping 212 matching lines @@
 }
 
 bool isLibraryPrefix(Expression node) =>
     node is SimpleIdentifier && node.staticElement is PrefixElement;
 
 LibraryElement _getLibrary(AnalysisContext c, String uri) =>
     c.computeLibraryElement(c.sourceFactory.forUri(uri));
 
 bool _isDartRuntime(LibraryElement l) =>
     l.isInSdk && l.source.uri.toString() == 'dart:_runtime';