Issue 22988. Improve checking for conflicts between parameters and local elements during extracting…

pkg/analysis_server/lib/src/services/refactoring/extract_method.dart

Index: pkg/analysis_server/lib/src/services/refactoring/extract_method.dart
diff --git a/pkg/analysis_server/lib/src/services/refactoring/extract_method.dart b/pkg/analysis_server/lib/src/services/refactoring/extract_method.dart
index da4e4e42d0884e9beca16cfbf15b33766b41589f..1167240910ca986bc99f24f563af545e601c85e1 100644
--- a/pkg/analysis_server/lib/src/services/refactoring/extract_method.dart
+++ b/pkg/analysis_server/lib/src/services/refactoring/extract_method.dart
@@ -77,7 +77,16 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl
   final List<int> offsets = <int>[];
   final List<int> lengths = <int>[];
 
-  Set<String> _usedNames = new Set<String>();
+  /**
+   * The map of local names to their visibility ranges.
+   */
+  Map<String, List<SourceRange>> _localNames = <String, List<SourceRange>>{};
+
+  /**
+   * The set of names that are referenced without any qualifier.
+   */
+  Set<String> _unqualifiedNames = new Set<String>();
+
   Set<String> _excludedNames = new Set<String>();
   List<RefactoringMethodParameter> _parameters = <RefactoringMethodParameter>[];
   Map<String, RefactoringMethodParameter> _parametersMap =
@@ -356,7 +365,7 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl
           return result;
         }
       }
-      if (_usedNames.contains(parameter.name)) {
+      if (_isParameterNameConflictWithBody(parameter)) {
         result.addError(format(
             "'{0}' is already used as a name in the selected code",
             parameter.name));
@@ -628,6 +637,26 @@ class ExtractMethodRefactoringImpl extends RefactoringImpl
     return analyzer.status.isOK;
   }
 
+  bool _isParameterNameConflictWithBody(RefactoringMethodParameter parameter) {
+    String id = parameter.id;
+    String name = parameter.name;
+    List<SourceRange> parameterRanges = _parameterReferencesMap[id];
+    List<SourceRange> otherRanges = _localNames[name];
+    for (SourceRange parameterRange in parameterRanges) {
+      if (otherRanges != null) {
+        for (SourceRange otherRange in otherRanges) {
+          if (parameterRange.intersects(otherRange)) {
+            return true;
+          }
+        }
+      }
+    }
+    if (_unqualifiedNames.contains(name)) {
+      return true;
+    }
+    return false;
+  }
+
   /**
    * Checks if [element] is referenced after [selectionRange].
    */
@@ -973,56 +1002,64 @@ class _InitializeOccurrencesVisitor extends GeneralizingAstVisitor<Object> {
   }
 }
 
-class _InitializeParametersVisitor extends GeneralizingAstVisitor<Object> {
+class _InitializeParametersVisitor extends GeneralizingAstVisitor {
   final ExtractMethodRefactoringImpl ref;
   final List<VariableElement> assignedUsedVariables;
 
   _InitializeParametersVisitor(this.ref, this.assignedUsedVariables);
 
   @override
-  Object visitSimpleIdentifier(SimpleIdentifier node) {
+  void visitSimpleIdentifier(SimpleIdentifier node) {
     SourceRange nodeRange = rangeNode(node);
-    if (ref.selectionRange.covers(nodeRange)) {
-      // analyze local variable
-      VariableElement variableElement =
-          getLocalOrParameterVariableElement(node);
-      if (variableElement != null) {
-        // name of the named expression
-        if (isNamedExpressionName(node)) {
-          return null;
-        }
-        // if declared outside, add parameter
-        if (!ref._isDeclaredInSelection(variableElement)) {
-          String variableName = variableElement.displayName;
-          // add parameter
-          RefactoringMethodParameter parameter =
-              ref._parametersMap[variableName];
-          if (parameter == null) {
-            DartType parameterType = node.bestType;
-            String parameterTypeCode = ref._getTypeCode(parameterType);
-            parameter = new RefactoringMethodParameter(
-                RefactoringMethodParameterKind.REQUIRED, parameterTypeCode,
-                variableName, id: variableName);
-            ref._parameters.add(parameter);
-            ref._parametersMap[variableName] = parameter;
-          }
-          // add reference to parameter
-          ref._addParameterReference(variableName, nodeRange);
+    if (!ref.selectionRange.covers(nodeRange)) {
+      return;
+    }
+    String name = node.name;
+    // analyze local variable
+    VariableElement variableElement = getLocalOrParameterVariableElement(node);
+    if (variableElement != null) {
+      // name of the named expression
+      if (isNamedExpressionName(node)) {
+        return;
+      }
+      // if declared outside, add parameter
+      if (!ref._isDeclaredInSelection(variableElement)) {
+        // add parameter
+        RefactoringMethodParameter parameter = ref._parametersMap[name];
+        if (parameter == null) {
+          DartType parameterType = node.bestType;
+          String parameterTypeCode = ref._getTypeCode(parameterType);
+          parameter = new RefactoringMethodParameter(
+              RefactoringMethodParameterKind.REQUIRED, parameterTypeCode, name,
+              id: name);
+          ref._parameters.add(parameter);
+          ref._parametersMap[name] = parameter;
         }
-        // remember, if assigned and used after selection
-        if (isLeftHandOfAssignment(node) &&
-            ref._isUsedAfterSelection(variableElement)) {
-          if (!assignedUsedVariables.contains(variableElement)) {
-            assignedUsedVariables.add(variableElement);
-          }
+        // add reference to parameter
+        ref._addParameterReference(name, nodeRange);
+      }
+      // remember, if assigned and used after selection
+      if (isLeftHandOfAssignment(node) &&
+          ref._isUsedAfterSelection(variableElement)) {
+        if (!assignedUsedVariables.contains(variableElement)) {
+          assignedUsedVariables.add(variableElement);
         }
       }
-      // remember declaration names
+    }
+    // remember information for conflicts checking
+    if (variableElement is LocalElement) {
+      // declared local elements
+      LocalElement localElement = variableElement as LocalElement;
       if (node.inDeclarationContext()) {
-        ref._usedNames.add(node.name);
+        ref._localNames.putIfAbsent(name, () => <SourceRange>[]);
+        ref._localNames[name].add(localElement.visibleRange);
+      }
+    } else {
+      // unqualified non-local names
+      if (!node.isQualified) {
+        ref._unqualifiedNames.add(name);
       }
     }
-    return null;
   }
 }