Cargo: Fix transitive dev-dependency classification (#1692)

Author: spatten

Changelog.md

@@ -1,5 +1,9 @@
 # FOSSA CLI Changelog
 
+## 3.17.9
+
+- Cargo: Fix transitive dev-dependency classification. Dependencies reachable only through dev-dep or build-dep roots are now correctly labeled as Development instead of Production ([#1692](https://github.com/fossas/fossa-cli/pull/1692)).
+
 ## 3.17.8
 
 - Vendored dependencies: archive uploads with an absolute `path` (as produced by the meta-fossa Yocto layer) no longer crash with a `permission denied` error while writing the tarball. ([#1713](https://github.com/fossas/fossa-cli/pull/1713))
@@ -33,8 +37,7 @@
 - Poetry: Support PEP 621 `[project].dependencies` for Poetry 2.x projects. Production dependencies declared in the standard `[project]` section are now correctly detected alongside legacy `[tool.poetry.dependencies]`. ([#1683](https://github.com/fossas/fossa-cli/pull/1683))
 
 ## 3.17.1
-
-- Node.js: Yarn and npm workspace packages now appear as individual build targets (e.g. `yarn@./:my-package`, `npm@./:my-package`), enabling per-package dependency scoping via `.fossa.yml`.
+- Node.js: Yarn and npm workspace packages now appear as individual build targets (e.g. `yarn@./:my-package`, `npm@./:my-package`), enabling per-package dependency scoping via `.fossa.yml`. ([#1643](https://github.com/fossas/fossa-cli/pull/1643))
 - Project edit: Fix 500 error when running `fossa project edit --policy` on existing projects ([#1688](https://github.com/fossas/fossa-cli/pull/1688))
 - UV: Add `directory` source type to uv.lock parser, fixing parse failures on projects with local directory dependencies ([#1690](https://github.com/fossas/fossa-cli/pull/1690))
 

src/Strategy/Cargo.hs

@@ -52,8 +52,9 @@ import Data.Foldable (for_, traverse_)
 import Data.Functor (void)
 import Data.List.NonEmpty qualified as NonEmpty
 import Data.Map.Strict qualified as Map
-import Data.Maybe (catMaybes, fromMaybe, isJust)
+import Data.Maybe (catMaybes, fromMaybe, isJust, isNothing)
 import Data.Set (Set)
+import Data.Set qualified as Set
 import Data.String.Conversion (toString, toText)
 import Data.Text (Text, breakOn)
 import Data.Text qualified as Text
@@ -74,7 +75,6 @@ import Effect.Exec (
   execThrow,
  )
 import Effect.Grapher (
-  LabeledGrapher,
   direct,
   edge,
   label,
@@ -485,36 +485,88 @@ toDependency emitGitBackedLocators sourceMap pkg =
         extractGitCommitHash sourceUrl
       _ -> Nothing
 
--- Possible values here are "build", "dev", and null.
--- Null refers to productions, while dev and build refer to development-time dependencies
--- Cargo does not differentiate test dependencies and dev dependencies,
--- so we just simplify it to Development.
-kindToLabel :: Maybe Text.Text -> CargoLabel
-kindToLabel (Just _) = CargoDepKind EnvDevelopment
-kindToLabel Nothing = CargoDepKind EnvProduction
-
-addLabel :: Has (LabeledGrapher PackageId CargoLabel) sig m => NodeDependency -> m ()
-addLabel dep = do
-  let packageId = nodePkg dep
-  traverse_ (label packageId . kindToLabel . nodeDepKind) $ nodeDepKinds dep
-
-addEdge :: Has (LabeledGrapher PackageId CargoLabel) sig m => ResolveNode -> m ()
-addEdge node = do
-  let parentId = resolveNodeId node
-  for_ (resolveNodeDeps node) $ \dep -> do
-    addLabel dep
-    edge parentId $ nodePkg dep
-
+-- A Cargo edge's kind ("build", "dev", or null) reflects the parent's manifest
+-- declaration, not the path taken to reach the parent. We classify each package
+-- by which workspace-rooted paths can reach it:
+--
+--   * Production: reachable from a workspace member via a path of null-kind
+--     edges only. These packages are linked into the release artifact.
+--
+--   * Development: any package reachable (via any edge) from the target of a
+--     non-null-kind edge. A "build" or "dev" edge marks the start of a subtree
+--     that never ships in the release binary, and every descendant of that
+--     subtree inherits Development.
+--
+-- A package can carry both labels when it's reachable by both kinds of paths.
+--
+-- We do not need a separate "dev-deps of prod-deps" case: Cargo only resolves
+-- dev-dependencies for workspace members, so non-workspace edges with kind
+-- "dev" do not appear in 'cargo metadata' output. The only non-null kind we
+-- see on a non-workspace edge is "build".
+--
+-- Cargo is the only strategy with per-edge kinds; others (pnpm, yarn, poetry)
+-- label nodes and propagate with 'hydrateDepEnvs'. That helper walks from a
+-- labeled node to every dependency it declares, regardless of edge kind, so
+-- a Production label on a workspace member would flow through a "dev" or
+-- "build" edge and mislabel the dev/build subtree as Production. We roll our
+-- own edge-filtered reachability here rather than generalize the shared helper.
 buildGraph :: Bool -> CargoMetadata -> Graphing Dependency
--- By construction, workspace members are the root nodes in the graph.
--- Use shrinkRoots to remove them and promote their direct dependencies to the
--- direct dependencies we report for the project.
 buildGraph emitGitBackedLocators meta = shrinkRoots $
   run . withLabeling (toDependency emitGitBackedLocators sourceMap) $ do
-    traverse_ direct $ metadataWorkspaceMembers meta
-    traverse_ addEdge $ resolvedNodes $ metadataResolve meta
+    traverse_ direct (metadataWorkspaceMembers meta)
+    for_ nodes $ \node ->
+      for_ (resolveNodeDeps node) $ \dep ->
+        edge (resolveNodeId node) (nodePkg dep)
+    for_ (Set.toList prodReachable) $ \pkg ->
+      label pkg (CargoDepKind EnvProduction)
+    for_ (Set.toList devReachable) $ \pkg ->
+      label pkg (CargoDepKind EnvDevelopment)
   where
     sourceMap = buildPackageSourceMap $ metadataPackages meta
+    nodes = resolvedNodes (metadataResolve meta)
+    workspaceMembers = Set.fromList (metadataWorkspaceMembers meta)
+
+    -- These predicates are not mutually exclusive: a dep declared in both
+    -- [dependencies] and [dev-dependencies] on the same parent carries both
+    -- a null and a non-null kind, so the edge feeds prodAdj *and* devSeeds.
+    isProdEdge dep = any (isNothing . nodeDepKind) (nodeDepKinds dep)
+    isDevEdge dep = any (isJust . nodeDepKind) (nodeDepKinds dep)
+
+    -- Adjacency containing only edges whose parent declares the child as a
+    -- normal dependency (at least one kind is null). Production reachability
+    -- must only traverse these — a build or dev edge breaks the release chain.
+    prodAdj =
+      Map.fromList $
+        map
+          (\node -> (resolveNodeId node, map nodePkg . filter isProdEdge . resolveNodeDeps $ node))
+          nodes
+
+    -- Every edge in the metadata graph, for Development reachability.
+    allAdj =
+      Map.fromList $
+        map (\node -> (resolveNodeId node, map nodePkg (resolveNodeDeps node))) nodes
+
+    -- Targets of any non-null-kind edge. Each seeds a Development subtree:
+    -- the target and all its transitive descendants are never linked into
+    -- a release build.
+    devSeeds =
+      Set.fromList $
+        map nodePkg $
+          concatMap (filter isDevEdge . resolveNodeDeps) nodes
+
+    prodReachable = reachable prodAdj workspaceMembers
+    devReachable = reachable allAdj devSeeds
+
+reachable :: Map.Map PackageId [PackageId] -> Set PackageId -> Set PackageId
+reachable adj = go Set.empty . Set.toList
+  where
+    go visited [] = visited
+    go visited (x : xs) =
+      if Set.member x visited
+        then go visited xs
+        else
+          let children = fromMaybe [] (Map.lookup x adj)
+           in go (Set.insert x visited) (children ++ xs)
 
 -- | Custom Parsec type alias
 type PkgSpecParser a = Parsec Void Text a

test/Cargo/MetadataSpec.hs

@@ -75,6 +75,8 @@ spec = do
 
   post1_77MetadataParseSpec
 
+  devDepTransitiveLabelingSpec
+
   extractGitCommitHashSpec
 
   gitCommitHashVersionSpec
@@ -201,6 +203,142 @@ post1_77MetadataParseSpec =
       expectDeps [ansiTermDep, clapDep, fooDep, barDepFallback] graph
       dependencyName barDepFallback `shouldBe` "bar"
 
+devKind :: Text -> NodeDepKind
+devKind k = NodeDepKind (Just k) Nothing
+
+devDepTransitiveLabelingSpec :: Test.Spec
+devDepTransitiveLabelingSpec =
+  Test.describe "dev-dep transitive labeling" $ do
+    Test.it "transitive deps of dev-deps are labeled Development" $ do
+      let wsId = PackageId "myapp" "0.1.0" "path+file:///path/to/myapp"
+          itoaId = mkPkgId "itoa" "1.0.18"
+          approxId = mkPkgId "approx" "0.5.1"
+          numTraitsId = mkPkgId "num-traits" "0.2.19"
+          autocfgId = mkPkgId "autocfg" "1.4.0"
+
+          wsNode =
+            ResolveNode
+              wsId
+              [ NodeDependency itoaId [nullKind]
+              , NodeDependency approxId [devKind "dev"]
+              ]
+          approxNode =
+            ResolveNode
+              approxId
+              [ NodeDependency numTraitsId [nullKind]
+              ]
+          numTraitsNode =
+            ResolveNode
+              numTraitsId
+              [ NodeDependency autocfgId [devKind "build"]
+              ]
+          itoaNode = ResolveNode itoaId []
+          autocfgNode = ResolveNode autocfgId []
+
+          meta = CargoMetadata [] [wsId] $ Resolve [wsNode, approxNode, numTraitsNode, itoaNode, autocfgNode]
+          graph = buildGraph False meta
+
+          itoaDep = mkDep "itoa" "1.0.18" CargoType [EnvProduction]
+          approxDep = mkDep "approx" "0.5.1" CargoType [EnvDevelopment]
+          numTraitsDep = mkDep "num-traits" "0.2.19" CargoType [EnvDevelopment]
+          autocfgDep = mkDep "autocfg" "1.4.0" CargoType [EnvDevelopment]
+
+      expectDeps [itoaDep, approxDep, numTraitsDep, autocfgDep] graph
+      expectDirect [itoaDep, approxDep] graph
+
+    Test.it "dep reachable from both prod and dev roots gets both labels" $ do
+      let wsId = PackageId "myapp" "0.1.0" "path+file:///path/to/myapp"
+          aId = mkPkgId "A" "1.0.0"
+          bId = mkPkgId "B" "1.0.0"
+          cId = mkPkgId "C" "1.0.0"
+
+          wsNode =
+            ResolveNode
+              wsId
+              [ NodeDependency aId [nullKind]
+              , NodeDependency bId [devKind "dev"]
+              ]
+          aNode = ResolveNode aId [NodeDependency cId [nullKind]]
+          bNode = ResolveNode bId [NodeDependency cId [nullKind]]
+          cNode = ResolveNode cId []
+
+          meta = CargoMetadata [] [wsId] $ Resolve [wsNode, aNode, bNode, cNode]
+          graph = buildGraph False meta
+
+          aDep = mkDep "A" "1.0.0" CargoType [EnvProduction]
+          bDep = mkDep "B" "1.0.0" CargoType [EnvDevelopment]
+          cDep = mkDep "C" "1.0.0" CargoType [EnvProduction, EnvDevelopment]
+
+      expectDeps [aDep, bDep, cDep] graph
+      expectDirect [aDep, bDep] graph
+
+    Test.it "direct dep declared as both normal and dev gets both labels" $ do
+      -- Cargo emits multiple NodeDepKinds when a dep is declared in both
+      -- [dependencies] and [dev-dependencies] on the same package.
+      let wsId = PackageId "myapp" "0.1.0" "path+file:///path/to/myapp"
+          dualId = mkPkgId "dual" "1.0.0"
+          transId = mkPkgId "trans" "1.0.0"
+
+          wsNode = ResolveNode wsId [NodeDependency dualId [nullKind, devKind "dev"]]
+          dualNode = ResolveNode dualId [NodeDependency transId [nullKind]]
+          transNode = ResolveNode transId []
+
+          meta = CargoMetadata [] [wsId] $ Resolve [wsNode, dualNode, transNode]
+          graph = buildGraph False meta
+
+          dualDep = mkDep "dual" "1.0.0" CargoType [EnvProduction, EnvDevelopment]
+          transDep = mkDep "trans" "1.0.0" CargoType [EnvProduction, EnvDevelopment]
+
+      expectDeps [dualDep, transDep] graph
+      expectDirect [dualDep] graph
+
+    Test.it "build-deps of prod-deps and their transitives are Development" $ do
+      -- Regression guard: build-script utilities (autocfg, cc, version_check,
+      -- etc.) are declared as [build-dependencies] of normal deps. They are
+      -- compiled during the build but never linked into the release artifact,
+      -- so they should be labeled Development, not Production — even though
+      -- their parent is a production dep.
+      let wsId = PackageId "myapp" "0.1.0" "path+file:///path/to/myapp"
+          serdeId = mkPkgId "serde" "1.0.0"
+          autocfgId = mkPkgId "autocfg" "1.0.0"
+          autocfgChildId = mkPkgId "autocfg-child" "1.0.0"
+
+          wsNode = ResolveNode wsId [NodeDependency serdeId [nullKind]]
+          serdeNode = ResolveNode serdeId [NodeDependency autocfgId [devKind "build"]]
+          autocfgNode = ResolveNode autocfgId [NodeDependency autocfgChildId [nullKind]]
+          childNode = ResolveNode autocfgChildId []
+
+          meta = CargoMetadata [] [wsId] $ Resolve [wsNode, serdeNode, autocfgNode, childNode]
+          graph = buildGraph False meta
+
+          serdeDep = mkDep "serde" "1.0.0" CargoType [EnvProduction]
+          autocfgDep = mkDep "autocfg" "1.0.0" CargoType [EnvDevelopment]
+          childDep = mkDep "autocfg-child" "1.0.0" CargoType [EnvDevelopment]
+
+      expectDeps [serdeDep, autocfgDep, childDep] graph
+      expectDirect [serdeDep] graph
+
+    Test.it "dep reachable via a normal prod path and a build-edge gets both labels" $ do
+      -- X is a direct normal dep of the workspace (Production) AND a build-dep
+      -- of a prod dep (Development via the build-edge split). Both labels
+      -- should be applied.
+      let wsId = PackageId "myapp" "0.1.0" "path+file:///path/to/myapp"
+          aId = mkPkgId "A" "1.0.0"
+          xId = mkPkgId "X" "1.0.0"
+
+          wsNode = ResolveNode wsId [NodeDependency aId [nullKind], NodeDependency xId [nullKind]]
+          aNode = ResolveNode aId [NodeDependency xId [devKind "build"]]
+          xNode = ResolveNode xId []
+
+          meta = CargoMetadata [] [wsId] $ Resolve [wsNode, aNode, xNode]
+          graph = buildGraph False meta
+
+          aDep = mkDep "A" "1.0.0" CargoType [EnvProduction]
+          xDep = mkDep "X" "1.0.0" CargoType [EnvProduction, EnvDevelopment]
+
+      expectDeps [aDep, xDep] graph
+      expectDirect [aDep, xDep] graph
+
 extractGitCommitHashSpec :: Test.Spec
 extractGitCommitHashSpec =
   Test.describe "extractGitCommitHash" $ do