use VariableElement instead of VariableElementImpl in _isStateless

lib/src/codegen/js_codegen.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.
 
 library dev_compiler.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet, HashMap;
 import 'dart:io' show Directory, File;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
@@ skipping 1673 matching lines @@
       ]);
     }
 
     _cascadeTarget = savedCascadeTemp;
     return result;
   }
 
   /// True is the expression can be evaluated multiple times without causing
   /// code execution. This is true for final fields. This can be true for local
   /// variables, if:
-  /// * they are not assigned within the [context].
+  ///
+  /// * they are not assigned within the [context] scope.
   /// * they are not assigned in a function closure anywhere.
+  ///
+  /// This method is used to avoid creating temporaries in cases where we know
+  /// we can safely re-evaluate [node] multiple times in [context]. This lets
+  /// us generate prettier code.
+  ///
+  /// This method is conservative: it should never return `true` unless it is
+  /// certain the [node] is stateless, because generated code may rely on the
+  /// correctness of a `true` value. However it may return `false` for things
+  /// that are in fact, stateless.
   bool _isStateless(Expression node, [AstNode context]) {
     if (node is SimpleIdentifier) {
       var e = node.staticElement;
       if (e is PropertyAccessorElement) e = e.variable;
-      if (e is VariableElementImpl && !e.isSynthetic) {
+      if (e is VariableElement && !e.isSynthetic) {
         if (e.isFinal) return true;
+
+        // TODO(jmesserly): remove this when isPotentiallyMutated* is available
+        // without the implementation class. Technically we shouldn't hit the
+        // ParameterMember case based on current usage of _isStateless, but this
+        // makes it clear we shouldn't rely on *Impl class.
+        if (e is Member) e = e.baseElement;
+
         if (e is LocalVariableElementImpl || e is ParameterElementImpl) {
           // make sure the local isn't mutated in the context.
           return !_isPotentiallyMutated(e, context);
         }
       }
     }
     return false;
   }
 
   @override
@@ skipping 470 matching lines @@
     return new JS.Identifier(jsLibraryName(library));
   }
 
   static bool _needsImplicitThis(Element e) =>
       e is PropertyAccessorElement && !e.variable.isStatic ||
           e is ClassMemberElement && !e.isStatic && e is! ConstructorElement;
 }
 
 /// Returns true if the local variable is potentially mutated within [context].
 /// This accounts for closures that may have been created outside of [context].
+// TODO(jmesserly): change type annotation to not be *Impl once
+// isPotentiallyMutated is available on VariableElement.

[Brian Wilkerson, 2015/04/06 16:03:23]

I think this support was just published as 0.24.1.

 bool _isPotentiallyMutated(VariableElementImpl e, [AstNode context]) {
   if (e.isPotentiallyMutatedInClosure) {
     // TODO(jmesserly): this returns true incorrectly in some cases, because
     // VariableResolverVisitor only checks that enclosingElement is not the
     // function element, but enclosingElement can be something else in some
     // cases (the block scope?). So it's more conservative than it could be.
     return true;
   }
   if (e.isPotentiallyMutatedInScope) {
     // Need to visit the context looking for assignment to this local.
@@ skipping 159 matching lines @@
 
     // TODO(jmesserly): in many cases marking the end will be unncessary.
     printer.mark(_location(node.end));
   }
 
   String _getIdentifier(AstNode node) {
     if (node is SimpleIdentifier) return node.name;
     return null;
   }
 }