-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pigeon] Enable warnings as errors for remaining languages #3317
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,12 +180,13 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
final HostDatatype hostDatatype = _getHostDatatype(root, field); | ||
String toWriteValue = ''; | ||
final String fieldName = field.name; | ||
final String safeCall = field.type.isNullable ? '?' : ''; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixes warnings about unnecessary safe calls on non-nullable values. |
||
if (!hostDatatype.isBuiltin && | ||
customClassNames.contains(field.type.baseName)) { | ||
toWriteValue = '$fieldName?.toList()'; | ||
toWriteValue = '$fieldName$safeCall.toList()'; | ||
} else if (!hostDatatype.isBuiltin && | ||
customEnumNames.contains(field.type.baseName)) { | ||
toWriteValue = '$fieldName?.raw'; | ||
toWriteValue = '$fieldName$safeCall.raw'; | ||
} else { | ||
toWriteValue = fieldName; | ||
} | ||
|
@@ -244,7 +245,8 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
indent.addln( | ||
'.let { if (it is Int) it.toLong() else it as Long? }'); | ||
} else { | ||
indent.writeln('val ${field.name} = $listValue as $fieldType?'); | ||
indent.writeln( | ||
'val ${field.name} = ${_cast(listValue, kotlinType: '$fieldType?')}'); | ||
} | ||
} else { | ||
if (!hostDatatype.isBuiltin && | ||
|
@@ -260,7 +262,8 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
indent | ||
.addln('.let { if (it is Int) it.toLong() else it as Long }'); | ||
} else { | ||
indent.writeln('val ${field.name} = $listValue as $fieldType'); | ||
indent.writeln( | ||
'val ${field.name} = ${_cast(listValue, kotlinType: fieldType)}'); | ||
} | ||
} | ||
}); | ||
|
@@ -380,13 +383,15 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
indent.writeln('callback()'); | ||
}); | ||
} else { | ||
final String forceUnwrap = func.returnType.isNullable ? '?' : ''; | ||
indent.addScoped('{', '}', () { | ||
if (func.returnType.baseName == 'int') { | ||
final String forceUnwrap = | ||
func.returnType.isNullable ? '?' : ''; | ||
indent.writeln( | ||
'val result = if (it is Int) it.toLong() else it as$forceUnwrap Long'); | ||
} else { | ||
indent.writeln('val result = it as$forceUnwrap $returnType'); | ||
indent.writeln( | ||
'val result = ${_cast('it', kotlinType: returnType, safeCast: func.returnType.isNullable)}'); | ||
} | ||
indent.writeln('callback(result)'); | ||
}); | ||
|
@@ -507,7 +512,6 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
|
||
indent.write('channel.setMessageHandler '); | ||
indent.addScoped('{ $messageVarName, reply ->', '}', () { | ||
indent.writeln('var wrapped = listOf<Any?>()'); | ||
final List<String> methodArguments = <String>[]; | ||
if (method.arguments.isNotEmpty) { | ||
indent.writeln('val args = message as List<Any?>'); | ||
|
@@ -543,6 +547,7 @@ class KotlinGenerator extends StructuredGenerator<KotlinOptions> { | |
}); | ||
}); | ||
} else { | ||
indent.writeln('var wrapped: List<Any?>'); | ||
tarrinneal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
indent.write('try '); | ||
indent.addScoped('{', '}', () { | ||
if (method.returnType.isVoid) { | ||
|
@@ -669,15 +674,15 @@ String _castForceUnwrap(String value, TypeDeclaration type, Root root) { | |
type.isNullable ? '$value == null ? null : ' : ''; | ||
return '$nullableConditionPrefix${_kotlinTypeForDartType(type)}.ofRaw($value as Int)$forceUnwrap'; | ||
} else { | ||
final String castUnwrap = type.isNullable ? '?' : ''; | ||
|
||
// The StandardMessageCodec can give us [Integer, Long] for | ||
// a Dart 'int'. To keep things simple we just use 64bit | ||
// longs in Pigeon with Kotlin. | ||
if (type.baseName == 'int') { | ||
final String castUnwrap = type.isNullable ? '?' : ''; | ||
return '$value.let { if (it is Int) it.toLong() else it as$castUnwrap Long }'; | ||
} else { | ||
return '$value as$castUnwrap ${_kotlinTypeForDartType(type)}'; | ||
return _cast(value, | ||
kotlinType: _kotlinTypeForDartType(type), safeCast: type.isNullable); | ||
} | ||
} | ||
} | ||
|
@@ -741,3 +746,13 @@ String _nullsafeKotlinTypeForDartType(TypeDeclaration type) { | |
final String nullSafe = type.isNullable ? '?' : ''; | ||
return '${_kotlinTypeForDartType(type)}$nullSafe'; | ||
} | ||
|
||
/// Returns an expression to cast [variable] to [kotlinType]. | ||
String _cast(String variable, | ||
{required String kotlinType, bool safeCast = false}) { | ||
// Special-case Any, since no-op casts cause warnings. | ||
if (kotlinType == 'Any?' || (safeCast && kotlinType == 'Any')) { | ||
return variable; | ||
} | ||
return '$variable as${safeCast ? '?' : ''} $kotlinType'; | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,7 +61,7 @@ public void asyncSuccess() { | |
(bytes) -> { | ||
bytes.rewind(); | ||
@SuppressWarnings("unchecked") | ||
ArrayList wrapped = (ArrayList) codec.decodeMessage(bytes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to fix a bunch of raw type warnings in our tests, and suppress a bunch of unchecked casts for most of the same lines. |
||
ArrayList<Object> wrapped = (ArrayList<Object>) codec.decodeMessage(bytes); | ||
assertTrue(wrapped.size() == 1); | ||
didCall[0] = true; | ||
}); | ||
|
@@ -88,7 +88,7 @@ public void asyncError() { | |
(bytes) -> { | ||
bytes.rewind(); | ||
@SuppressWarnings("unchecked") | ||
ArrayList wrapped = (ArrayList) codec.decodeMessage(bytes); | ||
ArrayList<Object> wrapped = (ArrayList<Object>) codec.decodeMessage(bytes); | ||
assertTrue(wrapped.size() > 1); | ||
assertEquals("java.lang.Exception: error", (String) wrapped.get(0)); | ||
didCall[0] = true; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cast changes in Java and Kotlin fix uncessary casts (
foo as Any?
,foo as? Any
in Kotlin,(Object) foo
in Java).