[red-knot] @override lint rule

[carljm]

Summary

Lots of TODOs and things to clean up here, but it demonstrates the working lint rule.

Test Plan

➜ cat main.py
from typing import override
from base import B

class C(B):
    @override
    def method(self): pass

➜ cat base.py
class B: pass

➜ cat typing.py
def override(func):
    return func

(We provide our own typing.py since we don't have typeshed vendored or type stub support yet.)

➜ ./target/debug/red_knot main.py
...
1   0.012086s TRACE red_knot Main Loop: Tick
[crates/red_knot/src/main.rs:157:21] diagnostics = [
    "Method C.method is decorated with `typing.override` but does not override any base class method",
]

If we add def method(self): pass to class B in base.py and run red_knot again, there is no lint error.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

Nice that you managed to get this rule working for today!

I think this PR shows a few issues that we need to address:

• SemanticLintContext probably must be generic over Db to prevent that we need to leak the internal storage layout on the Database API. We should solve this as part of this PR.
• Our Database is somewhat awkward because it requires setting the right HasJar and SemanticDb type bounds. I think salsa doesn't have this problem because it can fallback to the HasJarDyn trait to resolve an ingredient. We need a better solution for that, but that's out of scope for this PR
• My ultimate goal was to prevent that lint rules get access to db and instead force them to go through the context. I can see now how this is challenging with e.g. FunctionTypeId::name taking a db as argument. I still think that not giving lint rules access to the db is desired, but I'm not aware of a good solution right now.
• This is related to not giving access to db to the semantic lint rules. I think that it's very easy today to mix local ids from different files. Ideally, lint rules would only ever get local ids for the current file or otherwise global ids.

[carljm]

Thanks for the review!

SemanticLintContext probably must be generic over Db to prevent that we need to leak the internal storage layout on the Database API. We should solve this as part of this PR.

Why does using a trait object db require us to leak the internal storage layout on the database API? I think it led to that in this PR just because I was cutting corners to get things working, but I don't think it requires it. Instead it requires that all data access we need available to lint rules must be implemented as queries on the Db trait interface. These queries don't need to leak implementation details, but it will mean there will be a lot of them.

In some ways it seems like making everything generic over Db has more risk of leaking internals, because it means that the full Db interface is always available instead of just what we expose on the trait.

(I don't have strong opinions here, I'm still working out the pros and cons of different patterns and trying to understand the reasoning for different options.)

My ultimate goal was to prevent that lint rules get access to db and instead force them to go through the context. I can see now how this is challenging with e.g. FunctionTypeId::name taking a db as argument. I still think that not giving lint rules access to the db is desired, but I'm not aware of a good solution right now.

Yes, I think this will be pretty challenging with the interning-based storage approach, because pretty much anything you might want to do requires a db.

This is related to not giving access to db to the semantic lint rules. I think that it's very easy today to mix local ids from different files. Ideally, lint rules would only ever get local ids for the current file or otherwise global ids.

Yeah, I agree we should try to avoid this footgun, though it's not clear to me how to actually make it impossible. (Even a global ID will contain local IDs, and making those inaccessible will make those global IDs quite difficult or impossible to use in the code that actually needs to break them apart to do the queries. Maybe we can address this with crate visibility boundaries, if in future we break lint rules into a separate crate from the db.)

[carljm]

Addressed most review comments. Would like to discuss this more, as the right path isn't clear to me:

SemanticLintContext probably must be generic over Db to prevent that we need to leak the internal storage layout on the Database API. We should solve this as part of this PR.