feat(manager/gradle): rewrite and extend parser

[Churro]

Changes

Tl;dr

• New Gradle parser implementation based on @zharinov's great https://github.com/zharinov/good-enough-parser
• Covers all existing functionality + new Gradle patterns like dependencySet, mapOf and property("foo")
• Resolves at least 9 open issues related to the exiting implementation
• This PR is not meant to be merged, it is thought as a baseline for further, chunked PRs

Where is the catch?

• Matching template strings is currently tailored to existing tests but doesn't work with random combinations like "a${b}c${d}e${f}..."

  • Would require merge of feat: Add query support for string-values and symbols in string-trees zharinov/good-enough-parser#144 (pls @zharinov)
  • Would enable more flexible matching, e.g. "io.jsonwebtoken:jjwt-api:${property("jwtVersion")}" or "io.jsonwebtoken:jjwt-api:${extra["jwtVersion"]}"
  • Code demonstrating this is already there but commented to give you an idea what to expect (Update: resolved)

• With the new parser, handler functions can't work async, so apply from() can't call await parseGradle()

  • To still make it work, I went ahead using fs.getFileContentMap() to load all Gradle files upfront
  • Personally, I think this is viable as Gradle files are overall small. The alternative using fs.readFilesSync() seems less favorable to me but probably I'm overlooking something here and there is another approach I haven't thought about yet.
  • Positive thing: apply from() now plays nicely with ignorePaths

Next steps

• I'd very much appreciate your review of the concept itself, e.g. with regards to strict types

• I'd hope for @zharinov to accept feat: Add query support for string-values and symbols in string-trees zharinov/good-enough-parser#144 and publish the lib with this extension (Update: done)

  • fix: support for groovy variables with only one character zharinov/good-enough-parser#179 would be great too as otherwise single character variables like z = "1.2.3" won't be matched (Update: done)

• I've tested it extensively but, of course, it's hard to rule out all corner cases. So instead of a Big Bang merge, I'd suggest to do it piece-wise in co-existence with the existing parser

  • For example:
    • First replace variable assignments with the new parser + add support for Groovy maps and Kotlin mapOf
    • Replace simple dependency matching (= no interpolation) with the new parser
    • Add resolution of implicit Gradle plugins and so on
  • For each change / addition PR there would be a separate review + test repo + list of issues that are resolved

Please don't hesitate to share your thoughts on this plan too. For now, I didn't remove the tokenizer and other things that are obsolete with this re-implementation.

What's new?

• Improved support for Groovy's property syntax:

  • Dependencies can be defined e.g. as deps.support.design = '...' and later be referenced as deps["support"]["design"]
  • Support for property accessor methods (with optional project and rootProject prefix), e.g.:

    property("foo")
    project.getProperty("foo")
    project.ext.get("foo")

  • This also addresses Gradle-lite - does not detect property() call in kts #9184 (if feat: Add query support for string-values and symbols in string-trees zharinov/good-enough-parser#144 is merged)

• Support for Kotlin extra properties, e.g.:

  extra['foo'] = 'bar'
  project.extra.get("foo")
  rootProject.extra.get("foo")
  
  val springVersion by extra("3.1.0.RELEASE")
  val emailNotification by extra { "build@master.org" }

• Support for Groovy maps up to 3rd-level nesting, e.g.

  var versions = [
    graphqlDgs                    : "4.10.1",
    graphqlJava                   : "18.0",
    modelMapper                   : "3.1.0"
  ]

  Resolves:

  • Support map variables in Gradle parser #18091
  • Resolve dynamic deps versions in gradle #15359
  • Spotbugs library and plugin versions mixed up #13619

• Support for Kotlin's mapOf collections up to 3rd-level nesting, e.g.

  extra["deps"] = mapOf(
    "support" to mapOf(
      "appCompat" to "com.android.support:appcompat-v7:26.0.2",
      "junit" to mapOf(
        "jupiter" to "5.0.1"
      )
    )
  )

• Support for repository URLs set via setUrl(), e.g.:

  maven { setUrl("https://foo.bar/baz") }

• Support for alias definitions with Gradle version catalogs, e.g.:

  version("antlr", "4.10.1")
  alias("antlr").to("org.antlr", "antlr4").versionRef("antlr")
  alias("jackson").to ("com.fasterxml.jackson.core", "jackson-databind").version("2.13.0")

• Support for dependencySet collections (grouped into a single update), e.g.:

  dependencySet(group: 'org.apache.logging.log4j', version: '2.13.3') {
    entry 'log4j-api'
    entry 'log4j-core'
    entry 'log4j-slf4j-impl'
    entry 'log4j-jul'
  }

• Support for implicit Gradle plugins like pmd or checkstyle, e.g.:

  pmd {
    toolVersion = '6.36.0'
  }

  Resolves:

  • Update PMD tool version #12424
  • Update version of Java Lombok inside the Gradle configuration #13333
  • Gradle dependencies declared in build.gradle as ‘checkstyle’ with a variable as the version are not identified  #17657 -> partially, because the notation ${project.extensions.checkstyle.toolVersion} is not (yet) supported

Further Fixes

• Repo detected in publishing() block
  Should ignore publishing registries repositories #17067

• Gradle variable ordering breaks detected dependencies
  Gradle variable ordering breaks detected dependencies #18317

Future Work

Can be adapted (= reusing existing patterns) to match object { ... } definitions in buildSrc/*.kt files, see
#13295, #9410, #12397, #5480

Context

Documentation (please check one with an [x])

• I have updated the documentation, or
• No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

• Code inspection only, or
• Newly added/modified unit tests, or
• No unit tests but ran on a real repository, or
• Both unit tests + ran on a real repository

[zharinov]

Would be complicated to merge it by small steps, seems like parsing mechanism swapping is all-or-nothing?

[Churro]

@zharinov, thank you so much for merging the two PRs into https://github.com/zharinov/good-enough-parser 👍

My initial idea was to parse a Gradle file twice in parseGradle() and do parts with the new parser, others with the old one. Upon second thought, this won't work because variables can be re-assigned and using only the "last assignment" will incur wrong dep extractions.

Another approach could be the following:

• First create a pure refactoring PR, similar to how you did with bazel, swap the parser and maintain feature parity.
• Then do individual follow-up PRs that add support for further patterns like dependencySet and mapOf.

[zharinov]

I think, <drop-in replacement> + <further additions> is a good strategy

[rarkins]

I'm happy if @zharinov and @Churro are in agreement. Based on past experience I assume you don't mind that if we find regression errors, we'll hope to fix them rather than roll back?

[Churro]

Sure, I'm ready to address potential regression errors.

Aligned with what @zharinov wrote above, the idea is to first replace the existing parser (#18663), followed by a cleanup PR to remove the old parser + subsequent PRs that add dependencySet, mapOf, support for implicit Gradle plugins, etc

[rarkins]

So will most of this PR be split off into a drop in refactor PR?

It would be good if we can avoid any new PRs even if they are correct. That way we don't need to worry about rollbacks causing auto closes

[Churro]

So will most of this PR be split off into a drop in refactor PR?

That was the idea with PR #18663. First, to transition all existing functionality to the new parser and then to have subsequent PRs bringing in new features, link issues they resolve and provide evidence via test repos. In case a PR needs to be reverted that brought in e.g. mapOf functionality, it would auto-close only PRs for related deps.

It would be good if we can avoid any new PRs even if they are correct. That way we don't need to worry about rollbacks causing auto closes

Wouldn't this be the same situation if this fully-fledged PR here (or basically any PR that extracts more deps than before) was merged and needed to be rolled back? Test-wise the refactoring PR #18663 shows parity with the existing parser, so even if reverted, no auto close should happen as the current parser finds the very same set of deps.

Related to that, I assume that separate PRs would simplify code reviews with smaller changesets and more obvious reasoning why some changes were made. In any case, I'm ready to advance as you prefer.

P.S.: There are still 2-3 feature things that are not added yet here ATM (e.g. a solution for #8728) but where I thought subsequent PRs would be good enough.

[viceice]

so this has a behavior change and possibly closes PR's if some recursive applied files are not inside manager file list 🤔

[Churro]

so this has a behavior change and possibly closes PR's if some recursive applied files are not inside manager file list 🤔

That's correct. However, from a user perspective, if you already redefine fileMatch to not include such files, I'd also expect renovate to not process them via apply from either. So, perhaps, this change can also be seen as a correction move towards more intuitive renovate behavior.

[rarkins]

Should this now be deconflicted off main and ready for review, or is there some development work remaining, features to be divided up further, etc?

[Churro]

@rarkins, my plan would be as follows:

• Harden the state of the current impl with further tests: test(manager/gradle): add further tests #18977
• Add further PRs that address individual feature additions, e.g. dependencySet, mapOf, implicit gradle plugins etc. + demo repos and reference of issues that are resolved by them - basically as I'd do for other PRs as well.

If that's fine for you too, we could close this PR right now. If you'd prefer just one PR (at least for the changes pushed here so far), I'd be fine too and would then simply restructure this PR.

I'm working on further augmenting the parser implementation and currently have the following things on the radar (going into separate PRs I'd say):

• separate the extraction of registry URLs for plugins and regular dependencies - didn't add this here so far, since I didn't want to mix feature additions with behavioral changes
• matching + concatenated values (addressing Unsupported dependency in gradle #8728)
• matching object { ... } (addressing Support scan buildSrc's Dependencies? #13295, Support for Gradle composite builds #9410, Renovate doesn't scans the latest version based on gradle multi module project using gradle.kts #12397, multimodule gradle (kotlin) with versions defined as constants in buildSrc #5480)
• refactoring the existing implementation and split it feature-wise (feat(manager/gradle): reimplement parser using tree-based approach #18663 (comment))