Merged
Conversation
Adds support for analyzing bun.lock lockfiles, which use JSONC format (JSON with Comments). Includes a megaparsec-based JSONC stripper, bun lockfile parser with workspace and transitive dependency support, and documentation. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Bun does not extract peerDependencies from workspace package.json files, so the field is never populated in bun.lock. Package-level peerDependencies (used for transitive edges) are unaffected. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
jagonalez
commented
Feb 20, 2026
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Bun support: a JSONC stripping module (Data.Text.Jsonc) and tests; a ReadFS helper readContentsJsonc; a Bun lockfile parser and dependency-graph builder (Strategy.Node.Bun.BunLock) with tests; Node strategy integration (BunProjectType, Bun project constructor, analyzeBunLock, graph plumbing); small diagnostic message update; Cabal exposure of new modules; docs for Bun in Node.js references; and related test additions. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/Strategy/Node/Bun/BunLock.hs (2)
237-244: Avoid list comprehension.The coding guidelines specify avoiding list comprehensions in Haskell. Consider refactoring to use
mapMaybeor explicit recursion.♻️ Proposed fix
wsPackageNames :: Set.Set Text wsPackageNames = - Set.fromList - [ name - | pkg <- Map.elems (packages lockfile) - , isWorkspaceRef (pkgResolution pkg) - , let (name, _) = parseResolution (pkgResolution pkg) - ] + Set.fromList $ + mapMaybe extractWsName (Map.elems (packages lockfile)) + where + extractWsName pkg + | isWorkspaceRef (pkgResolution pkg) = + let (name, _) = parseResolution (pkgResolution pkg) + in Just name + | otherwise = NothingAs per coding guidelines: "In Haskell, avoid partial functions, list comprehensions, and match guards".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Strategy/Node/Bun/BunLock.hs` around lines 237 - 244, Refactor wsPackageNames to avoid the list comprehension by mapping over Map.elems (packages lockfile) and using mapMaybe (or a fold) to emit Just name only when isWorkspaceRef (pkgResolution pkg) holds; use parseResolution (pkgResolution pkg) to extract the (name, _) pair and then feed the resulting list into Set.fromList — reference symbols: wsPackageNames, packages, lockfile, pkgResolution, isWorkspaceRef, parseResolution, Set.fromList, Map.elems.
204-204: Minor: Haddock comment formatting.The Haddock comments on lines 204, 221, 229, and 233 have
-- \|instead of-- |. The backslash will prevent these from being recognized as Haddock documentation.📝 Proposed fix
- -- \| Convert a package to a Dependency, inferring environment from workspace declarations. + -- | Convert a package to a Dependency, inferring environment from workspace declarations.Apply similar fix to lines 221, 229, and 233.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Strategy/Node/Bun/BunLock.hs` at line 204, The Haddock comments in BunLock.hs use a backslash before the pipe (e.g., the comment above the function that "Convert a package to a Dependency, inferring environment from workspace declarations."), preventing them from being parsed as Haddock; update those comment prefixes from "-- \|" to "-- |" at the occurrences noted (the comment at the "Convert a package to a Dependency..." and the other Haddock lines referenced around the same area) so they become proper Haddock documentation for the corresponding functions/definitions.test/Bun/BunLockSpec.hs (1)
78-79: Avoid partial functionMap.!.Using
Map.!can throw an exception with an unhelpful error message if the key is missing. Consider usingMap.lookupwith explicit error handling orMap.!?to provide clearer test failure messages.This pattern appears throughout the test file (lines 78-79, 88-91, 95-96, 124-126, 130-131, 154, 158-165).
♻️ Proposed fix example
- wsName (workspaces lockfile Map.! "") `shouldBe'` "jsonc-test" - wsDependencies (workspaces lockfile Map.! "") `shouldBe'` Map.fromList [("lodash", "^4.17.21")] + case Map.lookup "" (workspaces lockfile) of + Nothing -> fail "Expected root workspace ''" + Just ws -> do + wsName ws `shouldBe'` "jsonc-test" + wsDependencies ws `shouldBe'` Map.fromList [("lodash", "^4.17.21")]As per coding guidelines: "In Haskell, avoid partial functions".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Bun/BunLockSpec.hs` around lines 78 - 79, The tests use the partial operator Map.! to access workspaces (e.g. expressions involving workspaces, lockfile, wsName and wsDependencies) which can throw unclear exceptions; replace each Map.! usage with safe lookup (Map.lookup or Map.!?) and assert the presence with an explicit failure message (for example assert that Map.lookup workspaces "" is Just ws or use shouldBe (Just expected) or an explicit case that calls expectation with a clear message) so failures show which key was missing and what was expected; update all occurrences (the checks for wsName and wsDependencies and the other listed lines) to perform a safe lookup on workspaces and then examine the retrieved value instead of using Map.! .
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Strategy/Node/Bun/BunLock.hs`:
- Around line 237-244: Refactor wsPackageNames to avoid the list comprehension
by mapping over Map.elems (packages lockfile) and using mapMaybe (or a fold) to
emit Just name only when isWorkspaceRef (pkgResolution pkg) holds; use
parseResolution (pkgResolution pkg) to extract the (name, _) pair and then feed
the resulting list into Set.fromList — reference symbols: wsPackageNames,
packages, lockfile, pkgResolution, isWorkspaceRef, parseResolution,
Set.fromList, Map.elems.
- Line 204: The Haddock comments in BunLock.hs use a backslash before the pipe
(e.g., the comment above the function that "Convert a package to a Dependency,
inferring environment from workspace declarations."), preventing them from being
parsed as Haddock; update those comment prefixes from "-- \|" to "-- |" at the
occurrences noted (the comment at the "Convert a package to a Dependency..." and
the other Haddock lines referenced around the same area) so they become proper
Haddock documentation for the corresponding functions/definitions.
In `@test/Bun/BunLockSpec.hs`:
- Around line 78-79: The tests use the partial operator Map.! to access
workspaces (e.g. expressions involving workspaces, lockfile, wsName and
wsDependencies) which can throw unclear exceptions; replace each Map.! usage
with safe lookup (Map.lookup or Map.!?) and assert the presence with an explicit
failure message (for example assert that Map.lookup workspaces "" is Just ws or
use shouldBe (Just expected) or an explicit case that calls expectation with a
clear message) so failures show which key was missing and what was expected;
update all occurrences (the checks for wsName and wsDependencies and the other
listed lines) to perform a safe lookup on workspaces and then examine the
retrieved value instead of using Map.! .
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Node/Bun/BunLock.hs`:
- Around line 138-147: Replace the guarded equations in parseResolution and
wsPackageName with explicit if/then/else expressions: locate the parseResolution
function and replace the top-level guard (| "@" `Text.isPrefixOf` res = ...)
with an if Text.isPrefixOf "@" res then ... else ... form preserving the same
let bindings and return values (i.e., construct ("@" <> scopeAndName, Text.drop
1 rest) in the then branch and (name, Text.drop 1 rest) in the else branch);
similarly locate wsPackageName and convert its guards into a single if/then/else
that selects the scoped-path parsing branch when the Text.isPrefixOf check is
true and the unscoped branch otherwise, keeping all existing local bindings and
results unchanged.
- Around line 205-227: Rewrite envOf to eliminate guard syntax and to compute
environments transitively: build two sets of package names reachable from
workspace production roots and from workspace dev roots (by traversing the
lockfile dependency graph from each workspace's declared deps), then determine a
package's environments by checking membership in those reachable sets (could be
prod-only, dev-only, or both). Replace the current envOf/BunPackage ->
DepEnvironment API with a function that returns a Set DepEnvironment (or adjust
toDependencyAs accordingly) and update toDependencyAs (and its use of
dependencyEnvironments) to assign the unioned set instead of Set.singleton;
remove the name-only direct lookup via devDepNames and use the reachability sets
derived from the graph to ensure transitive propagation.
…test Consolidate toDependency/toDependencyAs/envOf into toDependency, toDependencyWithEnv, and mkDep so parseResolution is called once per package. Add test verifying transitive deps of dev deps are labeled as production, consistent with npm v3 and pnpm. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/Node/Bun/BunLock.hs`:
- Around line 173-182: The guarded-case in buildGraph's traversal (the
Map.lookup childName branch in BunLock.hs) should be rewritten to avoid match
guards: replace the case with a plain lookup result match, then inside the Just
childPkg arm use an if isWorkspaceRef (pkgResolution childPkg) then pure () else
edge parentDep (toDependency childPkg); keep the Nothing -> pure () arm
unchanged and preserve the existing parentDep, deep parentDep, and
transitiveDepNames logic so behavior is identical but without guarded
alternatives.
In `@test/Bun/BunLockSpec.hs`:
- Around line 211-219: The functions shouldContainDep and shouldNotContain
currently use guarded equations; replace those guards with explicit conditional
expressions using if ... then ... else. Update shouldContainDep :: [Dependency]
-> Dependency -> Expectation to use if dep `elem` deps then pure () else
expectationFailure (show (dependencyName dep) ++ " not found in direct
dependencies"), and update shouldNotContain :: (Eq a, Show a) => [a] -> a ->
Expectation to use if x `elem` xs then expectationFailure (show x ++ " should
not be in " ++ show xs) else pure () so the logic and types (Dependency,
Expectation) remain unchanged.
- Around line 76-80: The tests use partial Map.! lookups on the workspaces map
(e.g. expressions referencing workspaces lockfile Map.! "") which can crash if
the key is missing; replace each Map.! usage with Map.lookup and explicitly
handle the Maybe (for example assert Map.lookup "" (workspaces lockfile)
`shouldSatisfy` isJust then extract the Just value or assert it equals Just
expected), updating assertions that call wsName and wsDependencies to first
obtain the workspace via Map.lookup; apply this change to all occurrences around
parseBunLockEffect, lockfileVersion, workspaces, wsName, and wsDependencies in
the file.
- Around line 204-210: parseBunLockIO currently uses String-based readFile and
then toText; change it to read Text directly: import Data.Text.IO as TextIO
(readFile) and use TextIO.readFile in parseBunLockIO so `contents` is Text, then
call stripJsonc on contents (remove the toText conversion) and keep using
encodeUtf8 on the stripped Text before eitherDecodeStrict; the function name to
locate is parseBunLockIO and helper functions are stripJsonc and
eitherDecodeStrict/encodeUtf8.
---
Duplicate comments:
In `@src/Strategy/Node/Bun/BunLock.hs`:
- Around line 139-147: The functions parseResolution and wsPackageName currently
use Haskell guard syntax (e.g., | "@" `Text.isPrefixOf` res = ...) which the
guidelines disallow; replace each guarded equation with an explicit if/then/else
expression: for parseResolution, test Text.isPrefixOf "@" res with if ... then
build the scoped-package branch (compute withoutAt, scopeAndName/rest and return
("@" <> scopeAndName, Text.drop 1 rest)) else compute the non-scoped branch
(breakOn "@" res and return (name, Text.drop 1 rest)); do the analogous
transformation in wsPackageName so both functions use no guards and preserve the
same tuple return shapes and intermediate names (withoutAt, scopeAndName, rest,
name) and behavior.
Replace readFile (String) + toText with TextIO.readFile (Text) to match the convention used by other test files in the codebase. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/Bun/BunLockSpec.hs`:
- Around line 212-220: The functions shouldContainDep and shouldNotContain use
pattern guards which are disallowed; replace each guarded definition with an
explicit if/then/else expression: in shouldContainDep check "if dep `elem` deps
then pure () else expectationFailure ..." and in shouldNotContain check "if x
`elem` xs then expectationFailure ... else pure ()", preserving the same failure
messages and types (Expectation) and keeping the same function names and
argument order.
- Around line 79-80: The tests use partial Map.! lookups on workspaces lockfile
(e.g., wsName (workspaces lockfile Map.! "") and wsDependencies (workspaces
lockfile Map.! "")); replace these with Map.lookup and handle the Nothing case
explicitly so failures produce clear messages instead of crashes: call
Map.lookup "" (workspaces lockfile), pattern-match or use maybe/when to fail
with a descriptive expectation (e.g., "expected workspace '' to exist") and then
assert wsName and wsDependencies on the retrieved value; apply the same change
to all other occurrences where Map.! is used in these specs (including the other
workspace/package assertions).
- Line 12: Replace the use of String/ByteString for parse errors with Text
across the spec (remove the Data.String.Conversion (encodeUtf8) import), change
the parse-producing functions/values to return Text (e.g., any parse* or
parseError bindings referenced in this file and the block around lines 199-210),
and only convert to String at the Hspec boundary by passing Text.unpack (or
equivalent) into expectationFailure; update calls to expectationFailure to
accept the converted Text and remove encodeUtf8 usage.
csasarak
approved these changes
Feb 20, 2026
Exclude file, link, workspace, root, and module resolution types from the dependency graph, consistent with how npm, pnpm, and yarn handle local packages. Git/github packages are now reported as GitType. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Strategy/Node/Bun/BunLock.hs (1)
15-43: Consider aligning import style with “explicit + qualified” guideline (new module).A lot of imports are explicit, but several are still unqualified (e.g.,
DepTypes,Effect.Grapher). If the repo’s style is “qualified with full names” everywhere, it’s easiest to fix in a brand-new module before it spreads.As per coding guidelines, “Use explicit imports in Haskell, qualified with full names”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Strategy/Node/Bun/BunLock.hs` around lines 15 - 43, The file mixes unqualified module imports (e.g., DepTypes, Effect.Grapher, Graphing, Path) with explicit imports; update these to follow the repo guideline “explicit imports qualified with full names”: import DepTypes, Effect.Grapher, Graphing, and Path as qualified modules (e.g., import DepTypes qualified as DepTypes) and list only the used symbols explicitly where possible, then update all references in this module (types like DepEnvironment, DepType, Dependency, VerConstraint and functions like deep, direct, edge, evalGrapher, run, and any Path/Graphing usages) to use the qualified module prefixes to match the new imports.test/Bun/BunLockSpec.hs (1)
54-70: Add a couple fixture/assertion cases to covergit+and additional non-npm/non-git resolution prefixes.Right now, the tests exercise
github:andworkspace:/file:parsing, butStrategy.Node.Bun.BunLock.resolutionToDepalso treatsgit+specially and is intended to exclude other non-registry types. Adding 1–2 targeted cases would reduce regressions in the filtering logic.Based on learnings, “Applies to **/*.{hs,rs} : Code should be thoroughly tested with appropriate unit tests”.
Also applies to: 99-118, 137-148, 171-184
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/Bun/BunLockSpec.hs` around lines 54 - 70, Add 1–2 unit tests to parseResolutionSpec to cover the missing resolution prefixes: call parseResolution with a git+ URL (e.g., "pkg@git+https://github.com/user/repo.git#commit") and with another non-registry prefix your parsing must exclude (e.g., "pkg@bitbucket:user/repo" or a custom "pkg@custom:ref"), and assert the expected (name, resolution) tuples; this ensures parseResolution (and downstream Strategy.Node.Bun.BunLock.resolutionToDep) correctly handles/remains consistent for git+ and other non-npm/non-git prefixes.
🤖 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/Node/Bun/BunLock.hs`:
- Around line 165-178: The graph splits identical packages because Dependency's
equality/ordering includes dependencyEnvironments while markDirectDeps uses
toDependencyWithEnv and transitive traversal uses toDependency, creating
separate vertices; fix by making package identity ignore dependencyEnvironments
(e.g., implement a canonical key function over (dependencyType, dependencyName,
dependencyVersion) used for graph keys and sorting instead of derived Eq/Ord on
Dependency) or by normalizing/merging environments at construction (ensure
toDependency and toDependencyWithEnv produce the same base identity or union
environments when inserting edges); update usages in markDirectDeps, mkDep,
toDependency, and toDependencyWithEnv to rely on the chosen canonical identity
so vertices collapse correctly.
- Around line 199-235: The isLocalRef function does not treat the "npm:" prefix
as a local/unsupported resolution, so npm-aliased package specs like
"npm:`@scope/pkg`@1.2.3" will be mis-classified; update isLocalRef (referenced by
resolutionToDep and used by toDependency/toDependencyWithEnv) to also return
True when Text.isPrefixOf "npm:" v so these npm-aliased resolutions are filtered
out as local/unsupported.
---
Duplicate comments:
In `@src/Strategy/Node/Bun/BunLock.hs`:
- Around line 136-145: The parseResolution function uses pattern guards to
detect scoped packages; replace the guard-based pattern with an explicit
if/then/else that checks Text.isPrefixOf "@" on the input (res) and performs the
same logic (drop the leading '@', split on the next '@' to return (scopeOrName,
version) or the unscoped branch), preserving the exact string operations and
returns; do the same for resolutionToDep (the similar guarded code around lines
220-226) by converting its guards into equivalent if/then/else branches while
keeping function names and behavior unchanged.
In `@test/Bun/BunLockSpec.hs`:
- Around line 216-224: The two functions shouldContainDep and shouldNotContain
use guarded equations which violate style rules; replace each guard with an
explicit conditional (e.g., if elem dep deps then pure () else
expectationFailure ...) or a case on elem to preserve behavior and
messages—ensure shouldContainDep still calls show (dependencyName dep) in the
failure message and shouldNotContain still shows x and xs; update the function
bodies for shouldContainDep and shouldNotContain accordingly.
---
Nitpick comments:
In `@src/Strategy/Node/Bun/BunLock.hs`:
- Around line 15-43: The file mixes unqualified module imports (e.g., DepTypes,
Effect.Grapher, Graphing, Path) with explicit imports; update these to follow
the repo guideline “explicit imports qualified with full names”: import
DepTypes, Effect.Grapher, Graphing, and Path as qualified modules (e.g., import
DepTypes qualified as DepTypes) and list only the used symbols explicitly where
possible, then update all references in this module (types like DepEnvironment,
DepType, Dependency, VerConstraint and functions like deep, direct, edge,
evalGrapher, run, and any Path/Graphing usages) to use the qualified module
prefixes to match the new imports.
In `@test/Bun/BunLockSpec.hs`:
- Around line 54-70: Add 1–2 unit tests to parseResolutionSpec to cover the
missing resolution prefixes: call parseResolution with a git+ URL (e.g.,
"pkg@git+https://github.com/user/repo.git#commit") and with another non-registry
prefix your parsing must exclude (e.g., "pkg@bitbucket:user/repo" or a custom
"pkg@custom:ref"), and assert the expected (name, resolution) tuples; this
ensures parseResolution (and downstream
Strategy.Node.Bun.BunLock.resolutionToDep) correctly handles/remains consistent
for git+ and other non-npm/non-git prefixes.
Switch from plain Grapher to LabeledGrapher so that vertices are environment-agnostic and environments accumulate as labels. This fixes duplicate vertices when the same package appears in both prod and dev across different workspaces. Also adds git dependency support (github:/git+ resolutions), a mixed-envs test fixture verifying environment merging, simplifies readContentsJsonc per review feedback, and updates changelog/docs. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The field was parsed but never used in analysis. Remove it per review feedback to avoid keeping dead code. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This reverts commit 61a5ce9.
# Conflicts: # Changelog.md
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
PR adds support for analyzing bun projects, we'll eventually need support for fossabot and it looks like there's a few requests from customer for support.
Acceptance criteria
Testing plan
Risks
Metrics
Is this change something that can or should be tracked? If so, can we do it today? And how? If its easy, do it
References
Add links to any referenced GitHub issues, Zendesk tickets, Jira tickets, Slack threads, etc.
Example:
Checklist
docs/.docs/README.msand gave consideration to how discoverable or not my documentation is.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. You may also need to update these if you have added/removed new dependency type (e.g.pip) or analysis target type (e.g.poetry).docs/references/subcommands/<subcommand>.md.