Extract RemoveUnusedElement and RemoveUnusedField producers.

There is an exception that we see internally, and I'd like to try to
refactor the code a bit.

R=brianwilkerson@google.com

Change-Id: I276e009437fd3cac82391126bde95c73225bca0c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/138784
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>

Author: scheglov

pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart

@@ -53,6 +53,8 @@ abstract class CorrectionProducer {
 
   TypeProvider get typeProvider => _context.typeProvider;
 
+  CompilationUnit get unit => _context.unit;
+
   CorrectionUtils get utils => _context.utils;
 
   Future<void> compute(DartChangeBuilder builder);
@@ -94,6 +96,7 @@ class CorrectionProducerContext {
   final int selectionLength;
   final int selectionEnd;
 
+  final CompilationUnit unit;
   final CorrectionUtils utils;
   final String file;
 
@@ -118,6 +121,7 @@ class CorrectionProducerContext {
         sessionHelper = AnalysisSessionHelper(resolvedResult.session),
         typeProvider = resolvedResult.typeProvider,
         selectionEnd = (selectionOffset ?? 0) + (selectionLength ?? 0),
+        unit = resolvedResult.unit,
         utils = CorrectionUtils(resolvedResult);
 
   AstNode get node => _node;

pkg/analysis_server/lib/src/services/correction/dart/remove_unused.dart

@@ -0,0 +1,154 @@
+// Copyright (c) 2020, 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 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/source/source_range.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:analyzer_plugin/utilities/range_factory.dart';
+
+class RemoveUnusedElement extends _RemoveUnused {
+  @override
+  FixKind get fixKind => DartFixKind.REMOVE_UNUSED_ELEMENT;
+
+  @override
+  Future<void> compute(DartChangeBuilder builder) async {
+    final sourceRanges = <SourceRange>[];
+    final referencedNode = node.parent;
+    if (referencedNode is ClassDeclaration ||
+        referencedNode is EnumDeclaration ||
+        referencedNode is FunctionDeclaration ||
+        referencedNode is FunctionTypeAlias ||
+        referencedNode is MethodDeclaration ||
+        referencedNode is VariableDeclaration) {
+      final element = referencedNode is Declaration
+          ? referencedNode.declaredElement
+          : (referencedNode as NamedCompilationUnitMember).declaredElement;
+      final references = _findAllReferences(unit, element);
+      // todo (pq): consider filtering for references that are limited to within the class.
+      if (references.length == 1) {
+        var sourceRange;
+        if (referencedNode is VariableDeclaration) {
+          VariableDeclarationList parent = referencedNode.parent;
+          if (parent.variables.length == 1) {
+            sourceRange = utils.getLinesRange(range.node(parent.parent));
+          } else {
+            sourceRange = range.nodeInList(parent.variables, referencedNode);
+          }
+        } else {
+          sourceRange = utils.getLinesRange(range.node(referencedNode));
+        }
+        sourceRanges.add(sourceRange);
+      }
+    }
+
+    await builder.addFileEdit(file, (builder) {
+      for (var sourceRange in sourceRanges) {
+        builder.addDeletion(sourceRange);
+      }
+    });
+  }
+}
+
+class RemoveUnusedField extends _RemoveUnused {
+  @override
+  FixKind get fixKind => DartFixKind.REMOVE_UNUSED_FIELD;
+
+  @override
+  Future<void> compute(DartChangeBuilder builder) async {
+    final declaration = node.parent;
+    if (declaration is! VariableDeclaration) {
+      return;
+    }
+    final element = (declaration as VariableDeclaration).declaredElement;
+    if (element is! FieldElement) {
+      return;
+    }
+
+    final sourceRanges = <SourceRange>[];
+    final references = _findAllReferences(unit, element);
+    for (var reference in references) {
+      // todo (pq): consider scoping this to parent or parent.parent.
+      final referenceNode = reference.thisOrAncestorMatching((node) =>
+          node is VariableDeclaration ||
+          node is ExpressionStatement ||
+          node is ConstructorFieldInitializer ||
+          node is FieldFormalParameter);
+      if (referenceNode == null) {
+        return;
+      }
+      var sourceRange;
+      if (referenceNode is VariableDeclaration) {
+        VariableDeclarationList parent = referenceNode.parent;
+        if (parent.variables.length == 1) {
+          sourceRange = utils.getLinesRange(range.node(parent.parent));
+        } else {
+          sourceRange = range.nodeInList(parent.variables, referenceNode);
+        }
+      } else if (referenceNode is ConstructorFieldInitializer) {
+        ConstructorDeclaration cons =
+            referenceNode.parent as ConstructorDeclaration;
+        // A() : _f = 0;
+        if (cons.initializers.length == 1) {
+          sourceRange = range.endEnd(cons.parameters, referenceNode);
+        } else {
+          sourceRange = range.nodeInList(cons.initializers, referenceNode);
+        }
+      } else if (referenceNode is FieldFormalParameter) {
+        FormalParameterList params =
+            referenceNode.parent as FormalParameterList;
+        if (params.parameters.length == 1) {
+          sourceRange =
+              range.endStart(params.leftParenthesis, params.rightParenthesis);
+        } else {
+          sourceRange = range.nodeInList(params.parameters, referenceNode);
+        }
+      } else {
+        sourceRange = utils.getLinesRange(range.node(referenceNode));
+      }
+      sourceRanges.add(sourceRange);
+    }
+
+    await builder.addFileEdit(file, (builder) {
+      for (var sourceRange in sourceRanges) {
+        builder.addDeletion(sourceRange);
+      }
+    });
+  }
+}
+
+class _ElementReferenceCollector extends RecursiveAstVisitor<void> {
+  final Element element;
+  final List<SimpleIdentifier> references = [];
+
+  _ElementReferenceCollector(this.element);
+
+  @override
+  void visitSimpleIdentifier(SimpleIdentifier node) {
+    final staticElement = node.staticElement;
+    if (staticElement == element) {
+      references.add(node);
+    } else if (staticElement is PropertyAccessorElement) {
+      if (staticElement.variable == element) {
+        references.add(node);
+      }
+    } else if (staticElement is FieldFormalParameterElement) {
+      if (staticElement.field == element) {
+        references.add(node);
+      }
+    }
+  }
+}
+
+abstract class _RemoveUnused extends CorrectionProducer {
+  List<SimpleIdentifier> _findAllReferences(AstNode root, Element element) {
+    var collector = _ElementReferenceCollector(element);
+    root.accept(collector);
+    return collector.references;
+  }
+}

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -21,6 +21,7 @@ import 'package:analysis_server/src/services/correction/dart/convert_to_where_ty
 import 'package:analysis_server/src/services/correction/dart/inline_typedef.dart';
 import 'package:analysis_server/src/services/correction/dart/remove_dead_if_null.dart';
 import 'package:analysis_server/src/services/correction/dart/remove_if_null_operator.dart';
+import 'package:analysis_server/src/services/correction/dart/remove_unused.dart';
 import 'package:analysis_server/src/services/correction/dart/replace_with_eight_digit_hex.dart';
 import 'package:analysis_server/src/services/correction/dart/wrap_in_future.dart';
 import 'package:analysis_server/src/services/correction/fix.dart';
@@ -35,7 +36,6 @@ import 'package:analyzer/dart/analysis/features.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/precedence.dart';
 import 'package:analyzer/dart/ast/token.dart';
-import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/nullability_suffix.dart';
 import 'package:analyzer/dart/element/type.dart';
@@ -374,12 +374,6 @@ class FixProcessor extends BaseProcessor {
     if (errorCode == HintCode.UNUSED_CATCH_STACK) {
       await _addFix_removeUnusedCatchStack();
     }
-    if (errorCode == HintCode.UNUSED_ELEMENT) {
-      await _addFix_removeUnusedElement();
-    }
-    if (errorCode == HintCode.UNUSED_FIELD) {
-      await _addFix_removeUnusedField();
-    }
     if (errorCode == HintCode.UNUSED_IMPORT) {
       await _addFix_removeUnusedImport();
     }
@@ -3752,110 +3746,6 @@ class FixProcessor extends BaseProcessor {
     }
   }
 
-  Future<void> _addFix_removeUnusedElement() async {
-    final sourceRanges = <SourceRange>[];
-    final referencedNode = node.parent;
-    if (referencedNode is ClassDeclaration ||
-        referencedNode is EnumDeclaration ||
-        referencedNode is FunctionDeclaration ||
-        referencedNode is FunctionTypeAlias ||
-        referencedNode is MethodDeclaration ||
-        referencedNode is VariableDeclaration) {
-      final element = referencedNode is Declaration
-          ? referencedNode.declaredElement
-          : (referencedNode as NamedCompilationUnitMember).declaredElement;
-      final references = _findAllReferences(unit, element);
-      // todo (pq): consider filtering for references that are limited to within the class.
-      if (references.length == 1) {
-        var sourceRange;
-        if (referencedNode is VariableDeclaration) {
-          VariableDeclarationList parent = referencedNode.parent;
-          if (parent.variables.length == 1) {
-            sourceRange = utils.getLinesRange(range.node(parent.parent));
-          } else {
-            sourceRange = range.nodeInList(parent.variables, referencedNode);
-          }
-        } else {
-          sourceRange = utils.getLinesRange(range.node(referencedNode));
-        }
-        sourceRanges.add(sourceRange);
-      }
-    }
-
-    if (sourceRanges.isNotEmpty) {
-      final changeBuilder = _newDartChangeBuilder();
-      await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
-        for (var sourceRange in sourceRanges) {
-          builder.addDeletion(sourceRange);
-        }
-      });
-      _addFixFromBuilder(changeBuilder, DartFixKind.REMOVE_UNUSED_ELEMENT);
-    }
-  }
-
-  Future<void> _addFix_removeUnusedField() async {
-    final declaration = node.parent;
-    if (declaration is! VariableDeclaration) {
-      return;
-    }
-    final element = (declaration as VariableDeclaration).declaredElement;
-    if (element is! FieldElement) {
-      return;
-    }
-
-    final sourceRanges = <SourceRange>[];
-    final references = _findAllReferences(unit, element);
-    for (var reference in references) {
-      // todo (pq): consider scoping this to parent or parent.parent.
-      final referenceNode = reference.thisOrAncestorMatching((node) =>
-          node is VariableDeclaration ||
-          node is ExpressionStatement ||
-          node is ConstructorFieldInitializer ||
-          node is FieldFormalParameter);
-      if (referenceNode == null) {
-        return;
-      }
-      var sourceRange;
-      if (referenceNode is VariableDeclaration) {
-        VariableDeclarationList parent = referenceNode.parent;
-        if (parent.variables.length == 1) {
-          sourceRange = utils.getLinesRange(range.node(parent.parent));
-        } else {
-          sourceRange = range.nodeInList(parent.variables, referenceNode);
-        }
-      } else if (referenceNode is ConstructorFieldInitializer) {
-        ConstructorDeclaration cons =
-            referenceNode.parent as ConstructorDeclaration;
-        // A() : _f = 0;
-        if (cons.initializers.length == 1) {
-          sourceRange = range.endEnd(cons.parameters, referenceNode);
-        } else {
-          sourceRange = range.nodeInList(cons.initializers, referenceNode);
-        }
-      } else if (referenceNode is FieldFormalParameter) {
-        FormalParameterList params =
-            referenceNode.parent as FormalParameterList;
-        if (params.parameters.length == 1) {
-          sourceRange =
-              range.endStart(params.leftParenthesis, params.rightParenthesis);
-        } else {
-          sourceRange = range.nodeInList(params.parameters, referenceNode);
-        }
-      } else {
-        sourceRange = utils.getLinesRange(range.node(referenceNode));
-      }
-      sourceRanges.add(sourceRange);
-    }
-
-    final changeBuilder = _newDartChangeBuilder();
-    await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
-      for (var sourceRange in sourceRanges) {
-        builder.addDeletion(sourceRange);
-      }
-    });
-    _addFixFromBuilder(changeBuilder, DartFixKind.REMOVE_UNUSED_FIELD);
-  }
-
   Future<void> _addFix_removeUnusedImport() async {
     // prepare ImportDirective
     ImportDirective importDirective =
@@ -4714,7 +4604,11 @@ class FixProcessor extends BaseProcessor {
     }
 
     var errorCode = error.errorCode;
-    if (errorCode == StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION) {
+    if (errorCode == HintCode.UNUSED_ELEMENT) {
+      await compute(RemoveUnusedElement());
+    } else if (errorCode == HintCode.UNUSED_FIELD) {
+      await compute(RemoveUnusedField());
+    } else if (errorCode == StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION) {
       await compute(RemoveDeadIfNull());
     } else if (errorCode is LintCode) {
       String name = errorCode.name;
@@ -4844,12 +4738,6 @@ class FixProcessor extends BaseProcessor {
     _addFixFromBuilder(changeBuilder, DartFixKind.CREATE_METHOD, args: [name]);
   }
 
-  List<SimpleIdentifier> _findAllReferences(AstNode root, Element element) {
-    var collector = _ElementReferenceCollector(element);
-    root.accept(collector);
-    return collector.references;
-  }
-
   /// Return the class, enum or mixin declaration for the given [element].
   Future<ClassOrMixinDeclaration> _getClassDeclaration(
       ClassElement element) async {
@@ -5235,32 +5123,6 @@ class _ClosestElementFinder {
   }
 }
 
-class _ElementReferenceCollector extends RecursiveAstVisitor<void> {
-  final Element element;
-  final List<SimpleIdentifier> references = [];
-
-  _ElementReferenceCollector(this.element);
-
-  @override
-  void visitSimpleIdentifier(SimpleIdentifier node) {
-    final staticElement = node.staticElement;
-    if (staticElement == element) {
-      references.add(node);
-    }
-    // Implicit Setter.
-    else if (staticElement is PropertyAccessorElement) {
-      if (staticElement.variable == element) {
-        references.add(node);
-      }
-      // Field Formals.
-    } else if (staticElement is FieldFormalParameterElement) {
-      if (staticElement.field == element) {
-        references.add(node);
-      }
-    }
-  }
-}
-
 /// [ExecutableElement], its parameters, and operations on them.
 class _ExecutableParameters {
   final AnalysisSessionHelper sessionHelper;