Skip to content

Make compatible with the configuration cache - #25

Draft
mernst wants to merge 13 commits into
mainfrom
config-cache
Draft

mernst wants to merge 13 commits into
mainfrom
config-cache

Conversation

@mernst

@mernst mernst commented Aug 7, 2026

Copy link
Copy Markdown
Member

No description provided.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The plugin now uses provider-backed version resolution and deferred Java compilation configuration. It manages manifest outputs and supports late project configuration. Compiler arguments, task enablement, dependencies, and delombok integration use lazy configuration. Functional tests cover configuration-cache reuse and invalidation, disabling behavior, processor configuration, project properties, manifest cleanup, dependency handling, fork state, and Lombok integration. The README documents configuration-cache compatibility.

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.83% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch config-cache

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/functionalTest/kotlin/org/checkerframework/plugin/gradle/ConfigurationCacheFunctionalTest.kt`:
- Around line 142-150: Update both invalidation tests in
ConfigurationCacheFunctionalTest.kt at lines 142-150 and 187-192: after each
secondResult check, assert that secondResult.output does not contain
CONFIGURATION_CACHE_REUSED and does contain CONFIGURATION_CACHE_STORED, while
preserving the existing compile outcome and diagnostics assertions.

In
`@src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt`:
- Around line 236-238: Update the annotation processor path assignment in the
checker task configuration to append the compile task’s path to the existing
checker task path rather than replacing it. Preserve the manifest directory
added by configureJavaCompileTasks so it remains on the delombok task’s tracked
inputs and changes to checkers invalidate stale results.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d4afd615-97aa-45fb-9e11-21d8387e7db4

📥 Commits

Reviewing files that changed from the base of the PR and between de659cf and e7b3850.

📒 Files selected for processing (8)
  • README.md
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFGroovyPluginFunctionalTest.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFPluginFunctionalTest.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/ConfigurationCacheFunctionalTest.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/Fixtures.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/OtherPluginsFunctionalTest.kt
  • src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt
  • src/main/kotlin/org/checkerframework/plugin/gradle/WriteCheckerManifestTask.kt

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/gradle.yml:
- Around line 61-65: Add job-level permissions to the test-gradle job, setting
contents to read only. Keep the existing Gradle matrix and job behavior
unchanged, and do not broaden permissions beyond the required repository-content
read access.

In
`@src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt`:
- Around line 217-236: Add functional coverage for the plugin configuration
migration by creating a test that declares a dependency constraint and an
exclude rule on checkerFramework, then resolves dependencies and asserts both
rules affect the selected dependency graph. Place the test alongside existing
checkerFramework functional tests and verify the constraint and exclusion
behavior through the resolved configuration.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b819f2c7-085c-4fec-9e7d-a24d60a7835d

📥 Commits

Reviewing files that changed from the base of the PR and between 7aa7e7b and 113e946.

📒 Files selected for processing (8)
  • .github/workflows/gradle.yml
  • build.gradle.kts
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFGroovyPluginFunctionalTest.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFPluginFunctionalTest.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/Fixtures.kt
  • src/functionalTest/kotlin/org/checkerframework/plugin/gradle/KotlinPluginFunctionalTest.kt
  • src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt
  • src/main/kotlin/org/checkerframework/plugin/gradle/WriteCheckerManifestTask.kt

Comment on lines +61 to +65
# Up to date as of 2026-08-07. Only the latest patch version for each minor version.
# Below Gradle 8.2.1, only the Groovy tests run: an older Gradle's Kotlin DSL cannot read
# the Kotlin metadata of this plugin, which is compiled by a newer Kotlin compiler. See
# minimumKotlinDslGradleVersion in the functional tests.
gradle: ['9.7.0', '9.6.1', '9.5.1', '9.4.1', '9.3.1', '9.2.1', '9.1.0', '9.0.0', '8.14.5', '8.13', '8.12.1', '8.11.1', '8.10.2', '8.9', '8.8', '8.7', '8.6', '8.5', '8.4', '8.3', '8.2.1', '8.1.1', '8.0.2', '7.6.6', '7.5.1', '7.4.2', '7.3.3']

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Set explicit least-privilege permissions for test-gradle.

This job executes ./gradlew from checked-out repository code, but it does not declare permissions. The effective GITHUB_TOKEN scope then depends on repository or organization defaults. If those defaults allow writes, this test job has more access than required. Add contents: read at the job level.

Proposed workflow change
   test-gradle:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
     strategy:

GitHub recommends granting only the minimum required token permissions. (docs.github.com)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/gradle.yml around lines 61 - 65, Add job-level permissions
to the test-gradle job, setting contents to read only. Keep the existing Gradle
matrix and job behavior unchanged, and do not broaden permissions beyond the
required repository-content read access.

Source: Linters/SAST tools

Comment on lines +217 to +236
// A constraint is shared rather than copied, which is what extendsFrom does as well. A
// constraint cannot be copied faithfully: DependencyConstraint has no `copy` method, and
// recreating a constraint from its group and name turns a constraint on a project into a
// constraint on an external module, which no longer selects the project.
cfConfigurationValue.allDependencyConstraints.forEach {
targetConfiguration.dependencyConstraints.add(it)
}
// The exclude rules are read from the whole hierarchy, because Configuration.getExcludeRules
// returns only a configuration's own rules, whereas resolution applies the exclude rules of
// every configuration in the hierarchy.
cfConfigurationValue.hierarchy.forEach { configuration ->
configuration.excludeRules.forEach {
val excludeRule = HashMap<String, String>()
it.group?.let { group -> excludeRule["group"] = group }
it.module?.let { module -> excludeRule["module"] = module }
if (excludeRule.isNotEmpty()) {
targetConfiguration.exclude(excludeRule)
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find functional tests that cover constraints or exclude rules on the checkerFramework configuration.
rg -n -C 5 'dependencyConstraints|constraints\s*\{|exclude\s*\(|excludeRules' --glob 'src/functionalTest/**' --glob 'src/test/**'

Repository: typetools/checker-framework-gradle-plugin

Length of output: 179


🏁 Script executed:

#!/bin/bash
set -e

echo "== files =="
git ls-files | rg '(^|/)(CheckerFrameworkPlugin\.kt|build\.gradle|build\.gradle\.kts|.*Test.*|.*Spec.*|.*Functional.*|.*FunctionalTest.*)$|src/functionalTest|src/test' | head -200

echo "== plugin relevant section =="
fd -a 'CheckerFrameworkPlugin.kt' . | while read -r f; do
  echo "--- $f"
  wc -l "$f"
  sed -n '1,280p' "$f" | nl -ba | sed -n '180,255p'
done

echo "== search for addCFDependencies / checkerFramework config =="
rg -n -C 4 'addCFDependencies|withDependencies|checkerFramework|annotationProcessor|dependencyConstraints|exclude\.\\(' --glob '!build/**' --glob '!out/**' .

Repository: typetools/checker-framework-gradle-plugin

Length of output: 1402


🏁 Script executed:

#!/bin/bash
set -e

echo "== CheckerFrameworkPlugin relevant section =="
sed -n '200,245p' src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt

echo "== all constraints/exclude mentions =="
rg -n -C 3 'dependencyConstraints|exclude|withDependencies|checkerFramework|annotationProcessor' src/main/kotlin src/functionalTest || true

echo "== functional test files =="
sed -n '1,220p' src/functionalText/kotlin/org/checkerframework/plugin/gradle/CFPluginFunctionalTest.kt 2>/dev/null || sed -n '1,260p' src/functionalTest/kotlin/org/checkerframework/plugin/gradle/CFPluginFunctionalTest.kt || true

Repository: typetools/checker-framework-gradle-plugin

Length of output: 50397


🌐 Web query:

Gradle Configuration.withDependencies DependencySet documentation mutate dependencyConstraints exclude after resolution

💡 Result:

The withDependencies method in the Gradle API is used to execute an action on a Configuration before it participates in dependency resolution [1][2]. While it allows for programmatic modification of dependencies, it has specific constraints and recommended patterns. Mutating Dependencies and Constraints Although the method is named withDependencies, the official documentation advises against using the provided callback to mutate existing dependencies [1][3]. Instead, you should use Provider-accepting methods (such as addLater) on the DependencySet to add dependencies lazily [1][3]. For version management or forcing versions, use dependency constraints rather than mutating existing dependencies [1][3]. Adding Excludes While withDependencies can technically be used to add excludes to a configuration, it is generally better to handle exclusions via component metadata rules if you want to influence transitive dependencies for a specific library [1][3][4]. For general configuration-wide exclusions, use the standard Configuration.exclude method [5]. After Resolution The withDependencies action must be registered before the configuration is resolved [1][3]. Once a configuration has participated in resolution (e.g., when it is resolved, published, or consumed as a variant), the withDependencies actions will have already been executed and cannot be used to modify the dependency set [1][3]. Key Takeaways 1. Timing: Actions are executed exactly once, immediately before the configuration is first used for resolution, publication, or consumption [1][3]. 2. Best Practices: Favor lazy APIs (e.g., addLater, Provider-based APIs) over modifying the DependencySet inside a withDependencies block [1][3]. 3. Constraints: To affect versions, use dependency constraints [1][3]. 4. Mutation Hierarchy: While it is possible to mutate a configuration's hierarchy (extendsFrom) using this method, it is highly discouraged [1][3]. For most use cases, especially those involving transitive dependency management, component metadata rules or direct resolution strategy modifications (via ResolutionStrategy) are the more robust and recommended approaches [5][4].

Citations:


🌐 Web query:

Gradle Configuration.getExcludeRules exclude rules resolution hierarchy documentation

💡 Result:

In Gradle, the Configuration.getExcludeRules method returns the set of exclude rules applied to a configuration [1][2]. These rules are used to exclude transitive dependencies from the dependency graph for that specific configuration [1][3]. Regarding the resolution hierarchy and logic for these exclusions, Gradle's behavior differs significantly from other build tools like Maven [3][4]. The key principles are: 1. Global vs. Per-Dependency Exclusions: You can apply exclude rules at the configuration level (affecting all dependencies within that configuration via Configuration.exclude) or at the individual dependency level (via ModuleDependency.exclude) [1][5][6]. 2. Dependency Graph Evaluation: Gradle evaluates the entire dependency graph when applying exclusions [3][4]. An exclusion rule is only effective if all paths leading to a specific dependency in the dependency graph agree on the exclusion [3][5]. 3. Effective Exclusion Requirement: If a dependency is excluded by one path but required by another path within the same configuration, the dependency will not be excluded [3][5][6]. Consequently, to fully exclude a transitive dependency that is brought in by multiple upstream dependencies, you must apply the exclusion rule to every path that introduces it [3][5]. Because of this behavior, Gradle documentation recommends that if you need to strictly remove a dependency regardless of how it is requested, alternative approaches—such as component metadata rules (to fix incorrect metadata) or dependency constraints—may be more appropriate and robust than manual exclusions [5][6][4].

Citations:


Add functional coverage for checkerFramework constraints and excludes.

The functional tests do not cover dependencyConstraints or exclude rules on the plugin configuration. Add a test that declares one of each and checks that they affect resolution; otherwise this migration from extendsFrom remains untested for those exclusion mechanisms.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/kotlin/org/checkerframework/plugin/gradle/CheckerFrameworkPlugin.kt`
around lines 217 - 236, Add functional coverage for the plugin configuration
migration by creating a test that declares a dependency constraint and an
exclude rule on checkerFramework, then resolves dependencies and asserts both
rules affect the selected dependency graph. Place the test alongside existing
checkerFramework functional tests and verify the constraint and exclusion
behavior through the resolved configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant