From 396d7862b4e3c4bf5dfa87927301d884503b0557 Mon Sep 17 00:00:00 2001
From: Paul Berry <paulberry@google.com>
Date: Fri, 21 Oct 2016 11:13:55 -0700
Subject: [PATCH] More improvements to DeclarationResolver.

This CL makes the following improvements:

- When visiting an executable element, local variables and labels are
  rebuilt rather than matching them up to the old element model.  This
  ensures that if the element model was resynthesized from an API
  summary (which doesn't contain local variables or labels), these
  elements are not lost.

- When matching up existing elements, if the elements did not
  previously contain offsets, offsets are recorded.  If they did
  previously contain offsets, the offsets are compared to verify that
  they match.  This ensures that if the element model was
  resynthesized from an API summary (which doesn't contain offsets),
  the offsets will be correct.

R=brianwilkerson@google.com, scheglov@google.com

Review URL: https://codereview.chromium.org/2435313002 .
---
 .../lib/src/dart/element/builder.dart         |  51 ++++----
 .../lib/src/dart/element/element.dart         |  62 +++++----
 .../src/generated/declaration_resolver.dart   | 121 +++++++++++-------
 .../generated/declaration_resolver_test.dart  |  77 ++++++++++-
 .../test/src/context/context_test.dart        |   6 +-
 5 files changed, 215 insertions(+), 102 deletions(-)

diff --git a/pkg/analyzer/lib/src/dart/element/builder.dart b/pkg/analyzer/lib/src/dart/element/builder.dart
index b45f44e9742..b421e75f13d 100644
--- a/pkg/analyzer/lib/src/dart/element/builder.dart
+++ b/pkg/analyzer/lib/src/dart/element/builder.dart
@@ -1078,8 +1078,11 @@ class LocalElementBuilder extends _BaseElementBuilder {
       CompilationUnitElementImpl compilationUnitElement)
       : super(initialHolder, compilationUnitElement);
 
-  @override
-  Object visitCatchClause(CatchClause node) {
+  /**
+   * Builds the variable elements associated with [node] and stores them in
+   * the element holder.
+   */
+  void buildCatchVariableElements(CatchClause node) {
     SimpleIdentifier exceptionParameter = node.exceptionParameter;
     if (exceptionParameter != null) {
       // exception
@@ -1102,6 +1105,26 @@ class LocalElementBuilder extends _BaseElementBuilder {
         stackTraceParameter.staticElement = stackTrace;
       }
     }
+  }
+
+  /**
+   * Builds the label elements associated with [labels] and stores them in the
+   * element holder.
+   */
+  void buildLabelElements(
+      NodeList<Label> labels, bool onSwitchStatement, bool onSwitchMember) {
+    for (Label label in labels) {
+      SimpleIdentifier labelName = label.label;
+      LabelElementImpl element = new LabelElementImpl.forNode(
+          labelName, onSwitchStatement, onSwitchMember);
+      labelName.staticElement = element;
+      _currentHolder.addLabel(element);
+    }
+  }
+
+  @override
+  Object visitCatchClause(CatchClause node) {
+    buildCatchVariableElements(node);
     return super.visitCatchClause(node);
   }
 
@@ -1226,37 +1249,19 @@ class LocalElementBuilder extends _BaseElementBuilder {
   @override
   Object visitLabeledStatement(LabeledStatement node) {
     bool onSwitchStatement = node.statement is SwitchStatement;
-    for (Label label in node.labels) {
-      SimpleIdentifier labelName = label.label;
-      LabelElementImpl element =
-          new LabelElementImpl.forNode(labelName, onSwitchStatement, false);
-      _currentHolder.addLabel(element);
-      labelName.staticElement = element;
-    }
+    buildLabelElements(node.labels, onSwitchStatement, false);
     return super.visitLabeledStatement(node);
   }
 
   @override
   Object visitSwitchCase(SwitchCase node) {
-    for (Label label in node.labels) {
-      SimpleIdentifier labelName = label.label;
-      LabelElementImpl element =
-          new LabelElementImpl.forNode(labelName, false, true);
-      _currentHolder.addLabel(element);
-      labelName.staticElement = element;
-    }
+    buildLabelElements(node.labels, false, true);
     return super.visitSwitchCase(node);
   }
 
   @override
   Object visitSwitchDefault(SwitchDefault node) {
-    for (Label label in node.labels) {
-      SimpleIdentifier labelName = label.label;
-      LabelElementImpl element =
-          new LabelElementImpl.forNode(labelName, false, true);
-      _currentHolder.addLabel(element);
-      labelName.staticElement = element;
-    }
+    buildLabelElements(node.labels, false, true);
     return super.visitSwitchDefault(node);
   }
 
diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart
index ba4eeda8520..ddfc3b43be0 100644
--- a/pkg/analyzer/lib/src/dart/element/element.dart
+++ b/pkg/analyzer/lib/src/dart/element/element.dart
@@ -870,10 +870,11 @@ class ClassElementImpl extends AbstractClassElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedClass != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedClass != null) {
       return _unlinkedClass.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -1852,10 +1853,11 @@ class ConstFieldElementImpl_EnumValue extends ConstFieldElementImpl_ofEnum {
 
   @override
   int get nameOffset {
-    if (_unlinkedEnumValue != null) {
+    int offset = super.nameOffset;
+    if (offset == -1 && _unlinkedEnumValue != null) {
       return _unlinkedEnumValue.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -3478,10 +3480,11 @@ class EnumElementImpl extends AbstractClassElementImpl {
 
   @override
   int get nameOffset {
-    if (_unlinkedEnum != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedEnum != null && _unlinkedEnum.nameOffset != 0) {
       return _unlinkedEnum.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -3827,10 +3830,11 @@ abstract class ExecutableElementImpl extends ElementImpl
 
   @override
   int get nameOffset {
-    if (serializedExecutable != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && serializedExecutable != null) {
       return serializedExecutable.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -4104,10 +4108,11 @@ class ExportElementImpl extends UriReferencedElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedExportNonPublic != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedExportNonPublic != null) {
       return _unlinkedExportNonPublic.offset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -4677,10 +4682,11 @@ class FunctionTypeAliasElementImpl extends ElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedTypedef != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedTypedef != null) {
       return _unlinkedTypedef.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -5014,13 +5020,14 @@ class ImportElementImpl extends UriReferencedElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedImport != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedImport != null) {
       if (_unlinkedImport.isImplicit) {
         return -1;
       }
       return _unlinkedImport.offset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   PrefixElement get prefix {
@@ -5223,10 +5230,13 @@ class LabelElementImpl extends ElementImpl implements LabelElement {
 
   @override
   int get nameOffset {
-    if (_unlinkedLabel != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 &&
+        _unlinkedLabel != null &&
+        _unlinkedLabel.nameOffset != 0) {
       return _unlinkedLabel.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -6764,10 +6774,11 @@ abstract class NonParameterVariableElementImpl extends VariableElementImpl {
 
   @override
   int get nameOffset {
-    if (_unlinkedVariable != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedVariable != null) {
       return _unlinkedVariable.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -7068,13 +7079,14 @@ class ParameterElementImpl extends VariableElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedParam != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedParam != null) {
       if (isSynthetic) {
         return -1;
       }
       return _unlinkedParam.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -7390,10 +7402,11 @@ class PrefixElementImpl extends ElementImpl implements PrefixElement {
 
   @override
   int get nameOffset {
-    if (_unlinkedImport != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedImport != null) {
       return _unlinkedImport.prefixOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   @override
@@ -8038,10 +8051,11 @@ class TypeParameterElementImpl extends ElementImpl
 
   @override
   int get nameOffset {
-    if (_unlinkedTypeParam != null) {
+    int offset = super.nameOffset;
+    if (offset == 0 && _unlinkedTypeParam != null) {
       return _unlinkedTypeParam.nameOffset;
     }
-    return super.nameOffset;
+    return offset;
   }
 
   TypeParameterType get type {
diff --git a/pkg/analyzer/lib/src/generated/declaration_resolver.dart b/pkg/analyzer/lib/src/generated/declaration_resolver.dart
index df4afc29c81..4d24b8b615f 100644
--- a/pkg/analyzer/lib/src/generated/declaration_resolver.dart
+++ b/pkg/analyzer/lib/src/generated/declaration_resolver.dart
@@ -65,14 +65,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitCatchClause(CatchClause node) {
-    SimpleIdentifier exceptionParameter = node.exceptionParameter;
-    if (exceptionParameter != null) {
-      _match(exceptionParameter, _walker.getVariable());
-      SimpleIdentifier stackTraceParameter = node.stackTraceParameter;
-      if (stackTraceParameter != null) {
-        _match(stackTraceParameter, _walker.getVariable());
-      }
-    }
+    _walker.elementBuilder.buildCatchVariableElements(node);
     return super.visitCatchClause(node);
   }
 
@@ -98,8 +91,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitConstructorDeclaration(ConstructorDeclaration node) {
-    ConstructorElement element = _match(node.name, _walker.getConstructor());
-    _walk(new ElementWalker.forExecutable(element), () {
+    ConstructorElement element = _match(node.name, _walker.getConstructor(),
+        offset: node.name?.offset ?? node.returnType.offset);
+    _walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
       node.element = element;
       super.visitConstructorDeclaration(node);
     });
@@ -109,9 +103,8 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitDeclaredIdentifier(DeclaredIdentifier node) {
-    VariableElement element = _match(node.identifier, _walker.getVariable());
-    super.visitDeclaredIdentifier(node);
-    _resolveMetadata(node, node.metadata, element);
+    // Declared identifiers can only occur inside executable elements.
+    _walker.elementBuilder.visitDeclaredIdentifier(node);
     return null;
   }
 
@@ -121,7 +114,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
         _match(node.parameter.identifier, _walker.getParameter());
     Expression defaultValue = node.defaultValue;
     if (defaultValue != null) {
-      _walk(new ElementWalker.forExecutable(element.initializer), () {
+      _walk(
+          new ElementWalker.forExecutable(element.initializer, _enclosingUnit),
+          () {
         defaultValue.accept(this);
       });
     }
@@ -194,7 +189,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
       }
     }
     node.functionExpression.element = element;
-    _walk(new ElementWalker.forExecutable(element), () {
+    _walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
       super.visitFunctionDeclaration(node);
     });
     _resolveMetadata(node, node.metadata, element);
@@ -205,8 +200,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
   Object visitFunctionExpression(FunctionExpression node) {
     if (node.parent is! FunctionDeclaration) {
       FunctionElement element = _walker.getFunction();
+      _matchOffset(element, node.offset);
       node.element = element;
-      _walk(new ElementWalker.forExecutable(element), () {
+      _walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
         super.visitFunctionExpression(node);
       });
       return null;
@@ -250,9 +246,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitLabeledStatement(LabeledStatement node) {
-    for (Label label in node.labels) {
-      _match(label.label, _walker.getLabel());
-    }
+    bool onSwitchStatement = node.statement is SwitchStatement;
+    _walker.elementBuilder
+        .buildLabelElements(node.labels, onSwitchStatement, false);
     return super.visitLabeledStatement(node);
   }
 
@@ -287,7 +283,7 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
             elementName: nameOfMethod + '=');
       }
     }
-    _walk(new ElementWalker.forExecutable(element), () {
+    _walk(new ElementWalker.forExecutable(element, _enclosingUnit), () {
       super.visitMethodDeclaration(node);
     });
     _resolveMetadata(node, node.metadata, element);
@@ -325,17 +321,13 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitSwitchCase(SwitchCase node) {
-    for (Label label in node.labels) {
-      _match(label.label, _walker.getLabel());
-    }
+    _walker.elementBuilder.buildLabelElements(node.labels, false, true);
     return super.visitSwitchCase(node);
   }
 
   @override
   Object visitSwitchDefault(SwitchDefault node) {
-    for (Label label in node.labels) {
-      _match(label.label, _walker.getLabel());
-    }
+    _walker.elementBuilder.buildLabelElements(node.labels, false, true);
     return super.visitSwitchDefault(node);
   }
 
@@ -359,7 +351,9 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
     VariableElement element = _match(node.name, _walker.getVariable());
     Expression initializer = node.initializer;
     if (initializer != null) {
-      _walk(new ElementWalker.forExecutable(element.initializer), () {
+      _walk(
+          new ElementWalker.forExecutable(element.initializer, _enclosingUnit),
+          () {
         super.visitVariableDeclaration(node);
       });
       return null;
@@ -370,12 +364,16 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
 
   @override
   Object visitVariableDeclarationList(VariableDeclarationList node) {
-    super.visitVariableDeclarationList(node);
-    if (node.parent is! FieldDeclaration &&
-        node.parent is! TopLevelVariableDeclaration) {
-      _resolveMetadata(node, node.metadata, node.variables[0].element);
+    if (_walker.elementBuilder != null) {
+      return _walker.elementBuilder.visitVariableDeclarationList(node);
+    } else {
+      super.visitVariableDeclarationList(node);
+      if (node.parent is! FieldDeclaration &&
+          node.parent is! TopLevelVariableDeclaration) {
+        _resolveMetadata(node, node.metadata, node.variables[0].element);
+      }
+      return null;
     }
-    return null;
   }
 
   /**
@@ -390,16 +388,26 @@ class DeclarationResolver extends RecursiveAstVisitor<Object> {
    */
   Element/*=E*/ _match/*<E extends Element>*/(
       SimpleIdentifier identifier, Element/*=E*/ element,
-      {String elementName}) {
+      {String elementName, int offset}) {
     elementName ??= identifier?.name ?? '';
+    offset ??= identifier.offset;
     if (element.name != elementName) {
       throw new StateError(
           'Expected an element matching `$elementName`, got `${element.name}`');
     }
     identifier?.staticElement = element;
+    _matchOffset(element, offset);
     return element;
   }
 
+  void _matchOffset(Element element, int offset) {
+    if (element.nameOffset != 0 && element.nameOffset != offset) {
+      throw new StateError('Element offset mismatch');
+    } else {
+      (element as ElementImpl).nameOffset = offset;
+    }
+  }
+
   /**
    * Associate each of the annotation [nodes] with the corresponding
    * [ElementAnnotation] in [annotations]. If there is a problem, report it
@@ -461,6 +469,19 @@ class ElementWalker {
    */
   final Element element;
 
+  /**
+   * If [element] is an executable element, an element builder which is
+   * accumulating the executable element's local variables and labels.
+   * Otherwise `null`.
+   */
+  LocalElementBuilder elementBuilder;
+
+  /**
+   * If [element] is an executable element, the element holder associated with
+   * [elementBuilder].  Otherwise `null`.
+   */
+  ElementHolder _elementHolder;
+
   List<PropertyAccessorElement> _accessors;
   int _accessorIndex = 0;
   List<ClassElement> _classes;
@@ -471,8 +492,6 @@ class ElementWalker {
   int _enumIndex = 0;
   List<ExecutableElement> _functions;
   int _functionIndex = 0;
-  List<LabelElement> _labels;
-  int _labelIndex = 0;
   List<ParameterElement> _parameters;
   int _parameterIndex = 0;
   List<FunctionTypeAliasElement> _typedefs;
@@ -514,13 +533,9 @@ class ElementWalker {
    * Creates an [ElementWalker] which walks the child elements of a compilation
    * unit element.
    */
-  ElementWalker.forExecutable(ExecutableElement element)
-      : element = element,
-        _functions = element.functions,
-        _labels = element.labels,
-        _parameters = element.parameters,
-        _typeParameters = element.typeParameters,
-        _variables = element.localVariables;
+  ElementWalker.forExecutable(
+      ExecutableElement element, CompilationUnitElement compilationUnit)
+      : this._forExecutable(element, compilationUnit, new ElementHolder());
 
   /**
    * Creates an [ElementWalker] which walks the child elements of a parameter
@@ -540,6 +555,16 @@ class ElementWalker {
         _parameters = element.parameters,
         _typeParameters = element.typeParameters;
 
+  ElementWalker._forExecutable(ExecutableElement element,
+      CompilationUnitElement compilationUnit, ElementHolder elementHolder)
+      : element = element,
+        elementBuilder =
+            new LocalElementBuilder(elementHolder, compilationUnit),
+        _elementHolder = elementHolder,
+        _functions = element.functions,
+        _parameters = element.parameters,
+        _typeParameters = element.typeParameters;
+
   /**
    * Returns the next non-synthetic child of [element] which is an accessor;
    * throws an [IndexError] if there are no more.
@@ -571,12 +596,6 @@ class ElementWalker {
    */
   ExecutableElement getFunction() => _functions[_functionIndex++];
 
-  /**
-   * Returns the next non-synthetic child of [element] which is a label; throws
-   * an [IndexError] if there are no more.
-   */
-  LabelElement getLabel() => _labels[_labelIndex++];
-
   /**
    * Returns the next non-synthetic child of [element] which is a parameter;
    * throws an [IndexError] if there are no more.
@@ -620,11 +639,15 @@ class ElementWalker {
     check(_constructors, _constructorIndex);
     check(_enums, _enumIndex);
     check(_functions, _functionIndex);
-    check(_labels, _labelIndex);
     check(_parameters, _parameterIndex);
     check(_typedefs, _typedefIndex);
     check(_typeParameters, _typeParameterIndex);
     check(_variables, _variableIndex);
+    Element element = this.element;
+    if (element is ExecutableElementImpl) {
+      element.labels = _elementHolder.labels;
+      element.localVariables = _elementHolder.localVariables;
+    }
   }
 
   static bool _isNotSynthetic(Element e) => !e.isSynthetic;
diff --git a/pkg/analyzer/test/generated/declaration_resolver_test.dart b/pkg/analyzer/test/generated/declaration_resolver_test.dart
index 9226efeb25f..74232ac7fe5 100644
--- a/pkg/analyzer/test/generated/declaration_resolver_test.dart
+++ b/pkg/analyzer/test/generated/declaration_resolver_test.dart
@@ -45,13 +45,16 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
   CompilationUnit unit;
   CompilationUnit unit2;
 
-  void checkMetadata(String search) {
+  void checkMetadata(String search, {bool expectDifferent: false}) {
     NodeList<Annotation> metadata = _findMetadata(unit, search);
     NodeList<Annotation> metadata2 = _findMetadata(unit2, search);
     expect(metadata, isNotEmpty);
     for (int i = 0; i < metadata.length; i++) {
-      expect(
-          metadata2[i].elementAnnotation, same(metadata[i].elementAnnotation));
+      Matcher expectation = same(metadata[i].elementAnnotation);
+      if (expectDifferent) {
+        expectation = isNot(expectation);
+      }
+      expect(metadata2[i].elementAnnotation, expectation);
     }
   }
 
@@ -83,7 +86,7 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
 
   void test_metadata_declaredIdentifier() {
     setupCode('f(x, y) { for (@a var x in y) {} }');
-    checkMetadata('var');
+    checkMetadata('var', expectDifferent: true);
   }
 
   void test_metadata_enumDeclaration() {
@@ -175,7 +178,7 @@ class DeclarationResolverMetadataTest extends ResolverTestCase {
 
   void test_metadata_localVariableDeclaration() {
     setupCode('f() { @a int x; }');
-    checkMetadata('x');
+    checkMetadata('x', expectDifferent: true);
   }
 
   void test_metadata_methodDeclaration_getter() {
@@ -258,6 +261,70 @@ class DeclarationResolverTest extends ResolverTestCase {
     super.setUp();
   }
 
+  void test_closure_inside_catch_block() {
+    String code = '''
+f() {
+  try {
+  } catch (e) {
+    return () => null;
+  }
+}
+''';
+    CompilationUnit unit = resolveSource(code);
+    // re-resolve
+    _cloneResolveUnit(unit);
+    // no other validations than built into DeclarationResolver
+  }
+
+  void test_closure_inside_labeled_statement() {
+    String code = '''
+f(b) {
+  foo: while (true) {
+    if (b) {
+      break foo;
+    }
+    return () => null;
+  }
+}
+''';
+    CompilationUnit unit = resolveSource(code);
+    // re-resolve
+    _cloneResolveUnit(unit);
+    // no other validations than built into DeclarationResolver
+  }
+
+  void test_closure_inside_switch_case() {
+    String code = '''
+void f(k, m) {
+  switch (k) {
+    case 0:
+      m.forEach((key, value) {});
+    break;
+  }
+}
+''';
+    CompilationUnit unit = resolveSource(code);
+    // re-resolve
+    _cloneResolveUnit(unit);
+    // no other validations than built into DeclarationResolver
+  }
+
+  void test_closure_inside_switch_default() {
+    String code = '''
+void f(k, m) {
+  switch (k) {
+    default:
+      m.forEach((key, value) {});
+    break;
+  }
+}
+''';
+    CompilationUnit unit = resolveSource(code);
+    // re-resolve
+    _cloneResolveUnit(unit);
+    // no other validations than built into DeclarationResolver
+  }
+
   void test_enumConstant_partiallyResolved() {
     String code = r'''
 enum Fruit {apple, pear}
diff --git a/pkg/analyzer/test/src/context/context_test.dart b/pkg/analyzer/test/src/context/context_test.dart
index 19352ab88d4..7c6c668d674 100644
--- a/pkg/analyzer/test/src/context/context_test.dart
+++ b/pkg/analyzer/test/src/context/context_test.dart
@@ -5257,7 +5257,11 @@ class _ElementComparer extends GeneralizingElementVisitor {
   @override
   void visitElement(Element element) {
     Element previousElement = previousElements[element];
-    if (!identical(previousElement, element)) {
+    bool expectIdentical = element is! LocalVariableElement;
+    bool ok = expectIdentical
+        ? identical(previousElement, element)
+        : previousElement == element;
+    if (!ok) {
       if (overwrittenCount == 0) {
         buffer.writeln();
       }