Skip to content

Ruby: add support for extracting overlay databases #19684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nickrolfe
Copy link
Contributor

There will need to be followup changes on the QL side before querying will work, but this is enough to be able to build an overlay database for Ruby.

@nickrolfe nickrolfe added the Ruby label Jun 6, 2025
@github-actions github-actions bot added QL-for-QL Rust Pull requests that update Rust code labels Jun 6, 2025
@nickrolfe nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from 6795153 to 33c2550 Compare June 6, 2025 13:26
nickrolfe added 3 commits June 6, 2025 15:03
This is required for overlay support.
Supports only a minimal subset of the project layout specification;
enough to work with the transformers produced by the CLI when building
an overlay database.
@nickrolfe nickrolfe force-pushed the nickrolfe/ruby-overlay-extraction branch from 33c2550 to fb89f22 Compare June 6, 2025 14:03
@nickrolfe nickrolfe marked this pull request as ready for review June 10, 2025 16:45
@Copilot Copilot AI review requested due to automatic review settings June 10, 2025 16:45
@nickrolfe nickrolfe requested review from a team as code owners June 10, 2025 16:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for overlay database extraction in Ruby by introducing a new databaseMetadata relation and path‐prefix transformation logic.

  • Introduces a --add-metadata-relation flag to the shared extractor generator and implements create_database_metadata().
  • Adds PathTransformer and normalize_and_transform_path in the shared extractor for prefix rewrites.
  • Updates the Ruby extractor to skip unchanged files, load overlay changes, apply path transformations, and emit an empty base‐metadata file.

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
shared/tree-sitter-extractor/src/generator/mod.rs Added add_metadata_relation flag and create_database_metadata
shared/tree-sitter-extractor/src/file_paths.rs Implemented PathTransformer, load_path_transformer, and path normalization with transformation
shared/tree-sitter-extractor/src/extractor/*.rs Propagated transformer through file population and trap writing
ruby/ql/lib/ruby.dbscheme Added new databaseMetadata relation definition
config/dbscheme-fragments.json Registered the Database metadata fragment
ruby/extractor/src/extractor.rs Added overlay‐change filtering, transformer usage, and metadata output
ruby/extractor/src/generator.rs Enabled metadata relation in the Ruby generator
Comments suppressed due to low confidence (1)

ruby/extractor/src/extractor.rs:347

  • [nitpick] New get_overlay_changed_files logic and load_path_transformer are not covered by existing tests. Consider adding unit tests for parsing the JSON changes file and path transformation behavior.
fn get_overlay_changed_files() -> Option<HashSet<PathBuf>> {

/// Normalizes the path according the common CodeQL specification. Assumes that
/// `path` has already been canonicalized using `std::fs::canonicalize`.
pub fn normalize_path(path: &Path) -> String {
/// This represents the minimum supported path transofmration that is needed to support extracting
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo in the doc comment: 'transofmration' should be 'transformation'.

Suggested change
/// This represents the minimum supported path transofmration that is needed to support extracting
/// This represents the minimum supported path transformation that is needed to support extracting

Copilot uses AI. Check for mistakes.

Comment on lines +364 to +370
.map(|change| {
change
.as_str()
.map(|s| PathBuf::from(s).canonicalize().ok())
.flatten()
})
.collect()
Copy link
Preview

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collecting an iterator of Option<PathBuf> will not produce HashSet<PathBuf>. You should use filter_map to drop None entries and wrap the resulting HashSet in Some(...) before returning.

Suggested change
.map(|change| {
change
.as_str()
.map(|s| PathBuf::from(s).canonicalize().ok())
.flatten()
})
.collect()
.filter_map(|change| {
change
.as_str()
.and_then(|s| PathBuf::from(s).canonicalize().ok())
})
.collect::<HashSet<PathBuf>>()
.into()

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QL-for-QL Ruby Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant