Skip to content

Added test showing UpgradeDependencyVersion does not upgrade buildscript.ext {} when using dependencies.gradle #5547

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

JohannisK
Copy link
Contributor

What's changed?

  • Added test showing UpgradeDependencyVersion does not upgrade buildscript.ext {} when using dependencies.gradle

What's your motivation?

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Jun 3, 2025
@JohannisK JohannisK self-assigned this Jun 3, 2025
@JohannisK JohannisK added the bug Something isn't working label Jun 3, 2025
@timtebeek timtebeek marked this pull request as draft June 3, 2025 08:43
@shanman190
Copy link
Contributor

I'm not sure if this test case captures a valid Gradle build. For some reason I think the buildscript.ext is treated separately from project.ext without overlap.

Secondarily, variable substitution was intentionally limited to the current build descriptor that contains a dependency that utilizes the variable (effectively enforcing scoping rules) and gradle.properties because otherwise we have no linkage to knowing that we're updating the correct variable or just some other random one that just so happens to have the same name.

@JohannisK
Copy link
Contributor Author

JohannisK commented Jun 4, 2025

I'm not sure if this test case captures a valid Gradle build. For some reason I think the buildscript.ext is treated separately from project.ext without overlap.

Secondarily, variable substitution was intentionally limited to the current build descriptor that contains a dependency that utilizes the variable (effectively enforcing scoping rules) and gradle.properties because otherwise we have no linkage to knowing that we're updating the correct variable or just some other random one that just so happens to have the same name.

I'm not that big of a gradle expert, but what I found so far is that variables declared in buildscript.ext {} should apply to dependencies in dependencies.gradle. The reported issue also contained examples showing the scenario is used.

I could expand on the solution, by actually keeping track of files where dependencies are declared in the scanner (so I know implementation "com.google.guava:guava:${guavaVersion}" was declared in dependencies.gradle) and then retrace those steps when I encounter apply from: 'dependencies.gradle' in the build.gradle

@shanman190
Copy link
Contributor

Ok, so researched this a little bit more.

buildscript { // receiver = org.gradle.api.Project
  ext { // receiver = org.gradle.api.Project (extension = ext, implementation class = org.gradle.api.plugins.ExtraPropertiesExtension)
    guavaVersion = "29.0-jre" // receiver = org.gradle.api.plugins.ExtraPropertiesExtension
  }
}

So any of these properties are going to be project specific and the imported standalone script would have it's version be directly dependent upon the current project's state of their extension.

So as an illustration:

settings.gradle

include("projectA")
include("projectB")

projectA/build.gradle

ext {
  guavaVersion = "29.0-jre"
}

apply from: "dependencies.gradle"

projectB/build.gradle

ext {
  guavaVersion = "30.0-jre"
}

apply from: "dependencies.gradle"

The above project is valid, but each project would include a different version of Guava because the extra properties are evaluated individually within the context of the script application. This is one of the challenges with respect to global dataflow analysis in terms of Gradle scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants