Remove AST-node dependency from FunctionType and ClassType

[MichaReiser]

Summary

This PR improves Red Knot's incremental computation by:

• Making FunctionType::return_type and ClassType::explicit_bases salsa queries. I suspect this is where the main performance improvement comes from. Making these two functions salsa queries prevents that the class or function's node (accessed through definition.node) is marked as a dependency at the callee side. Instead, the callee only depends on whatever the return type or bases are.
• Removing FunctionType::definition and ClassType::definition and instead use semantic_index.definition(..) to lookup the type's definition lazily. This ensures that deserializing a type doesn't require deserializing the AST as well.
• Remove ScopeId::node and move it to Scope::node. Same as for FunctionType::definition. Moving the Node from the ScopeId to the definition allows deserializing the ScopeId without deserializing the AST node. This is important because both FunctionType and ClassType have a ScopeId field.

Fixes #14054

Test Plan

cargo test

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[codspeed-hq]

CodSpeed Performance Report

Merging #14087 will improve performances by 12.36%

_{Comparing micha/remove-ast-dependencies-from-type (eb97246) with main (9dddd73)}

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	micha/remove-ast-dependencies-from-type	Change
⚡	red_knot_check_file[incremental]	4.6 ms	4.1 ms	+12.36%

[MichaReiser]

Lol what

[MichaReiser]

The issue without this being a salsa query is that the calling-query suddenly depends on the function's ast (and, therefore, the file).

In the case of our benchmark: _parser.py calls match_to_datetime which is defined in _re.py. Now, it shouldn't be necessary to recheck _parser.py because no type in _re.py changed. However, Salsa had to recheck every callsite because the return_type query accessed the definition.kind(db) which is the AST node of the match_to_datetime and its AST has changed (not really, but we use no_eq...)

I verified that making return_ty is the reason for the perf improvement by removing the #[salsa::tracked].

[AlexWaygood]

Nice!

[AlexWaygood]

LGTM other than my remaining docs nits and one further optional comment:

[carljm]

Code changes here look great, thanks for sorting this out!

My main concern is that I think the guidance here is going to prove quite difficult to remember and follow manually and solely via code review; it's just very easy for these things to leak. It seems like it should be possible to write tests that are similar to what happen in our benchmark, just verifying that if you make a no-op change to one file, it doesn't cause re-execution of queries in another file that shouldn't need to re-execute. I think a few tests like that could go a really long way in helping us avoid regression here.

[MichaReiser]

@carljm I agree that a test would be nice. I added one for call inference although I'm not sure how valuable it is because it is rather fragile (a slight change to inference will break the test itself).

I don't know how to write the ideal test that:

• Exercises most language features
• Asserts that infer_definition_types or infer_expression_types is never called for any other file than the file to which the comment was added.

The problem here is that salsa doesn't provide the necessary reflection API to answer the second question. We could log all events but that test is likely going to break with every inference test AND it's kind of impossible to tell from a ExpressionId to which file it belongs.

I don't mind adding more tests if you have suggestions on how to test this, but I'll leave it at what I have for now. I do think that our benchmarks do a decent job at this anyway. I would hope that we would investigate a 12% perf regression ;)

I don't think there's a way to test deserialization in a meaningful before salsa adds persistent caching support.