Gradle: Add additional development and test configurations for common plugins#1685
Gradle: Add additional development and test configurations for common plugins#1685
Conversation
WalkthroughThis pull request enhances Gradle configuration mapping to classify additional plugin-related configurations as development or test dependencies. The implementation refactors 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/references/strategies/languages/gradle/gradle.md (1)
244-255:⚠️ Potential issue | 🟡 MinorTest configuration docs are now partially updated but still incomplete.
The list omitstestFixtures*configurations that are still classified as test insrc/Strategy/Gradle/Common.hs(Lines 59-66). Please either document all supported configs or explicitly label this as a non-exhaustive subset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/references/strategies/languages/gradle/gradle.md` around lines 244 - 255, The docs list of test configurations in gradle.md is incomplete because the code in src/Strategy/Gradle/Common.hs still treats testFixtures* configurations as test deps; update the docs to either add the missing testFixtures entries (e.g., testFixturesImplementation, testFixturesCompileOnly, testFixturesRuntimeOnly, testFixturesCompileClasspath, testFixturesRuntimeClasspath) to the enumerated list (the same list that currently shows testImplementation, testCompileOnly, testRuntimeOnly, testCompileClasspath, testRuntimeClasspath, etc.) or explicitly mark the section as a non-exhaustive subset and point readers to the canonical source in Common.hs that defines the full classification.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Strategy/Gradle/Common.hs`:
- Around line 50-53: Add explicit unit tests verifying that each new config-name
string literal (e.g., "aggregateCodeCoverageReportResults", "jacocoAgent",
"jacocoAggregation", "jacocoAnt" and the similar literals at lines 68-74) is
classified to the intended environment rather than falling through to EnvOther;
locate the classification function in Common.hs (e.g., the mapping or function
that returns EnvOther) and write tests that call that classifier with each
literal and assert the expected non-EnvOther value, including negative tests to
ensure typos would fail.
- Around line 40-46: Replace the match-guard form in configNameToLabel with an
explicit if/then/else chain that evaluates the same predicates in order: check x
`elem` knownDevConfigs, then x `elem` knownTestConfigs, then
isDefaultAndroidDevConfig x, then isDefaultAndroidTestConfig x, and finally
default to Env (EnvOther x); return EnvDevelopment/EnvTesting or EnvOther
accordingly, preserving the same semantics and using the existing symbols
(knownDevConfigs, knownTestConfigs, isDefaultAndroidDevConfig,
isDefaultAndroidTestConfig, Env, EnvDevelopment, EnvTesting, EnvOther) to locate
the code to change.
---
Outside diff comments:
In `@docs/references/strategies/languages/gradle/gradle.md`:
- Around line 244-255: The docs list of test configurations in gradle.md is
incomplete because the code in src/Strategy/Gradle/Common.hs still treats
testFixtures* configurations as test deps; update the docs to either add the
missing testFixtures entries (e.g., testFixturesImplementation,
testFixturesCompileOnly, testFixturesRuntimeOnly, testFixturesCompileClasspath,
testFixturesRuntimeClasspath) to the enumerated list (the same list that
currently shows testImplementation, testCompileOnly, testRuntimeOnly,
testCompileClasspath, testRuntimeClasspath, etc.) or explicitly mark the section
as a non-exhaustive subset and point readers to the canonical source in
Common.hs that defines the full classification.
🪄 Autofix (Beta)
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: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: bcd95f74-54b7-4868-975d-56b8b055cf51
📒 Files selected for processing (3)
Changelog.mddocs/references/strategies/languages/gradle/gradle.mdsrc/Strategy/Gradle/Common.hs
| configNameToLabel conf = | ||
| case conf of | ||
| "compileOnly" -> Env EnvDevelopment | ||
| x | x `elem` knownDevConfigs -> Env EnvDevelopment | ||
| x | x `elem` knownTestConfigs -> Env EnvTesting | ||
| x | isDefaultAndroidDevConfig x -> Env EnvDevelopment | ||
| x | isDefaultAndroidTestConfig x -> Env EnvTesting | ||
| x -> Env $ EnvOther x |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n src/Strategy/Gradle/Common.hs | head -60Repository: fossas/fossa-cli
Length of output: 2395
Replace match guards in configNameToLabel.
This function uses match guards (| syntax), which violates the Haskell guideline: "avoid partial functions, list comprehensions, and match guards."
Suggested refactor
configNameToLabel :: Text -> GradleLabel
configNameToLabel conf =
- case conf of
- x | x `elem` knownDevConfigs -> Env EnvDevelopment
- x | x `elem` knownTestConfigs -> Env EnvTesting
- x | isDefaultAndroidDevConfig x -> Env EnvDevelopment
- x | isDefaultAndroidTestConfig x -> Env EnvTesting
- x -> Env $ EnvOther x
+ if conf `elem` knownDevConfigs
+ then Env EnvDevelopment
+ else
+ if conf `elem` knownTestConfigs
+ then Env EnvTesting
+ else
+ if isDefaultAndroidDevConfig conf
+ then Env EnvDevelopment
+ else
+ if isDefaultAndroidTestConfig conf
+ then Env EnvTesting
+ else Env (EnvOther conf)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Strategy/Gradle/Common.hs` around lines 40 - 46, Replace the match-guard
form in configNameToLabel with an explicit if/then/else chain that evaluates the
same predicates in order: check x `elem` knownDevConfigs, then x `elem`
knownTestConfigs, then isDefaultAndroidDevConfig x, then
isDefaultAndroidTestConfig x, and finally default to Env (EnvOther x); return
EnvDevelopment/EnvTesting or EnvOther accordingly, preserving the same semantics
and using the existing symbols (knownDevConfigs, knownTestConfigs,
isDefaultAndroidDevConfig, isDefaultAndroidTestConfig, Env, EnvDevelopment,
EnvTesting, EnvOther) to locate the code to change.
| [ "aggregateCodeCoverageReportResults" | ||
| , "jacocoAgent" | ||
| , "jacocoAggregation" | ||
| , "jacocoAnt" |
There was a problem hiding this comment.
Add explicit tests for the new config-name mappings.
These new string literals are on the classification path; a typo/regression silently falls through to EnvOther, which directly impacts alert labeling behavior.
As per coding guidelines, "Code should be thoroughly tested with appropriate unit tests".
Also applies to: 68-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Strategy/Gradle/Common.hs` around lines 50 - 53, Add explicit unit tests
verifying that each new config-name string literal (e.g.,
"aggregateCodeCoverageReportResults", "jacocoAgent", "jacocoAggregation",
"jacocoAnt" and the similar literals at lines 68-74) is classified to the
intended environment rather than falling through to EnvOther; locate the
classification function in Common.hs (e.g., the mapping or function that returns
EnvOther) and write tests that call that classifier with each literal and assert
the expected non-EnvOther value, including negative tests to ensure typos would
fail.
3 participants
Overview
Classifies several Gradle configurations from the SpotBugs and JaCoCo plugins as development-only or test-only, reducing false-positive security alerts on dependencies that never ship to production.
This is a re-push of #1684 from a branch on the main repo so that CI can run (the original PR was from a first-time contributor and CI was gated).
Original PR: #1684 by @jeffalder
Original commits preserved with their authorship.
Changes:
spotbugs,spotbugsPlugins,spotbugsSlf4jaggregateCodeCoverageReportResults,jacocoAgent,jacocoAggregation,jacocoAntconfigNameToLabelto use a list lookup for dev configs (mirroring the existing pattern for test configs)Acceptance criteria
When users run
fossa analyzeon a Gradle project with the SpotBugs or JaCoCo plugins applied, dependencies from those configurations should no longer surface as production security alerts.Testing plan
configNameToLabelbranchesRisks
Minimal -- additive change to two known-config lists. Worst case: a user who currently sees these deps as
EnvOtherin their FOSSA results will stop seeing them.Metrics
N/A
References
Checklist
docs/.Changelog.md. If this PR did not mark a release, I added my changes into an## Unreleasedsection at the top..fossa.ymlorfossa-deps.{json.yml}, I updateddocs/references/files/*.schema.jsonAND I have updated example files used byfossa initcommand.docs/references/subcommands/<subcommand>.md.