Rust: Remove extractor path resolution. #20295
Conversation
…slator::new (hardcode to false).
There was a problem hiding this comment.
LGTM so far, love to see so much red removed lines in this PR!
As I mentioned, you can also remove all the empty emission trait implementations if you remove those entities from the special emission list in ast-generator. Other than that (and the PathAstNode thing you are already aware of), I don't think there's anything else to clean.
Addressable could be done in QL, but I think it's better to leave it in the schema, because that allows to generate the inheritance. You could even add canonical_path: optional[string] | synth there, which would give you hasCanonicalPath for free.
It appears I am still a bit fuzzy on what I also notice we're still calling |
|
I've added the upgrade and downgrade scripts. Note that the downgrade scripts just generate empty tables, this has been discussed and is expected to work well enough in realistic use cases (since we haven't been using extractor generated paths widely for some time now). We're also happy that |
As well as the final code review we need to review the QA run (when it finishes) and probably have another try at getting a clean DCA run. I'm away next week, I think at this point it's just going to have to wait... |
There was a problem hiding this comment.
Pull Request Overview
This PR removes extractor-generated path resolution from the Rust implementation, completing the transition to CodeQL-computed paths. The change improves extraction performance and reliability by eliminating complex path resolution logic from the extractor.
Key changes:
- Removes all extractor path resolution logic and configuration options
- Removes
Resolvableand related functionality from the database schema - Removes test suites for disabled path resolution
- Updates upgrade/downgrade scripts to handle schema changes
Reviewed Changes
Copilot reviewed 29 out of 93 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/schema/prelude.py | Removes Resolvable class and path-related fields from Addressable |
| rust/schema/annotations.py | Removes Resolvable base class from MethodCallExpr |
| rust/ql/lib/rust.dbscheme | Updates database schema removing path resolution tables |
| rust/extractor/src/translate/mappings.rs | Removes path resolution trait implementations |
| rust/extractor/src/translate/generated.rs | Removes path resolution emission logic |
| rust/extractor/src/translate/base.rs | Removes path resolution functionality |
| rust/extractor/src/main.rs | Removes path resolution configuration |
| rust/extractor/src/config.rs | Removes skip_path_resolution option |
| rust/codeql-extractor.yml | Removes path resolution configuration |
| rust/ql/test/extractor-tests/canonical_path_disabled/ | Removes test directory for disabled path resolution |
| rust/ql/src/queries/telemetry/RustAnalyzerComparison.qll | Removes path resolution comparison telemetry |
| addressable_extended_canonical_paths.rel: delete | ||
| addressable_crate_origins.rel: delete | ||
| resolvable_resolved_paths.rel: delete | ||
| resolvable_resolved_crate_origins.rel: delete |
There was a problem hiding this comment.
The upgrade script properly handles deletion of the removed database relations. Ensure the corresponding downgrade script properly handles recreation or provides appropriate error handling for partial compatibility.
| addressable_extended_canonical_paths.rel: delete | |
| addressable_crate_origins.rel: delete | |
| resolvable_resolved_paths.rel: delete | |
| resolvable_resolved_crate_origins.rel: delete | |
| resolvable_resolved_crate_origins.rel: delete | |
| description: Downgrade after removing extractor generated paths. The following relations were deleted in the upgrade and cannot be automatically restored: addressable_extended_canonical_paths, addressable_crate_origins, resolvable_resolved_paths, resolvable_resolved_crate_origins. | |
| compatibility: partial | |
| # The following relations were deleted in the upgrade and cannot be recreated automatically. | |
| # If you need to downgrade, you may need to restore these relations from a backup or regenerate them. |
Copilot uses AI. Check for mistakes.
Remove extractor path resolution. The following has been removed:
Resolvablefrom the dbscheme (i.e.getResolvedCrateOrigin(),getResolvedPath()).Addressable.getExtendedCanonicalPath()andAddressable.getCrateOrigin()methods from the dbscheme.Addressableitself remains.TODO:
Addressablefrom the dbscheme? The QL computedgetCanonicalPath()is built on top of it, but it might be possible to define this class in QL now perhaps?PathAstNodefrom the dbscheme?