diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 58bc3d89814f..d7b145f2e30e 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -1714,6 +1714,8 @@ FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE: status: noFix FfiCode.MUST_BE_A_SUBTYPE: status: noFix +FfiCode.MUST_RETURN_VOID: + status: noFix FfiCode.NON_CONSTANT_TYPE_ARGUMENT: status: noFix FfiCode.NON_NATIVE_FUNCTION_TYPE_ARGUMENT_TO_POINTER: diff --git a/pkg/analyzer/lib/src/dart/error/ffi_code.g.dart b/pkg/analyzer/lib/src/dart/error/ffi_code.g.dart index a954353ec992..3e050fac6eaa 100644 --- a/pkg/analyzer/lib/src/dart/error/ffi_code.g.dart +++ b/pkg/analyzer/lib/src/dart/error/ffi_code.g.dart @@ -316,6 +316,15 @@ class FfiCode extends AnalyzerErrorCode { hasPublishedDocs: true, ); + /// Parameters: + /// 0: the return type that should be 'void'. + static const FfiCode MUST_RETURN_VOID = FfiCode( + 'MUST_RETURN_VOID', + "The return type of the function passed to 'RawVoidCallback' must be " + "'void' rather than '{0}'.", + correctionMessage: "Try changing the return type to 'void'.", + ); + /// Parameters: /// 0: the name of the function, method, or constructor having type arguments static const FfiCode NON_CONSTANT_TYPE_ARGUMENT = FfiCode( diff --git a/pkg/analyzer/lib/src/error/error_code_values.g.dart b/pkg/analyzer/lib/src/error/error_code_values.g.dart index 3a3eebfd1cb3..4c606c981cd0 100644 --- a/pkg/analyzer/lib/src/error/error_code_values.g.dart +++ b/pkg/analyzer/lib/src/error/error_code_values.g.dart @@ -574,6 +574,7 @@ const List errorCodeValues = [ FfiCode.MISSING_SIZE_ANNOTATION_CARRAY, FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE, FfiCode.MUST_BE_A_SUBTYPE, + FfiCode.MUST_RETURN_VOID, FfiCode.NON_CONSTANT_TYPE_ARGUMENT, FfiCode.NON_NATIVE_FUNCTION_TYPE_ARGUMENT_TO_POINTER, FfiCode.NON_POSITIVE_ARRAY_DIMENSION, diff --git a/pkg/analyzer/lib/src/generated/ffi_verifier.dart b/pkg/analyzer/lib/src/generated/ffi_verifier.dart index 58adcc8c045a..12a2dec1cdad 100644 --- a/pkg/analyzer/lib/src/generated/ffi_verifier.dart +++ b/pkg/analyzer/lib/src/generated/ffi_verifier.dart @@ -30,6 +30,7 @@ class FfiVerifier extends RecursiveAstVisitor { static const _dartFfiLibraryName = 'dart.ffi'; static const _finalizableClassName = 'Finalizable'; static const _isLeafParamName = 'isLeaf'; + static const _rawVoidCallback = 'RawVoidCallback'; static const _opaqueClassName = 'Opaque'; static const Set _primitiveIntegerNativeTypesFixedSize = { @@ -227,6 +228,8 @@ class FfiVerifier extends RecursiveAstVisitor { FfiCode.CREATION_OF_STRUCT_OR_UNION, node.constructorName, ); + } else if (class_.isRawVoidCallback) { + _validateRawVoidCallback(node); } super.visitInstanceCreationExpression(node); @@ -1181,6 +1184,37 @@ class FfiVerifier extends RecursiveAstVisitor { } } + /// Validate the invocation of the static method `RawVoidCallback(f)`. + void _validateRawVoidCallback(InstanceCreationExpression node) { + var argCount = node.argumentList.arguments.length; + if (argCount != 1) { + // There are other diagnostics reported against the invocation and the + // diagnostics generated below might be inaccurate, so don't report them. + return; + } + + var typeArg = (node.staticType as ParameterizedType).typeArguments[0]; + if (!_isValidFfiNativeFunctionType(typeArg)) { + _errorReporter.reportErrorForNode(FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE, + node.constructorName, [typeArg, _rawVoidCallback]); + return; + } + + var f = node.argumentList.arguments[0]; + var funcType = f.typeOrThrow; + if (!_validateCompatibleFunctionTypes(funcType, typeArg)) { + _errorReporter.reportErrorForNode( + FfiCode.MUST_BE_A_SUBTYPE, f, [funcType, typeArg, _rawVoidCallback]); + return; + } + + // TODO(brianwilkerson) Validate that `f` is a top-level function. + var retType = (funcType as FunctionType).returnType; + if (retType is! VoidType) { + _errorReporter.reportErrorForNode(FfiCode.MUST_RETURN_VOID, f, [retType]); + } + } + void _validateRefIndexed(IndexExpression node) { var targetType = node.realTarget.staticType; if (!_isValidFfiNativeType(targetType, @@ -1467,6 +1501,14 @@ extension on Element? { element.isFfiClass; } + /// Return `true` if this represents the class `RawVoidCallback`. + bool get isRawVoidCallback { + final element = this; + return element is ClassElement && + element.name == FfiVerifier._rawVoidCallback && + element.isFfiClass; + } + /// Return `true` if this represents the class `Struct`. bool get isStruct { final element = this; @@ -1489,7 +1531,7 @@ extension on Element? { element.isFfiClass; } - /// Return `true` if this represents a subclass of the class `Struct`. + /// Return `true` if this represents a subclass of the class `Union`. bool get isUnionSubclass { final element = this; return element is ClassElement && element.supertype.isUnion; diff --git a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart index 0cbe55a50978..78dad18c2b1c 100644 --- a/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart +++ b/pkg/analyzer/lib/src/test_utilities/mock_sdk.dart @@ -775,6 +775,14 @@ final class Pointer extends NativeType { final Pointer nullptr = Pointer.fromAddress(0); +class RawVoidCallback { + RawVoidCallback(@DartRepresentationOf('T') Function callback) {} + + Pointer> get pointer; + + void close(); +} + extension NativeFunctionPointer on Pointer> { external DF asFunction({bool isLeaf = false}); diff --git a/pkg/analyzer/messages.yaml b/pkg/analyzer/messages.yaml index d280a586bcc1..08f512e8cb26 100644 --- a/pkg/analyzer/messages.yaml +++ b/pkg/analyzer/messages.yaml @@ -19010,6 +19010,54 @@ FfiCode: Pointer.fromFunction(f, 5); } ``` + MUST_RETURN_VOID: + problemMessage: "The return type of the function passed to 'RawVoidCallback' must be 'void' rather than '{0}'." + correctionMessage: "Try changing the return type to 'void'." + comment: |- + Parameters: + 0: the return type that should be 'void'. + hasPublishedDocs: false + documentation: |- + #### Description + + The analyzer produces this diagnostic when you pass a function + that doesn't return `void` to the `RawVoidCallback` constructor. + + `RawVoidCallback` creates a native callback that can be invoked + from any thread. The native code that invokes the callback sends a message + back to the isolate that created the callback, and doesn't wait for a + response. So it isn't possible to return a result from the callback. + + For more information about FFI, see [C interop using dart:ffi][ffi]. + + #### Example + + The following code produces this diagnostic because the function + `f` returns `int` rather than `void`. + + ```dart + import 'dart:ffi'; + + int f(int i) => i * 2; + + void g() { + RawVoidCallback([!f!]); + } + ``` + + #### Common fixes + + Change the return type of the function to `void`. + + ```dart + import 'dart:ffi'; + + void f(int i) => print(i * 2); + + void g() { + RawVoidCallback(f); + } + ``` NON_CONSTANT_TYPE_ARGUMENT: problemMessage: "The type arguments to '{0}' must be known at compile time, so they can't be type parameters." correctionMessage: Try changing the type argument to be a constant type. diff --git a/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart b/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart new file mode 100644 index 000000000000..9197c4c77408 --- /dev/null +++ b/pkg/analyzer/test/src/diagnostics/ffi_async_callback_test.dart @@ -0,0 +1,87 @@ +// Copyright (c) 2023, 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:analyzer/src/dart/error/ffi_code.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../dart/resolution/context_collection_resolution.dart'; + +main() { + defineReflectiveSuite(() { + defineReflectiveTests(FfiRawVoidCallbacksMustReturnVoid); + }); +} + +@reflectiveTest +class FfiRawVoidCallbacksMustReturnVoid extends PubPackageResolutionTest { + test_RawVoidCallback_inferred() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +void f(int i) => i * 2; +void g() { + RawVoidCallback? callback; + callback = RawVoidCallback(f); + callback.close(); +} +''', []); + } + + test_RawVoidCallback_mustBeANativeFunctionType() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +void f(int i) => i * 2; +void g() { + RawVoidCallback(f); +} +''', [ + error(FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE, 56, 35), + ]); + } + + test_RawVoidCallback_mustBeASubtype() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +void f(int i) => i * 2; +void g() { + RawVoidCallback(f); +} +''', [ + error(FfiCode.MUST_BE_A_SUBTYPE, 95, 1), + ]); + } + + test_RawVoidCallback_mustHaveTypeArgs() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +int f(int i) => i * 2; +void g() { + RawVoidCallback(f); +} +''', [ + error(FfiCode.MUST_BE_A_NATIVE_FUNCTION_TYPE, 55, 15), + ]); + } + + test_RawVoidCallback_mustReturnVoid() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +int f(int i) => i * 2; +void g() { + RawVoidCallback(f); +} +''', [ + error(FfiCode.MUST_RETURN_VOID, 94, 1), + ]); + } + + test_RawVoidCallback_ok() async { + await assertErrorsInCode(r''' +import 'dart:ffi'; +void f(int i) => i * 2; +void g() { + RawVoidCallback(f); +} +''', []); + } +} diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart index 7d64e481d472..f064ad0b4e44 100644 --- a/pkg/analyzer/test/src/diagnostics/test_all.dart +++ b/pkg/analyzer/test/src/diagnostics/test_all.dart @@ -243,6 +243,7 @@ import 'extra_annotation_on_struct_field_test.dart' import 'extra_positional_arguments_test.dart' as extra_positional_arguments; import 'extra_size_annotation_carray_test.dart' as extra_size_annotation_carray; import 'extraneous_modifier_test.dart' as extraneous_modifier; +import 'ffi_async_callback_test.dart' as ffi_async_callback_test; import 'ffi_leaf_call_must_not_use_handle_test.dart' as ffi_leaf_call_must_not_use_handle; import 'ffi_native_test.dart' as ffi_native_test; @@ -1046,6 +1047,7 @@ main() { extra_positional_arguments.main(); extra_size_annotation_carray.main(); extraneous_modifier.main(); + ffi_async_callback_test.main(); ffi_leaf_call_must_not_use_handle.main(); ffi_native_test.main(); field_in_struct_with_initializer.main(); diff --git a/pkg/analyzer/tool/diagnostics/diagnostics.md b/pkg/analyzer/tool/diagnostics/diagnostics.md index 6e316587be15..3760d7fd3570 100644 --- a/pkg/analyzer/tool/diagnostics/diagnostics.md +++ b/pkg/analyzer/tool/diagnostics/diagnostics.md @@ -13434,6 +13434,52 @@ class B extends A { } {% endprettify %} +### must_return_void + +_The return type of the function passed to 'RawVoidCallback' must be 'void' +rather than '{0}'._ + +#### Description + +The analyzer produces this diagnostic when you pass a function +that doesn't return `void` to the `RawVoidCallback` constructor. + +`RawVoidCallback` creates a native callback that can be invoked +from any thread. The native code that invokes the callback sends a message +back to the isolate that created the callback, and doesn't wait for a +response. So it isn't possible to return a result from the callback. + +For more information about FFI, see [C interop using dart:ffi][ffi]. + +#### Example + +The following code produces this diagnostic because the function +`f` returns `int` rather than `void`. + +{% prettify dart tag=pre+code %} +import 'dart:ffi'; + +int f(int i) => i * 2; + +void g() { + RawVoidCallback([!f!]); +} +{% endprettify %} + +#### Common fixes + +Change the return type of the function to `void`. + +{% prettify dart tag=pre+code %} +import 'dart:ffi'; + +void f(int i) => print(i * 2); + +void g() { + RawVoidCallback(f); +} +{% endprettify %} + ### name_not_string _The value of the 'name' field is required to be a string._