Fix gradle bumping dependency substitutions issue

[AbhishekBhaskar]

What are you trying to accomplish?

Fixes #15423.

Gradle resolutionStrategy.dependencySubstitution rules use the same coordinate syntax as real dependencies, so Dependabot treated the substituted (source) coordinate as a dependency. A rule like:

substitute module("io.airlift:aircompressor:2.0.2") using module("io.airlift:aircompressor:2.0.3")

got rewritten to substitute module("...:2.0.3") using module("...:2.0.3") — collapsing the substitution into a no-op.

This PR makes Dependabot ignore substitution rules:

• Parser: prepared_content strips dependencySubstitution { ... } blocks, so no coordinate inside them is parsed as a dependency.
• Updater: original_buildfile_declarations skips declarations whose source line is inside a substitution block, so a real dependency sharing the same coordinate/version isn't rewritten into the substitution rule.

Both share FileParser.mask_literals_and_comments, which blanks the contents of string literals and comments (preserving length and newline positions) so brace counting finds the real block boundaries. It handles quoted strings, Groovy slashy/dollar-slashy strings, and line/nested block comments, and blocks are removed before the legacy comment stripping runs.

Anything you want to highlight for special attention from reviewers?

• The block-match regex is unanchored so it also matches the dotted form resolutionStrategy.dependencySubstitution {.
• The updater excludes lines by whether they fall inside a substitution block, so multiline substitute(...).using(...) formatting is handled, not just single-line rules.
• Blocks are deleted by exact offsets rather than a global gsub, which also handles empty dependencySubstitution {} blocks.

How will you know you've accomplished your goal?

• New parser spec asserts substitution coordinates are excluded and substituted versions aren't picked up.
• New updater specs update a real dependency sharing a coordinate with single-line and multiline substitution sources and assert the substitution lines are untouched.
• The fixture covers braces inside quoted/slashy/dollar-slashy reasons, a // inside a string, comment braces, a nested block comment, and an empty block.
• Gradle parser + updater specs pass, rubocop is clean, and Sorbet reports no errors.

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[Copilot]

Pull request overview

Prevents Gradle dependency substitution rules from being parsed or rewritten as dependencies.

Changes:

• Strips dependencySubstitution blocks during parsing.
• Skips single-line substitution rules during updates.
• Adds parser/updater regression coverage and a fixture.

Show a summary per file

File	Description
gradle/lib/dependabot/gradle/file_parser.rb	Excludes substitution blocks from parsing.
gradle/lib/dependabot/gradle/file_updater.rb	Avoids rewriting detected substitution lines.
gradle/spec/dependabot/gradle/file_parser_spec.rb	Tests parser behavior.
gradle/spec/dependabot/gradle/file_updater_spec.rb	Tests preserving substitution rules.
gradle/spec/fixtures/buildfiles/dependency_substitution.gradle	Provides regression fixture.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

gradle/lib/dependabot/gradle/file_updater.rb:267

• This excludes whole lines rather than only the substitution block. Gradle permits compact statements such as resolutionStrategy.dependencySubstitution { ... }; dependencies { implementation "g:a:1" }; the parser preserves and discovers the real declaration after removing the exact block range, but the updater skips that shared line and raises Expected content to change!. Please retain the character offsets and inspect/update only the portions outside substitution blocks so valid declarations on a boundary line remain updateable without rewriting the rule.

        content.lines.each_with_index.filter_map do |line, index|
          next if substitution_ranges.any? { |range| range.cover?(index) }

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

gradle/lib/dependabot/gradle/file_parser.rb:47

• This unbounded match also recognizes suffixes of unrelated identifiers, so a valid custom closure such as notdependencySubstitution { implementation("g:n:1") } is deleted and its real dependency disappears from parsing. Require an identifier boundary before the Gradle API name while still allowing the dotted resolutionStrategy.dependencySubstitution form.

This issue also appears on line 121 of the same file.

      DEPENDENCY_SUBSTITUTION_DECLARATION_REGEX = /dependencySubstitution\s*\{/

gradle/lib/dependabot/gradle/file_updater.rb:26

• This has the same suffix false positive as the parser regex: a custom block named notdependencySubstitution causes every declaration on its covered lines to be skipped by the updater. Add an identifier boundary so only the Gradle dependencySubstitution member is recognized.

This issue also appears on line 266 of the same file.

      SUBSTITUTION_BLOCK_START_REGEX = /dependencySubstitution\s*\{/

gradle/lib/dependabot/gradle/file_updater.rb:267

• This line-based exclusion also drops real declarations that happen to share a line with the substitution block. For example, a valid compact script such as configurations.all { resolutionStrategy.dependencySubstitution { ... } }; dependencies { implementation("g:n:1") } is parsed successfully, but the updater skips the entire line and raises Expected content to change!. Track substitution character offsets and exclude only matches within those offsets rather than excluding whole lines.

        content.lines.each_with_index.filter_map do |line, index|
          next if substitution_ranges.any? { |range| range.cover?(index) }

gradle/lib/dependabot/gradle/file_parser.rb:123

• The first matching triple-quote sequence is not necessarily the delimiter: Groovy triple-quoted strings can contain an escaped delimiter such as \""". In a substitution reason, this closes the mask early, exposes any following brace, and can make the block boundary fail so substitution coordinates are parsed or rewritten again. Locate an unescaped closing delimiter (accounting for an odd backslash run) instead of using the first raw index match.

        if three == '"""' || three == "'''"
          close = content.index(three, index + 3)
          close ? close + 3 : length

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[thavaahariharangit]

LGTM

[Copilot]

Copilot review overview

Review tier: Balanced
Findings: 1 · 2

New issues introduced by this change (3)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This scanner combines incompatible Groovy and Kotlin lexical rules without knowing the build-file…
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This suffix match also treats unrelated custom Gradle closure names such as…
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — This suffix match also marks unrelated custom Gradle closures such as `mydependencySubstitution {…

Suppressed comments (2)

gradle/lib/dependabot/gradle/file_updater.rb:267

• The exclusion is line-based, so it also drops declarations outside the block when they share its opening or closing line. For example, resolutionStrategy.dependencySubstitution {}; dependencies { implementation("g:a:1") } is valid and the parser still finds g:a:1, but this updater finds no declaration and raises Expected content to change!. Exclude only the exact block spans (or mask those spans while preserving the remaining text on each line).

        content.lines.each_with_index.filter_map do |line, index|
          next if substitution_ranges.any? { |range| range.cover?(index) }

gradle/lib/dependabot/gradle/file_parser.rb:692

• Nested matches produce overlapping ranges, but deleting the inner range first shifts the string before the outer range is applied. The outer range then consumes content after its actual closing brace, potentially removing a following real dependency from parser input. Since removing an outer substitution block already removes nested occurrences, skip matches whose start is covered by a previously recorded outer range before mutating the string.

        block_ranges.reverse_each { |range| result[range] = "" }

[AbhishekBhaskar]

Fixed

[Copilot]

Copilot review overview

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 2

New issues introduced by this change (2)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — This scan also runs for non-script requirements because buildfiles contains every non-support…
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This heuristic misclassifies valid no-parentheses Groovy calls such as because /reason {/: the…

Issues resolved since last review (3)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — This suffix match also marks unrelated custom Gradle closures such as `mydependencySubstitution {… View resolved comment
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This suffix match also treats unrelated custom Gradle closure names such as… View resolved comment
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This scanner combines incompatible Groovy and Kotlin lexical rules without knowing the build-file… View resolved comment

Suppressed comments (1)

gradle/lib/dependabot/gradle/file_updater.rb:267

• A range marks the entire opening and closing lines, not just the block's offsets. Valid one-line Groovy such as resolutionStrategy.dependencySubstitution {}; dependencies { implementation "g:a:1" } is parsed as a real dependency after the block is removed, but this updater skips its only declaration and raises Expected content to change!. Track/mask character spans on each line and update only declarations outside those spans; simply retaining the whole line would still let the later global replacement modify the substitution text.

          next if substitution_ranges.any? { |range| range.cover?(index) }

[kbukum1]

I think fileparser now becamee more complicated. Not sure if there is better way for doing all parsing on the file. Maybe we should have helper or something instead of having bunch of if-elses. I am approving. If it is feasable it will be great to have follow-up to kind of proper helper utility where it helps for parsing properly.

[Copilot]

Copilot review overview

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — Skipping the entire covered line also skips real declarations that appear after a substitution…

Issues resolved since last review (2)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This heuristic misclassifies valid no-parentheses Groovy calls such as because /reason {/: the… View resolved comment
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — This scan also runs for non-script requirements because buildfiles contains every non-support… View resolved comment

Suppressed comments (1)

gradle/lib/dependabot/gradle/file_parser.rb:226

• This previous-character heuristic misses valid Groovy slashy strings after keywords. In def reason = { return /tracking {/ } inside a substitution block, the opening slash follows the n in return, so its { remains structural; the closing slash is then mistaken for a new unterminated slashy string, masking the real block close. Both callers consequently fail to exclude the substitution coordinates. Please recognize value-introducing keyword contexts such as return (or use a lexical scanner) and cover this case.

      def self.slashy_string_start?(content, index)
        position = index - 1
        position -= 1 while position >= 0 && (content[position] == " " || content[position] == "\t")
        return true if position.negative?

        !T.must(content[position]).match?(/[\w)\]}]/)

[Copilot]

Copilot review overview

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 3

New issues introduced by this change (3)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — Valid Groovy call syntax also permits the closure inside parentheses, e.g.…
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — The updater has the same gap for the valid Groovy form `resolutionStrategy.dependencySubstitution({…
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This treats any Groovy / after a newline as the start of a slashy string because newline is not…

Pre-existing issues (1)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — Skipping the entire covered line also skips real declarations that appear after a substitution… View comment

[Copilot]

Copilot review overview

🔵 Needs a closer look

Review tier: Balanced
Findings: 3

Pre-existing issues (3)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — This treats any Groovy / after a newline as the start of a slashy string because newline is not… View comment
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — The updater has the same gap for the valid Groovy form `resolutionStrategy.dependencySubstitution({… View comment
	gradle/​lib/​dependabot/​gradle/​file_parser.rb — Valid Groovy call syntax also permits the closure inside parentheses, e.g.… View comment

Issues resolved since last review (1)

Severity	Finding
	gradle/​lib/​dependabot/​gradle/​file_updater.rb — Skipping the entire covered line also skips real declarations that appear after a substitution… View resolved comment

Suppressed comments (3)

gradle/lib/dependabot/gradle/file_parser.rb:47

• Valid Gradle/Groovy DSL can pass the closure in parentheses (resolutionStrategy.dependencySubstitution({ ... })). This regex does not match the ({ form, so those substitution coordinates remain in prepared_content and are still parsed as dependencies. Accept an optional opening parenthesis before the block brace.

      DEPENDENCY_SUBSTITUTION_DECLARATION_REGEX = /\bdependencySubstitution\s*\{/

gradle/lib/dependabot/gradle/file_updater.rb:26

• The parser-side fix also needs to be mirrored here: dependencySubstitution({ ... }) is valid Gradle/Groovy syntax, but this pattern misses it. The updater will therefore treat lines in that block as ordinary declarations and can still rewrite the source coordinate.

      SUBSTITUTION_BLOCK_START_REGEX = /\bdependencySubstitution\s*\{/

gradle/lib/dependabot/gradle/file_updater.rb:286

• Returning an allowed line does not confine the later replacement to that occurrence: update_version_in_buildfile calls content.gsub(declaration, ...) globally. With valid multiline forms such as implementation(\n "g:a:1"\n) and substitute module(\n "g:a:1"\n), identically indented coordinate lines cause the real declaration selected here to rewrite the matching line inside the excluded substitution block too. Carry occurrence offsets/ranges into the replacement (or mask protected ranges during replacement) instead of returning raw line text for a global gsub.

          line if evaluated.include?(T.must(requirement.requirement_string))