Strategy/Scala: route sbt 1.4+ with explicit DependencyTreePlugin to built-in command (TKT-15490)#1711
Strategy/Scala: route sbt 1.4+ with explicit DependencyTreePlugin to built-in command (TKT-15490)#1711zlav wants to merge 5 commits into
Conversation
… task When a project explicitly enables `addDependencyTreePlugin` on sbt 1.4+, fossa-cli detected `sbt.plugins.DependencyTreePlugin` and dispatched to the same path used by the legacy `net.virtualvoid.sbt.graph.DependencyGraphPlugin`, which runs the lowercase `dependencyBrowseTreeHtml` task. sbt 1.4+ only exposes the uppercase `dependencyBrowseTreeHTML`, so the task failed and the analyzer silently dropped deep dependencies. Distinguish the two plugins at detection time and pick the correct task casing per plugin. TKT-15490 / ANE-2718.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR refactors Scala sbt plugin detection from boolean tuple results to structured types. New types 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Strategy/Scala.hs (1)
206-212: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider making context and error messages task-agnostic.
The context message on line 206 and error message on line 212 both hardcode
dependencyBrowseTreeHTML, but with the new routing logic the actual task executed could be eitherdependencyBrowseTreeHTML(modern) ordependencyBrowseTreeHtml(legacy) depending on the detected plugin kind. While these messages appear in a code path that runs after tree JSON generation (and thus don't directly cause incorrect behavior), using a generic message like "Analyzing sbt dependencies using dependency tree JSON" would be more accurate and less confusing for users who might see the legacy task invoked.📝 Suggested message updates
-analyzeWithDepTreeJson (ScalaProject _ treeJson closure) = context "Analyzing sbt dependencies using dependencyBrowseTreeHTML" $ do +analyzeWithDepTreeJson (ScalaProject _ treeJson closure) = context "Analyzing sbt dependencies using dependency tree JSON" $ do treeJson' <- errCtx MissingFullDependencyPluginCtx $ errHelp MissingFullDependencyPluginHelp $ errDoc sbtDepsGraphPluginUrl $ errDoc scalaFossaDocUrl $ - fromMaybeText "Could not retrieve output from sbt dependencyBrowseTreeHTML" treeJson + fromMaybeText "Could not retrieve dependency tree JSON output from sbt" treeJson🤖 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/Strategy/Scala.hs` around lines 206 - 212, The context and error messages in analyzeWithDepTreeJson (pattern ScalaProject _ treeJson closure) hardcode dependencyBrowseTreeHTML which can be either dependencyBrowseTreeHTML or dependencyBrowseTreeHtml; update the context string and the fromMaybeText error message to be task-agnostic (e.g., "Analyzing sbt dependencies using dependency tree JSON" and "Could not retrieve output from sbt dependency tree JSON") so they accurately reflect either modern or legacy task execution while still referencing treeJson/ScalaProject/analyzeWithDepTreeJson to locate the change.
🤖 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.
Outside diff comments:
In `@src/Strategy/Scala.hs`:
- Around line 206-212: The context and error messages in analyzeWithDepTreeJson
(pattern ScalaProject _ treeJson closure) hardcode dependencyBrowseTreeHTML
which can be either dependencyBrowseTreeHTML or dependencyBrowseTreeHtml; update
the context string and the fromMaybeText error message to be task-agnostic
(e.g., "Analyzing sbt dependencies using dependency tree JSON" and "Could not
retrieve output from sbt dependency tree JSON") so they accurately reflect
either modern or legacy task execution while still referencing
treeJson/ScalaProject/analyzeWithDepTreeJson to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: f374c435-f539-45fd-b422-6fda86503657
📒 Files selected for processing (4)
Changelog.mdsrc/Strategy/Scala.hssrc/Strategy/Scala/Plugin.hstest/Scala/PluginSpec.hs
The context label and error text mentioned dependencyBrowseTreeHTML, but the routing change can also pick the legacy dependencyBrowseTreeHtml task. Reword to "dependency tree JSON" so the message is correct in both cases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`sbt plugins` lists user-disabled plugins (`disablePlugins(...)`) with a ": disabled in <scope>" suffix. The pre-existing substring match on the bare FQCN counted those as present, which routed `findProjects` to a task the active plugin set does not provide — same shape as the TKT-15490 regression, different trigger. Detection now requires "<FQCN>: enabled in" verbatim. Fixtures cover the two disabled cases (mini disabled, modern disabled), both expected to classify as Nothing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Overview
On sbt 1.4+ projects that load
DependencyTreePluginviaaddDependencyTreePlugin(without the auto-loadedMiniDependencyTreePlugin),fossa analyzeran the lowercase taskdependencyBrowseTreeHtml. Modern sbt only exposes the uppercasedependencyBrowseTreeHTML, so the task failed and deep-dependency analysis silently fell back to a poms-only graph.The detection logic now distinguishes the legacy
net.virtualvoid.sbt.graph.DependencyGraphPlugin(sbt < 1.4, lowercase task) from the modernsbt.plugins.DependencyTreePlugin(sbt 1.4+, uppercase task) and picks the right task name per kind. The built-inMiniDependencyTreePluginpath is unchanged and still preferred when present.Acceptance criteria
fossa analyzeproduces a complete deep-dependency graph on sbt 1.4+ projects that enableaddDependencyTreePlugin.Testing plan
Scala.PluginSpecdetection cases still pass.cabal test unit-tests --test-options="--match \"detectDependencyPlugins\""— 7/7 pass.Risks
hasDependencyPluginsnow returns aDependencyPluginsDetectedrecord instead of(Bool, Bool). One internal call site (Strategy.Scala.findProjects) updated in this PR.Metrics
N/A.
References
-batchand-no-colorsfor sbt for compatibility with older SBT. #1356 (commit215a74f8).Checklist
docs/(Changelog.md## Unreleased)..fossa.ymlorfossa-depsschema changes.