sanic-org / sanic-routing

Internal handler routing for Sanic beginning with v21.3.
https://sanicframework.org/
MIT License
13 stars 11 forks source link

Incorrect handling of `pattern` argument for `register_pattern` #56

Closed ahankinson closed 2 years ago

ahankinson commented 2 years ago

The type annotation on register_pattern gives Pattern as the acceptable type.

However, an exception is thrown if you do provide a pattern with re.compile, complaining that it must be a str.

So:

app.router.register_pattern(
    "nestr",
    nonempty_str,
    re.compile(r"^[^/]+$")
)

passes the type checker, but fails with an exception, while

app.router.register_pattern(
    "nestr",
    nonempty_str,
    r"^[^/]+$"
)

does not raise an exception, but highlights the pattern argument as a mismatching type.

The relevant lines are:

https://github.com/sanic-org/sanic-routing/blob/main/sanic_routing/router.py#L241-L243

https://github.com/sanic-org/sanic-routing/blob/main/sanic_routing/router.py#L272-L276

ahankinson commented 2 years ago

I can make a PR, but would appreciate some direction: I can either make it raise on non-Pattern arguments, or change the function annotation to str.

ahopkins commented 2 years ago

Yeah, I noticed that today actually.

I think ideally we would accept both by annotation and at runtime. IIRC, the strings are compiled in that register method.

If you are up for the challenge, I'd love to see a PR to fix.