From 0fe448a99643e149acb2e7e32d7a30eba7dd646d Mon Sep 17 00:00:00 2001 From: danrubel Date: Mon, 29 Oct 2018 01:50:27 +0000 Subject: [PATCH] Revise the analysis server edit.dartfix protocol This updates the edit.dartfix protocol to separate location from the text describing the changes so that the dartfix client can choose what location information to display and how it should be displayed. Change-Id: Ic56a4cb187538571d60136d6de9265d879fccb53 Reviewed-on: https://dart-review.googlesource.com/c/81780 Commit-Queue: Dan Rubel Reviewed-by: Brian Wilkerson --- pkg/analysis_server/doc/api.html | 7 +- .../lib/protocol/protocol_constants.dart | 7 +- .../lib/protocol/protocol_generated.dart | 232 +++++++++++++----- .../lib/src/edit/edit_dartfix.dart | 57 ++--- .../src/edit/fix/prefer_int_literals_fix.dart | 12 +- .../lib/src/edit/fix/prefer_mixin_fix.dart | 9 +- .../support/integration_test_methods.dart | 17 +- .../support/protocol_matchers.dart | 24 +- .../java/types/DartFixSuggestion.java | 128 ++++++++++ .../types/RuntimeCompletionExpression.java | 6 +- pkg/analysis_server/tool/spec/spec_input.html | 44 +++- .../lib/src/protocol/protocol_constants.dart | 7 +- .../lib/src/protocol/protocol_generated.dart | 232 +++++++++++++----- pkg/dartfix/lib/src/driver.dart | 39 +-- 14 files changed, 591 insertions(+), 230 deletions(-) create mode 100644 pkg/analysis_server/tool/spec/generated/java/types/DartFixSuggestion.java diff --git a/pkg/analysis_server/doc/api.html b/pkg/analysis_server/doc/api.html index 504713e6685..0c7d34b8efc 100644 --- a/pkg/analysis_server/doc/api.html +++ b/pkg/analysis_server/doc/api.html @@ -2658,6 +2658,7 @@ a:focus, a:hover { +
AddContentOverlay: object
@@ -4379,9 +4380,9 @@ a:focus, a:hover {
RuntimeCompletionExpression: object

An expression for which we want to know its runtime type. - In expressions like `a.b.c.where((e) => e.^)` we want to know the - runtime type of `a.b.c` to enforce it statically at the time when we - compute completion suggestions, and get better type for `e`. + In expressions like 'a.b.c.where((e) => e.^)' we want to know the + runtime type of 'a.b.c' to enforce it statically at the time when we + compute completion suggestions, and get better type for 'e'.

offset: int
diff --git a/pkg/analysis_server/lib/protocol/protocol_constants.dart b/pkg/analysis_server/lib/protocol/protocol_constants.dart index 9d85d08bdd2..1280460f470 100644 --- a/pkg/analysis_server/lib/protocol/protocol_constants.dart +++ b/pkg/analysis_server/lib/protocol/protocol_constants.dart @@ -173,11 +173,10 @@ const String EDIT_REQUEST_ORGANIZE_DIRECTIVES = 'edit.organizeDirectives'; const String EDIT_REQUEST_ORGANIZE_DIRECTIVES_FILE = 'file'; const String EDIT_REQUEST_SORT_MEMBERS = 'edit.sortMembers'; const String EDIT_REQUEST_SORT_MEMBERS_FILE = 'file'; -const String EDIT_RESPONSE_DARTFIX_DESCRIPTION_OF_FIXES = 'descriptionOfFixes'; -const String EDIT_RESPONSE_DARTFIX_FIXES = 'fixes'; +const String EDIT_RESPONSE_DARTFIX_EDITS = 'edits'; const String EDIT_RESPONSE_DARTFIX_HAS_ERRORS = 'hasErrors'; -const String EDIT_RESPONSE_DARTFIX_OTHER_RECOMMENDATIONS = - 'otherRecommendations'; +const String EDIT_RESPONSE_DARTFIX_OTHER_SUGGESTIONS = 'otherSuggestions'; +const String EDIT_RESPONSE_DARTFIX_SUGGESTIONS = 'suggestions'; const String EDIT_RESPONSE_FORMAT_EDITS = 'edits'; const String EDIT_RESPONSE_FORMAT_SELECTION_LENGTH = 'selectionLength'; const String EDIT_RESPONSE_FORMAT_SELECTION_OFFSET = 'selectionOffset'; diff --git a/pkg/analysis_server/lib/protocol/protocol_generated.dart b/pkg/analysis_server/lib/protocol/protocol_generated.dart index cb7d9469bd3..3a0a480f94d 100644 --- a/pkg/analysis_server/lib/protocol/protocol_generated.dart +++ b/pkg/analysis_server/lib/protocol/protocol_generated.dart @@ -5931,6 +5931,105 @@ class ConvertMethodToGetterOptions extends RefactoringOptions } } +/** + * DartFixSuggestion + * + * { + * "description": String + * "location": optional Location + * } + * + * Clients may not extend, implement or mix-in this class. + */ +class DartFixSuggestion implements HasToJson { + String _description; + + Location _location; + + /** + * A human readable description of the suggested change. + */ + String get description => _description; + + /** + * A human readable description of the suggested change. + */ + void set description(String value) { + assert(value != null); + this._description = value; + } + + /** + * The location of the suggested change. + */ + Location get location => _location; + + /** + * The location of the suggested change. + */ + void set location(Location value) { + this._location = value; + } + + DartFixSuggestion(String description, {Location location}) { + this.description = description; + this.location = location; + } + + factory DartFixSuggestion.fromJson( + JsonDecoder jsonDecoder, String jsonPath, Object json) { + if (json == null) { + json = {}; + } + if (json is Map) { + String description; + if (json.containsKey("description")) { + description = jsonDecoder.decodeString( + jsonPath + ".description", json["description"]); + } else { + throw jsonDecoder.mismatch(jsonPath, "description"); + } + Location location; + if (json.containsKey("location")) { + location = new Location.fromJson( + jsonDecoder, jsonPath + ".location", json["location"]); + } + return new DartFixSuggestion(description, location: location); + } else { + throw jsonDecoder.mismatch(jsonPath, "DartFixSuggestion", json); + } + } + + @override + Map toJson() { + Map result = {}; + result["description"] = description; + if (location != null) { + result["location"] = location.toJson(); + } + return result; + } + + @override + String toString() => json.encode(toJson()); + + @override + bool operator ==(other) { + if (other is DartFixSuggestion) { + return description == other.description && location == other.location; + } + return false; + } + + @override + int get hashCode { + int hash = 0; + hash = JenkinsSmiHash.combine(hash, description.hashCode); + hash = JenkinsSmiHash.combine(hash, location.hashCode); + return JenkinsSmiHash.finish(hash); + } +} + /** * diagnostic.getDiagnostics params * @@ -6268,60 +6367,60 @@ class EditDartfixParams implements RequestParams { * edit.dartfix result * * { - * "descriptionOfFixes": List - * "otherRecommendations": List + * "suggestions": List + * "otherSuggestions": List * "hasErrors": bool - * "fixes": List + * "edits": List * } * * Clients may not extend, implement or mix-in this class. */ class EditDartfixResult implements ResponseResult { - List _descriptionOfFixes; + List _suggestions; - List _otherRecommendations; + List _otherSuggestions; bool _hasErrors; - List _fixes; + List _edits; /** - * A list of human readable changes made by applying the fixes. + * A list of recommended changes that can be automatically made by applying + * the 'edits' included in this response. */ - List get descriptionOfFixes => _descriptionOfFixes; + List get suggestions => _suggestions; /** - * A list of human readable changes made by applying the fixes. + * A list of recommended changes that can be automatically made by applying + * the 'edits' included in this response. */ - void set descriptionOfFixes(List value) { + void set suggestions(List value) { assert(value != null); - this._descriptionOfFixes = value; + this._suggestions = value; } /** - * A list of human readable recommended changes that cannot be made - * automatically. + * A list of recommended changes that could not be automatically made. */ - List get otherRecommendations => _otherRecommendations; + List get otherSuggestions => _otherSuggestions; /** - * A list of human readable recommended changes that cannot be made - * automatically. + * A list of recommended changes that could not be automatically made. */ - void set otherRecommendations(List value) { + void set otherSuggestions(List value) { assert(value != null); - this._otherRecommendations = value; + this._otherSuggestions = value; } /** * True if the analyzed source contains errors that might impact the - * correctness of the recommended fixes that can be automatically applied. + * correctness of the recommended changes that can be automatically applied. */ bool get hasErrors => _hasErrors; /** * True if the analyzed source contains errors that might impact the - * correctness of the recommended fixes that can be automatically applied. + * correctness of the recommended changes that can be automatically applied. */ void set hasErrors(bool value) { assert(value != null); @@ -6329,27 +6428,27 @@ class EditDartfixResult implements ResponseResult { } /** - * The suggested fixes. + * A list of source edits to apply the recommended changes. */ - List get fixes => _fixes; + List get edits => _edits; /** - * The suggested fixes. + * A list of source edits to apply the recommended changes. */ - void set fixes(List value) { + void set edits(List value) { assert(value != null); - this._fixes = value; + this._edits = value; } EditDartfixResult( - List descriptionOfFixes, - List otherRecommendations, + List suggestions, + List otherSuggestions, bool hasErrors, - List fixes) { - this.descriptionOfFixes = descriptionOfFixes; - this.otherRecommendations = otherRecommendations; + List edits) { + this.suggestions = suggestions; + this.otherSuggestions = otherSuggestions; this.hasErrors = hasErrors; - this.fixes = fixes; + this.edits = edits; } factory EditDartfixResult.fromJson( @@ -6358,23 +6457,25 @@ class EditDartfixResult implements ResponseResult { json = {}; } if (json is Map) { - List descriptionOfFixes; - if (json.containsKey("descriptionOfFixes")) { - descriptionOfFixes = jsonDecoder.decodeList( - jsonPath + ".descriptionOfFixes", - json["descriptionOfFixes"], - jsonDecoder.decodeString); + List suggestions; + if (json.containsKey("suggestions")) { + suggestions = jsonDecoder.decodeList( + jsonPath + ".suggestions", + json["suggestions"], + (String jsonPath, Object json) => + new DartFixSuggestion.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "descriptionOfFixes"); + throw jsonDecoder.mismatch(jsonPath, "suggestions"); } - List otherRecommendations; - if (json.containsKey("otherRecommendations")) { - otherRecommendations = jsonDecoder.decodeList( - jsonPath + ".otherRecommendations", - json["otherRecommendations"], - jsonDecoder.decodeString); + List otherSuggestions; + if (json.containsKey("otherSuggestions")) { + otherSuggestions = jsonDecoder.decodeList( + jsonPath + ".otherSuggestions", + json["otherSuggestions"], + (String jsonPath, Object json) => + new DartFixSuggestion.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "otherRecommendations"); + throw jsonDecoder.mismatch(jsonPath, "otherSuggestions"); } bool hasErrors; if (json.containsKey("hasErrors")) { @@ -6383,18 +6484,18 @@ class EditDartfixResult implements ResponseResult { } else { throw jsonDecoder.mismatch(jsonPath, "hasErrors"); } - List fixes; - if (json.containsKey("fixes")) { - fixes = jsonDecoder.decodeList( - jsonPath + ".fixes", - json["fixes"], + List edits; + if (json.containsKey("edits")) { + edits = jsonDecoder.decodeList( + jsonPath + ".edits", + json["edits"], (String jsonPath, Object json) => new SourceFileEdit.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "fixes"); + throw jsonDecoder.mismatch(jsonPath, "edits"); } return new EditDartfixResult( - descriptionOfFixes, otherRecommendations, hasErrors, fixes); + suggestions, otherSuggestions, hasErrors, edits); } else { throw jsonDecoder.mismatch(jsonPath, "edit.dartfix result", json); } @@ -6410,11 +6511,14 @@ class EditDartfixResult implements ResponseResult { @override Map toJson() { Map result = {}; - result["descriptionOfFixes"] = descriptionOfFixes; - result["otherRecommendations"] = otherRecommendations; + result["suggestions"] = + suggestions.map((DartFixSuggestion value) => value.toJson()).toList(); + result["otherSuggestions"] = otherSuggestions + .map((DartFixSuggestion value) => value.toJson()) + .toList(); result["hasErrors"] = hasErrors; - result["fixes"] = - fixes.map((SourceFileEdit value) => value.toJson()).toList(); + result["edits"] = + edits.map((SourceFileEdit value) => value.toJson()).toList(); return result; } @@ -6429,12 +6533,12 @@ class EditDartfixResult implements ResponseResult { @override bool operator ==(other) { if (other is EditDartfixResult) { - return listEqual(descriptionOfFixes, other.descriptionOfFixes, - (String a, String b) => a == b) && - listEqual(otherRecommendations, other.otherRecommendations, - (String a, String b) => a == b) && + return listEqual(suggestions, other.suggestions, + (DartFixSuggestion a, DartFixSuggestion b) => a == b) && + listEqual(otherSuggestions, other.otherSuggestions, + (DartFixSuggestion a, DartFixSuggestion b) => a == b) && hasErrors == other.hasErrors && - listEqual(fixes, other.fixes, + listEqual(edits, other.edits, (SourceFileEdit a, SourceFileEdit b) => a == b); } return false; @@ -6443,10 +6547,10 @@ class EditDartfixResult implements ResponseResult { @override int get hashCode { int hash = 0; - hash = JenkinsSmiHash.combine(hash, descriptionOfFixes.hashCode); - hash = JenkinsSmiHash.combine(hash, otherRecommendations.hashCode); + hash = JenkinsSmiHash.combine(hash, suggestions.hashCode); + hash = JenkinsSmiHash.combine(hash, otherSuggestions.hashCode); hash = JenkinsSmiHash.combine(hash, hasErrors.hashCode); - hash = JenkinsSmiHash.combine(hash, fixes.hashCode); + hash = JenkinsSmiHash.combine(hash, edits.hashCode); return JenkinsSmiHash.finish(hash); } } diff --git a/pkg/analysis_server/lib/src/edit/edit_dartfix.dart b/pkg/analysis_server/lib/src/edit/edit_dartfix.dart index 3a8e3449d0a..6673fb475f3 100644 --- a/pkg/analysis_server/lib/src/edit/edit_dartfix.dart +++ b/pkg/analysis_server/lib/src/edit/edit_dartfix.dart @@ -23,7 +23,7 @@ import 'package:analyzer/src/lint/linter_visitor.dart'; import 'package:analyzer/src/lint/registry.dart'; import 'package:analyzer/src/services/lint.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart' - show SourceChange, SourceEdit, SourceFileEdit; + show Location, SourceChange, SourceEdit, SourceFileEdit; import 'package:front_end/src/fasta/fasta_codes.dart'; import 'package:front_end/src/scanner/token.dart'; import 'package:source_span/src/span.dart'; @@ -34,14 +34,14 @@ class EditDartFix { final fixFolders = []; final fixFiles = []; - List descriptionOfFixes; - List otherRecommendations; + List suggestions; + List otherSuggestions; SourceChange sourceChange; EditDartFix(this.server, this.request); - void addFix(String description, SourceChange change) { - descriptionOfFixes.add(description); + void addFix(String description, Location location, SourceChange change) { + suggestions.add(new DartFixSuggestion(description, location: location)); for (SourceFileEdit fileEdit in change.edits) { for (SourceEdit sourceEdit in fileEdit.edits) { sourceChange.addEdit(fileEdit.file, fileEdit.fileStamp, sourceEdit); @@ -49,8 +49,9 @@ class EditDartFix { } } - void addRecommendation(String recommendation) { - otherRecommendations.add(recommendation); + void addRecommendation(String description, [Location location]) { + otherSuggestions + .add(new DartFixSuggestion(description, location: location)); } Future compute() async { @@ -125,8 +126,8 @@ class EditDartFix { for (String rootPath in contextManager.includedPaths) { resources.add(resourceProvider.getResource(rootPath)); } - descriptionOfFixes = []; - otherRecommendations = []; + suggestions = []; + otherSuggestions = []; sourceChange = new SourceChange('dartfix'); bool hasErrors = false; while (resources.isNotEmpty) { @@ -186,8 +187,8 @@ class EditDartFix { await fix.applyRemainingFixes(); } - return new EditDartfixResult(descriptionOfFixes, otherRecommendations, - hasErrors, sourceChange.edits) + return new EditDartfixResult( + suggestions, otherSuggestions, hasErrors, sourceChange.edits) .toResponse(request.id); } @@ -204,7 +205,6 @@ class EditDartFix { return false; } - final location = '${locationDescription(result, error.offset)}'; final dartContext = new DartFixContextImpl( new FixContextImpl( server.resourceProvider, result.driver, error, result.errors), @@ -212,12 +212,13 @@ class EditDartFix { result.unit); final processor = new FixProcessor(dartContext); Fix fix = await processor.computeFix(); + final location = locationFor(result, error.offset, error.length); if (fix != null) { - addFix('${fix.change.message} in $location', fix.change); + addFix(fix.change.message, location, fix.change); } else { // TODO(danrubel): Determine why the fix could not be applied // and report that in the description. - addRecommendation('Could not fix "${error.message}" in $location'); + addRecommendation('Could not fix "${error.message}"', location); } return true; } @@ -240,29 +241,11 @@ class EditDartFix { return false; } - /// Return a human readable description of the specified offset and file. - String locationDescription(AnalysisResult result, int offset) { - // TODO(danrubel): Pass the location back to the client along with the - // message indicating what was or was not automatically fixed - // rather than interpreting and integrating the location into the message. - final description = new StringBuffer(); - // Determine the relative path - for (Folder folder in fixFolders) { - if (folder.contains(result.path)) { - description.write(server.resourceProvider.pathContext - .relative(result.path, from: folder.path)); - break; - } - } - if (description.isEmpty) { - description.write(result.path); - } - // Determine the line and column number - if (offset >= 0) { - final loc = result.unit.lineInfo.getLocation(offset); - description.write(':${loc.lineNumber}'); - } - return description.toString(); + Location locationFor(AnalysisResult result, int offset, int length) { + final locInfo = result.unit.lineInfo.getLocation(offset); + final location = new Location( + result.path, offset, length, locInfo.lineNumber, locInfo.columnNumber); + return location; } } diff --git a/pkg/analysis_server/lib/src/edit/fix/prefer_int_literals_fix.dart b/pkg/analysis_server/lib/src/edit/fix/prefer_int_literals_fix.dart index 81758f9d243..52dda1f517f 100644 --- a/pkg/analysis_server/lib/src/edit/fix/prefer_int_literals_fix.dart +++ b/pkg/analysis_server/lib/src/edit/fix/prefer_int_literals_fix.dart @@ -23,18 +23,18 @@ class PreferIntLiteralsFix extends LinterFix { new EditDartFixAssistContext(dartFix, source, result.unit, literal)); List assists = await processor.computeAssist(DartAssistKind.CONVERT_TO_INT_LITERAL); - final location = dartFix.locationDescription(result, literal.offset); + final location = + dartFix.locationFor(result, literal.offset, literal.length); if (assists.isNotEmpty) { for (Assist assist in assists) { - dartFix.addFix( - 'Replace a double literal with an int literal in $location', - assist.change); + dartFix.addFix('Replace a double literal with an int literal', + location, assist.change); } } else { // TODO(danrubel): If assists is empty, then determine why // assist could not be performed and report that in the description. - dartFix.addRecommendation('Could not replace' - ' a double literal with an int literal in $location'); + dartFix.addRecommendation( + 'Could not replace a double literal with an int literal', location); } } } diff --git a/pkg/analysis_server/lib/src/edit/fix/prefer_mixin_fix.dart b/pkg/analysis_server/lib/src/edit/fix/prefer_mixin_fix.dart index 5e2aad1c85e..52f924a3744 100644 --- a/pkg/analysis_server/lib/src/edit/fix/prefer_mixin_fix.dart +++ b/pkg/analysis_server/lib/src/edit/fix/prefer_mixin_fix.dart @@ -41,11 +41,11 @@ class PreferMixinFix extends LinterFix { dartFix, elem.source, result.unit, declaration.name)); List assists = await processor .computeAssist(DartAssistKind.CONVERT_CLASS_TO_MIXIN); - final location = dartFix.locationDescription(result, elem.nameOffset); + final location = + dartFix.locationFor(result, elem.nameOffset, elem.nameLength); if (assists.isNotEmpty) { for (Assist assist in assists) { - dartFix.addFix( - 'Convert ${elem.displayName} to a mixin in $location', + dartFix.addFix('Convert ${elem.displayName} to a mixin', location, assist.change); } } else { @@ -53,7 +53,8 @@ class PreferMixinFix extends LinterFix { // assist could not be performed and report that in the description. dartFix.addRecommendation( 'Could not convert ${elem.displayName} to a mixin' - ' because the class contains a constructor in $location'); + ' because the class contains a constructor', + location); } } } diff --git a/pkg/analysis_server/test/integration/support/integration_test_methods.dart b/pkg/analysis_server/test/integration/support/integration_test_methods.dart index 3976d6c68e0..401723d8c7e 100644 --- a/pkg/analysis_server/test/integration/support/integration_test_methods.dart +++ b/pkg/analysis_server/test/integration/support/integration_test_methods.dart @@ -1517,23 +1517,24 @@ abstract class IntegrationTestMixin { * * Returns * - * descriptionOfFixes: List + * suggestions: List * - * A list of human readable changes made by applying the fixes. + * A list of recommended changes that can be automatically made by applying + * the 'edits' included in this response. * - * otherRecommendations: List + * otherSuggestions: List * - * A list of human readable recommended changes that cannot be made - * automatically. + * A list of recommended changes that could not be automatically made. * * hasErrors: bool * * True if the analyzed source contains errors that might impact the - * correctness of the recommended fixes that can be automatically applied. + * correctness of the recommended changes that can be automatically + * applied. * - * fixes: List + * edits: List * - * The suggested fixes. + * A list of source edits to apply the recommended changes. */ Future sendEditDartfix(List included) async { var params = new EditDartfixParams(included).toJson(); diff --git a/pkg/analysis_server/test/integration/support/protocol_matchers.dart b/pkg/analysis_server/test/integration/support/protocol_matchers.dart index db9505f2632..13ba9213204 100644 --- a/pkg/analysis_server/test/integration/support/protocol_matchers.dart +++ b/pkg/analysis_server/test/integration/support/protocol_matchers.dart @@ -297,6 +297,18 @@ final Matcher isContextData = "cacheEntryExceptions": isListOf(isString) })); +/** + * DartFixSuggestion + * + * { + * "description": String + * "location": optional Location + * } + */ +final Matcher isDartFixSuggestion = new LazyMatcher(() => new MatchesJsonObject( + "DartFixSuggestion", {"description": isString}, + optionalFields: {"location": isLocation})); + /** * Element * @@ -2131,18 +2143,18 @@ final Matcher isEditDartfixParams = new LazyMatcher(() => new MatchesJsonObject( * edit.dartfix result * * { - * "descriptionOfFixes": List - * "otherRecommendations": List + * "suggestions": List + * "otherSuggestions": List * "hasErrors": bool - * "fixes": List + * "edits": List * } */ final Matcher isEditDartfixResult = new LazyMatcher(() => new MatchesJsonObject("edit.dartfix result", { - "descriptionOfFixes": isListOf(isString), - "otherRecommendations": isListOf(isString), + "suggestions": isListOf(isDartFixSuggestion), + "otherSuggestions": isListOf(isDartFixSuggestion), "hasErrors": isBool, - "fixes": isListOf(isSourceFileEdit) + "edits": isListOf(isSourceFileEdit) })); /** diff --git a/pkg/analysis_server/tool/spec/generated/java/types/DartFixSuggestion.java b/pkg/analysis_server/tool/spec/generated/java/types/DartFixSuggestion.java new file mode 100644 index 00000000000..7a106f82f01 --- /dev/null +++ b/pkg/analysis_server/tool/spec/generated/java/types/DartFixSuggestion.java @@ -0,0 +1,128 @@ +/* + * Copyright (c) 2018, 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 file has been automatically generated. Please do not edit it manually. + * To regenerate the file, use the script "pkg/analysis_server/tool/spec/generate_files". + */ +package org.dartlang.analysis.server.protocol; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import com.google.common.collect.Lists; +import com.google.dart.server.utilities.general.JsonUtilities; +import com.google.dart.server.utilities.general.ObjectUtilities; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonPrimitive; +import org.apache.commons.lang3.builder.HashCodeBuilder; +import java.util.ArrayList; +import java.util.Iterator; +import org.apache.commons.lang3.StringUtils; + +/** + * A suggestion from an edit.dartfix request. + * + * @coverage dart.server.generated.types + */ +@SuppressWarnings("unused") +public class DartFixSuggestion { + + public static final DartFixSuggestion[] EMPTY_ARRAY = new DartFixSuggestion[0]; + + public static final List EMPTY_LIST = Lists.newArrayList(); + + /** + * A human readable description of the suggested change. + */ + private final String description; + + /** + * The location of the suggested change. + */ + private final Location location; + + /** + * Constructor for {@link DartFixSuggestion}. + */ + public DartFixSuggestion(String description, Location location) { + this.description = description; + this.location = location; + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof DartFixSuggestion) { + DartFixSuggestion other = (DartFixSuggestion) obj; + return + ObjectUtilities.equals(other.description, description) && + ObjectUtilities.equals(other.location, location); + } + return false; + } + + public static DartFixSuggestion fromJson(JsonObject jsonObject) { + String description = jsonObject.get("description").getAsString(); + Location location = jsonObject.get("location") == null ? null : Location.fromJson(jsonObject.get("location").getAsJsonObject()); + return new DartFixSuggestion(description, location); + } + + public static List fromJsonArray(JsonArray jsonArray) { + if (jsonArray == null) { + return EMPTY_LIST; + } + ArrayList list = new ArrayList(jsonArray.size()); + Iterator iterator = jsonArray.iterator(); + while (iterator.hasNext()) { + list.add(fromJson(iterator.next().getAsJsonObject())); + } + return list; + } + + /** + * A human readable description of the suggested change. + */ + public String getDescription() { + return description; + } + + /** + * The location of the suggested change. + */ + public Location getLocation() { + return location; + } + + @Override + public int hashCode() { + HashCodeBuilder builder = new HashCodeBuilder(); + builder.append(description); + builder.append(location); + return builder.toHashCode(); + } + + public JsonObject toJson() { + JsonObject jsonObject = new JsonObject(); + jsonObject.addProperty("description", description); + if (location != null) { + jsonObject.add("location", location.toJson()); + } + return jsonObject; + } + + @Override + public String toString() { + StringBuilder builder = new StringBuilder(); + builder.append("["); + builder.append("description="); + builder.append(description + ", "); + builder.append("location="); + builder.append(location); + builder.append("]"); + return builder.toString(); + } + +} diff --git a/pkg/analysis_server/tool/spec/generated/java/types/RuntimeCompletionExpression.java b/pkg/analysis_server/tool/spec/generated/java/types/RuntimeCompletionExpression.java index eb6b0f0400c..b2d6b705f59 100644 --- a/pkg/analysis_server/tool/spec/generated/java/types/RuntimeCompletionExpression.java +++ b/pkg/analysis_server/tool/spec/generated/java/types/RuntimeCompletionExpression.java @@ -24,9 +24,9 @@ import java.util.Iterator; import org.apache.commons.lang3.StringUtils; /** - * An expression for which we want to know its runtime type. In expressions like `a.b.c.where((e) - * => e.^)` we want to know the runtime type of `a.b.c` to enforce it statically at the time - * when we compute completion suggestions, and get better type for `e`. + * An expression for which we want to know its runtime type. In expressions like 'a.b.c.where((e) + * => e.^)' we want to know the runtime type of 'a.b.c' to enforce it statically at the time + * when we compute completion suggestions, and get better type for 'e'. * * @coverage dart.server.generated.types */ diff --git a/pkg/analysis_server/tool/spec/spec_input.html b/pkg/analysis_server/tool/spec/spec_input.html index c8f98641c6c..bb90c8a424f 100644 --- a/pkg/analysis_server/tool/spec/spec_input.html +++ b/pkg/analysis_server/tool/spec/spec_input.html @@ -1958,35 +1958,36 @@ - + - String + DartFixSuggestion

- A list of human readable changes made by applying the fixes. + A list of recommended changes that can be automatically made + by applying the 'edits' included in this response.

- + - String + DartFixSuggestion

- A list of human readable recommended changes that cannot be made automatically. + A list of recommended changes that could not be automatically made.

bool

True if the analyzed source contains errors that might impact the correctness - of the recommended fixes that can be automatically applied. + of the recommended changes that can be automatically applied.

- + SourceFileEdit

- The suggested fixes. + A list of source edits to apply the recommended changes.

@@ -3313,9 +3314,9 @@

An expression for which we want to know its runtime type. - In expressions like `a.b.c.where((e) => e.^)` we want to know the - runtime type of `a.b.c` to enforce it statically at the time when we - compute completion suggestions, and get better type for `e`. + In expressions like 'a.b.c.where((e) => e.^)' we want to know the + runtime type of 'a.b.c' to enforce it statically at the time when we + compute completion suggestions, and get better type for 'e'.

@@ -4232,6 +4233,25 @@ request.

+ +

+ A suggestion from an edit.dartfix request. +

+ + + String +

+ A human readable description of the suggested change. +

+ + + Location +

+ The location of the suggested change. +

+ + +

A single result from a search request. diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart index 9d85d08bdd2..1280460f470 100644 --- a/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart +++ b/pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart @@ -173,11 +173,10 @@ const String EDIT_REQUEST_ORGANIZE_DIRECTIVES = 'edit.organizeDirectives'; const String EDIT_REQUEST_ORGANIZE_DIRECTIVES_FILE = 'file'; const String EDIT_REQUEST_SORT_MEMBERS = 'edit.sortMembers'; const String EDIT_REQUEST_SORT_MEMBERS_FILE = 'file'; -const String EDIT_RESPONSE_DARTFIX_DESCRIPTION_OF_FIXES = 'descriptionOfFixes'; -const String EDIT_RESPONSE_DARTFIX_FIXES = 'fixes'; +const String EDIT_RESPONSE_DARTFIX_EDITS = 'edits'; const String EDIT_RESPONSE_DARTFIX_HAS_ERRORS = 'hasErrors'; -const String EDIT_RESPONSE_DARTFIX_OTHER_RECOMMENDATIONS = - 'otherRecommendations'; +const String EDIT_RESPONSE_DARTFIX_OTHER_SUGGESTIONS = 'otherSuggestions'; +const String EDIT_RESPONSE_DARTFIX_SUGGESTIONS = 'suggestions'; const String EDIT_RESPONSE_FORMAT_EDITS = 'edits'; const String EDIT_RESPONSE_FORMAT_SELECTION_LENGTH = 'selectionLength'; const String EDIT_RESPONSE_FORMAT_SELECTION_OFFSET = 'selectionOffset'; diff --git a/pkg/analysis_server_client/lib/src/protocol/protocol_generated.dart b/pkg/analysis_server_client/lib/src/protocol/protocol_generated.dart index 2bbf169d208..9fabb4c83ad 100644 --- a/pkg/analysis_server_client/lib/src/protocol/protocol_generated.dart +++ b/pkg/analysis_server_client/lib/src/protocol/protocol_generated.dart @@ -5931,6 +5931,105 @@ class ConvertMethodToGetterOptions extends RefactoringOptions } } +/** + * DartFixSuggestion + * + * { + * "description": String + * "location": optional Location + * } + * + * Clients may not extend, implement or mix-in this class. + */ +class DartFixSuggestion implements HasToJson { + String _description; + + Location _location; + + /** + * A human readable description of the suggested change. + */ + String get description => _description; + + /** + * A human readable description of the suggested change. + */ + void set description(String value) { + assert(value != null); + this._description = value; + } + + /** + * The location of the suggested change. + */ + Location get location => _location; + + /** + * The location of the suggested change. + */ + void set location(Location value) { + this._location = value; + } + + DartFixSuggestion(String description, {Location location}) { + this.description = description; + this.location = location; + } + + factory DartFixSuggestion.fromJson( + JsonDecoder jsonDecoder, String jsonPath, Object json) { + if (json == null) { + json = {}; + } + if (json is Map) { + String description; + if (json.containsKey("description")) { + description = jsonDecoder.decodeString( + jsonPath + ".description", json["description"]); + } else { + throw jsonDecoder.mismatch(jsonPath, "description"); + } + Location location; + if (json.containsKey("location")) { + location = new Location.fromJson( + jsonDecoder, jsonPath + ".location", json["location"]); + } + return new DartFixSuggestion(description, location: location); + } else { + throw jsonDecoder.mismatch(jsonPath, "DartFixSuggestion", json); + } + } + + @override + Map toJson() { + Map result = {}; + result["description"] = description; + if (location != null) { + result["location"] = location.toJson(); + } + return result; + } + + @override + String toString() => json.encode(toJson()); + + @override + bool operator ==(other) { + if (other is DartFixSuggestion) { + return description == other.description && location == other.location; + } + return false; + } + + @override + int get hashCode { + int hash = 0; + hash = JenkinsSmiHash.combine(hash, description.hashCode); + hash = JenkinsSmiHash.combine(hash, location.hashCode); + return JenkinsSmiHash.finish(hash); + } +} + /** * diagnostic.getDiagnostics params * @@ -6268,60 +6367,60 @@ class EditDartfixParams implements RequestParams { * edit.dartfix result * * { - * "descriptionOfFixes": List - * "otherRecommendations": List + * "suggestions": List + * "otherSuggestions": List * "hasErrors": bool - * "fixes": List + * "edits": List * } * * Clients may not extend, implement or mix-in this class. */ class EditDartfixResult implements ResponseResult { - List _descriptionOfFixes; + List _suggestions; - List _otherRecommendations; + List _otherSuggestions; bool _hasErrors; - List _fixes; + List _edits; /** - * A list of human readable changes made by applying the fixes. + * A list of recommended changes that can be automatically made by applying + * the 'edits' included in this response. */ - List get descriptionOfFixes => _descriptionOfFixes; + List get suggestions => _suggestions; /** - * A list of human readable changes made by applying the fixes. + * A list of recommended changes that can be automatically made by applying + * the 'edits' included in this response. */ - void set descriptionOfFixes(List value) { + void set suggestions(List value) { assert(value != null); - this._descriptionOfFixes = value; + this._suggestions = value; } /** - * A list of human readable recommended changes that cannot be made - * automatically. + * A list of recommended changes that could not be automatically made. */ - List get otherRecommendations => _otherRecommendations; + List get otherSuggestions => _otherSuggestions; /** - * A list of human readable recommended changes that cannot be made - * automatically. + * A list of recommended changes that could not be automatically made. */ - void set otherRecommendations(List value) { + void set otherSuggestions(List value) { assert(value != null); - this._otherRecommendations = value; + this._otherSuggestions = value; } /** * True if the analyzed source contains errors that might impact the - * correctness of the recommended fixes that can be automatically applied. + * correctness of the recommended changes that can be automatically applied. */ bool get hasErrors => _hasErrors; /** * True if the analyzed source contains errors that might impact the - * correctness of the recommended fixes that can be automatically applied. + * correctness of the recommended changes that can be automatically applied. */ void set hasErrors(bool value) { assert(value != null); @@ -6329,27 +6428,27 @@ class EditDartfixResult implements ResponseResult { } /** - * The suggested fixes. + * A list of source edits to apply the recommended changes. */ - List get fixes => _fixes; + List get edits => _edits; /** - * The suggested fixes. + * A list of source edits to apply the recommended changes. */ - void set fixes(List value) { + void set edits(List value) { assert(value != null); - this._fixes = value; + this._edits = value; } EditDartfixResult( - List descriptionOfFixes, - List otherRecommendations, + List suggestions, + List otherSuggestions, bool hasErrors, - List fixes) { - this.descriptionOfFixes = descriptionOfFixes; - this.otherRecommendations = otherRecommendations; + List edits) { + this.suggestions = suggestions; + this.otherSuggestions = otherSuggestions; this.hasErrors = hasErrors; - this.fixes = fixes; + this.edits = edits; } factory EditDartfixResult.fromJson( @@ -6358,23 +6457,25 @@ class EditDartfixResult implements ResponseResult { json = {}; } if (json is Map) { - List descriptionOfFixes; - if (json.containsKey("descriptionOfFixes")) { - descriptionOfFixes = jsonDecoder.decodeList( - jsonPath + ".descriptionOfFixes", - json["descriptionOfFixes"], - jsonDecoder.decodeString); + List suggestions; + if (json.containsKey("suggestions")) { + suggestions = jsonDecoder.decodeList( + jsonPath + ".suggestions", + json["suggestions"], + (String jsonPath, Object json) => + new DartFixSuggestion.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "descriptionOfFixes"); + throw jsonDecoder.mismatch(jsonPath, "suggestions"); } - List otherRecommendations; - if (json.containsKey("otherRecommendations")) { - otherRecommendations = jsonDecoder.decodeList( - jsonPath + ".otherRecommendations", - json["otherRecommendations"], - jsonDecoder.decodeString); + List otherSuggestions; + if (json.containsKey("otherSuggestions")) { + otherSuggestions = jsonDecoder.decodeList( + jsonPath + ".otherSuggestions", + json["otherSuggestions"], + (String jsonPath, Object json) => + new DartFixSuggestion.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "otherRecommendations"); + throw jsonDecoder.mismatch(jsonPath, "otherSuggestions"); } bool hasErrors; if (json.containsKey("hasErrors")) { @@ -6383,18 +6484,18 @@ class EditDartfixResult implements ResponseResult { } else { throw jsonDecoder.mismatch(jsonPath, "hasErrors"); } - List fixes; - if (json.containsKey("fixes")) { - fixes = jsonDecoder.decodeList( - jsonPath + ".fixes", - json["fixes"], + List edits; + if (json.containsKey("edits")) { + edits = jsonDecoder.decodeList( + jsonPath + ".edits", + json["edits"], (String jsonPath, Object json) => new SourceFileEdit.fromJson(jsonDecoder, jsonPath, json)); } else { - throw jsonDecoder.mismatch(jsonPath, "fixes"); + throw jsonDecoder.mismatch(jsonPath, "edits"); } return new EditDartfixResult( - descriptionOfFixes, otherRecommendations, hasErrors, fixes); + suggestions, otherSuggestions, hasErrors, edits); } else { throw jsonDecoder.mismatch(jsonPath, "edit.dartfix result", json); } @@ -6410,11 +6511,14 @@ class EditDartfixResult implements ResponseResult { @override Map toJson() { Map result = {}; - result["descriptionOfFixes"] = descriptionOfFixes; - result["otherRecommendations"] = otherRecommendations; + result["suggestions"] = + suggestions.map((DartFixSuggestion value) => value.toJson()).toList(); + result["otherSuggestions"] = otherSuggestions + .map((DartFixSuggestion value) => value.toJson()) + .toList(); result["hasErrors"] = hasErrors; - result["fixes"] = - fixes.map((SourceFileEdit value) => value.toJson()).toList(); + result["edits"] = + edits.map((SourceFileEdit value) => value.toJson()).toList(); return result; } @@ -6429,12 +6533,12 @@ class EditDartfixResult implements ResponseResult { @override bool operator ==(other) { if (other is EditDartfixResult) { - return listEqual(descriptionOfFixes, other.descriptionOfFixes, - (String a, String b) => a == b) && - listEqual(otherRecommendations, other.otherRecommendations, - (String a, String b) => a == b) && + return listEqual(suggestions, other.suggestions, + (DartFixSuggestion a, DartFixSuggestion b) => a == b) && + listEqual(otherSuggestions, other.otherSuggestions, + (DartFixSuggestion a, DartFixSuggestion b) => a == b) && hasErrors == other.hasErrors && - listEqual(fixes, other.fixes, + listEqual(edits, other.edits, (SourceFileEdit a, SourceFileEdit b) => a == b); } return false; @@ -6443,10 +6547,10 @@ class EditDartfixResult implements ResponseResult { @override int get hashCode { int hash = 0; - hash = JenkinsSmiHash.combine(hash, descriptionOfFixes.hashCode); - hash = JenkinsSmiHash.combine(hash, otherRecommendations.hashCode); + hash = JenkinsSmiHash.combine(hash, suggestions.hashCode); + hash = JenkinsSmiHash.combine(hash, otherSuggestions.hashCode); hash = JenkinsSmiHash.combine(hash, hasErrors.hashCode); - hash = JenkinsSmiHash.combine(hash, fixes.hashCode); + hash = JenkinsSmiHash.combine(hash, edits.hashCode); return JenkinsSmiHash.finish(hash); } } diff --git a/pkg/dartfix/lib/src/driver.dart b/pkg/dartfix/lib/src/driver.dart index 80b6c11d105..80e7a05ac49 100644 --- a/pkg/dartfix/lib/src/driver.dart +++ b/pkg/dartfix/lib/src/driver.dart @@ -122,25 +122,23 @@ class Driver { } Future applyFixes(EditDartfixResult result) async { - showDescriptions('Recommended changes', result.descriptionOfFixes); - showDescriptions( - 'Recommended changes that cannot not be automatically applied', - result.otherRecommendations, - ); - if (result.descriptionOfFixes.isEmpty) { + showDescriptions('Recommended changes', result.suggestions); + showDescriptions('Recommended changes that cannot be automatically applied', + result.otherSuggestions); + if (result.suggestions.isEmpty) { logger.stdout(''); - logger.stdout(result.otherRecommendations.isNotEmpty - ? 'No recommended changes that cannot be automatically applied.' + logger.stdout(result.otherSuggestions.isNotEmpty + ? 'None of the recommended changes can be automatically applied.' : 'No recommended changes.'); return; } logger.stdout(''); logger.stdout(ansi.emphasized('Files to be changed:')); - for (SourceFileEdit fileEdit in result.fixes) { + for (SourceFileEdit fileEdit in result.edits) { logger.stdout(' ${_relativePath(fileEdit.file)}'); } if (shouldApplyChanges(result)) { - for (SourceFileEdit fileEdit in result.fixes) { + for (SourceFileEdit fileEdit in result.edits) { final file = new File(fileEdit.file); String code = await file.readAsString(); for (SourceEdit edit in fileEdit.edits) { @@ -152,13 +150,16 @@ class Driver { } } - void showDescriptions(String title, List descriptions) { - if (descriptions.isNotEmpty) { + void showDescriptions(String title, List suggestions) { + if (suggestions.isNotEmpty) { logger.stdout(''); logger.stdout(ansi.emphasized('$title:')); - List sorted = new List.from(descriptions)..sort(); - for (String line in sorted) { - logger.stdout(' $line'); + List sorted = new List.from(suggestions) + ..sort(compareSuggestions); + for (DartFixSuggestion suggestion in sorted) { + Location loc = suggestion.location; + logger.stdout(' ${_toSentenceFragment(suggestion.description)}' + '${loc == null ? "" : " • ${loc.startLine}:${loc.startColumn}"}'); } } } @@ -327,6 +328,14 @@ class Driver { } } + int compareSuggestions(DartFixSuggestion s1, DartFixSuggestion s2) { + int result = s1.description.compareTo(s2.description); + if (result != 0) { + return result; + } + return (s2.location?.offset ?? 0) - (s1.location?.offset ?? 0); + } + bool shouldFilterError(AnalysisError error) { // Do not show TODOs or errors that will be automatically fixed.