From 408bfb79ac6e0bc99ae3214add1979748b1466cc Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 11:03:09 -0700 Subject: [PATCH 01/10] Pnpm: scope analysis to individual workspace members (ANE-3125) A pnpm workspace collapsed into a single result whose dependencies were the union of every member's, with no record of which member each came from. The importers section of pnpm-lock.yaml carries that information -- its keys are the members' paths -- but buildGraphCore discarded the key and merged all importers into one graph, and discovery exposed no per-member build targets to select with. Expose each workspace member (and the root) as a build target, as yarn and npm already do, and thread the selection through to the lockfile analyzer. The importer keys are resolved from the selected target names via the package.json manifests, sharing the mapping npm v3 uses and differing only in how the two formats spell the workspace root ("." vs ""). A scoped graph is pruned to what the selected importers reach; an unscoped one is left exactly as before. fossa analyze --only-target 'pnpm@./:browser' Two things this needed beyond the yarn/npm port: Workspace links. pnpm records a dependency on a sibling member as `version: link:../other` rather than as a packages entry, so the sibling's dependencies live only under its own importer key. Merging every importer hid that; scoping would have lost them. expandWorkspaceLinks follows each link to the importer it names, transitively, so a scoped result stays complete. Unnamed workspace roots. findWorkspaceBuildTargets gave up entirely when the root package.json had no name field, which withheld targets from every member as well. pnpm keeps its workspace configuration in pnpm-workspace.yaml, so its roots are frequently nameless. Fall back to the root directory's own basename. This applies to yarn and npm too, where it can only add targets that were previously withheld. Also stop reporting workspace-reference specifiers as versions. Analyzing a member directory on its own falls back to a package.json-only npm strategy that cannot resolve `catalog:`, `workspace:` or `link:`, and it was emitting the raw specifier as the version -- locators like `npm+left-pad$catalog:`, a dependency pinned to a version that exists in no registry. Skip those with a warning. `file:` is left as it is: equally unresolvable, but long-standing npm behavior and a separate decision. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015zRtempnk5Led4xWQTqVfb --- Changelog.md | 4 + docs/references/files/fossa-yml.md | 14 ++ .../strategies/languages/nodejs/pnpm.md | 64 +++++++- .../Analysis/PnpmWorkspaceSpec.hs | 107 +++++++++++++ spectrometer.cabal | 1 + src/Strategy/Node.hs | 98 ++++++++---- src/Strategy/Node/PackageJson.hs | 42 ++++- src/Strategy/Node/Pnpm/PnpmLock.hs | 145 ++++++++++++++++-- test/Node/NodeSpec.hs | 58 ++++++- .../pnpm-workspaces/browser/package.json | 8 + .../testdata/pnpm-workspaces/package.json | 7 + .../testdata/pnpm-workspaces/pnpm-lock.yaml | 82 ++++++++++ .../pnpm-workspaces/pnpm-workspace.yaml | 7 + .../pnpm-workspaces/server/package.json | 7 + .../pnpm-workspaces/shared/package.json | 7 + test/Pnpm/PnpmLockSpec.hs | 86 ++++++++++- 16 files changed, 690 insertions(+), 47 deletions(-) create mode 100644 integration-test/Analysis/PnpmWorkspaceSpec.hs create mode 100644 test/Node/testdata/pnpm-workspaces/browser/package.json create mode 100644 test/Node/testdata/pnpm-workspaces/package.json create mode 100644 test/Node/testdata/pnpm-workspaces/pnpm-lock.yaml create mode 100644 test/Node/testdata/pnpm-workspaces/pnpm-workspace.yaml create mode 100644 test/Node/testdata/pnpm-workspaces/server/package.json create mode 100644 test/Node/testdata/pnpm-workspaces/shared/package.json diff --git a/Changelog.md b/Changelog.md index 08a06be75d..54b097c467 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,10 @@ ## Unreleased +- Pnpm: workspace members are now individual build targets, so a single member can be analyzed on its own with `--only-target 'pnpm@./:my-package'` or a `target:` entry in `.fossa.yml`. Previously every member's dependencies were merged into one result with no way to scope them. Dependencies a selected member reaches through the workspace protocol (`link:` in the lockfile) are included. With no target filter the result is unchanged. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) +- Node: a workspace root whose `package.json` has no `name` field no longer suppresses build targets for the whole workspace; the root directory's name is used for the root target. This most often affected pnpm, whose workspace configuration lives in `pnpm-workspace.yaml`. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) +- Node: dependencies whose version is a workspace reference (`catalog:`, `workspace:`, `link:`) are no longer reported at that literal string as their version, which produced locators like `npm+left-pad$catalog:` for packages that do not exist. They are skipped with a warning when analysis falls back to a `package.json`-only strategy that cannot resolve them. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) + - Diagnostics: When an error or warning group contains multiple errors, each error's `Traceback:` header is now printed on its own line instead of being glued onto the last line of the preceding error message (e.g. `...none passed validationTraceback:`). ([#1758](https://github.com/fossas/fossa-cli/pull/1758)) ## 3.18.2 diff --git a/docs/references/files/fossa-yml.md b/docs/references/files/fossa-yml.md index 1911773efd..2283c780b5 100644 --- a/docs/references/files/fossa-yml.md +++ b/docs/references/files/fossa-yml.md @@ -290,6 +290,20 @@ Targets are listed in the following formats for both `only` and `exclude` lists. - type: pipenv (all pipenv type targets at any path) ``` +Some project types divide a single project into named build targets — for +example each workspace package of a yarn, npm, or pnpm monorepo. Add a `target` +field alongside `path` to select one of them: + +```yaml + - type: pnpm + path: ./ + target: browser +``` + +This is the `.fossa.yml` equivalent of `--only-target 'pnpm@./:browser'`. Run +`fossa list-targets` to see which targets a project has; a project with no named +targets is selected by `type` and `path` alone. + #### `targets.only:` The list of `only` targets that should be scanned. When used alongside `paths.only`, the intersection of the two lists is taken to find targets for scanning diff --git a/docs/references/strategies/languages/nodejs/pnpm.md b/docs/references/strategies/languages/nodejs/pnpm.md index 05d74cedfc..ad6e16a51a 100644 --- a/docs/references/strategies/languages/nodejs/pnpm.md +++ b/docs/references/strategies/languages/nodejs/pnpm.md @@ -27,7 +27,10 @@ in `pnpm-lock.yaml` to analyze the dependency graph. > 📘 Important Note > -> Anything defined in the `importers` section will be ignored. In order to scan individual targts, the workspace needs to have individual/separate lock files. +> The `importers` section is the source of direct dependencies. By default every +> importer's dependencies are merged into one result for the whole workspace. To +> report on a single workspace member, select it as a build target — see +> [Workspace Build Targets](#workspace-build-targets) below. An example is provided below: @@ -157,6 +160,60 @@ CLI will infer the package name and version using `/${dependencyName}/${dependen * Optional dependencies are included in the analysis by default. They can be ignored in FOSSA UI. * `fossa-cli` supports lockFileVersion: 4.x, 5.x, 6.x, 7.x, 8.x, and 9.x. +### Workspace Build Targets + +Each workspace member, and the workspace root, is exposed as an individual build +target. A workspace whose `pnpm-workspace.yaml` lists `browser` and `server` +produces: + +``` +pnpm@./:my-workspace +pnpm@./:browser +pnpm@./:server +``` + +The target name is the member's `name` from its `package.json`, so a member +named `@acme/browser` is selected as `pnpm@./:@acme/browser`. Run +`fossa list-targets` to see the exact names. If the workspace root's +`package.json` has no `name` field — common for pnpm, since the workspace +configuration lives in `pnpm-workspace.yaml` — the root directory's own name is +used for the root target. + +Selecting a subset reports only those members' dependencies: + +```bash +fossa analyze --only-target 'pnpm@./:browser' +``` + +or, equivalently, in `.fossa.yml`: + +```yaml +version: 3 +targets: + only: + - type: pnpm + path: ./ + target: browser +``` + +When a selected member depends on a sibling member through the +[workspace protocol](https://pnpm.io/workspaces#workspace-protocol), pnpm records +that in the lockfile as `version: link:`. The sibling's own dependencies +are included in the result, since the selected member does depend on them. + +With no target filtering, all targets are selected and every workspace member's +dependencies are included, which is the behavior of every release before this +feature. + +> 📘 Note +> +> A pnpm workspace is a single FOSSA project rooted at the workspace root, and +> stays one project no matter which targets are selected. Build targets scope +> what that project reports; they do not split it into several projects. Running +> `fossa analyze` from inside a member directory does not scope the scan either +> — without the lockfile in scope, analysis falls back to a `package.json`-only +> npm strategy with a partial graph. + ### Catalogs pnpm [catalogs](https://pnpm.io/catalogs) (introduced in pnpm 9.5) are supported. @@ -164,6 +221,11 @@ When `catalog:` or `catalog:` specifiers are used in `package.json`, the resolved versions from `pnpm-lock.yaml` are used for analysis. No additional configuration is needed. +Resolving a catalog specifier requires the lockfile, which lives at the workspace +root. Analyzing a member directory on its own therefore cannot resolve them, and +such dependencies are skipped with a warning rather than reported at the literal +version `catalog:`. Analyze from the workspace root to include them. + # F.A.Q diff --git a/integration-test/Analysis/PnpmWorkspaceSpec.hs b/integration-test/Analysis/PnpmWorkspaceSpec.hs new file mode 100644 index 0000000000..974d2a5726 --- /dev/null +++ b/integration-test/Analysis/PnpmWorkspaceSpec.hs @@ -0,0 +1,107 @@ +{-# LANGUAGE TemplateHaskell #-} + +-- | End-to-end coverage for target-level dependency scoping of pnpm workspaces: +-- discovery over a vendored workspace fixture, then analysis per selected build +-- target. +module Analysis.PnpmWorkspaceSpec (spec) where + +import Analysis.FixtureUtils (FixtureEnvironment (LocalEnvironment), testRunner, withResult) +import App.Fossa.Analyze.Types (AnalyzeProject (analyzeProject)) +import App.Types (Mode (NonStrict)) +import Control.Carrier.Debug (ignoreDebug) +import Control.Carrier.Reader (runReader) +import Data.Set (Set) +import Data.Set qualified as Set +import Data.Set.NonEmpty qualified as NonEmptySet +import Data.Text (Text) +import DepTypes (Dependency (dependencyName)) +import Graphing (Graphing) +import Graphing qualified +import Path (Dir, Path, Rel, mkRelDir, ()) +import Path.IO qualified as PIO +import Test.Hspec (Spec, beforeAll, describe, it, shouldBe, shouldSatisfy) +import Types ( + BuildTarget (BuildTarget), + DependencyResults (dependencyGraph), + DiscoveredProject (projectBuildTargets, projectData, projectType), + DiscoveredProjectType (PnpmProjectType), + FoundTargets (FoundTargets, ProjectWithoutTargets), + ) + +import Strategy.Node qualified as Node + +fixtureDir :: Path Rel Dir +fixtureDir = $(mkRelDir "test/Node/testdata/pnpm-workspaces/") + +-- | The fixture's root package.json has no @name@ field, which is typical of a +-- pnpm workspace root since the workspace configuration lives in +-- pnpm-workspace.yaml. Its target name is therefore the root directory's own +-- basename. +allTargetNames :: [Text] +allTargetNames = + [ "pnpm-workspaces" + , "@fossa-test/browser" + , "@fossa-test/server" + , "@fossa-test/shared" + ] + +data FixtureGraphs = FixtureGraphs + { discoveredTargets :: FoundTargets + , wholeGraph :: Graphing Dependency + , rootGraph :: Graphing Dependency + , browserGraph :: Graphing Dependency + , serverGraph :: Graphing Dependency + , sharedGraph :: Graphing Dependency + } + +mkTargets :: [Text] -> FoundTargets +mkTargets = maybe ProjectWithoutTargets FoundTargets . NonEmptySet.nonEmpty . Set.fromList . map BuildTarget + +depNames :: Graphing Dependency -> Set Text +depNames = Set.fromList . map dependencyName . Graphing.vertexList + +analyzeFixture :: IO FixtureGraphs +analyzeFixture = do + currentDir <- PIO.getCurrentDir + let scanDir = currentDir fixtureDir + discovered <- testRunner (Node.discover scanDir) LocalEnvironment + withResult discovered $ \_ projects -> case projects of + [project] -> do + projectType project `shouldBe` PnpmProjectType + let analyzeWith targets = do + analyzed <- testRunner (ignoreDebug $ runReader NonStrict $ analyzeProject targets (projectData project)) LocalEnvironment + withResult analyzed $ \_ depResults -> pure (dependencyGraph depResults) + FixtureGraphs (projectBuildTargets project) + <$> analyzeWith (projectBuildTargets project) + <*> analyzeWith (mkTargets ["pnpm-workspaces"]) + <*> analyzeWith (mkTargets ["@fossa-test/browser"]) + <*> analyzeWith (mkTargets ["@fossa-test/server"]) + <*> analyzeWith (mkTargets ["@fossa-test/shared"]) + projects' -> fail ("expected exactly one discovered project, got " <> show (length projects')) + +spec :: Spec +spec = beforeAll analyzeFixture $ + describe "pnpm workspace" $ do + it "should expose the root and each workspace member as build targets" $ \fixture -> + discoveredTargets fixture `shouldBe` mkTargets allTargetNames + + it "should report only the selected member's dependencies" $ \fixture -> do + -- left-pad reaches browser through a catalog: specifier; is-odd belongs + -- only to server, and colorjs only to the root. + depNames (browserGraph fixture) `shouldSatisfy` Set.member "left-pad" + depNames (browserGraph fixture) `shouldSatisfy` (\names -> not (any (`Set.member` names) ["is-odd", "is-number", "colorjs"])) + + depNames (serverGraph fixture) `shouldBe` Set.fromList ["is-odd", "is-number"] + depNames (rootGraph fixture) `shouldBe` Set.fromList ["colorjs"] + + it "should follow a workspace link into the sibling it names" $ \fixture -> do + -- browser depends on the shared member via `version: link:../shared`. + -- Its dependencies, and their transitives, belong in browser's result; + -- the workspace package itself is not a reportable dependency. + depNames (browserGraph fixture) `shouldBe` Set.fromList ["left-pad", "uri-js", "punycode"] + depNames (sharedGraph fixture) `shouldBe` Set.fromList ["uri-js", "punycode"] + + it "should analyze the whole workspace when all targets are selected" $ \fixture -> do + depNames (wholeGraph fixture) `shouldBe` Set.fromList ["colorjs", "left-pad", "is-odd", "is-number", "uri-js", "punycode"] + [rootGraph fixture, browserGraph fixture, serverGraph fixture, sharedGraph fixture] + `shouldSatisfy` all ((`Set.isSubsetOf` depNames (wholeGraph fixture)) . depNames) diff --git a/spectrometer.cabal b/spectrometer.cabal index e53f507243..bad6cdd832 100644 --- a/spectrometer.cabal +++ b/spectrometer.cabal @@ -790,6 +790,7 @@ test-suite integration-tests Analysis.NpmLockV3WorkspaceSpec Analysis.NugetSpec Analysis.PnpmSpec + Analysis.PnpmWorkspaceSpec Analysis.Python.PipenvSpec Analysis.Python.PoetrySpec Analysis.Python.SetuptoolsSpec diff --git a/src/Strategy/Node.hs b/src/Strategy/Node.hs index 67472f45a4..715010ecf0 100644 --- a/src/Strategy/Node.hs +++ b/src/Strategy/Node.hs @@ -10,12 +10,15 @@ module Strategy.Node ( findWorkspaceBuildTargets, extractDepListsForTargets, resolveNpmV3WorkspacePaths, + resolvePnpmImporterKeys, + workspaceRootTargetName, ) where import Algebra.Graph.AdjacencyMap qualified as AM import Algebra.Graph.AdjacencyMap.Extra qualified as AME import App.Fossa.Analyze.LicenseAnalyze (LicenseAnalyzeProject, licenseAnalyzeProject) import App.Fossa.Analyze.Types (AnalyzeProject (analyzeProject, analyzeProjectStaticOnly)) +import Control.Applicative ((<|>)) import Control.Carrier.Diagnostics (errDoc) import Control.Effect.Diagnostics ( Diagnostics, @@ -72,6 +75,7 @@ import Path ( File, Path, Rel, + dirname, mkRelFile, parent, stripProperPrefix, @@ -163,6 +167,7 @@ mkProject project = do projectBuildTargets' = case project of Yarn _ _ -> findWorkspaceBuildTargets graph NPMLock _ _ -> findWorkspaceBuildTargets graph + Pnpm _ _ -> findWorkspaceBuildTargets graph _ -> ProjectWithoutTargets Manifest rootManifest <- fromEitherShow $ findWorkspaceRootManifest graph pure $ @@ -175,23 +180,40 @@ mkProject project = do -- | Build targets from workspace package names (root + members). -- If the workspace graph has children (i.e., workspace members), each --- package name (including the root) becomes a 'BuildTarget'. If there --- are no workspace children (single-package project), returns --- 'ProjectWithoutTargets'. +-- package name becomes a 'BuildTarget', along with the root's own name from +-- 'workspaceRootTargetName'. If there are no workspace children +-- (single-package project), returns 'ProjectWithoutTargets'. findWorkspaceBuildTargets :: PkgJsonGraph -> FoundTargets -findWorkspaceBuildTargets graph@PkgJsonGraph{..} = +findWorkspaceBuildTargets graph = let WorkspacePackageNames childNames = findWorkspaceNames graph in if Set.null childNames then ProjectWithoutTargets - else - let rootName = findWorkspaceRootManifest graph >>= \m -> maybe (Left "no name") Right (packageName =<< Map.lookup m jsonLookup) - in case rootName of - -- If the root package.json has no name field, fall back to - -- ProjectWithoutTargets so its deps aren't silently dropped. - Left _ -> ProjectWithoutTargets - Right n -> - let allNames = Set.insert n childNames - in maybe ProjectWithoutTargets FoundTargets (NonEmptySet.nonEmpty (Set.map BuildTarget allNames)) + else case workspaceRootTargetName graph of + Nothing -> ProjectWithoutTargets + Just n -> + let allNames = Set.insert n childNames + in maybe ProjectWithoutTargets FoundTargets (NonEmptySet.nonEmpty (Set.map BuildTarget allNames)) + +-- | The build target name for a workspace root: the @name@ field of its +-- package.json, or the root directory's own basename when it declares none. +-- +-- The fallback matters most for pnpm, which keeps its workspace configuration +-- in pnpm-workspace.yaml rather than in package.json, so a pnpm workspace root +-- is frequently a bare @{ "private": true }@ with no name at all. Without the +-- fallback such a root produced no targets, and because 'findWorkspaceBuildTargets' +-- is all-or-nothing that withheld targets from every workspace member as well. +-- +-- 'Nothing' means the graph has no single root manifest, or the root sits at a +-- filesystem root with no basename to borrow. +workspaceRootTargetName :: PkgJsonGraph -> Maybe Text +workspaceRootTargetName graph@PkgJsonGraph{jsonLookup} = do + manifest@(Manifest rootManifest) <- either (const Nothing) Just $ findWorkspaceRootManifest graph + (packageName =<< Map.lookup manifest jsonLookup) <|> rootDirName rootManifest + where + rootDirName :: Path Abs File -> Maybe Text + rootDirName m = + let name = toText . FP.dropTrailingPathSeparator . toFilePath . dirname $ parent m + in if name `elem` ["", ".", "/"] then Nothing else Just name instance AnalyzeProject NodeProject where analyzeProject = getDeps @@ -207,13 +229,13 @@ getDeps :: m DependencyResults getDeps targets (Yarn yarnLockFile graph) = analyzeYarn targets yarnLockFile graph getDeps targets (NPMLock packageLockFile graph) = analyzeNpmLock targets packageLockFile graph -getDeps _ (Pnpm pnpmLockFile _) = analyzePnpmLock pnpmLockFile +getDeps targets (Pnpm pnpmLockFile graph) = analyzePnpmLock targets pnpmLockFile graph getDeps _ (Bun bunLockFile _) = analyzeBunLock bunLockFile getDeps _ (NPM graph) = analyzeNpm graph -analyzePnpmLock :: (Has Diagnostics sig m, Has ReadFS sig m, Has Logger sig m) => Manifest -> m DependencyResults -analyzePnpmLock (Manifest pnpmLockFile) = do - result <- PnpmLock.analyze pnpmLockFile +analyzePnpmLock :: (Has Diagnostics sig m, Has ReadFS sig m, Has Logger sig m) => FoundTargets -> Manifest -> PkgJsonGraph -> m DependencyResults +analyzePnpmLock targets (Manifest pnpmLockFile) graph = do + result <- PnpmLock.analyze (resolvePnpmImporterKeys targets graph) pnpmLockFile pure $ DependencyResults result Complete [pnpmLockFile] analyzeBunLock :: (Has Diagnostics sig m, Has ReadFS sig m) => Manifest -> m DependencyResults @@ -229,7 +251,7 @@ analyzeNpmLock targets (Manifest npmLockFile) graph = do NpmLockV1Compatible -> PackageLock.analyze npmLockFile (extractDepListsForTargets targets graph) (findWorkspaceNames graph) pure $ DependencyResults result Complete [npmLockFile] -analyzeNpm :: (Has Diagnostics sig m) => PkgJsonGraph -> m DependencyResults +analyzeNpm :: (Has Diagnostics sig m, Has Logger sig m) => PkgJsonGraph -> m DependencyResults analyzeNpm wsGraph = do void . recover @@ -314,29 +336,53 @@ findWorkspaceNames PkgJsonGraph{..} = -- path keys npm v3 lockfiles use: @""@ for the root, @"packages/a"@ for a -- member. 'Nothing' means no target filter, so no scoping is applied. resolveNpmV3WorkspacePaths :: FoundTargets -> PkgJsonGraph -> Maybe (Set Text) -resolveNpmV3WorkspacePaths ProjectWithoutTargets _ = Nothing -resolveNpmV3WorkspacePaths (FoundTargets targets) graph@PkgJsonGraph{..} = +resolveNpmV3WorkspacePaths = resolveWorkspacePathKeys "" + +-- | Map selected build targets (workspace package names) to the importer keys +-- @pnpm-lock.yaml@ uses: @"."@ for the root, @"packages/a"@ for a member. +-- 'Nothing' means no target filter, so no scoping is applied. +-- +-- Identical to 'resolveNpmV3WorkspacePaths' apart from how the two lockfile +-- formats spell the workspace root. +resolvePnpmImporterKeys :: FoundTargets -> PkgJsonGraph -> Maybe (Set Text) +resolvePnpmImporterKeys = resolveWorkspacePathKeys "." + +-- | Shared implementation of 'resolveNpmV3WorkspacePaths' and +-- 'resolvePnpmImporterKeys', parameterized by the key the lockfile format uses +-- for the workspace root. +resolveWorkspacePathKeys :: Text -> FoundTargets -> PkgJsonGraph -> Maybe (Set Text) +resolveWorkspacePathKeys _ ProjectWithoutTargets _ = Nothing +resolveWorkspacePathKeys rootKey (FoundTargets targets) graph@PkgJsonGraph{..} = case findWorkspaceRootManifest graph of Left _ -> Nothing - Right (Manifest rootManifest) -> + Right rootManifest -> Just . Set.fromList . map snd $ filter ((`Set.member` targetNames) . fst) namePathPairs where - rootDir = parent rootManifest + rootDir = parent $ unManifest rootManifest targetNames = Set.map unBuildTarget (NonEmptySet.toSet targets) namePathPairs :: [(Text, Text)] namePathPairs = mapMaybe - (\(Manifest m, pj) -> (,) <$> packageName pj <*> manifestToWorkspacePath m) + (\(manifest, pj) -> (,) <$> manifestTargetName manifest pj <*> manifestToWorkspacePath (unManifest manifest)) (Map.toList jsonLookup) + -- Must agree with 'findWorkspaceBuildTargets' on what each manifest's + -- target is called, including the root's basename fallback; a target + -- offered by list-targets but unresolvable here would silently select + -- nothing. + manifestTargetName :: Manifest -> PackageJson -> Maybe Text + manifestTargetName manifest pj + | manifest == rootManifest = workspaceRootTargetName graph + | otherwise = packageName pj + manifestToWorkspacePath :: Path Abs File -> Maybe Text manifestToWorkspacePath m = let manifestDir = parent m in if manifestDir == rootDir - then Just "" - else -- npm keys workspaces with forward slashes on every OS, so - -- normalize the platform separator before matching. + then Just rootKey + else -- npm and pnpm both key workspaces with forward slashes on + -- every OS, so normalize the platform separator before matching. fmap (Text.replace "\\" "/" . toText . FP.dropTrailingPathSeparator . toFilePath) (stripProperPrefix rootDir manifestDir) extractDepLists :: PkgJsonGraph -> FlatDeps diff --git a/src/Strategy/Node/PackageJson.hs b/src/Strategy/Node/PackageJson.hs index 413b6736ca..ef6bbe6689 100644 --- a/src/Strategy/Node/PackageJson.hs +++ b/src/Strategy/Node/PackageJson.hs @@ -27,6 +27,7 @@ import Control.Effect.Diagnostics ( context, run, ) +import Control.Monad (unless) import Data.Aeson ( FromJSON (parseJSON), KeyValue ((.=)), @@ -46,6 +47,7 @@ import Data.Set (Set) import Data.String.Conversion (ToText (toText)) import Data.Tagged (Tagged) import Data.Text (Text) +import Data.Text qualified as Text import DepTypes ( DepEnvironment (..), DepType (NodeJSType), @@ -59,16 +61,50 @@ import Effect.Grapher ( label, withLabeling, ) +import Effect.Logger (Logger, logWarn, pretty) import GHC.Generics (Generic) import Graphing (Graphing) import Path (Abs, File, Path, Rel) newtype WorkspacePackageNames = WorkspacePackageNames (Set Text) -analyze :: (Has Diagnostics sig m) => [PackageJson] -> m (Graphing Dependency) +analyze :: (Has Diagnostics sig m, Has Logger sig m) => [PackageJson] -> m (Graphing Dependency) analyze manifests = do + let unresolvable = concatMap unresolvableSpecifiers manifests + unless (null unresolvable) $ + logWarn . pretty $ + "Skipping " + <> toText (show (length unresolvable)) + <> " dependencies whose version is a workspace reference this strategy cannot resolve without a lockfile (" + <> Text.intercalate ", " unresolvable + <> "). Analyze from the workspace root to include them." context "Building dependency graph" . pure $ foldMap buildGraph manifests +-- | Specifier protocols that name a location in the workspace rather than a +-- version range. +-- +-- @catalog:@ (pnpm catalogs), @workspace:@ (the workspace protocol) and +-- @link:@ are all resolved from files this strategy does not read — the +-- lockfile, or pnpm-workspace.yaml. Recording the raw specifier as the version +-- produced locators like @npm+left-pad$catalog:@, a dependency pinned to the +-- literal version "catalog:", which does not exist in any registry. +-- +-- @file:@ is deliberately not in this list. It is equally unresolvable, but it +-- long predates the workspace protocols and npm projects have been reporting +-- it this way for years; changing that is a separate decision. +workspaceProtocols :: [Text] +workspaceProtocols = ["catalog:", "workspace:", "link:"] + +isWorkspaceReference :: Text -> Bool +isWorkspaceReference constraint = any (`Text.isPrefixOf` constraint) workspaceProtocols + +-- | @name\@specifier@ for every dependency dropped by 'buildGraph', for warning. +unresolvableSpecifiers :: PackageJson -> [Text] +unresolvableSpecifiers PackageJson{..} = + map (\(name, constraint) -> name <> "@" <> constraint) + . filter (isWorkspaceReference . snd) + $ Map.toList packageDeps <> Map.toList packageDevDeps + type NodeGrapher = LabeledGrapher NodePackage NodePackageLabel newtype NodePackageLabel = NodePackageEnv DepEnvironment @@ -76,8 +112,8 @@ newtype NodePackageLabel = NodePackageEnv DepEnvironment buildGraph :: PackageJson -> Graphing Dependency buildGraph PackageJson{..} = run . withLabeling toDependency $ do - _ <- Map.traverseWithKey (addDep EnvProduction) packageDeps - _ <- Map.traverseWithKey (addDep EnvDevelopment) packageDevDeps + _ <- Map.traverseWithKey (addDep EnvProduction) (Map.filter (not . isWorkspaceReference) packageDeps) + _ <- Map.traverseWithKey (addDep EnvDevelopment) (Map.filter (not . isWorkspaceReference) packageDevDeps) pure () where addDep :: Has NodeGrapher sig m => DepEnvironment -> Text -> Text -> m () diff --git a/src/Strategy/Node/Pnpm/PnpmLock.hs b/src/Strategy/Node/Pnpm/PnpmLock.hs index 8f631c7162..6d23ebe2a0 100644 --- a/src/Strategy/Node/Pnpm/PnpmLock.hs +++ b/src/Strategy/Node/Pnpm/PnpmLock.hs @@ -4,6 +4,7 @@ module Strategy.Node.Pnpm.PnpmLock ( -- * for testing buildGraph, parsePnpmLockfile, + resolveImporterKey, ) where import Control.Applicative ((<|>)) @@ -13,9 +14,10 @@ import Data.ByteString (ByteString) import Data.Either (partitionEithers) import Data.Foldable (for_) import Data.HashMap.Strict qualified as HashMap +import Data.List (foldl') import Data.Map (Map, toList) import Data.Map qualified as Map -import Data.Maybe (fromMaybe) +import Data.Maybe (fromMaybe, mapMaybe) import Data.Set qualified as Set import Data.String.Conversion (toString, toText) import Data.Text (Text) @@ -172,8 +174,12 @@ toResolvedDependency toEnv pkgs mkPkg depName depVersion = do -- -- | Core graph-building logic shared across all lockfile versions. -buildGraphCore :: BuildGraphConfig -> PnpmLockfileBase -> Graphing Dependency -buildGraphCore BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgcLabelingMode, bgcSnapshotEdges, bgcCatalogs} base = +-- +-- The first argument is the set of importer keys to treat as direct-dependency +-- sources, or 'Nothing' to use every importer in the lockfile. See +-- 'scopedImporters'. +buildGraphCore :: Maybe (Set.Set Text) -> BuildGraphConfig -> PnpmLockfileBase -> Graphing Dependency +buildGraphCore selection BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgcLabelingMode, bgcSnapshotEdges, bgcCatalogs} base = let getPkgNameVersion = bgcGetPkgNameVersion mkPkgKey = bgcMkPkgKey toEnv = bgcToEnv @@ -182,10 +188,17 @@ buildGraphCore BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgc catalogs = bgcCatalogs pkgs = lockfilePackages base snapshotEdgesHM = HashMap.fromList snapshotEdges - in withoutLocalPackages . hydrateDepEnvs $ + importers = maybe (lockfileImporters base) (Map.restrictKeys (lockfileImporters base)) selection + -- Every entry in `packages` is added as a deep node below, so a scoped + -- graph would otherwise still carry the whole workspace's dependencies, + -- just with a smaller direct set. Prune to what the selected importers + -- can actually reach. Unscoped analysis skips this so its output is + -- unchanged. + pruneIfScoped = maybe id (const Graphing.pruneUnreachable) selection + in pruneIfScoped . withoutLocalPackages . hydrateDepEnvs $ run . withLabeling applyLabels $ do -- Direct dependencies from each importer (workspace package). - for_ (toList (lockfileImporters base)) $ \(_, projectImporters) -> do + for_ (toList importers) $ \(_, projectImporters) -> do for_ (Map.toList $ directDependencies projectImporters) $ \(depName, ProjectMapDepMetadata depVersion) -> let resolvedVersion = resolveCatalogVersion catalogs depName depVersion in for_ (toResolvedDependency toEnv pkgs mkPkgKey depName resolvedVersion) $ \dep -> do @@ -221,6 +234,95 @@ buildGraphCore BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgc for_ deepDependencies $ \(deepName, deepVersion) -> do maybe (pure ()) (edge parentDep) (toResolvedDependency toEnv pkgs mkPkgKey deepName deepVersion) +-- +-- Workspace scoping +-- + +-- | The base fields of a lockfile, whatever its version. +lockfileBaseOf :: PnpmLockfile -> PnpmLockfileBase +lockfileBaseOf (LockfileV4Or5 (PnpmLockfileV4Or5 base)) = base +lockfileBaseOf (LockfileV678 (PnpmLockfileV678 base)) = base +lockfileBaseOf (LockfileV9 v) = lockfileBase v + +-- | The importers whose direct dependencies should be graphed, given the +-- importer keys resolved from the selected build targets by +-- 'Strategy.Node.resolvePnpmImporterKeys'. +-- +-- 'Nothing' means the analysis is unscoped, which must reproduce pre-scoping +-- output exactly. That happens both when no target filter is applied and when +-- the selection turns out to cover every importer in the lockfile, which is +-- the default case where all targets are selected. +-- +-- A selection that matches no importer yields @Just Set.empty@: nothing is +-- direct, so pruning leaves an empty graph. 'analyze' warns when that happens +-- rather than quietly falling back to the whole workspace. +scopedImporters :: Maybe (Set.Set Text) -> PnpmLockfileBase -> Maybe (Set.Set Text) +scopedImporters Nothing _ = Nothing +scopedImporters (Just keys) base = + if selected == allImporters then Nothing else Just selected + where + allImporters = Map.keysSet (lockfileImporters base) + selected = expandWorkspaceLinks base (keys `Set.intersection` allImporters) + +-- | Grow a selection of importer keys to include the workspace importers that +-- those importers link to, transitively. +-- +-- pnpm records a dependency on a sibling workspace package as +-- @version: link:\@ rather than as an entry in @packages@, so +-- the sibling's own dependencies live under its importer key and nowhere else. +-- Unscoped analysis merges every importer, so those dependencies land in the +-- graph regardless of who declared them; once a selection is applied they would +-- disappear. Following the links keeps a scoped result complete. +expandWorkspaceLinks :: PnpmLockfileBase -> Set.Set Text -> Set.Set Text +expandWorkspaceLinks base = go Set.empty . Set.toList + where + importers = lockfileImporters base + + go :: Set.Set Text -> [Text] -> Set.Set Text + go seen [] = seen + go seen (key : rest) + | key `Set.member` seen = go seen rest + | otherwise = go (Set.insert key seen) (linkedFrom key <> rest) + + linkedFrom :: Text -> [Text] + linkedFrom key = case Map.lookup key importers of + Nothing -> [] + Just projectMap -> + mapMaybe (linkTarget key . version) $ + Map.elems (directDependencies projectMap) <> Map.elems (directDevDependencies projectMap) + + -- A link is only followed when it names an importer the lockfile actually + -- has; a @link:@ pointing outside the workspace resolves to nothing. + linkTarget :: Text -> Text -> Maybe Text + linkTarget fromKey ver = do + relPath <- Text.stripPrefix "link:" ver + let key = resolveImporterKey fromKey relPath + if key `Map.member` importers then Just key else Nothing + +-- | Resolve a path relative to an importer back into importer-key form: +-- forward slashes, @.@ and @..@ segments collapsed, and @"."@ for the +-- workspace root. +-- +-- >> resolveImporterKey "browser" "../server" = "server" +-- >> resolveImporterKey "apps/web" "../../libs/ui" = "libs/ui" +-- >> resolveImporterKey "browser" "../" = "." +resolveImporterKey :: Text -> Text -> Text +resolveImporterKey fromKey relPath = toKey $ foldl' step [] segments + where + segments :: [Text] + segments = + concatMap (filter (not . Text.null) . Text.splitOn "/" . Text.replace "\\" "/") [fromKey, relPath] + + -- The accumulator is in reverse order, so ".." drops its head. + step :: [Text] -> Text -> [Text] + step acc ".." = drop 1 acc + step acc "." = acc + step acc segment = segment : acc + + toKey :: [Text] -> Text + toKey [] = "." + toKey acc = Text.intercalate "/" (reverse acc) + -- -- Top-level dispatch -- @@ -228,10 +330,16 @@ buildGraphCore BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgc -- | Build the dependency graph, labeling direct deps with their environment -- (prod\/dev). hydrateDepEnvs then propagates those environments to all -- transitive successors. -buildGraph :: PnpmLockfile -> Graphing Dependency -buildGraph (LockfileV4Or5 (PnpmLockfileV4Or5 base)) = buildGraphCore buildGraphConfigV4or5 base -buildGraph (LockfileV678 (PnpmLockfileV678 base)) = buildGraphCore buildGraphConfigV678 base -buildGraph (LockfileV9 v) = buildGraphCore (buildGraphConfigV9 v) (lockfileBase v) +-- +-- The first argument scopes the graph to a set of workspace importer keys; see +-- 'scopedImporters'. +buildGraph :: Maybe (Set.Set Text) -> PnpmLockfile -> Graphing Dependency +buildGraph selection lockfile = case lockfile of + LockfileV4Or5 (PnpmLockfileV4Or5 base) -> withSelection buildGraphConfigV4or5 base + LockfileV678 (PnpmLockfileV678 base) -> withSelection buildGraphConfigV678 base + LockfileV9 v -> withSelection (buildGraphConfigV9 v) (lockfileBase v) + where + withSelection config base = buildGraphCore (scopedImporters selection base) config base -- | Parse the contents of a pnpm-lock.yaml file. -- @@ -251,8 +359,12 @@ parsePnpmLockfile contents = case decodeAllEither' contents of ([], []) -> Left "no YAML documents found" (errs, []) -> Left . Text.intercalate "\n" $ map toText errs -analyze :: (Has ReadFS sig m, Has Logger sig m, Has Diagnostics sig m) => Path Abs File -> m (Graphing Dependency) -analyze file = context "Analyzing Pnpm Lockfile" $ do +-- | Analyze a pnpm lockfile, optionally scoped to the given workspace importer +-- keys (@"."@ for the root, @"packages/a"@ for a member), as resolved from the +-- selected build targets by 'Strategy.Node.resolvePnpmImporterKeys'. 'Nothing' +-- means no target filter is applied, so the whole workspace is graphed. +analyze :: (Has ReadFS sig m, Has Logger sig m, Has Diagnostics sig m) => Maybe (Set.Set Text) -> Path Abs File -> m (Graphing Dependency) +analyze selectedImporters file = context "Analyzing Pnpm Lockfile" $ do pnpmLockFile <- context "Parsing pnpm-lock file" $ context ("Parsing YAML file '" <> toText (toString file) <> "'") $ do contents <- readContentsBS file @@ -271,4 +383,13 @@ analyze file = context "Analyzing Pnpm Lockfile" $ do LockfileV678 _ -> pure () LockfileV9 _ -> pure () - context "Building dependency graph" $ pure $ buildGraph pnpmLockFile + case (selectedImporters, scopedImporters selectedImporters (lockfileBaseOf pnpmLockFile)) of + (Just keys, Just selected) + | Set.null selected -> + logWarn . pretty $ + "Target filter (resolved importer keys: " + <> Text.intercalate ", " (Set.toList keys) + <> ") did not match any importer in the pnpm lockfile; reporting an empty dependency graph." + _ -> pure () + + context "Building dependency graph" $ pure $ buildGraph selectedImporters pnpmLockFile diff --git a/test/Node/NodeSpec.hs b/test/Node/NodeSpec.hs index f26d65bbce..77c769115c 100644 --- a/test/Node/NodeSpec.hs +++ b/test/Node/NodeSpec.hs @@ -14,7 +14,7 @@ import DepTypes (DepEnvironment (EnvProduction), Dependency (dependencyEnvironme import Graphing qualified import Path (Abs, Dir, Path, mkRelDir, mkRelFile, ()) import Path.IO (getCurrentDir) -import Strategy.Node (NodeProject (NPMLock), discover, extractDepListsForTargets, findWorkspaceBuildTargets, getDeps, pkgGraph, resolveNpmV3WorkspacePaths) +import Strategy.Node (NodeProject (NPMLock), discover, extractDepListsForTargets, findWorkspaceBuildTargets, getDeps, pkgGraph, resolveNpmV3WorkspacePaths, resolvePnpmImporterKeys, workspaceRootTargetName) import Strategy.Node.PackageJson ( FlatDeps (..), Manifest (..), @@ -61,6 +61,8 @@ spec = do workspaceBuildTargetsSpec currDir extractDepListsForTargetsSpec currDir resolveNpmV3WorkspacePathsSpec currDir + resolvePnpmImporterKeysSpec currDir + unnamedWorkspaceRootSpec currDir discoveredWorkSpaceProj :: Path Abs Dir -> DiscoveredProject NodeProject discoveredWorkSpaceProj currDir = @@ -311,6 +313,60 @@ resolveNpmV3WorkspacePathsSpec currDir = describe "resolveNpmV3WorkspacePaths" $ it "resolves no paths when no target matches a manifest" $ forTargets ["does-not-exist"] `shouldBe` Just Set.empty +resolvePnpmImporterKeysSpec :: Path Abs Dir -> Spec +resolvePnpmImporterKeysSpec currDir = describe "resolvePnpmImporterKeys" $ do + let graph = workspaceGraphWithDeps currDir + forTargets names = + resolvePnpmImporterKeys + (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList (map BuildTarget names)) + graph + + it "returns Nothing when unscoped" $ + resolvePnpmImporterKeys ProjectWithoutTargets graph `shouldBe` Nothing + + it "maps the root target to the \".\" importer key" $ + -- pnpm spells the workspace root "." where npm spells it "". + forTargets ["workspace-test"] `shouldBe` Just (Set.fromList ["."]) + + it "maps a workspace name to its root-relative importer key" $ + forTargets ["pkg-b"] `shouldBe` Just (Set.fromList ["nested/pkg-b"]) + + it "maps every selected target" $ + forTargets ["workspace-test", "pkg-a", "pkg-b"] + `shouldBe` Just (Set.fromList [".", "pkg-a", "nested/pkg-b"]) + + it "resolves no importers when no target matches a manifest" $ + forTargets ["does-not-exist"] `shouldBe` Just Set.empty + +-- | A pnpm workspace root usually keeps its configuration in +-- pnpm-workspace.yaml, so its package.json commonly has no @name@. Such a root +-- must still yield build targets, and the name it is given must be the one the +-- importer-key resolution understands. +unnamedWorkspaceRootSpec :: Path Abs Dir -> Spec +unnamedWorkspaceRootSpec currDir = describe "workspace root without a name" $ do + let graph = unnamedRootWorkspaceGraph currDir + + it "names the root target after the root directory" $ + workspaceRootTargetName graph `shouldBe` Just "workspace-test" + + it "still exposes the root and every member as build targets" $ + findWorkspaceBuildTargets graph + `shouldBe` (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList (map BuildTarget ["workspace-test", "pkg-a", "pkg-b"])) + + it "resolves the fallback root target back to the root importer" $ + resolvePnpmImporterKeys + (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList [BuildTarget "workspace-test"]) + graph + `shouldBe` Just (Set.fromList ["."]) + +-- | 'workspaceGraphWithDeps' with the root's @name@ field removed. +unnamedRootWorkspaceGraph :: Path Abs Dir -> PkgJsonGraph +unnamedRootWorkspaceGraph currDir = + graph{jsonLookup = Map.adjust (\pj -> pj{packageName = Nothing}) (Manifest rootManifest) (jsonLookup graph)} + where + graph = workspaceGraphWithDeps currDir + rootManifest = currDir $(mkRelFile "test/Node/testdata/workspace-test/package.json") + -- | A workspace graph with actual dependencies for testing extractDepListsForTargets. workspaceGraphWithDeps :: Path Abs Dir -> PkgJsonGraph workspaceGraphWithDeps currDir = diff --git a/test/Node/testdata/pnpm-workspaces/browser/package.json b/test/Node/testdata/pnpm-workspaces/browser/package.json new file mode 100644 index 0000000000..5548756c68 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/browser/package.json @@ -0,0 +1,8 @@ +{ + "name": "@fossa-test/browser", + "version": "1.0.0", + "dependencies": { + "left-pad": "catalog:", + "@fossa-test/shared": "workspace:*" + } +} diff --git a/test/Node/testdata/pnpm-workspaces/package.json b/test/Node/testdata/pnpm-workspaces/package.json new file mode 100644 index 0000000000..394e5c4fb8 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/package.json @@ -0,0 +1,7 @@ +{ + "version": "1.0.0", + "private": true, + "devDependencies": { + "colorjs": "^0.1.9" + } +} diff --git a/test/Node/testdata/pnpm-workspaces/pnpm-lock.yaml b/test/Node/testdata/pnpm-workspaces/pnpm-lock.yaml new file mode 100644 index 0000000000..86f50017e8 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/pnpm-lock.yaml @@ -0,0 +1,82 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +catalogs: + default: + left-pad: + specifier: 1.3.0 + version: 1.3.0 + +importers: + + .: + devDependencies: + colorjs: + specifier: ^0.1.9 + version: 0.1.9 + + browser: + dependencies: + '@fossa-test/shared': + specifier: workspace:* + version: link:../shared + left-pad: + specifier: 'catalog:' + version: 1.3.0 + + server: + dependencies: + is-odd: + specifier: 3.0.1 + version: 3.0.1 + + shared: + dependencies: + uri-js: + specifier: ^4.4.1 + version: 4.4.1 + +packages: + + colorjs@0.1.9: + resolution: {integrity: sha512-filDwoNvVLqcLB4zMmWa65rgNHW/ff26wn3/q+XpYrI9EFts6fwnLIPYGOr8G+9cECMsxfCm9a2QzXdf+imY7A==} + + is-number@6.0.0: + resolution: {integrity: sha512-Wu1VHeILBK8KAWJUAiSZQX94GmOE45Rg6/538fKwiloUu21KncEkYGPqob2oSZ5mUT73vLGrHQjKw3KMPwfDzg==} + engines: {node: '>=0.10.0'} + + is-odd@3.0.1: + resolution: {integrity: sha512-CQpnWPrDwmP1+SMHXZhtLtJv90yiyVfluGsX5iNCVkrhQtU3TQHsUWPG9wkdk9Lgd5yNpAg9jQEo90CBaXgWMA==} + engines: {node: '>=4'} + + left-pad@1.3.0: + resolution: {integrity: sha512-XI5MPzVNApjAyhQzphX8BkmKsKUxD4LdyK24iZeQGinBN9yTQT3bFlCBy/aVx2HrNcqQGsdot8ghrjyrvMCoEA==} + deprecated: use String.prototype.padStart() + + punycode@2.3.1: + resolution: {integrity: sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg==} + engines: {node: '>=6'} + + uri-js@4.4.1: + resolution: {integrity: sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==} + +snapshots: + + colorjs@0.1.9: {} + + is-number@6.0.0: {} + + is-odd@3.0.1: + dependencies: + is-number: 6.0.0 + + left-pad@1.3.0: {} + + punycode@2.3.1: {} + + uri-js@4.4.1: + dependencies: + punycode: 2.3.1 diff --git a/test/Node/testdata/pnpm-workspaces/pnpm-workspace.yaml b/test/Node/testdata/pnpm-workspaces/pnpm-workspace.yaml new file mode 100644 index 0000000000..711ea52464 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/pnpm-workspace.yaml @@ -0,0 +1,7 @@ +packages: + - 'browser' + - 'server' + - 'shared' + +catalog: + left-pad: 1.3.0 diff --git a/test/Node/testdata/pnpm-workspaces/server/package.json b/test/Node/testdata/pnpm-workspaces/server/package.json new file mode 100644 index 0000000000..7990e86bf8 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/server/package.json @@ -0,0 +1,7 @@ +{ + "name": "@fossa-test/server", + "version": "1.0.0", + "dependencies": { + "is-odd": "3.0.1" + } +} diff --git a/test/Node/testdata/pnpm-workspaces/shared/package.json b/test/Node/testdata/pnpm-workspaces/shared/package.json new file mode 100644 index 0000000000..9476f5c167 --- /dev/null +++ b/test/Node/testdata/pnpm-workspaces/shared/package.json @@ -0,0 +1,7 @@ +{ + "name": "@fossa-test/shared", + "version": "1.0.0", + "dependencies": { + "uri-js": "^4.4.1" + } +} diff --git a/test/Pnpm/PnpmLockSpec.hs b/test/Pnpm/PnpmLockSpec.hs index e6d37cf211..ed72082b62 100644 --- a/test/Pnpm/PnpmLockSpec.hs +++ b/test/Pnpm/PnpmLockSpec.hs @@ -17,14 +17,15 @@ import DepTypes ( ) import GraphUtil ( expectDep, + expectDeps, expectDirect, expectEdge, ) import Graphing (Graphing) import Path (Abs, File, Path, mkRelFile, ()) import Path.IO (getCurrentDir) -import Strategy.Node.Pnpm.PnpmLock (buildGraph, parsePnpmLockfile) -import Test.Hspec (Expectation, Spec, describe, expectationFailure, it, runIO) +import Strategy.Node.Pnpm.PnpmLock (buildGraph, parsePnpmLockfile, resolveImporterKey) +import Test.Hspec (Expectation, Spec, describe, expectationFailure, it, runIO, shouldBe) mkProdDep :: Text -> Dependency mkProdDep nameAtVersion = mkDep nameAtVersion (Just EnvProduction) @@ -89,10 +90,16 @@ lodash = mempty checkGraph :: Path Abs File -> (Graphing Dependency -> Spec) -> Spec -checkGraph pathToFixture buildGraphSpec = do +checkGraph = checkScopedGraph Nothing + +-- | Like 'checkGraph', but scoping the graph to a set of workspace importer +-- keys, as 'Strategy.Node.resolvePnpmImporterKeys' would from selected build +-- targets. +checkScopedGraph :: Maybe (Set.Set Text) -> Path Abs File -> (Graphing Dependency -> Spec) -> Spec +checkScopedGraph selection pathToFixture buildGraphSpec = do lockFileContents <- runIO $ BS.readFile (toString pathToFixture) case parsePnpmLockfile lockFileContents of - Right pnpmLock -> buildGraphSpec (buildGraph pnpmLock) + Right pnpmLock -> buildGraphSpec (buildGraph selection pnpmLock) Left err -> describe "pnpm-lock" $ it "should parse lockfile" (expectationFailure $ toString err) @@ -143,6 +150,77 @@ spec = do describe "works with pnpm v11 multi-document lockfile" $ checkGraph pnpmLockV11MultiDoc pnpmLockV9LocalDepSpec + -- Workspace scoping. The fixture has four importers: the root (colorjs), + -- browser (left-pad, plus a link: to shared), server (is-odd -> is-number) + -- and shared (uri-js -> punycode). + let pnpmWorkspace = currentDir $(mkRelFile "test/Node/testdata/pnpm-workspaces/pnpm-lock.yaml") + + describe "workspace scoping" $ do + describe "unscoped" $ + checkScopedGraph Nothing pnpmWorkspace $ \graph -> + it "should merge every importer's direct dependencies" $ + expectDirect + [ mkDevDep "colorjs@0.1.9" + , mkProdDep "left-pad@1.3.0" + , mkProdDep "is-odd@3.0.1" + , mkProdDep "uri-js@4.4.1" + ] + graph + + describe "scoped to one member" $ + checkScopedGraph (Just $ Set.fromList ["server"]) pnpmWorkspace $ \graph -> do + it "should keep the selected member's dependencies and their transitives" $ do + expectDirect [mkProdDep "is-odd@3.0.1"] graph + expectDep (mkProdDep "is-number@6.0.0") graph + + it "should drop every other importer's dependencies" $ + expectDeps [mkProdDep "is-odd@3.0.1", mkProdDep "is-number@6.0.0"] graph + + describe "scoped to a member that links to a sibling" $ + checkScopedGraph (Just $ Set.fromList ["browser"]) pnpmWorkspace $ \graph -> do + it "should include the linked sibling's dependencies" $ + -- browser declares `@fossa-test/shared: link:../shared`, so shared's + -- own dependencies are part of browser's result. They are reported as + -- direct because the lockfile records no per-importer provenance. + expectDirect [mkProdDep "left-pad@1.3.0", mkProdDep "uri-js@4.4.1"] graph + + it "should not emit the workspace link itself as a dependency" $ + expectDeps + [ mkProdDep "left-pad@1.3.0" + , mkProdDep "uri-js@4.4.1" + , mkProdDep "punycode@2.3.1" + ] + graph + + describe "scoped to every importer" $ + checkScopedGraph (Just $ Set.fromList [".", "browser", "server", "shared"]) pnpmWorkspace $ \graph -> + it "should match the unscoped graph" $ + expectDirect + [ mkDevDep "colorjs@0.1.9" + , mkProdDep "left-pad@1.3.0" + , mkProdDep "is-odd@3.0.1" + , mkProdDep "uri-js@4.4.1" + ] + graph + + describe "scoped to an importer the lockfile does not have" $ + checkScopedGraph (Just $ Set.fromList ["nonexistent"]) pnpmWorkspace $ \graph -> + it "should report an empty graph rather than the whole workspace" $ + expectDeps [] graph + + describe "resolveImporterKey" $ do + it "should resolve a sibling link to the sibling's importer key" $ + resolveImporterKey "browser" "../shared" `shouldBe` "shared" + + it "should collapse repeated parent segments" $ + resolveImporterKey "apps/web" "../../libs/ui" `shouldBe` "libs/ui" + + it "should resolve a link back to the workspace root" $ + resolveImporterKey "browser" ".." `shouldBe` "." + + it "should resolve a link relative to the root importer" $ + resolveImporterKey "." "packages/a" `shouldBe` "packages/a" + pnpmLockGraphSpec :: Graphing Dependency -> Spec pnpmLockGraphSpec graph = do let hasEdge :: Dependency -> Dependency -> Expectation From 6c0ebacd3b3b598f366a76bde5cd143fba482670 Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 11:06:57 -0700 Subject: [PATCH 02/10] Document the target filter field in the .fossa.yml schema The parser has accepted `target:` on a target filter since before workspace build targets existed, but neither the reference doc nor the JSON schema mentioned it, so the yaml form of --only-target was undiscoverable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015zRtempnk5Led4xWQTqVfb --- docs/references/files/fossa-yml.v3.schema.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/references/files/fossa-yml.v3.schema.json b/docs/references/files/fossa-yml.v3.schema.json index 46e2b4f90e..301b5afa98 100644 --- a/docs/references/files/fossa-yml.v3.schema.json +++ b/docs/references/files/fossa-yml.v3.schema.json @@ -281,6 +281,10 @@ "path": { "type": "string", "description": "Associated path with target type (if any)" + }, + "target": { + "type": "string", + "description": "A named build target within the project at `path`, for project types that have them (for example each workspace package of a yarn, npm, or pnpm monorepo). Requires `path`. Run `fossa list-targets` to see a project's target names." } } }, From 3e3c78e24b5905f784c6c562dbda9ab4564264ed Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 11:09:46 -0700 Subject: [PATCH 03/10] Fill in the changelog PR links and close the docs gaps around build targets Fills in #1763 for the three Unreleased entries. The .fossa.yml reference described list-targets output as `type@path`, which has been incomplete since yarn and npm gained per-workspace targets; it now names the `type@path:target` form too. The package.json strategy doc says nothing about the workspace-reference specifiers it now skips, so add a section covering them and point at the workspace build targets of the three lockfile strategies as the way to scope a scan instead. The yarn and npm docs get the root-name fallback, which applies to them as much as to pnpm. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015zRtempnk5Led4xWQTqVfb --- Changelog.md | 7 ++--- docs/references/files/fossa-yml.md | 2 +- .../languages/nodejs/npm-lockfile.md | 3 ++ .../languages/nodejs/packagejson.md | 17 +++++++++++ .../strategies/languages/nodejs/pnpm.md | 28 +++++++++---------- .../strategies/languages/nodejs/yarn.md | 3 ++ 6 files changed, 41 insertions(+), 19 deletions(-) diff --git a/Changelog.md b/Changelog.md index 54b097c467..0d9e4972cc 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,10 +2,9 @@ ## Unreleased -- Pnpm: workspace members are now individual build targets, so a single member can be analyzed on its own with `--only-target 'pnpm@./:my-package'` or a `target:` entry in `.fossa.yml`. Previously every member's dependencies were merged into one result with no way to scope them. Dependencies a selected member reaches through the workspace protocol (`link:` in the lockfile) are included. With no target filter the result is unchanged. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) -- Node: a workspace root whose `package.json` has no `name` field no longer suppresses build targets for the whole workspace; the root directory's name is used for the root target. This most often affected pnpm, whose workspace configuration lives in `pnpm-workspace.yaml`. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) -- Node: dependencies whose version is a workspace reference (`catalog:`, `workspace:`, `link:`) are no longer reported at that literal string as their version, which produced locators like `npm+left-pad$catalog:` for packages that do not exist. They are skipped with a warning when analysis falls back to a `package.json`-only strategy that cannot resolve them. ([#TODO](https://github.com/fossas/fossa-cli/pull/TODO)) - +- Pnpm: workspace members are now individual build targets, so a single member can be analyzed on its own with `--only-target 'pnpm@./:my-package'` or a `target:` entry in `.fossa.yml`. Previously every member's dependencies were merged into one result with no way to scope them. Dependencies a selected member reaches through the workspace protocol (`link:` in the lockfile) are included. With no target filter the result is unchanged. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) +- Node: a workspace root whose `package.json` has no `name` field no longer suppresses build targets for the whole workspace; the root directory's name is used for the root target. This most often affected pnpm, whose workspace configuration lives in `pnpm-workspace.yaml`. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) +- Node: dependencies whose version is a workspace reference (`catalog:`, `workspace:`, `link:`) are no longer reported at that literal string as their version, which produced locators like `npm+left-pad$catalog:` for packages that do not exist. They are skipped with a warning when analysis falls back to a `package.json`-only strategy that cannot resolve them. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) - Diagnostics: When an error or warning group contains multiple errors, each error's `Traceback:` header is now printed on its own line instead of being glued onto the last line of the preceding error message (e.g. `...none passed validationTraceback:`). ([#1758](https://github.com/fossas/fossa-cli/pull/1758)) ## 3.18.2 diff --git a/docs/references/files/fossa-yml.md b/docs/references/files/fossa-yml.md index 2283c780b5..917c5ae2c1 100644 --- a/docs/references/files/fossa-yml.md +++ b/docs/references/files/fossa-yml.md @@ -383,7 +383,7 @@ For detailed walkthrough, and example please refer to [analysis target configura #### Project target configuration example -Run the command `fossa list-targets` to determine the analysis targets present in your project. The output will look similar to the following with the targets in format `type@path` (You may see that duplicated lines for "Found target" and "Found project"): +Run the command `fossa list-targets` to determine the analysis targets present in your project. The output will look similar to the following with the targets in format `type@path`, or `type@path:target` for a project divided into named build targets (You may see that duplicated lines for "Found target" and "Found project"): ``` Found target: bundler@prod/docker diff --git a/docs/references/strategies/languages/nodejs/npm-lockfile.md b/docs/references/strategies/languages/nodejs/npm-lockfile.md index 3d2ff43333..9b4c3dd5ac 100644 --- a/docs/references/strategies/languages/nodejs/npm-lockfile.md +++ b/docs/references/strategies/languages/nodejs/npm-lockfile.md @@ -33,6 +33,9 @@ npm@./:web When a subset of targets is selected, only those packages' dependencies are included in the analysis. +If the workspace root's `package.json` has no `name` field, the root directory's +own name is used for the root target. + When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. diff --git a/docs/references/strategies/languages/nodejs/packagejson.md b/docs/references/strategies/languages/nodejs/packagejson.md index 9fcf0a0cf1..bec4a025c0 100644 --- a/docs/references/strategies/languages/nodejs/packagejson.md +++ b/docs/references/strategies/languages/nodejs/packagejson.md @@ -23,3 +23,20 @@ As of _v2.19.x_, we also combine `package.json` files that are members of the same workspace. The files are treated as though all dependencies were found from the same file, though we report the origins of the deps as a set of all files. + +### Workspace references + +A version specifier can name another package in the same workspace rather than a +version range: `catalog:` and `catalog:` +([pnpm catalogs](https://pnpm.io/catalogs)), `workspace:` +([the workspace protocol](https://pnpm.io/workspaces#workspace-protocol)), and +`link:`. Resolving those needs the lockfile or `pnpm-workspace.yaml`, neither of +which this strategy reads, so such dependencies are skipped and a warning names +them. + +This strategy is a fallback used when no lockfile is in scope — most often when +`fossa analyze` is run from inside a workspace member's own directory. Analyze +from the workspace root instead, where the lockfile resolves these specifiers to +real versions. To report on one member, see workspace build targets for +[pnpm](pnpm.md#workspace-build-targets), [npm](npm-lockfile.md#workspace-build-targets), +or [yarn](yarn.md#workspace-build-targets). diff --git a/docs/references/strategies/languages/nodejs/pnpm.md b/docs/references/strategies/languages/nodejs/pnpm.md index ad6e16a51a..e9b0823f0a 100644 --- a/docs/references/strategies/languages/nodejs/pnpm.md +++ b/docs/references/strategies/languages/nodejs/pnpm.md @@ -163,8 +163,8 @@ CLI will infer the package name and version using `/${dependencyName}/${dependen ### Workspace Build Targets Each workspace member, and the workspace root, is exposed as an individual build -target. A workspace whose `pnpm-workspace.yaml` lists `browser` and `server` -produces: +target. A workspace named `my-workspace` whose `pnpm-workspace.yaml` lists +`browser` and `server` produces: ``` pnpm@./:my-workspace @@ -172,7 +172,7 @@ pnpm@./:browser pnpm@./:server ``` -The target name is the member's `name` from its `package.json`, so a member +A target's name is that package's `name` from its `package.json`, so a member named `@acme/browser` is selected as `pnpm@./:@acme/browser`. Run `fossa list-targets` to see the exact names. If the workspace root's `package.json` has no `name` field — common for pnpm, since the workspace @@ -185,7 +185,7 @@ Selecting a subset reports only those members' dependencies: fossa analyze --only-target 'pnpm@./:browser' ``` -or, equivalently, in `.fossa.yml`: +The same selection in `.fossa.yml`: ```yaml version: 3 @@ -199,20 +199,20 @@ targets: When a selected member depends on a sibling member through the [workspace protocol](https://pnpm.io/workspaces#workspace-protocol), pnpm records that in the lockfile as `version: link:`. The sibling's own dependencies -are included in the result, since the selected member does depend on them. +are part of the selected member's result, because the selected member depends on +them. -With no target filtering, all targets are selected and every workspace member's -dependencies are included, which is the behavior of every release before this -feature. +With no target filtering, all targets are selected and every member's +dependencies are included. > 📘 Note > -> A pnpm workspace is a single FOSSA project rooted at the workspace root, and -> stays one project no matter which targets are selected. Build targets scope -> what that project reports; they do not split it into several projects. Running -> `fossa analyze` from inside a member directory does not scope the scan either -> — without the lockfile in scope, analysis falls back to a `package.json`-only -> npm strategy with a partial graph. +> A pnpm workspace is one FOSSA project rooted at the workspace root, and stays +> one project no matter which targets are selected — build targets scope what +> that project reports rather than splitting it into several projects. Running +> `fossa analyze` from inside a member's directory is not a way to scope a scan: +> without the lockfile in scope, analysis falls back to the +> [package.json strategy](packagejson.md) and its partial graph. ### Catalogs diff --git a/docs/references/strategies/languages/nodejs/yarn.md b/docs/references/strategies/languages/nodejs/yarn.md index 8340ef1d99..6e262cf588 100644 --- a/docs/references/strategies/languages/nodejs/yarn.md +++ b/docs/references/strategies/languages/nodejs/yarn.md @@ -55,6 +55,9 @@ yarn@./:lib-core When a subset of targets is selected, only those packages' dependencies are included in the analysis. +If the workspace root's `package.json` has no `name` field, the root directory's +own name is used for the root target. + When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. From 52a8772f684d4a6744cde19d09d4b44deedb4ae3 Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 12:21:46 -0700 Subject: [PATCH 04/10] Keep an unnamed workspace root's dependencies in yarn and npm v1 analysis The root directory-name fallback in workspaceRootTargetName gave an unnamed root a build target, but extractDepListsForTargets still selected manifests by their package name, which the root does not have. With no target filter every target is selected, so yarn and npm v1 workspaces with an unnamed root silently lost the root's own dependencies. manifestTargetNames is now the one mapping from manifest to target name, and findWorkspaceBuildTargets, resolveWorkspacePathKeys and extractDepListsForTargets all read from it so they cannot disagree. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0122XfZKg7Yidpc1HEEvBSJD --- src/Strategy/Node.hs | 63 ++++++++++++++++++++++++++----------------- test/Node/NodeSpec.hs | 14 ++++++++-- 2 files changed, 51 insertions(+), 26 deletions(-) diff --git a/src/Strategy/Node.hs b/src/Strategy/Node.hs index 715010ecf0..9c63b1f09b 100644 --- a/src/Strategy/Node.hs +++ b/src/Strategy/Node.hs @@ -179,21 +179,43 @@ mkProject project = do } -- | Build targets from workspace package names (root + members). --- If the workspace graph has children (i.e., workspace members), each --- package name becomes a 'BuildTarget', along with the root's own name from --- 'workspaceRootTargetName'. If there are no workspace children --- (single-package project), returns 'ProjectWithoutTargets'. +-- If the workspace graph has children (i.e., workspace members), every name +-- in 'manifestTargetNames' becomes a 'BuildTarget'. If there are no workspace +-- children (single-package project), or the root has no name to offer, +-- returns 'ProjectWithoutTargets'. findWorkspaceBuildTargets :: PkgJsonGraph -> FoundTargets findWorkspaceBuildTargets graph = let WorkspacePackageNames childNames = findWorkspaceNames graph in if Set.null childNames then ProjectWithoutTargets else case workspaceRootTargetName graph of + -- With no target naming the root there would be no way to select + -- the root's own dependencies, so offer no targets rather than lose + -- them. Nothing -> ProjectWithoutTargets - Just n -> - let allNames = Set.insert n childNames + Just _ -> + let allNames = Set.fromList . Map.elems $ manifestTargetNames graph in maybe ProjectWithoutTargets FoundTargets (NonEmptySet.nonEmpty (Set.map BuildTarget allNames)) +-- | The name each manifest in the graph is offered under as a build target: +-- its @name@ field, except for the workspace root, which may instead borrow +-- its directory name ('workspaceRootTargetName'). +-- +-- Everything that turns selected build targets back into manifests must read +-- from this map. Matching on @packageName@ alone would never find a root that +-- took the fallback, and so would silently drop its dependencies whenever +-- every target is selected, which is the default. +manifestTargetNames :: PkgJsonGraph -> Map Manifest Text +manifestTargetNames graph@PkgJsonGraph{jsonLookup} = Map.mapMaybeWithKey targetName jsonLookup + where + root = either (const Nothing) Just (findWorkspaceRootManifest graph) + + targetName :: Manifest -> PackageJson -> Maybe Text + targetName manifest pj = + if Just manifest == root + then workspaceRootTargetName graph + else packageName pj + -- | The build target name for a workspace root: the @name@ field of its -- package.json, or the root directory's own basename when it declares none. -- @@ -352,7 +374,7 @@ resolvePnpmImporterKeys = resolveWorkspacePathKeys "." -- for the workspace root. resolveWorkspacePathKeys :: Text -> FoundTargets -> PkgJsonGraph -> Maybe (Set Text) resolveWorkspacePathKeys _ ProjectWithoutTargets _ = Nothing -resolveWorkspacePathKeys rootKey (FoundTargets targets) graph@PkgJsonGraph{..} = +resolveWorkspacePathKeys rootKey (FoundTargets targets) graph = case findWorkspaceRootManifest graph of Left _ -> Nothing Right rootManifest -> @@ -364,17 +386,8 @@ resolveWorkspacePathKeys rootKey (FoundTargets targets) graph@PkgJsonGraph{..} = namePathPairs :: [(Text, Text)] namePathPairs = mapMaybe - (\(manifest, pj) -> (,) <$> manifestTargetName manifest pj <*> manifestToWorkspacePath (unManifest manifest)) - (Map.toList jsonLookup) - - -- Must agree with 'findWorkspaceBuildTargets' on what each manifest's - -- target is called, including the root's basename fallback; a target - -- offered by list-targets but unresolvable here would silently select - -- nothing. - manifestTargetName :: Manifest -> PackageJson -> Maybe Text - manifestTargetName manifest pj - | manifest == rootManifest = workspaceRootTargetName graph - | otherwise = packageName pj + (\(manifest, name) -> (name,) <$> manifestToWorkspacePath (unManifest manifest)) + (Map.toList $ manifestTargetNames graph) manifestToWorkspacePath :: Path Abs File -> Maybe Text manifestToWorkspacePath m = @@ -400,20 +413,22 @@ extractDepLists PkgJsonGraph{..} = foldMap extractSingle $ Map.elems jsonLookup -- | Like 'extractDepLists', but scoped to the selected workspace targets. -- When 'ProjectWithoutTargets', includes all deps. --- When 'FoundTargets', only includes deps from packages whose --- package name matches a selected target (root or workspace member). +-- When 'FoundTargets', only includes deps from manifests whose target name +-- (see 'manifestTargetNames') matches a selected target (root or workspace +-- member). extractDepListsForTargets :: FoundTargets -> PkgJsonGraph -> FlatDeps extractDepListsForTargets ProjectWithoutTargets graph = extractDepLists graph -extractDepListsForTargets (FoundTargets targets) PkgJsonGraph{..} = +extractDepListsForTargets (FoundTargets targets) graph@PkgJsonGraph{..} = foldMap extractSingle selectedPackageJsons where targetNames :: Set Text targetNames = Set.map unBuildTarget (NonEmptySet.toSet targets) + selectedManifests :: Set Manifest + selectedManifests = Map.keysSet . Map.filter (`Set.member` targetNames) $ manifestTargetNames graph + selectedPackageJsons :: [PackageJson] - selectedPackageJsons = - filter (maybe False (`Set.member` targetNames) . packageName) $ - Map.elems jsonLookup + selectedPackageJsons = Map.elems $ Map.restrictKeys jsonLookup selectedManifests mapToSet :: Map Text Text -> Set NodePackage mapToSet = Set.fromList . map (uncurry NodePackage) . Map.toList diff --git a/test/Node/NodeSpec.hs b/test/Node/NodeSpec.hs index 77c769115c..ec340d8b83 100644 --- a/test/Node/NodeSpec.hs +++ b/test/Node/NodeSpec.hs @@ -340,8 +340,8 @@ resolvePnpmImporterKeysSpec currDir = describe "resolvePnpmImporterKeys" $ do -- | A pnpm workspace root usually keeps its configuration in -- pnpm-workspace.yaml, so its package.json commonly has no @name@. Such a root --- must still yield build targets, and the name it is given must be the one the --- importer-key resolution understands. +-- must still yield build targets, and the name it is given must be the one +-- that importer-key resolution and manifest selection understand. unnamedWorkspaceRootSpec :: Path Abs Dir -> Spec unnamedWorkspaceRootSpec currDir = describe "workspace root without a name" $ do let graph = unnamedRootWorkspaceGraph currDir @@ -359,6 +359,16 @@ unnamedWorkspaceRootSpec currDir = describe "workspace root without a name" $ do graph `shouldBe` Just (Set.fromList ["."]) + it "keeps the root's dependencies when every target is selected" $ + -- With no target filter, analysis receives every target. That is what + -- yarn and npm v1 lockfile analysis see, so the root must survive it. + extractDepListsForTargets (findWorkspaceBuildTargets graph) graph + `shouldBe` extractDepListsForTargets ProjectWithoutTargets graph + + it "selects the root's dependencies by its fallback target" $ + directDeps (extractDepListsForTargets (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList [BuildTarget "workspace-test"]) graph) + `shouldBe` applyTag @Production (Set.fromList [NodePackage "husky" "^8.0.0"]) + -- | 'workspaceGraphWithDeps' with the root's @name@ field removed. unnamedRootWorkspaceGraph :: Path Abs Dir -> PkgJsonGraph unnamedRootWorkspaceGraph currDir = From 3e854762926ff6a5967d7b8d3acd89cd2ffe529b Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 12:34:17 -0700 Subject: [PATCH 05/10] Require a named workspace root for build targets, and say so Drop the directory-name fallback for a workspace root with no `name`. The name it produced varied with where the repository was checked out (`project` on CircleCI, `app` in a Docker build), so a committed .fossa.yml target or a CI --exclude-target naming the root could not be trusted to mean the same thing everywhere. pnpm now follows the rule yarn and npm have had since #1643: an unnamed root yields no build targets, and the whole workspace is analyzed as one unit. Because that is easy to mistake for a bug, discovery warns when a workspace has members but its root has no name, and says what to add. This supersedes 52a8772f: with no fallback, matching manifests by package name is correct again, so that indirection is removed. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0122XfZKg7Yidpc1HEEvBSJD --- Changelog.md | 1 - .../languages/nodejs/npm-lockfile.md | 4 +- .../strategies/languages/nodejs/pnpm.md | 7 +- .../strategies/languages/nodejs/yarn.md | 4 +- .../Analysis/PnpmWorkspaceSpec.hs | 10 +- src/Strategy/Node.hs | 117 +++++++----------- src/Strategy/Node/Errors.hs | 18 ++- test/Node/NodeSpec.hs | 35 ++---- .../testdata/pnpm-workspaces/package.json | 1 + 9 files changed, 85 insertions(+), 112 deletions(-) diff --git a/Changelog.md b/Changelog.md index 0d9e4972cc..bd100ac0d8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,7 +3,6 @@ ## Unreleased - Pnpm: workspace members are now individual build targets, so a single member can be analyzed on its own with `--only-target 'pnpm@./:my-package'` or a `target:` entry in `.fossa.yml`. Previously every member's dependencies were merged into one result with no way to scope them. Dependencies a selected member reaches through the workspace protocol (`link:` in the lockfile) are included. With no target filter the result is unchanged. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) -- Node: a workspace root whose `package.json` has no `name` field no longer suppresses build targets for the whole workspace; the root directory's name is used for the root target. This most often affected pnpm, whose workspace configuration lives in `pnpm-workspace.yaml`. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) - Node: dependencies whose version is a workspace reference (`catalog:`, `workspace:`, `link:`) are no longer reported at that literal string as their version, which produced locators like `npm+left-pad$catalog:` for packages that do not exist. They are skipped with a warning when analysis falls back to a `package.json`-only strategy that cannot resolve them. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) - Diagnostics: When an error or warning group contains multiple errors, each error's `Traceback:` header is now printed on its own line instead of being glued onto the last line of the preceding error message (e.g. `...none passed validationTraceback:`). ([#1758](https://github.com/fossas/fossa-cli/pull/1758)) diff --git a/docs/references/strategies/languages/nodejs/npm-lockfile.md b/docs/references/strategies/languages/nodejs/npm-lockfile.md index 9b4c3dd5ac..56a40f42a1 100644 --- a/docs/references/strategies/languages/nodejs/npm-lockfile.md +++ b/docs/references/strategies/languages/nodejs/npm-lockfile.md @@ -33,8 +33,8 @@ npm@./:web When a subset of targets is selected, only those packages' dependencies are included in the analysis. -If the workspace root's `package.json` has no `name` field, the root directory's -own name is used for the root target. +The workspace root's `package.json` needs a `name` field too: without one, no +build targets are offered and the whole workspace is analyzed as a single unit. When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. diff --git a/docs/references/strategies/languages/nodejs/pnpm.md b/docs/references/strategies/languages/nodejs/pnpm.md index e9b0823f0a..ec7f157bc8 100644 --- a/docs/references/strategies/languages/nodejs/pnpm.md +++ b/docs/references/strategies/languages/nodejs/pnpm.md @@ -174,10 +174,9 @@ pnpm@./:server A target's name is that package's `name` from its `package.json`, so a member named `@acme/browser` is selected as `pnpm@./:@acme/browser`. Run -`fossa list-targets` to see the exact names. If the workspace root's -`package.json` has no `name` field — common for pnpm, since the workspace -configuration lives in `pnpm-workspace.yaml` — the root directory's own name is -used for the root target. +`fossa list-targets` to see the exact names. The workspace root needs a `name` +too: without one, no build targets are offered and the whole workspace is +analyzed as a single unit. Selecting a subset reports only those members' dependencies: diff --git a/docs/references/strategies/languages/nodejs/yarn.md b/docs/references/strategies/languages/nodejs/yarn.md index 6e262cf588..d661d38a4c 100644 --- a/docs/references/strategies/languages/nodejs/yarn.md +++ b/docs/references/strategies/languages/nodejs/yarn.md @@ -55,8 +55,8 @@ yarn@./:lib-core When a subset of targets is selected, only those packages' dependencies are included in the analysis. -If the workspace root's `package.json` has no `name` field, the root directory's -own name is used for the root target. +The workspace root's `package.json` needs a `name` field too: without one, no +build targets are offered and the whole workspace is analyzed as a single unit. When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. diff --git a/integration-test/Analysis/PnpmWorkspaceSpec.hs b/integration-test/Analysis/PnpmWorkspaceSpec.hs index 974d2a5726..17f566c5c8 100644 --- a/integration-test/Analysis/PnpmWorkspaceSpec.hs +++ b/integration-test/Analysis/PnpmWorkspaceSpec.hs @@ -33,13 +33,11 @@ import Strategy.Node qualified as Node fixtureDir :: Path Rel Dir fixtureDir = $(mkRelDir "test/Node/testdata/pnpm-workspaces/") --- | The fixture's root package.json has no @name@ field, which is typical of a --- pnpm workspace root since the workspace configuration lives in --- pnpm-workspace.yaml. Its target name is therefore the root directory's own --- basename. +-- | Every package in the fixture, the root included, is named in its +-- package.json. A workspace root without a @name@ yields no build targets. allTargetNames :: [Text] allTargetNames = - [ "pnpm-workspaces" + [ "@fossa-test/workspace" , "@fossa-test/browser" , "@fossa-test/server" , "@fossa-test/shared" @@ -73,7 +71,7 @@ analyzeFixture = do withResult analyzed $ \_ depResults -> pure (dependencyGraph depResults) FixtureGraphs (projectBuildTargets project) <$> analyzeWith (projectBuildTargets project) - <*> analyzeWith (mkTargets ["pnpm-workspaces"]) + <*> analyzeWith (mkTargets ["@fossa-test/workspace"]) <*> analyzeWith (mkTargets ["@fossa-test/browser"]) <*> analyzeWith (mkTargets ["@fossa-test/server"]) <*> analyzeWith (mkTargets ["@fossa-test/shared"]) diff --git a/src/Strategy/Node.hs b/src/Strategy/Node.hs index 9c63b1f09b..a7b6f4e5fe 100644 --- a/src/Strategy/Node.hs +++ b/src/Strategy/Node.hs @@ -11,14 +11,12 @@ module Strategy.Node ( extractDepListsForTargets, resolveNpmV3WorkspacePaths, resolvePnpmImporterKeys, - workspaceRootTargetName, ) where import Algebra.Graph.AdjacencyMap qualified as AM import Algebra.Graph.AdjacencyMap.Extra qualified as AME import App.Fossa.Analyze.LicenseAnalyze (LicenseAnalyzeProject, licenseAnalyzeProject) import App.Fossa.Analyze.Types (AnalyzeProject (analyzeProject, analyzeProjectStaticOnly)) -import Control.Applicative ((<|>)) import Control.Carrier.Diagnostics (errDoc) import Control.Effect.Diagnostics ( Diagnostics, @@ -30,16 +28,17 @@ import Control.Effect.Diagnostics ( fromEitherShow, fromMaybe, recover, + warn, warnOnErr, ) import Control.Effect.Reader (Reader) -import Control.Monad (void, (<=<)) +import Control.Monad (void, when, (<=<)) import Data.Glob (Glob) import Data.Glob qualified as Glob import Data.List.Extra (singleton) import Data.Map (Map, toList) import Data.Map.Strict qualified as Map -import Data.Maybe (catMaybes, isJust, mapMaybe) +import Data.Maybe (catMaybes, isJust, isNothing, mapMaybe) import Data.Set (Set) import Data.Set qualified as Set import Data.Set.NonEmpty qualified as NonEmptySet @@ -75,7 +74,6 @@ import Path ( File, Path, Rel, - dirname, mkRelFile, parent, stripProperPrefix, @@ -83,7 +81,7 @@ import Path ( (), ) import Strategy.Node.Bun.BunLock qualified as BunLock -import Strategy.Node.Errors (CyclicPackageJson (CyclicPackageJson), MissingNodeLockFile (..), fossaNodeDocUrl, npmLockFileDocUrl, yarnLockfileDocUrl, yarnV2LockfileDocUrl) +import Strategy.Node.Errors (CyclicPackageJson (CyclicPackageJson), MissingNodeLockFile (..), UnnamedWorkspaceRoot (UnnamedWorkspaceRoot), fossaNodeDocUrl, npmLockFileDocUrl, yarnLockfileDocUrl, yarnV2LockfileDocUrl) import Strategy.Node.Npm.PackageLock qualified as PackageLock import Strategy.Node.Npm.PackageLockV3 qualified as PackageLockV3 import Strategy.Node.PackageJson ( @@ -164,12 +162,18 @@ mkProject project = do -- Only expose build targets for project types whose getDeps actually -- honors them. Otherwise users see per-package targets in list-targets -- but filtering has no effect on analysis. - projectBuildTargets' = case project of - Yarn _ _ -> findWorkspaceBuildTargets graph - NPMLock _ _ -> findWorkspaceBuildTargets graph - Pnpm _ _ -> findWorkspaceBuildTargets graph - _ -> ProjectWithoutTargets + honorsTargets = case project of + Yarn _ _ -> True + NPMLock _ _ -> True + Pnpm _ _ -> True + _ -> False + projectBuildTargets' = if honorsTargets then findWorkspaceBuildTargets graph else ProjectWithoutTargets Manifest rootManifest <- fromEitherShow $ findWorkspaceRootManifest graph + -- A workspace whose root has no name gets no targets at all (see + -- 'findWorkspaceBuildTargets'), which looks like a bug from the outside: + -- list-targets shows only the project. Say why, and what fixes it. + when (honorsTargets && hasUnnamedWorkspaceRoot graph) $ + warn (UnnamedWorkspaceRoot rootManifest) pure $ DiscoveredProject { projectType = typename @@ -179,63 +183,38 @@ mkProject project = do } -- | Build targets from workspace package names (root + members). --- If the workspace graph has children (i.e., workspace members), every name --- in 'manifestTargetNames' becomes a 'BuildTarget'. If there are no workspace --- children (single-package project), or the root has no name to offer, --- returns 'ProjectWithoutTargets'. +-- If the workspace graph has children (i.e., workspace members), each +-- package name becomes a 'BuildTarget', along with the root's own name. +-- If there are no workspace children (single-package project), or the root +-- declares no @name@, returns 'ProjectWithoutTargets'. findWorkspaceBuildTargets :: PkgJsonGraph -> FoundTargets findWorkspaceBuildTargets graph = let WorkspacePackageNames childNames = findWorkspaceNames graph in if Set.null childNames then ProjectWithoutTargets - else case workspaceRootTargetName graph of - -- With no target naming the root there would be no way to select - -- the root's own dependencies, so offer no targets rather than lose - -- them. + else case workspaceRootName graph of + -- Everything that resolves selected targets back to manifests + -- matches on the package name, so a nameless root could never be + -- selected and its dependencies would be dropped by any selection, + -- including the default of every target. Offer no targets instead; + -- 'mkProject' warns so the user knows why. Nothing -> ProjectWithoutTargets - Just _ -> - let allNames = Set.fromList . Map.elems $ manifestTargetNames graph + Just n -> + let allNames = Set.insert n childNames in maybe ProjectWithoutTargets FoundTargets (NonEmptySet.nonEmpty (Set.map BuildTarget allNames)) --- | The name each manifest in the graph is offered under as a build target: --- its @name@ field, except for the workspace root, which may instead borrow --- its directory name ('workspaceRootTargetName'). --- --- Everything that turns selected build targets back into manifests must read --- from this map. Matching on @packageName@ alone would never find a root that --- took the fallback, and so would silently drop its dependencies whenever --- every target is selected, which is the default. -manifestTargetNames :: PkgJsonGraph -> Map Manifest Text -manifestTargetNames graph@PkgJsonGraph{jsonLookup} = Map.mapMaybeWithKey targetName jsonLookup - where - root = either (const Nothing) Just (findWorkspaceRootManifest graph) +-- | The @name@ of the workspace root's package.json, if it declares one. +workspaceRootName :: PkgJsonGraph -> Maybe Text +workspaceRootName graph@PkgJsonGraph{jsonLookup} = do + root <- either (const Nothing) Just $ findWorkspaceRootManifest graph + packageName =<< Map.lookup root jsonLookup - targetName :: Manifest -> PackageJson -> Maybe Text - targetName manifest pj = - if Just manifest == root - then workspaceRootTargetName graph - else packageName pj - --- | The build target name for a workspace root: the @name@ field of its --- package.json, or the root directory's own basename when it declares none. --- --- The fallback matters most for pnpm, which keeps its workspace configuration --- in pnpm-workspace.yaml rather than in package.json, so a pnpm workspace root --- is frequently a bare @{ "private": true }@ with no name at all. Without the --- fallback such a root produced no targets, and because 'findWorkspaceBuildTargets' --- is all-or-nothing that withheld targets from every workspace member as well. --- --- 'Nothing' means the graph has no single root manifest, or the root sits at a --- filesystem root with no basename to borrow. -workspaceRootTargetName :: PkgJsonGraph -> Maybe Text -workspaceRootTargetName graph@PkgJsonGraph{jsonLookup} = do - manifest@(Manifest rootManifest) <- either (const Nothing) Just $ findWorkspaceRootManifest graph - (packageName =<< Map.lookup manifest jsonLookup) <|> rootDirName rootManifest - where - rootDirName :: Path Abs File -> Maybe Text - rootDirName m = - let name = toText . FP.dropTrailingPathSeparator . toFilePath . dirname $ parent m - in if name `elem` ["", ".", "/"] then Nothing else Just name +-- | True when the graph has workspace members but its root declares no +-- @name@: the one shape of workspace that yields no build targets. +hasUnnamedWorkspaceRoot :: PkgJsonGraph -> Bool +hasUnnamedWorkspaceRoot graph = + let WorkspacePackageNames childNames = findWorkspaceNames graph + in not (Set.null childNames) && isNothing (workspaceRootName graph) instance AnalyzeProject NodeProject where analyzeProject = getDeps @@ -374,7 +353,7 @@ resolvePnpmImporterKeys = resolveWorkspacePathKeys "." -- for the workspace root. resolveWorkspacePathKeys :: Text -> FoundTargets -> PkgJsonGraph -> Maybe (Set Text) resolveWorkspacePathKeys _ ProjectWithoutTargets _ = Nothing -resolveWorkspacePathKeys rootKey (FoundTargets targets) graph = +resolveWorkspacePathKeys rootKey (FoundTargets targets) graph@PkgJsonGraph{..} = case findWorkspaceRootManifest graph of Left _ -> Nothing Right rootManifest -> @@ -386,8 +365,8 @@ resolveWorkspacePathKeys rootKey (FoundTargets targets) graph = namePathPairs :: [(Text, Text)] namePathPairs = mapMaybe - (\(manifest, name) -> (name,) <$> manifestToWorkspacePath (unManifest manifest)) - (Map.toList $ manifestTargetNames graph) + (\(manifest, pj) -> (,) <$> packageName pj <*> manifestToWorkspacePath (unManifest manifest)) + (Map.toList jsonLookup) manifestToWorkspacePath :: Path Abs File -> Maybe Text manifestToWorkspacePath m = @@ -413,22 +392,20 @@ extractDepLists PkgJsonGraph{..} = foldMap extractSingle $ Map.elems jsonLookup -- | Like 'extractDepLists', but scoped to the selected workspace targets. -- When 'ProjectWithoutTargets', includes all deps. --- When 'FoundTargets', only includes deps from manifests whose target name --- (see 'manifestTargetNames') matches a selected target (root or workspace --- member). +-- When 'FoundTargets', only includes deps from packages whose +-- package name matches a selected target (root or workspace member). extractDepListsForTargets :: FoundTargets -> PkgJsonGraph -> FlatDeps extractDepListsForTargets ProjectWithoutTargets graph = extractDepLists graph -extractDepListsForTargets (FoundTargets targets) graph@PkgJsonGraph{..} = +extractDepListsForTargets (FoundTargets targets) PkgJsonGraph{..} = foldMap extractSingle selectedPackageJsons where targetNames :: Set Text targetNames = Set.map unBuildTarget (NonEmptySet.toSet targets) - selectedManifests :: Set Manifest - selectedManifests = Map.keysSet . Map.filter (`Set.member` targetNames) $ manifestTargetNames graph - selectedPackageJsons :: [PackageJson] - selectedPackageJsons = Map.elems $ Map.restrictKeys jsonLookup selectedManifests + selectedPackageJsons = + filter (maybe False (`Set.member` targetNames) . packageName) $ + Map.elems jsonLookup mapToSet :: Map Text Text -> Set NodePackage mapToSet = Set.fromList . map (uncurry NodePackage) . Map.toList diff --git a/src/Strategy/Node/Errors.hs b/src/Strategy/Node/Errors.hs index bedf3951d0..6f4656f129 100644 --- a/src/Strategy/Node/Errors.hs +++ b/src/Strategy/Node/Errors.hs @@ -1,6 +1,7 @@ module Strategy.Node.Errors ( MissingNodeLockFile (..), CyclicPackageJson (..), + UnnamedWorkspaceRoot (..), fossaNodeDocUrl, npmLockFileDocUrl, yarnLockfileDocUrl, @@ -12,7 +13,8 @@ import Data.Text (Text) import Diag.Diagnostic (ToDiagnostic, renderDiagnostic) import Effect.Logger (renderIt) import Errata (Errata (..)) -import Prettyprinter (indent, vsep) +import Path (Abs, File, Path, toFilePath) +import Prettyprinter (indent, pretty, vsep) yarnLockfileDocUrl :: Text yarnLockfileDocUrl = "https://classic.yarnpkg.com/lang/en/docs/yarn-lock/" @@ -32,6 +34,20 @@ instance ToDiagnostic CyclicPackageJson where let header = "Detected cyclic references between package.json files in the workspace" Errata (Just header) [] Nothing +-- | A workspace with members whose root package.json declares no @name@, so +-- no build targets can be offered for it. +newtype UnnamedWorkspaceRoot = UnnamedWorkspaceRoot (Path Abs File) + +instance ToDiagnostic UnnamedWorkspaceRoot where + renderDiagnostic (UnnamedWorkspaceRoot rootManifest) = do + let header = + renderIt $ + vsep + [ "The workspace root " <> pretty (toFilePath rootManifest) <> " has no `name` field, so its members are not offered as build targets and the whole workspace is analyzed as one unit." + , indent 2 "Add a `name` to that package.json to select members individually with `--only-target` or `targets.only` in .fossa.yml." + ] + Errata (Just header) [] Nothing + data MissingNodeLockFile = MissingNodeLockFileCtx | MissingNodeLockFileHelp diff --git a/test/Node/NodeSpec.hs b/test/Node/NodeSpec.hs index ec340d8b83..21bf93d23e 100644 --- a/test/Node/NodeSpec.hs +++ b/test/Node/NodeSpec.hs @@ -14,7 +14,7 @@ import DepTypes (DepEnvironment (EnvProduction), Dependency (dependencyEnvironme import Graphing qualified import Path (Abs, Dir, Path, mkRelDir, mkRelFile, ()) import Path.IO (getCurrentDir) -import Strategy.Node (NodeProject (NPMLock), discover, extractDepListsForTargets, findWorkspaceBuildTargets, getDeps, pkgGraph, resolveNpmV3WorkspacePaths, resolvePnpmImporterKeys, workspaceRootTargetName) +import Strategy.Node (NodeProject (NPMLock), discover, extractDepListsForTargets, findWorkspaceBuildTargets, getDeps, pkgGraph, resolveNpmV3WorkspacePaths, resolvePnpmImporterKeys) import Strategy.Node.PackageJson ( FlatDeps (..), Manifest (..), @@ -338,36 +338,19 @@ resolvePnpmImporterKeysSpec currDir = describe "resolvePnpmImporterKeys" $ do it "resolves no importers when no target matches a manifest" $ forTargets ["does-not-exist"] `shouldBe` Just Set.empty --- | A pnpm workspace root usually keeps its configuration in --- pnpm-workspace.yaml, so its package.json commonly has no @name@. Such a root --- must still yield build targets, and the name it is given must be the one --- that importer-key resolution and manifest selection understand. +-- | A workspace root with no @name@ cannot be selected by any target filter, +-- so no targets are offered at all and the whole workspace is analyzed as one. +-- That is what keeps the root's own dependencies from being dropped. unnamedWorkspaceRootSpec :: Path Abs Dir -> Spec unnamedWorkspaceRootSpec currDir = describe "workspace root without a name" $ do let graph = unnamedRootWorkspaceGraph currDir - it "names the root target after the root directory" $ - workspaceRootTargetName graph `shouldBe` Just "workspace-test" - - it "still exposes the root and every member as build targets" $ - findWorkspaceBuildTargets graph - `shouldBe` (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList (map BuildTarget ["workspace-test", "pkg-a", "pkg-b"])) - - it "resolves the fallback root target back to the root importer" $ - resolvePnpmImporterKeys - (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList [BuildTarget "workspace-test"]) - graph - `shouldBe` Just (Set.fromList ["."]) - - it "keeps the root's dependencies when every target is selected" $ - -- With no target filter, analysis receives every target. That is what - -- yarn and npm v1 lockfile analysis see, so the root must survive it. - extractDepListsForTargets (findWorkspaceBuildTargets graph) graph - `shouldBe` extractDepListsForTargets ProjectWithoutTargets graph + it "offers no build targets" $ + findWorkspaceBuildTargets graph `shouldBe` ProjectWithoutTargets - it "selects the root's dependencies by its fallback target" $ - directDeps (extractDepListsForTargets (maybe ProjectWithoutTargets FoundTargets . nonEmpty $ Set.fromList [BuildTarget "workspace-test"]) graph) - `shouldBe` applyTag @Production (Set.fromList [NodePackage "husky" "^8.0.0"]) + it "still analyzes every manifest, root included" $ + directDeps (extractDepListsForTargets (findWorkspaceBuildTargets graph) graph) + `shouldBe` applyTag @Production (Set.fromList [NodePackage "husky" "^8.0.0", NodePackage "lodash" "^4.0.0", NodePackage "express" "^4.0.0"]) -- | 'workspaceGraphWithDeps' with the root's @name@ field removed. unnamedRootWorkspaceGraph :: Path Abs Dir -> PkgJsonGraph diff --git a/test/Node/testdata/pnpm-workspaces/package.json b/test/Node/testdata/pnpm-workspaces/package.json index 394e5c4fb8..2725ad8ea1 100644 --- a/test/Node/testdata/pnpm-workspaces/package.json +++ b/test/Node/testdata/pnpm-workspaces/package.json @@ -1,4 +1,5 @@ { + "name": "@fossa-test/workspace", "version": "1.0.0", "private": true, "devDependencies": { From 3bedca28d892acb72aa65f01523d84d378cf3a43 Mon Sep 17 00:00:00 2001 From: spatten Date: Wed, 2 Sep 2026 12:39:10 -0700 Subject: [PATCH 06/10] Document the root-name requirement, the warning, and the list-targets form The three strategy pages now say what an unnamed workspace root looks like in practice (a bare type@./ target, one analysis unit, a warning that names the fix) instead of only stating the rule. list-targets.md shows the type@path:target form that workspace projects produce, which fossa-yml.md referred to but the subcommand page never illustrated. The changelog gains a line for the warning, since it is user-visible for yarn and npm workspaces that were previously silent. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0122XfZKg7Yidpc1HEEvBSJD --- Changelog.md | 1 + .../strategies/languages/nodejs/npm-lockfile.md | 6 ++++-- docs/references/strategies/languages/nodejs/pnpm.md | 10 +++++++--- docs/references/strategies/languages/nodejs/yarn.md | 6 ++++-- docs/references/subcommands/list-targets.md | 11 +++++++++++ 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/Changelog.md b/Changelog.md index bd100ac0d8..6cd304d9ea 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ ## Unreleased - Pnpm: workspace members are now individual build targets, so a single member can be analyzed on its own with `--only-target 'pnpm@./:my-package'` or a `target:` entry in `.fossa.yml`. Previously every member's dependencies were merged into one result with no way to scope them. Dependencies a selected member reaches through the workspace protocol (`link:` in the lockfile) are included. With no target filter the result is unchanged. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) +- Node: when a yarn, npm, or pnpm workspace root has no `name` in its `package.json`, discovery now warns that no build targets can be offered for the workspace and that adding a `name` enables them. Previously the targets were withheld silently. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) - Node: dependencies whose version is a workspace reference (`catalog:`, `workspace:`, `link:`) are no longer reported at that literal string as their version, which produced locators like `npm+left-pad$catalog:` for packages that do not exist. They are skipped with a warning when analysis falls back to a `package.json`-only strategy that cannot resolve them. ([#1763](https://github.com/fossas/fossa-cli/pull/1763)) - Diagnostics: When an error or warning group contains multiple errors, each error's `Traceback:` header is now printed on its own line instead of being glued onto the last line of the preceding error message (e.g. `...none passed validationTraceback:`). ([#1758](https://github.com/fossas/fossa-cli/pull/1758)) diff --git a/docs/references/strategies/languages/nodejs/npm-lockfile.md b/docs/references/strategies/languages/nodejs/npm-lockfile.md index 56a40f42a1..dc5a454977 100644 --- a/docs/references/strategies/languages/nodejs/npm-lockfile.md +++ b/docs/references/strategies/languages/nodejs/npm-lockfile.md @@ -33,8 +33,10 @@ npm@./:web When a subset of targets is selected, only those packages' dependencies are included in the analysis. -The workspace root's `package.json` needs a `name` field too: without one, no -build targets are offered and the whole workspace is analyzed as a single unit. +The workspace root's `package.json` needs a `name` field too. Without one, no +build targets are offered: `fossa list-targets` shows a bare `npm@./`, the +whole workspace is analyzed as a single unit, and a warning explains that adding +a `name` to the root `package.json` enables per-package targets. When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. diff --git a/docs/references/strategies/languages/nodejs/pnpm.md b/docs/references/strategies/languages/nodejs/pnpm.md index ec7f157bc8..0e2924ea05 100644 --- a/docs/references/strategies/languages/nodejs/pnpm.md +++ b/docs/references/strategies/languages/nodejs/pnpm.md @@ -174,9 +174,13 @@ pnpm@./:server A target's name is that package's `name` from its `package.json`, so a member named `@acme/browser` is selected as `pnpm@./:@acme/browser`. Run -`fossa list-targets` to see the exact names. The workspace root needs a `name` -too: without one, no build targets are offered and the whole workspace is -analyzed as a single unit. +`fossa list-targets` to see the exact names. + +The workspace root needs a `name` too. Without one, no build targets are +offered: `fossa list-targets` shows a bare `pnpm@./`, the whole workspace is +analyzed as a single unit, and a warning explains that adding a `name` to the +root `package.json` enables per-member targets. A root that is `private: true` +can carry any name; it is never published. Selecting a subset reports only those members' dependencies: diff --git a/docs/references/strategies/languages/nodejs/yarn.md b/docs/references/strategies/languages/nodejs/yarn.md index d661d38a4c..eace8a777a 100644 --- a/docs/references/strategies/languages/nodejs/yarn.md +++ b/docs/references/strategies/languages/nodejs/yarn.md @@ -55,8 +55,10 @@ yarn@./:lib-core When a subset of targets is selected, only those packages' dependencies are included in the analysis. -The workspace root's `package.json` needs a `name` field too: without one, no -build targets are offered and the whole workspace is analyzed as a single unit. +The workspace root's `package.json` needs a `name` field too. Without one, no +build targets are offered: `fossa list-targets` shows a bare `yarn@./`, the +whole workspace is analyzed as a single unit, and a warning explains that adding +a `name` to the root `package.json` enables per-package targets. When no filtering is applied, all targets are selected and all dependencies from every workspace package are included in the analysis. diff --git a/docs/references/subcommands/list-targets.md b/docs/references/subcommands/list-targets.md index ef807f517e..7346aabbce 100644 --- a/docs/references/subcommands/list-targets.md +++ b/docs/references/subcommands/list-targets.md @@ -16,6 +16,17 @@ $ fossa list-targets This output tells us that when `fossa analyze` is run, we will be analyzing `cabal`, `cocoapods`, `pipenv`, and `yarn` projects. This can be useful to determine if there are targets you expect or don't expect to see. +A project that is divided into named build targets, such as a yarn, npm, or pnpm workspace, is listed once per target in the form `type@path:target`: + +```bash +$ fossa list-targets +[ INFO] Found target: yarn@./:my-monorepo +[ INFO] Found target: yarn@./:app +[ INFO] Found target: yarn@./:lib-core +``` + +Each of those can be selected on its own with `fossa analyze --only-target 'yarn@./:app'` or a `target:` entry under `targets` in `.fossa.yml`; see [analysis target configuration](../files/fossa-yml.md#analysis-target-configuration). + #### Command output formats The list-targets command supports the following formats (via `fossa list-targets --format=