diff --git a/src/nexusrpc/handler/_decorators.py b/src/nexusrpc/handler/_decorators.py index 8b9fb20..6bcbaa7 100644 --- a/src/nexusrpc/handler/_decorators.py +++ b/src/nexusrpc/handler/_decorators.py @@ -266,7 +266,14 @@ def _start(ctx: StartOperationContext, input: Any) -> Any: _start.__doc__ = start.__doc__ return nexusrpc.handler._syncio.SyncOperationHandler(_start) - # TODO(preview): these types should only need to be inspected if the user has not supplied a service definition. + # Type inspection happens here at @sync_operation decoration time, before we know + # if a service definition will be provided to @service_handler. While we could + # theoretically defer this inspection until @service_handler time (and skip it when + # a service definition provides the types), doing so would require storing the + # original method reference and implementing lazy evaluation. The added complexity + # isn't worth the marginal performance benefit. When types are None and a service + # definition is provided, type validation is simply skipped (see + # validate_operation_handler_methods in _operation_handler.py). input_type, output_type = get_start_method_input_and_output_type_annotations( # type: ignore[var-annotated] start # type: ignore[arg-type] ) diff --git a/src/nexusrpc/handler/_operation_handler.py b/src/nexusrpc/handler/_operation_handler.py index 5a9c16a..bb1c79d 100644 --- a/src/nexusrpc/handler/_operation_handler.py +++ b/src/nexusrpc/handler/_operation_handler.py @@ -213,7 +213,10 @@ def validate_operation_handler_methods( f"is '{op_defn.name}'. Operation handlers may not override the name of an operation " f"in the service definition." ) - # Input type is contravariant: op handler input must be superclass of op defn output + # Input type is contravariant: op handler input must be superclass of op defn input. + # If handler's input_type is None (missing annotation), skip validation - the handler + # relies on the service definition for type information. This supports handlers without + # explicit type annotations when a service definition is provided. if ( op.input_type is not None and Any not in (op.input_type, op_defn.input_type) @@ -230,7 +233,9 @@ def validate_operation_handler_methods( f"superclass of the operation definition input type." ) - # Output type is covariant: op handler output must be subclass of op defn output + # Output type is covariant: op handler output must be subclass of op defn output. + # If handler's output_type is None (missing annotation), skip validation - the handler + # relies on the service definition for type information. if ( op.output_type is not None and Any not in (op.output_type, op_defn.output_type) diff --git a/src/nexusrpc/handler/_util.py b/src/nexusrpc/handler/_util.py index a95801d..99e8ca5 100644 --- a/src/nexusrpc/handler/_util.py +++ b/src/nexusrpc/handler/_util.py @@ -20,7 +20,6 @@ ServiceHandlerT = TypeVar("ServiceHandlerT") -# TODO(preview): is it ever valid for this to return None (as opposed to NoneType)? def get_start_method_input_and_output_type_annotations( start: Callable[ [ServiceHandlerT, StartOperationContext, InputT], @@ -30,9 +29,26 @@ def get_start_method_input_and_output_type_annotations( Optional[type[InputT]], Optional[type[OutputT]], ]: - """Return operation input and output types. + """Extract input and output type annotations from a start method. - `start` must be a type-annotated start method that returns a synchronous result. + Args: + start: A start method with signature (self, ctx: StartOperationContext, input: I) -> O + + Returns: + A tuple of (input_type, output_type) where: + + - ``None`` means the type annotation is missing or could not be extracted. + This is valid when a service definition provides the types. + - ``type(None)`` (NoneType) means the annotation explicitly specified ``None`` + as the type (e.g., ``input: None`` or ``-> None``). + + When ``None`` is returned for either type, the caller should handle it based on + whether a service definition is available: + + - If a service definition is provided, the types from the service definition + will be used and type validation against the handler is skipped. + - If no service definition is provided, an error will be raised downstream + when attempting to create an ``OperationDefinition``. """ try: type_annotations = typing.get_type_hints(start) diff --git a/tests/handler/test_invalid_usage.py b/tests/handler/test_invalid_usage.py index cadad92..7e70a32 100644 --- a/tests/handler/test_invalid_usage.py +++ b/tests/handler/test_invalid_usage.py @@ -160,6 +160,27 @@ def op(self) -> OperationHandler: ... # type: ignore error_message = r"has no input type" +class SyncOperationNoTypeAnnotationsWithoutServiceDefinition(_TestCase): + """Test that @sync_operation with missing type annotations raises an error + when no service definition is provided. + + This tests the flow through get_start_method_input_and_output_type_annotations() + which returns (None, None) for missing types, followed by + OperationDefinition.from_operation() which raises ValueError. + """ + + @staticmethod + def build(): + @service_handler + class ServiceWithMissingTypes: + @sync_operation + async def op(self, ctx, input): ... # type: ignore[reportMissingParameterType] + + _ = ServiceWithMissingTypes + + error_message = r"has no input type" + + @pytest.mark.parametrize( "test_case", [ @@ -170,6 +191,7 @@ def op(self) -> OperationHandler: ... # type: ignore AsyncioHandlerWithSyncioOperation, ServiceDefinitionHasDuplicateMethodNames, OperationHandlerNoInputOutputTypeAnnotationsWithoutServiceDefinition, + SyncOperationNoTypeAnnotationsWithoutServiceDefinition, ], ) def test_invalid_usage(test_case: _TestCase):