Skip to content

codegen: fix silently-ignored per-type extern_path mappings (#111)#153

Merged
iainmcgin merged 1 commit into
anthropics:mainfrom
ogarciarevett:fix/extern-path-per-type-fqn
May 27, 2026
Merged

codegen: fix silently-ignored per-type extern_path mappings (#111)#153
iainmcgin merged 1 commit into
anthropics:mainfrom
ogarciarevett:fix/extern-path-per-type-fqn

Conversation

@ogarciarevett
Copy link
Copy Markdown
Contributor

Problem

Closes #111.

extern_path only matched at the proto package-prefix level. A mapping that
names a specific type FQN — e.g. .google.protobuf.Timestamp = ::pbjson_types::Timestamp, a normal prost/tonic idiom — was silently
ignored
: the file's package (google.protobuf) doesn't have
.google.protobuf.Timestamp as a prefix, so the type resolved to a
(non-existent) local module with no diagnostic.

This happened because resolution lived at file registration: CodeGenContext
computed one rust_module per file from resolve_extern_prefix(package, …) and
registered every top-level type under it. prost instead resolves at type
registration against the type's whole FQN (exact-match first, then longest
dotted prefix). Users migrating from prost reach for the per-type form and find
it does nothing.

Solution

Move extern resolution from per-file to per-type, layering prost-style matching
on top of the existing logic (buffa-codegen/src/context.rs):

  • resolve_extern_type(fqn, extern_paths) — exact FQN match first, then the
    longest dotted-prefix entry (a package or an enclosing type). For a prefix
    match the trailing proto segments become snake_case modules and the final
    segment is kept as the Rust type name. For package prefixes this produces
    byte-identical paths to before (guarded by a regression test).
  • resolve_type_path — applies resolution per type with priority: exact
    per-type FQN → file-level mapping (the internal descriptor.proto
    buffa-descriptor split) → package/prefix → local. Returns whether the type
    is extern, so local-module deconfliction (Module redefinition error when messages and packages share a name #135) is unaffected.
  • register_nested_types honors an exact per-type override on a nested FQN,
    and otherwise inherits the parent's resolved module — so a parent override
    cascades to nested types and enums.
  • The __buffa:: view/oneof boundary recovery in rust_type_relative_split
    keeps working because package_of still stores the proto package; it now
    clamps gracefully instead of asserting.
  • Config::extern_path docs describe the per-type form and precedence.

Behavior compatibility

Existing package-level mappings resolve identically to before
(test_extern_path_package_prefix_still_resolves pins this; a no-leak guard
confirms a per-type entry doesn't bleed onto unrelated types). No checked-in
generated code changes — this only affects resolution.

Limitations

An extern type referenced by a generated view must map to another
buffa-generated crate: the view path is composed as
<rust_path_root>::__buffa::view::…, which a non-buffa crate (e.g.
pbjson_types) doesn't provide. For such types, map per-type to a buffa crate
or disable views. This is documented on Config::extern_path. The
diagnostic-warning idea from the issue would make a good follow-up.

Test plan

  • Unit tests for resolve_extern_type: exact, exact-wins-over-prefix,
    package-prefix-appends-type, catch-all, no-match, dot-boundary no-match.
  • CodeGenContext tests: exact per-type, per-type-overrides-package,
    nested cascade, enum override, package-prefix regression guard, no-leak guard.
  • rust_type_relative_split per-type override (top-level + nested) —
    validates the __buffa:: boundary recovery.
  • Integration: cross_package_pertype.proto maps .basic.Person /
    .basic.Status per-type to crate::basic, round-trips end-to-end.
  • cargo fmt / clippy / cargo test --workspace clean; generated code
    (WKT + bootstrap) regenerates with zero drift.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 25, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ogarciarevett
Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request May 25, 2026
@iainmcgin
Copy link
Copy Markdown
Collaborator

[claude code] Thanks for the contribution — the implementation reviewed well: the resolution order (exact FQN → file-level → longest prefix → local) is correct, the package-prefix behaviour is unchanged from before, and the workspace builds and tests cleanly with the branch merged into current main.

I pushed one follow-up commit (6fdc28c) directly to this branch with documentation and CHANGELOG updates so the new behaviour is discoverable outside the builder rustdoc:

  • CHANGELOG.md — added an [Unreleased] / Fixed entry for extern_path only matches package prefixes — per-type FQN mappings are silently ignored #111, including a note that per-type entries which previously had no effect (e.g. typo'd FQNs) now take effect.
  • docs/guide.md — the "External type paths" section now documents per-type mappings with an example, the precedence rules, the nested-type cascade, and the view limitation; the build-method and protoc-opt tables mention the per-type form.
  • docs/migration-from-prost.md — the extern_path row now shows the per-type idiom and the views caveat for non-buffa target crates.
  • Config::extern_path rustdoc (buffa-build) — corrected the relative-rust_path description (it is emitted verbatim, not treated as a local reference), documented how nested types inherit an enclosing override (snake_case of the proto message name), and moved the view constraint under a # Limitations heading along with a note that misconfiguration surfaces at consumer compile time rather than generation time.
  • CodeGenConfig::extern_paths rustdoc (buffa-codegen) and the protoc-gen-buffa help/error text — now mention the type-FQN form alongside the package form.

No changes to your code itself. A couple of smaller items (an emit-time diagnostic when a view-referenced type is mapped to a non-buffa crate, and some extra test coverage for repeated/map/oneof references to overridden types) may get filed as separate follow-up issues rather than expanding this PR.

@iainmcgin iainmcgin enabled auto-merge (squash) May 27, 2026 02:47
…cs#111) (anthropics#153)

extern_path entries naming a single type FQN (the prost/tonic idiom,
e.g. .google.protobuf.Timestamp = ::pbjson_types::Timestamp) parsed but
never matched, because resolution only considered package prefixes at
file registration. Resolution now happens per type: an exact type-FQN
entry wins over the internal descriptor.proto routing, then the longest
matching package prefix, then local generation. Nested types inherit an
enclosing message's override. Package-prefix mappings produce
byte-identical output to before (regression-tested).

Also documents the per-type form across the guide, prost migration
notes, plugin help text, and Config::extern_path rustdoc, and adds the
CHANGELOG entry.

Co-authored-by: Omar Garcia <12629920+ogarciarevett@users.noreply.github.com>
Co-authored-by: Iain McGinniss <309153+iainmcgin@users.noreply.github.com>
@iainmcgin iainmcgin force-pushed the fix/extern-path-per-type-fqn branch from e18edb9 to 06be737 Compare May 27, 2026 03:38
@iainmcgin iainmcgin merged commit 3a8720a into anthropics:main May 27, 2026
7 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators May 27, 2026
@ogarciarevett ogarciarevett deleted the fix/extern-path-per-type-fqn branch May 27, 2026 13:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

extern_path only matches package prefixes — per-type FQN mappings are silently ignored

2 participants