Allow replacementOffset and Length overriding

pkg/analysis_server/lib/src/domain_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.
 
 import 'dart:async';
 
 import 'package:analysis_server/protocol/protocol.dart';
 import 'package:analysis_server/protocol/protocol_generated.dart';
 import 'package:analysis_server/src/analysis_server.dart';
 import 'package:analysis_server/src/constants.dart';
@@ skipping 103 matching lines @@
 
     //
     // Add the fixes produced by plugins to the server-generated fixes.
     //
     if (pluginFutures != null) {
       List<plugin.Response> responses = await waitForResponses(pluginFutures,
           requestParameters: requestParams);
       for (plugin.Response response in responses) {
         plugin.CompletionGetSuggestionsResult result =
             new plugin.CompletionGetSuggestionsResult.fromResponse(response);
+        if (suggestions.isEmpty) {
+          request.replacementOffset = result.replacementOffset;
+          request.replacementLength = result.replacementLength;

[Brian Wilkerson, 2017/05/30 18:30:22]

I think we only want to overwrite the current values if there are no current
values. I suspect that you're assuming that if there are no suggestions that
server won't have set these values into the request, but I'm not sure that's
always true. We could make it safe by replacing "=" with "??=".

[maxkim, 2017/05/30 19:10:32]

Adding the null-aware assignment is making the replacementOffset not be properly
override by the plugin's result.replacementOffset; I believe the length is
working fine since the value is null. 

I don't fully understand why the original `request.replacementOffset` and
`request.replacementLength` being null matters if suggestions is already empty.
By this point, the primary Dart autocompletion has had time to finish its
autocompletion calculations and if suggestions is empty, it means there is no
danger of overriding these two values. 

However, I do think I should add another check to make sure that the plugin has
suggestions in the first place before even entering this block. 

[Brian Wilkerson, 2017/05/30 19:18:48]

I'm probably just being too cautious. I'm willing to leave it as is.
That sounds like a good idea.

+        } else if (request.replacementOffset != result.replacementOffset &&
+            request.replacementLength != result.replacementLength) {
+          server.instrumentationService
+              .logError('Plugin completion-results dropped due to conflicting'
+                  ' replacement offset/length: ${result.toJson()}');
+          continue;
+        }
         suggestions.addAll(result.results);
       }
     }
     //
     // Return the result.
     //
     return new CompletionResult(
         request.replacementOffset, request.replacementLength, suggestions);
   }
 
@@ skipping 154 matching lines @@
   final int replacementOffset;
 
   /**
    * The suggested completions.
    */
   final List<CompletionSuggestion> suggestions;
 
   CompletionResult(
       this.replacementOffset, this.replacementLength, this.suggestions);
 }