There are two different axes of sync/async here, and it's important not to mix them up: the context manager could be sync or async, and the function that's being decorated could be sync or async. This issue is about the case where you have a sync context manager, and it's decorating an async function.

The current behavior is definitely super confusing and not at all useful.

In general, I'm also very skeptical about using iscoroutinefunction to automatically change semantics, because of the issue Andrew mentions about failing to handle complicated objects like functools.partial instances or sync functions that return awaitables. In this *specific* case though, I'm not sure... maybe it actually is OK?

What's special about this case is that when you write:

@sync_cm()
async def foo():
    ...

...the same person is writing both the @ line and the 'async def' line, and they're right next to each other in the source code. So the normal concern about some unexpected foreign object being passed in doesn't really apply. And in fact, if I did have an awaitable-returning-function:

@sync_cm()
def foo():
    return some_async_fn()

...then I think most people would actually *expect* this to be equivalent to:

def foo():
    with sync_cm():
        return some_async_fn()

Not very useful, but not surprising either. So maybe this is the rare case where switching on iscoroutinefunction is actually OK? The only cases where it would fail is if someone invokes the decorator form directly, e.g.

new_fn = sync_cm()(old_fn)

This seems pretty rare, but probably someone does do it, so I dunno.

If we think that's too magic, another option would be to make up some more explicit syntax, like:

@sync_cm().wrap_async
async def foo(): ...

...Oh, but I think this breaks the Guido's gut feeling rule (https://mail.python.org/pipermail/python-dev/2004-August/046711.html), so it would have to instead be:

@sync_cm.wrap_async()
async def foo(): ...

And even if we decide not to automatically switch on iscoroutinefunction, we should probably add a warning or something at least, because right now

@sync_cm()
async def ...

is definitely a mistake.

(Adding Nick to nosy as the contextlib maintainer.)