[anlaysis_server] Improve the error text/codes for property editor APIs

Change-Id: If3ad0fd11cc661e88104444afd14a5d79374cc1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406360
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Elliott Brooks <[email protected]>
Commit-Queue: Brian Wilkerson <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/constants.dart

@@ -323,6 +323,21 @@ abstract final class ServerErrorCodes {
   /// General state error.
   static const StateError = ErrorCodes(-32015);
 
+  /// A request was made that requires use of workspace/applyEdit but the
+  /// current editor does not support it.
+  static const EditsUnsupportedByEditor = ErrorCodes(-32016);
+
+  /// An editArgument request tried to modify an invocation at a position where
+  /// there was no invocation.
+  static const EditArgumentInvalidPosition = ErrorCodes(-32017);
+
+  /// An editArgument request tried to modify a parameter that does not exist or
+  /// is not editable.
+  static const EditArgumentInvalidParameter = ErrorCodes(-32018);
+
+  /// An editArgument request tried to set an argument value that is not valid.
+  static const EditArgumentInvalidValue = ErrorCodes(-32019);
+
   /// An error raised when the server detects that the server and client are out
   /// of sync and cannot recover. For example if a textDocument/didChange notification
   /// has invalid offsets, suggesting the client and server have become out of sync

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart

@@ -52,7 +52,7 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
 
     if (!editorClientCapabilities.applyEdit) {
       return error(
-        ErrorCodes.RequestFailed,
+        ServerErrorCodes.EditsUnsupportedByEditor,
         'The connected editor does not support applying edits',
       );
     }
@@ -86,8 +86,8 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
       var invocation = getInvocationInfo(result, offset);
       if (invocation == null) {
         return error(
-          ErrorCodes.RequestFailed,
-          'No invocation was found at the position',
+          ServerErrorCodes.EditArgumentInvalidPosition,
+          'No invocation was found at the provided position',
         );
       }
 
@@ -109,8 +109,8 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
       );
       if (parameter == null) {
         return error(
-          ErrorCodes.RequestFailed,
-          'Parameter "$parameterName" was not found in ${parameters.map((p) => p.name3).join(', ')}',
+          ServerErrorCodes.EditArgumentInvalidParameter,
+          "The parameter '$parameterName' was not found in this invocation. The available parameters are ${parameters.map((p) => p.name3).join(', ')}",
         );
       }
 
@@ -129,9 +129,12 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
         // This should never happen unless a client is broken, because either
         // they should have failed the version check, or they would've already
         // known this argument was not editable.
+        var notEditableReasonLower =
+            notEditableReason.substring(0, 1).toLowerCase() +
+            notEditableReason.substring(1);
         return error(
-          ErrorCodes.RequestFailed,
-          "Parameter '$parameterName' is not editable: $notEditableReason",
+          ServerErrorCodes.EditArgumentInvalidParameter,
+          "The parameter '$parameterName' is not editable because $notEditableReasonLower",
         );
       }
 
@@ -183,8 +186,8 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
         return success('null');
       } else {
         return error(
-          ErrorCodes.RequestFailed,
-          'Value for non-nullable parameter "${edit.name}" cannot be null',
+          ServerErrorCodes.EditArgumentInvalidValue,
+          "The value for the parameter '${edit.name}' cannot be null",
         );
       }
     }
@@ -213,14 +216,14 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
         return success(value.toString());
       } else {
         return error(
-          ErrorCodes.RequestFailed,
-          'Value for parameter "${edit.name}" should be one of ${allowedValues.map((v) => '"$v"').join(', ')} but was "$value"',
+          ServerErrorCodes.EditArgumentInvalidValue,
+          "The value for the parameter '${edit.name}' should be one of ${allowedValues.map((v) => "'$v'").join(', ')} but was '$value'",
         );
       }
     } else {
       return error(
-        ErrorCodes.RequestFailed,
-        'Value for parameter "${edit.name}" should be $type but was ${value.runtimeType}',
+        ServerErrorCodes.EditArgumentInvalidValue,
+        "The value for the parameter '${edit.name}' should be $type but was ${value.runtimeType}",
       );
     }
   }
@@ -312,7 +315,7 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
     if (editResponse.error != null) {
       return error(
         ServerErrorCodes.ClientFailedToApplyEdit,
-        'Client failed to apply workspace edit: $editDescription',
+        "The editor failed to apply the workspace edit '$editDescription'",
         editResponse.error.toString(),
       );
     }
@@ -331,7 +334,7 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
       // changed.
       return error(
         ServerErrorCodes.ClientFailedToApplyEdit,
-        'Client did not apply workspace edit: $editDescription '
+        "The editor did not apply the workspace edit '$editDescription' "
         '(reason: ${failureReason ?? 'not given'})',
         workspaceEdit.toString(),
       );

pkg/analysis_server/test/shared/shared_edit_argument_tests.dart

@@ -5,6 +5,7 @@
 import 'dart:async';
 
 import 'package:analysis_server/lsp_protocol/protocol.dart';
+import 'package:analysis_server/src/lsp/constants.dart';
 import 'package:analyzer/src/test_utilities/test_code_format.dart';
 import 'package:test/test.dart';
 
@@ -225,9 +226,10 @@ mixin SharedEditArgumentTests
       params: '([int? x, int y = 10, int? z])',
       originalArgs: '(1)',
       edit: ArgumentEdit(name: 'z', newValue: 2),
+      errorCode: ServerErrorCodes.EditArgumentInvalidParameter,
       message:
-          "Parameter 'z' is not editable: "
-          "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.",
+          "The parameter 'z' is not editable because "
+          "a value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.",
     );
   }
 
@@ -236,9 +238,10 @@ mixin SharedEditArgumentTests
       params: '([int? x = 10, int? y])',
       originalArgs: '()',
       edit: ArgumentEdit(name: 'y', newValue: 2),
+      errorCode: ServerErrorCodes.EditArgumentInvalidParameter,
       message:
-          "Parameter 'y' is not editable: "
-          "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
+          "The parameter 'y' is not editable because "
+          "a value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
     );
   }
 
@@ -268,9 +271,10 @@ mixin SharedEditArgumentTests
       params: '(int? x, int? y, int? z)',
       originalArgs: '(1)',
       edit: ArgumentEdit(name: 'z', newValue: 2),
+      errorCode: ServerErrorCodes.EditArgumentInvalidParameter,
       message:
-          "Parameter 'z' is not editable: "
-          "A value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.",
+          "The parameter 'z' is not editable because "
+          "a value for the 3rd parameter can't be added until a value for all preceding positional parameters have been added.",
     );
   }
 
@@ -280,9 +284,10 @@ mixin SharedEditArgumentTests
       params: '(int? x, int? y)',
       originalArgs: '()',
       edit: ArgumentEdit(name: 'y', newValue: 2),
+      errorCode: ServerErrorCodes.EditArgumentInvalidParameter,
       message:
-          "Parameter 'y' is not editable: "
-          "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
+          "The parameter 'y' is not editable because "
+          "a value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
     );
   }
 
@@ -292,9 +297,10 @@ mixin SharedEditArgumentTests
       params: '(int? x, int? y, { int? z })',
       originalArgs: '(z: 1)',
       edit: ArgumentEdit(name: 'y', newValue: 2),
+      errorCode: ServerErrorCodes.EditArgumentInvalidParameter,
       message:
-          "Parameter 'y' is not editable: "
-          "A value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
+          "The parameter 'y' is not editable because "
+          "a value for the 2nd parameter can't be added until a value for all preceding positional parameters have been added.",
     );
   }
 
@@ -358,7 +364,8 @@ mixin SharedEditArgumentTests
       params: '({ bool? x })',
       originalArgs: '(x: true)',
       edit: ArgumentEdit(name: 'x', newValue: 'invalid'),
-      message: 'Value for parameter "x" should be bool? but was String',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' should be bool? but was String",
     );
   }
 
@@ -376,7 +383,8 @@ mixin SharedEditArgumentTests
       params: '({ required bool x })',
       originalArgs: '(x: true)',
       edit: ArgumentEdit(name: 'x'),
-      message: 'Value for non-nullable parameter "x" cannot be null',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' cannot be null",
     );
   }
 
@@ -403,7 +411,9 @@ mixin SharedEditArgumentTests
       params: '({ double? x })',
       originalArgs: '(x: 1.1)',
       edit: ArgumentEdit(name: 'x', newValue: 'invalid'),
-      message: 'Value for parameter "x" should be double? but was String',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message:
+          "The value for the parameter 'x' should be double? but was String",
     );
   }
 
@@ -421,7 +431,8 @@ mixin SharedEditArgumentTests
       params: '({ required double x })',
       originalArgs: '(x: 1.0)',
       edit: ArgumentEdit(name: 'x'),
-      message: 'Value for non-nullable parameter "x" cannot be null',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' cannot be null",
     );
   }
 
@@ -467,8 +478,9 @@ mixin SharedEditArgumentTests
       params: '({ E? x })',
       originalArgs: '(x: E.one)',
       edit: ArgumentEdit(name: 'x', newValue: 'invalid'),
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
       message:
-          'Value for parameter "x" should be one of "E.one", "E.two" but was "invalid"',
+          "The value for the parameter 'x' should be one of 'E.one', 'E.two' but was 'invalid'",
     );
   }
 
@@ -488,7 +500,8 @@ mixin SharedEditArgumentTests
       params: '({ required E x })',
       originalArgs: '(x: E.one)',
       edit: ArgumentEdit(name: 'x'),
-      message: 'Value for non-nullable parameter "x" cannot be null',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' cannot be null",
     );
   }
 
@@ -520,7 +533,8 @@ const myConst = E.one;
       params: '({ int? x })',
       originalArgs: '(x: 1)',
       edit: ArgumentEdit(name: 'x', newValue: 'invalid'),
-      message: 'Value for parameter "x" should be int? but was String',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' should be int? but was String",
     );
   }
 
@@ -538,7 +552,8 @@ const myConst = E.one;
       params: '({ required int x })',
       originalArgs: '(x: 1)',
       edit: ArgumentEdit(name: 'x'),
-      message: 'Value for non-nullable parameter "x" cannot be null',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' cannot be null",
     );
   }
 
@@ -592,7 +607,8 @@ const myConst = E.one;
       params: '({ String? x })',
       originalArgs: "(x: 'a')",
       edit: ArgumentEdit(name: 'x', newValue: 123),
-      message: 'Value for parameter "x" should be String? but was int',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' should be String? but was int",
     );
   }
 
@@ -619,7 +635,8 @@ const myConst = E.one;
       params: '({ required String x })',
       originalArgs: "(x: 'a')",
       edit: ArgumentEdit(name: 'x'),
-      message: 'Value for non-nullable parameter "x" cannot be null',
+      errorCode: ServerErrorCodes.EditArgumentInvalidValue,
+      message: "The value for the parameter 'x' cannot be null",
     );
   }
 
@@ -720,6 +737,7 @@ const myConst = E.one;
       params: '({ required String x })',
       originalArgs: "(x: 'a')",
       edit: ArgumentEdit(name: 'x'),
+      errorCode: ServerErrorCodes.EditsUnsupportedByEditor,
       message: 'The connected editor does not support applying edits',
     );
   }
@@ -753,6 +771,7 @@ const myConst = E.one;
     required String params,
     required String originalArgs,
     required ArgumentEdit edit,
+    required ErrorCodes errorCode,
     required String message,
     String? additionalCode,
   }) async {
@@ -777,7 +796,7 @@ class MyWidget extends StatelessWidget {
 
     await expectLater(
       editArgument(testFileUri, code.position.position, edit),
-      throwsA(isResponseError(ErrorCodes.RequestFailed, message: message)),
+      throwsA(isResponseError(errorCode, message: message)),
     );
   }