Skip to content

refactor(fetch): harden the workspace-root descriptor before a second ecosystem uses it #94

Description

@justin13888

#83 (PR #90) replaced the ManifestKind::CargoToml gate in workspace_root_of with a per-kind descriptor:

pub struct WorkspaceRoots {
    pub root_names: &'static [&'static str],
    pub root_kind: ManifestKind,
    pub self_governing: bool,
}

Cargo is the only descriptor today, and it satisfies every assumption below by coincidence — root_kind == kind, and root_names has one entry. Review of #90 found four latent traps that only bite the second ecosystem to use this shape. None is reachable now; all are cheap to close while nothing depends on them.

1. self_governing silently assumes root_kind == kind

discover.rs recognizes a self-governing root with kind.declares_workspace(content) but recognizes a walked root with roots.root_kind.declares_workspace(content).

With self_governing: true and root_kind != kind, workspace_root_of accepts the manifest as its own root using kind's parser, then workspace_declarations(kind, self_content) parses that same text with root_kind's parser, gets Err, and falls back to an empty Vec. Every inherited entry in that root is then silently unresolved and reported as source: "inherited", constraint: null — a wrong answer that looks like a missing declaration.

Close it by stating the invariant on self_governing and using roots.root_kind.declares_workspace in both places, or with a debug_assert!.

2. One root_kind cannot cover a heterogeneous root_names list

Gradle version catalogs fit the current shape, so this does not block #82. The case that does not fit is already half-present in this codebase: a PackageJson member with "react": "catalog:" can root at pnpm-workspace.yaml, or — for Bun, whose bun.lock we already read — at the workspace root's own package.json.

Declaring root_names: ["pnpm-workspace.yaml", "package.json"] forces one kind over both: pick PackageJson and declares_workspace runs the JSON parser over YAML, returns false, and the pnpm root is silently walked past; pick PnpmWorkspaceYaml and a Bun root package.json is parsed as YAML.

Pair each name with its kind instead: root_names: &'static [(&'static str, ManifestKind)].

Also worth correcting while there: the field is documented as "file names", but gradle/libs.versions.toml is a relative path, and dir.join() already relies on that.

3. workspace_cache is keyed on the root path alone

check.rs memoizes declarations per root path. That is correct only while root path → root_kind is a function, which finding 2's shape would let someone violate by accident. The resulting failure is order-dependent — whichever member kind was checked first populates the cache — and therefore intermittent.

Key on (root, roots.root_kind).

4. workspace_declarations takes the member's kind, contradicting its own doc

The doc says the root is parsed as its own kind rather than as kind, but the parameter is the member kind, with the root kind re-derived inside. A public caller holding a root path and its text cannot call it without knowing which member kind produced it. Take the root's ManifestKind directly and have the callers pass roots.root_kind.

Note

Doing 2 first makes 1, 3, and 4 mostly fall out, since the parser for a given root name stops being ambiguous.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    coredependable-core (pure types/parsers/semver)workspaceWorkspace / monorepo support

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions