refactor: tweak type annotations to make Pyright pass

[jkmnt]

Summary of Changes

This is not intended to be merged as is, I'm not that familiar with the falcon codebase.

The code is passing the Pyright checks in basic mode.

In standard mode there are a few more errors. These are mostly related to the unbound variables defined only in some branch of if/else. Pyright is strict here and insist for variables to be always defined. I "fixed" one of these errors in CompiledRouter.

I run the tests. All of them are either green or skipped (?).

The type for headers of falcon.errors was made more strict: dict[str,str] | list[tuple[str, str] instead of Mapping[str, str] | Iterable[tuple[str, str]]. The catch is that dict is iterable too, so dict keyed by tuple of two strings (i.e. dict[tuple[str, str], Any]) is accepted, while should not.

I moved _logger to the separate module. The _logger was used in falcon.asgi/app, but root falcon module was never imported there.

I replaced hasattr() applied to some optional methods with getattr(). Pyright is strict here too: hasattr success means nothing :-)

[CaselIT]

Thanks for the change!

I haven't looked at the complete change yet, but I would avoid doing non-typing related changes, like the logger change.

will have another look later today.

[CaselIT]

I don't understand why we need this. it's more specific that HeaderArg

[jkmnt]

Ok, the reason is this code:

def _load_headers(headers: Optional[HeaderArg]) -> Headers:
    """Transform the headers to dict."""
    if headers is None:
        return {}
    if isinstance(headers, dict):
        return headers
    return dict(headers)

The header args were wide-typed as either Mapping[str, str] or Iterable[tuple[str, str]]. The isinstance(..., dict) is not enough to tell Mapping from Iterable[tuple]. I.e. for k,v in {('foo', 'bar') : 42, ('a', 'b'): 111} yields perfect ('foo', 'bar'), ('a', 'b') pairs matching the Iterable[tuple[str, str]]. That's the pyright complain.

The docs state the arguments to errors must be list or dict (not list-like or dict-like), I thought it would be the right thing to match the typing to runtime.

[CaselIT]

this seems overly pedantic. it's likely fine to narrow HeaderArg to use Sequence[Tuple...] that should solve the complain

[jkmnt]

I changed the the NarrowHeaderArg to Union[Mapping[], Sequence[]], pyright is ok with it.
Changing the very HeaderArg from Iterable to Sequence is wrong imho, since it's used in set_headers.
The docs state the set_headers accepts iterable of tuples but sequence will disallow using generators there.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (61c1047) to head (8465649).

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #2375   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         7725      7738   +13     
  Branches      1071      1071           
=========================================
+ Hits          7725      7738   +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[CaselIT]

I had also made some changes. Will create a new PR

[CaselIT]

I've pushed them here #2379

Everything should pass there. Could you take a look @jkmnt ?

thanks for getting the ball rolling!

[jkmnt]

No complains from pyright on my check in both standart and basic modes.
Personally I'm not a big fan of silencing the typechecker, but in internal code it's ok.

I think

meth: Anno = getattr(obj, 'meth_name', None); 
if meth:
    meth()

vs

 if hasattr(obj, 'meth_name'):
     ob.meth_name()

is still better from both the typecheck and performance standpoints: one attribute lookup vs two.
Though the try: obj.meth(); except ... if even faster )

I've pushed them here #2379

Everything should pass there. Could you take a look @jkmnt ?

thanks for getting the ball rolling!

[CaselIT]

I agree getatrr. I will cherry pick your chane in the other one