Handle generic types in LSP spec parsing
The regex wasn't picking up generate type args (in interface definitions or usages in field definitions). This adds <> characters to the regex and handles them in mapping of types (as well as adds some additional tests to cover this).
Change-Id: I8187b3d2abb2b35c0d7638839e3116700cfbf9e0
Reviewed-on: https://dart-review.googlesource.com/c/81004
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Danny Tuppeny <dantup@google.com>
diff --git a/pkg/analysis_server/lib/lsp_protocol/protocol_generated.dart b/pkg/analysis_server/lib/lsp_protocol/protocol_generated.dart
index 1c32810..d6fb0e5 100644
--- a/pkg/analysis_server/lib/lsp_protocol/protocol_generated.dart
+++ b/pkg/analysis_server/lib/lsp_protocol/protocol_generated.dart
@@ -3586,7 +3586,7 @@
}
class NotificationMessage implements Message {
- NotificationMessage(this.method, this.jsonrpc) {
+ NotificationMessage(this.method, this.params, this.jsonrpc) {
if (method == null) {
throw 'method is required but was not provided';
}
@@ -3596,8 +3596,14 @@
}
factory NotificationMessage.fromJson(Map<String, dynamic> json) {
final method = json['method'];
+ final params = (json['params'] is List &&
+ (json['params'].length == 0 ||
+ json['params'].every((item) => true)))
+ ? new Either2<List<dynamic>, dynamic>.t1(
+ json['params']?.map((item) => item)?.cast<dynamic>()?.toList())
+ : (json['params']);
final jsonrpc = json['jsonrpc'];
- return new NotificationMessage(method, jsonrpc);
+ return new NotificationMessage(method, params, jsonrpc);
}
final String jsonrpc;
@@ -3605,9 +3611,15 @@
/// The method to be invoked.
final String method;
+ /// The notification's params.
+ final Either2<List<dynamic>, dynamic> params;
+
Map<String, dynamic> toJson() {
Map<String, dynamic> __result = {};
__result['method'] = method ?? (throw 'method is required but was not set');
+ if (params != null) {
+ __result['params'] = params;
+ }
__result['jsonrpc'] =
jsonrpc ?? (throw 'jsonrpc is required but was not set');
return __result;
@@ -4140,7 +4152,7 @@
}
class RequestMessage implements Message {
- RequestMessage(this.id, this.method, this.jsonrpc) {
+ RequestMessage(this.id, this.method, this.params, this.jsonrpc) {
if (id == null) {
throw 'id is required but was not provided';
}
@@ -4158,8 +4170,14 @@
? new Either2<num, String>.t2(json['id'])
: (throw '''${json['id']} was not one of (number, string)'''));
final method = json['method'];
+ final params = (json['params'] is List &&
+ (json['params'].length == 0 ||
+ json['params'].every((item) => true)))
+ ? new Either2<List<dynamic>, dynamic>.t1(
+ json['params']?.map((item) => item)?.cast<dynamic>()?.toList())
+ : (json['params']);
final jsonrpc = json['jsonrpc'];
- return new RequestMessage(id, method, jsonrpc);
+ return new RequestMessage(id, method, params, jsonrpc);
}
/// The request id.
@@ -4169,10 +4187,16 @@
/// The method to be invoked.
final String method;
+ /// The method's params.
+ final Either2<List<dynamic>, dynamic> params;
+
Map<String, dynamic> toJson() {
Map<String, dynamic> __result = {};
__result['id'] = id ?? (throw 'id is required but was not set');
__result['method'] = method ?? (throw 'method is required but was not set');
+ if (params != null) {
+ __result['params'] = params;
+ }
__result['jsonrpc'] =
jsonrpc ?? (throw 'jsonrpc is required but was not set');
return __result;
@@ -4225,8 +4249,55 @@
bool operator ==(o) => o is ResourceOperationKind && o._value == _value;
}
+class ResponseError<D> {
+ ResponseError(this.code, this.message, this.data) {
+ if (code == null) {
+ throw 'code is required but was not provided';
+ }
+ if (message == null) {
+ throw 'message is required but was not provided';
+ }
+ }
+ factory ResponseError.fromJson(Map<String, dynamic> json) {
+ final code = json['code'];
+ final message = json['message'];
+ final data = json['data'];
+ return new ResponseError<D>(code, message, data);
+ }
+
+ /// A number indicating the error type that occurred.
+ final num code;
+
+ /// A Primitive or Structured value that contains
+ /// additional information about the error. Can be
+ /// omitted.
+ final D data;
+
+ /// A string providing a short description of the error.
+ final String message;
+
+ Map<String, dynamic> toJson() {
+ Map<String, dynamic> __result = {};
+ __result['code'] = code ?? (throw 'code is required but was not set');
+ __result['message'] =
+ message ?? (throw 'message is required but was not set');
+ if (data != null) {
+ __result['data'] = data;
+ }
+ return __result;
+ }
+
+ static bool canParse(Object obj) {
+ return obj is Map<String, dynamic> &&
+ obj.containsKey('code') &&
+ obj['code'] is num &&
+ obj.containsKey('message') &&
+ obj['message'] is String;
+ }
+}
+
class ResponseMessage implements Message {
- ResponseMessage(this.id, this.result, this.jsonrpc) {
+ ResponseMessage(this.id, this.result, this.error, this.jsonrpc) {
if (jsonrpc == null) {
throw 'jsonrpc is required but was not provided';
}
@@ -4238,10 +4309,14 @@
? new Either2<num, String>.t2(json['id'])
: (throw '''${json['id']} was not one of (number, string)'''));
final result = json['result'];
+ final error = json['error'];
final jsonrpc = json['jsonrpc'];
- return new ResponseMessage(id, result, jsonrpc);
+ return new ResponseMessage(id, result, error, jsonrpc);
}
+ /// The error object in case a request fails.
+ final ResponseError<dynamic> error;
+
/// The request id.
final Either2<num, String> id;
final String jsonrpc;
@@ -4256,6 +4331,9 @@
if (result != null) {
__result['result'] = result;
}
+ if (error != null) {
+ __result['error'] = error;
+ }
__result['jsonrpc'] =
jsonrpc ?? (throw 'jsonrpc is required but was not set');
return __result;
diff --git a/pkg/analysis_server/test/tool/lsp_spec/dart_test.dart b/pkg/analysis_server/test/tool/lsp_spec/dart_test.dart
new file mode 100644
index 0000000..068063c
--- /dev/null
+++ b/pkg/analysis_server/test/tool/lsp_spec/dart_test.dart
@@ -0,0 +1,48 @@
+// 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.
+
+import 'package:test/test.dart';
+
+import '../../../tool/lsp_spec/codegen_dart.dart';
+
+main() {
+ group('mapType', () {
+ test('handles basic types', () {
+ expect(mapType(['string']), equals('String'));
+ expect(mapType(['boolean']), equals('bool'));
+ expect(mapType(['any']), equals('dynamic'));
+ expect(mapType(['object']), equals('dynamic'));
+ expect(mapType(['int']), equals('int'));
+ expect(mapType(['num']), equals('num'));
+ });
+
+ test('handles union types', () {
+ expect(mapType(['string', 'int']), equals('Either2<String, int>'));
+ expect(mapType(['string | int']), equals('Either2<String, int>'));
+ });
+
+ test('handles arrays', () {
+ expect(mapType(['string[]']), equals('List<String>'));
+ expect(mapType(['Array<string>']), equals('List<String>'));
+ });
+
+ test('handles types with args', () {
+ expect(mapType(['Class<string[]>']), equals('Class<List<String>>'));
+ expect(mapType(['Array<string | num>']),
+ equals('List<Either2<String, num>>'));
+ });
+
+ test('handles complex nested types', () {
+ expect(
+ mapType([
+ 'Array<string>',
+ 'any[]',
+ 'Response<A>',
+ 'Request<Array<string | num>>'
+ ]),
+ equals(
+ 'Either4<List<String>, List<dynamic>, Response<A>, Request<List<Either2<String, num>>>>'));
+ });
+ });
+}
diff --git a/pkg/analysis_server/test/tool/lsp_spec/json_test.dart b/pkg/analysis_server/test/tool/lsp_spec/json_test.dart
index 646d49a..bc07300 100644
--- a/pkg/analysis_server/test/tool/lsp_spec/json_test.dart
+++ b/pkg/analysis_server/test/tool/lsp_spec/json_test.dart
@@ -18,7 +18,8 @@
});
test('returns correct output for union types', () {
- final message = new RequestMessage(new Either2.t1(1), "test", "test");
+ final message =
+ new RequestMessage(new Either2.t1(1), "test", null, "test");
String output = json.encode(message.toJson());
expect(output, equals('{"id":1,"method":"test","jsonrpc":"test"}'));
});
diff --git a/pkg/analysis_server/test/tool/lsp_spec/typescript_test.dart b/pkg/analysis_server/test/tool/lsp_spec/typescript_test.dart
index 77e3be7..1fad598 100644
--- a/pkg/analysis_server/test/tool/lsp_spec/typescript_test.dart
+++ b/pkg/analysis_server/test/tool/lsp_spec/typescript_test.dart
@@ -64,6 +64,48 @@
});
});
+ test('parses an interface with type args', () {
+ final String input = '''
+interface ResponseError<D> {
+ data?: D;
+}
+ ''';
+ final List<ApiItem> output = extractTypes(input);
+ expect(output, hasLength(1));
+ expect(output[0], const TypeMatcher<Interface>());
+ final Interface interface = output[0];
+ expect(interface.members, hasLength(1));
+ final Field field = interface.members.first;
+ expect(field, const TypeMatcher<Field>());
+ expect(field.name, equals('data'));
+ expect(field.allowsUndefined, true);
+ expect(field.allowsNull, false);
+ expect(field.types, equals(['D']));
+ });
+
+ test('parses an interface with Arrays in Array<T> format', () {
+ final String input = '''
+export interface RequestMessage {
+ /**
+ * The method's params.
+ */
+ params?: Array<any> | object;
+}
+ ''';
+ final List<ApiItem> output = extractTypes(input);
+ expect(output, hasLength(1));
+ expect(output[0], const TypeMatcher<Interface>());
+ final Interface interface = output[0];
+ expect(interface.members, hasLength(1));
+ final Field field = interface.members.first;
+ expect(field, const TypeMatcher<Field>());
+ expect(field.name, equals('params'));
+ expect(field.comment, equals('''The method's params.'''));
+ expect(field.allowsUndefined, true);
+ expect(field.allowsNull, false);
+ expect(field.types, equals(['Array<any>', 'object']));
+ });
+
test('flags nullable undefined values', () {
final String input = '''
export interface A {
diff --git a/pkg/analysis_server/tool/lsp_spec/codegen_dart.dart b/pkg/analysis_server/tool/lsp_spec/codegen_dart.dart
index 1f96c64..54b0ee1 100644
--- a/pkg/analysis_server/tool/lsp_spec/codegen_dart.dart
+++ b/pkg/analysis_server/tool/lsp_spec/codegen_dart.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:dart_style/dart_style.dart';
+import 'package:meta/meta.dart';
import 'typescript.dart';
@@ -28,6 +29,10 @@
return formattedCode.trim() + '\n'; // Ensure a single trailing newline.
}
+List<String> _extractTypesFromUnion(String type) {
+ return type.split('|').map((t) => t.trim()).toList();
+}
+
String _formatCode(String code) {
try {
code = formatter.format(code);
@@ -100,12 +105,14 @@
}
/// Maps a TypeScript type on to a Dart type, including following TypeAliases.
-String _mapType(List<String> types) {
+@visibleForTesting
+String mapType(List<String> types) {
const mapping = <String, String>{
'boolean': 'bool',
'string': 'String',
'number': 'num',
'any': 'dynamic',
+ 'object': 'dynamic',
// Special cases that are hard to parse or anonymous types.
'{ [uri: string]: TextEdit[]; }': 'Map<String, List<TextEdit>>',
'{ language: string; value: string }': 'MarkedStringWithLanguage'
@@ -114,21 +121,38 @@
throw 'Unions of more than 4 types are not supported.';
}
if (types.length >= 2) {
- final typeArgs = types.map((t) => _mapType([t])).join(', ');
+ final typeArgs = types.map((t) => mapType([t])).join(', ');
return 'Either${types.length}<$typeArgs>';
}
final type = types.first;
if (type.endsWith('[]')) {
- return 'List<${_mapType([type.substring(0, type.length - 2)])}>';
+ return 'List<${mapType([type.substring(0, type.length - 2)])}>';
+ } else if (type.startsWith('Array<') && type.endsWith('>')) {
+ return 'List<${mapType([type.substring(6, type.length - 1)])}>';
+ } else if (type.contains('<')) {
+ // For types with type args, we need to map the type and each type arg.
+ final declaredType = _stripTypeArgs(type);
+ final typeArgs = type
+ .substring(declaredType.length + 1, type.length - 1)
+ .split(',')
+ .map((t) => t.trim());
+ return '${mapType([
+ declaredType
+ ])}<${typeArgs.map((t) => mapType([t])).join(', ')}>';
+ } else if (type.contains('|')) {
+ // It's possible we ended up with nested unions that the parsing.
+ // TODO(dantup): This is now partly done during parsing and partly done
+ // here. Maybe consider removing from typescript.dart and just carrying a
+ // String through so the logic is all in one place in this function?
+ return mapType(_extractTypesFromUnion(type));
+ } else if (_typeAliases.containsKey(type)) {
+ return mapType([_typeAliases[type].baseType]);
+ } else if (mapping.containsKey(type)) {
+ return mapType([mapping[type]]);
+ } else {
+ return type;
}
- if (_typeAliases.containsKey(type)) {
- return _mapType([_typeAliases[type].baseType]);
- }
- if (mapping.containsKey(type)) {
- return _mapType([mapping[type]]);
- }
- return type;
}
String _rewriteCommentReference(String comment) {
@@ -144,6 +168,10 @@
});
}
+String _stripTypeArgs(String typeName) => typeName.contains('<')
+ ? typeName.substring(0, typeName.indexOf('<'))
+ : typeName;
+
Iterable<String> _wrapLines(List<String> lines, int maxLength) sync* {
lines = lines.map((l) => l.trimRight()).toList();
for (var line in lines) {
@@ -170,15 +198,15 @@
buffer
..writeIndentedln('static bool canParse(Object obj) {')
..indent()
- ..writeIndentedln('return obj is Map<String, dynamic>');
+ ..writeIndented('return obj is Map<String, dynamic>');
// In order to consider this valid for parsing, all fields that may not be
// undefined must be present and also type check for the correct type.
final requiredFields =
_getAllFields(interface).where((f) => !f.allowsUndefined);
for (var field in requiredFields) {
- buffer.write("&& obj.containsKey('${field.name}') && ");
+ buffer.write(" && obj.containsKey('${field.name}') && ");
_writeTypeCheckCondition(
- buffer, "obj['${field.name}']", _mapType(field.types));
+ buffer, "obj['${field.name}']", mapType(field.types));
}
buffer
..writeln(';')
@@ -197,7 +225,7 @@
return;
}
buffer
- ..writeIndented('${interface.name}(')
+ ..writeIndented('${_stripTypeArgs(interface.name)}(')
..write(allFields.map((field) => 'this.${field.name}').join(', '))
..write(')');
final fieldsWithValidation =
@@ -289,13 +317,13 @@
_writeDocCommentsAndAnnotations(buffer, field);
buffer
..writeIndented('final ')
- ..write(_mapType(field.types))
+ ..write(mapType(field.types))
..writeln(' ${field.name};');
}
void _writeFromJsonCode(
IndentableStringBuffer buffer, List<String> types, String valueCode) {
- final type = _mapType(types);
+ final type = mapType(types);
if (_isLiteral(type)) {
buffer.write("$valueCode");
} else if (_isSpecType(type)) {
@@ -316,22 +344,33 @@
void _writeFromJsonCodeForUnion(
IndentableStringBuffer buffer, List<String> types, String valueCode) {
- final unionTypeName = _mapType(types);
+ final unionTypeName = mapType(types);
// Write a check against each type, eg.:
// x is y ? new Either.tx(x) : (...)
+ var hasIncompleteCondition = false;
+ var unclosedParens = 0;
for (var i = 0; i < types.length; i++) {
- final dartType = _mapType([types[i]]);
+ final dartType = mapType([types[i]]);
- _writeTypeCheckCondition(buffer, valueCode, dartType);
- buffer.write(' ? new $unionTypeName.t${i + 1}(');
- _writeFromJsonCode(buffer, [dartType], valueCode); // Call recursively!
- buffer.write(') : (');
+ if (dartType != 'dynamic') {
+ _writeTypeCheckCondition(buffer, valueCode, dartType);
+ buffer.write(' ? new $unionTypeName.t${i + 1}(');
+ _writeFromJsonCode(buffer, [dartType], valueCode); // Call recursively!
+ buffer.write(') : (');
+ hasIncompleteCondition = true;
+ unclosedParens++;
+ } else {
+ _writeFromJsonCode(buffer, [dartType], valueCode);
+ hasIncompleteCondition = false;
+ }
}
// Fill the final parens with a throw because if we fell through all of the
// cases then the value we had didn't match any of the types in the union.
- buffer
- .write("throw '''\${$valueCode} was not one of (${types.join(', ')})'''");
- buffer.write(')' * types.length);
+ if (hasIncompleteCondition) {
+ buffer.write(
+ "throw '''\${$valueCode} was not one of (${types.join(', ')})'''");
+ }
+ buffer.write(')' * unclosedParens);
}
void _writeFromJsonConstructor(
@@ -342,7 +381,7 @@
}
buffer
..writeIndentedln(
- 'factory ${interface.name}.fromJson(Map<String, dynamic> json) {')
+ 'factory ${_stripTypeArgs(interface.name)}.fromJson(Map<String, dynamic> json) {')
..indent();
for (final field in allFields) {
buffer.writeIndented('final ${field.name} = ');
@@ -354,7 +393,7 @@
..write(allFields.map((field) => '${field.name}').join(', '))
..writeln(');')
..outdent()
- ..write('}');
+ ..writeIndented('}');
}
void _writeInterface(IndentableStringBuffer buffer, Interface interface) {
@@ -488,12 +527,17 @@
} else if (_isSpecType(dartType)) {
buffer.write('$dartType.canParse($valueCode)');
} else if (_isList(dartType)) {
- // TODO(dantup): If we're happy to assume we never have two lists in a union
- // we could simplify this to '$valueCode is List'.
- buffer.write(
- '($valueCode is List && ($valueCode.length == 0 || $valueCode.every((item) => ');
- _writeTypeCheckCondition(buffer, 'item', _getListType(dartType));
- buffer.write(')))');
+ final listType = _getListType(dartType);
+ buffer.write('($valueCode is List');
+ if (dartType != 'dynamic') {
+ // TODO(dantup): If we're happy to assume we never have two lists in a union
+ // we could skip this bit.
+ buffer
+ .write(' && ($valueCode.length == 0 || $valueCode.every((item) => ');
+ _writeTypeCheckCondition(buffer, 'item', listType);
+ buffer.write('))');
+ }
+ buffer.write(')');
} else if (_isUnion(dartType)) {
// To type check a union, we just recursively check against each of its types.
final unionTypes = _getUnionTypes(dartType);
@@ -502,7 +546,7 @@
if (i != 0) {
buffer.write(' || ');
}
- _writeTypeCheckCondition(buffer, valueCode, _mapType([unionTypes[i]]));
+ _writeTypeCheckCondition(buffer, valueCode, mapType([unionTypes[i]]));
}
buffer.write(')');
} else {
diff --git a/pkg/analysis_server/tool/lsp_spec/typescript.dart b/pkg/analysis_server/tool/lsp_spec/typescript.dart
index 286b0e2..017c666 100644
--- a/pkg/analysis_server/tool/lsp_spec/typescript.dart
+++ b/pkg/analysis_server/tool/lsp_spec/typescript.dart
@@ -159,8 +159,8 @@
: super(name, comment);
static List<Field> extractFrom(String interfaceName, String code) {
- final RegExp _fieldPattern = new RegExp(
- _comment + r'([\w\[\]]+\??)\s*:\s*([\w\[\] \|\{\}\(\):;]+)\s*(?:;|$)');
+ final RegExp _fieldPattern = new RegExp(_comment +
+ r'([\w\[\]]+\??)\s*:\s*([\w\[\] \|\{\}\(\)<>:;]+)\s*(?:;|$)');
final fields = _fieldPattern.allMatches(code).where((m) {
// Skip over the indexer in FormattingOptions since we don't need this
@@ -220,7 +220,7 @@
static List<Interface> extractFrom(String code) {
final RegExp _interfacePattern = new RegExp(_comment +
- r'(?:export\s+)?(?:interface|class)\s+(\w+)(?:\s+extends\s+([\w, ]+?))?\s*' +
+ r'(?:export\s+)?(?:interface|class)\s+([\w<>]+)(?:\s+extends\s+([\w, ]+?))?\s*' +
_blockBody);
final interfaces = _interfacePattern.allMatches(code).map((match) {
