Skip to content

Ruby: Diff-informed queries: phase 3 (non-trivial locations) #20080

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 1 commit into
base: main
Choose a base branch
from

Conversation

d10c
Copy link
Contributor

@d10c d10c commented Jul 17, 2025

This PR enables diff-informed mode on queries that select a location other than dataflow source or sink. This entails adding a non-trivial location override that returns the locations that are actually selected.

Prior work includes PRs like #19663, #19759, and #19817. This PR uses the same patch script as those PRs to find candidate queries to convert to diff-enabled. This is the final step in mass-enabling diff-informed queries on all the languages.

Commit-by-commit reviewing is recommended.

  • I have split the commits that add/modify tests from the ones that enable/disable diff-informed queries.
  • If the commit modifies a .qll file, in the commit message I've included links to the queries that depend on that .qll for easier reviewing.
  • Feel free to delegate parts of the review to others who may be more specialized in certain languages.

@github-actions github-actions bot added the Ruby label Jul 17, 2025
@d10c d10c added the no-change-note-required This PR does not need a change note label Jul 17, 2025
@d10c d10c marked this pull request as ready for review July 17, 2025 14:37
@Copilot Copilot AI review requested due to automatic review settings July 17, 2025 14:37
@d10c d10c requested a review from a team as a code owner July 17, 2025 14:37
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 diff-informed queries in Ruby by implementing location overrides for queries that select non-trivial locations (not just dataflow sources or sinks). It's part of phase 3 of enabling diff-informed mode across all languages, completing the mass-enablement process.

Key changes:

  • Adds observeDiffInformedIncrementalMode() predicate to disable diff-informed mode for specific queries where it's not suitable
  • Implements workaround for Ruby RegExp parsing issues when constants are folded

@@ -17,6 +17,10 @@ private module MissingFullAnchorConfig implements DataFlow::ConfigSig {
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }

predicate observeDiffInformedIncrementalMode() {
none() // can't be made diff-informed because the locations of Ruby RegExpTerms aren't correct when the regexp is parsed from a string arising from constant folding
Copy link
Preview

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

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

The comment should be more specific about what 'aren't correct' means. Consider clarifying whether the locations are missing, inaccurate, or point to the wrong source positions.

Suggested change
none() // can't be made diff-informed because the locations of Ruby RegExpTerms aren't correct when the regexp is parsed from a string arising from constant folding
none() // can't be made diff-informed because the locations of Ruby RegExpTerms are inaccurate or point to incorrect source positions when the regexp is parsed from a string arising from constant folding

Copilot uses AI. Check for mistakes.

@d10c d10c requested a review from michaelnebel July 17, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant