Remove unnecessary List creation

pkg/analysis_server/lib/src/services/generated/completion.dart

 // Copyright (c) 2014, 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.
 
 // This code was auto-generated, is not intended to be edited, and is subject to
 // significant change. Please see the README file for more information.
 
 library services.completion;
 
 import 'dart:collection';
@@ skipping 800 matching lines @@
     }
   }
 
   bool _filterAllows(Element element) => _filter._match(element);
 
   bool _filterDisallows(Element element) => !_filter._match(element);
 
   bool _filterDisallows2(String name) => !_filter._match2(name);
 
   List<Element> _findAllNotTypes(List<Element> elements) {
-    elements = [];
+    List<Element> elements = <Element>[];

[sra1, 2014/10/26 06:31:39]

This local is shadowing the parameter.
The original code was using a new list instead of passing in a list.

Both are suspicious - the parameter has no purpose, and the function always
returns a new empty growable list.
Is there a unit test that would catch this?

The code should probably be using 
  elements.removeWhere((e) => ...);
  return elements;
to mutate the input, or
  return elements.where((e) => ...).toList();
to return a new list with some elements filtered out.

If the code should be mutating, I would make the return type void and use a
cascade at the call site, like the 'sort' example in another comment. 

[Brian Wilkerson, 2014/10/26 15:15:46]

Good catch! The original Java code was mistranslated. The original makes a copy,
so I used 'where'.

I'll work with Dan and Konstantin to figure out how best to test this.

     for (JavaIterator<Element> I = new JavaIterator(elements); I.hasNext;) {
       Element element = I.next();
       ElementKind kind = element.kind;
       if (kind == ElementKind.FUNCTION || kind == ElementKind.TOP_LEVEL_VARIABLE || kind == ElementKind.GETTER || kind == ElementKind.SETTER) {
         continue;
       }
       I.remove();
     }
-    return new List.from(elements);
+    return elements;
   }
 
   List<Element> _findAllPrefixes() {
     LibraryElement lib = _context.compilationUnitElement.enclosingElement;
     return lib.prefixes;
   }
 
   List<Element> _findAllTypes(LibraryElement library, TopLevelNamesKind topKind) {
     List<Element> elements = _findTopLevelElements(library, topKind);
     return _findAllTypes2(elements);
   }
 
   List<Element> _findAllTypes2(List<Element> elements) {
-    elements = [];
+    List<Element> elements = <Element>[];

[sra1, 2014/10/26 06:31:39]

ditto

[Brian Wilkerson, 2014/10/26 15:15:46]

Same problem, same solution.

     for (JavaIterator<Element> I = new JavaIterator(elements); I.hasNext;) {
       Element element = I.next();
       ElementKind kind = element.kind;
       if (kind == ElementKind.CLASS || kind == ElementKind.FUNCTION_TYPE_ALIAS) {
         continue;
       }
       I.remove();
     }
-    return new List.from(elements);
+    return elements;
   }
 
   List<Element> _findTopLevelElements(LibraryElement library, TopLevelNamesKind topKind) {
     List<Element> elements = [];
     if (topKind == TopLevelNamesKind.DECLARED_AND_IMPORTS) {
       elements.addAll(CorrectionUtils.getTopLevelElements(library));
       for (ImportElement imp in library.imports) {
         elements.addAll(CorrectionUtils.getImportNamespace(imp).values);
       }
       _removeNotMatchingFilter(elements);
@@ skipping 69 matching lines @@
   bool get hasErrorBeforeCompletionLocation {
     List<AnalysisError> errors = _context.errors;
     if (errors == null || errors.length == 0) {
       return false;
     }
     return errors[0].offset <= _completionLocation();
   }
 
   List<ImportElement> _importsWithName(SimpleIdentifier libName) {
     String name = libName.name;
-    List<ImportElement> imports = [];
+    List<ImportElement> imports = <ImportElement>[];
     for (ImportElement imp in currentLibrary.imports) {
       PrefixElement prefix = imp.prefix;
       if (prefix != null) {
         String impName = prefix.displayName;
         if (name == impName) {
           imports.add(imp);
         }
       }
     }
-    return new List.from(imports);
+    return imports;
   }
 
   bool _isCompletingKeyword(Token keyword) {
     if (keyword == null) {
       return false;
     }
     int completionLoc = _context.selectionOffset;
     if (completionLoc >= keyword.offset && completionLoc <= keyword.end) {
       return true;
     }
@@ skipping 357 matching lines @@
             named = true;
           } else if (param.parameterKind == ParameterKind.POSITIONAL) {
             positional = true;
           }
           break;
         }
         params.add(param.displayName);
         types.add(param.type.toString());
       }
     }
-    prop.setParameterNames(new List.from(params));
-    prop.setParameterTypes(new List.from(types));
+    prop.setParameterNames(params);
+    prop.setParameterTypes(types);
     prop.setParameterStyle(posCount, named, positional);
   }
 
   SimpleIdentifier _typeDeclarationName(AstNode node) {
     AstNode parent = node;
     while (parent != null) {
       if (parent is ClassDeclaration) {
         return (parent as ClassDeclaration).name;
       }
       if (parent is ClassTypeAlias) {
@@ skipping 2353 matching lines @@
 
 class TopLevelNamesKind extends Enum<TopLevelNamesKind> {
   static const TopLevelNamesKind DECLARED_AND_IMPORTS = const TopLevelNamesKind('DECLARED_AND_IMPORTS', 0);
 
   static const TopLevelNamesKind DECLARED_AND_EXPORTS = const TopLevelNamesKind('DECLARED_AND_EXPORTS', 1);
 
   static const List<TopLevelNamesKind> values = const [DECLARED_AND_IMPORTS, DECLARED_AND_EXPORTS];
 
   const TopLevelNamesKind(String name, int ordinal) : super(name, ordinal);
 }