Skip to content

refactor: extract shared codegen helpers into internal/genhelper and dedup cpp/csharp/go loaders#167

Merged
Kybxd merged 4 commits into
masterfrom
refactor/shared-genhelper
Jun 17, 2026
Merged

refactor: extract shared codegen helpers into internal/genhelper and dedup cpp/csharp/go loaders#167
Kybxd merged 4 commits into
masterfrom
refactor/shared-genhelper

Conversation

@Kybxd

@Kybxd Kybxd commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Each protoc-gen-*-tableau-loader plugin previously carried its own near-identical
copies of the generated-file header logic and the map-key handling logic. This PR
consolidates those into a new cross-language package, internal/genhelper, and
rewires the C++ / C# / Go loaders to reuse it, removing a large amount of
duplicated code.

Net change: +234 / −365 across 15 files (≈130 lines of duplication removed).

Changes

New shared package: internal/genhelper

  • header.goProtocVersion() (extracts the protoc compiler version from
    the codegen request) and GenerateSourcePath() (writes the // source: /
    deprecation comment).
  • mapkey.go — shared MapKey / MapKeySlice and AddMapKey() with
    cross-level name/field-name deduplication. Go / C++ / C# alias it directly
    (type MapKey = genhelper.MapKey); the TS loader embeds it to add
    language-specific fields, so every loader shares the same base.

Loader dedup

  • cpp / csharp / go: replaced per-loader duplicated header & map-key helpers
    with the shared genhelper package (helper/helper.go slimmed down in all three).
  • csharp: dropped unused gen / messagerName params from
    genMessage / genMapGetters / generateFileContent (pure pass-through,
    never used in the C# code path).
  • removed the obsolete cmd/protoc-gen-go-tableau-loader/helper.go.

Shared internal helpers

  • internal/options: extracted GetWorksheetOptions() / IsWorksheet() and
    reused them across NeedGenOrderedMap / NeedGenIndex / NeedGenOrderedIndex /
    NeedGenFile; added the LangTS constant.
  • minor consistency updates in internal/index and internal/xproto.

Tests

  • test(csharp): aligned shared fixtures (HubFixture / LoadTests) with the
    refactored helpers.

Notes

  • Generated output is byte-for-byte unchanged — this is a pure source-level
    refactor with no behavioral changes.
  • The TypeScript loader is not included here; it lives on the ts-loader
    branch and reuses the same internal/genhelper. This branch is intentionally
    scoped to the non-TS (cpp/csharp/go + shared internal) changes so it can be
    reviewed and merged independently.

Verification

  • go build ./...
  • go vet ./cmd/... ./internal/... ✅ (no warnings)

Kybxd added 2 commits June 16, 2026 16:32
…dedup cpp/csharp/go loaders

- add internal/genhelper with shared file-header (ProtocVersion/header) and map-key (MapKey/MapKeySlice) helpers

- replace per-loader duplicated helper code in cpp/csharp/go by reusing the shared genhelper package

- internal/options: extract GetWorksheetOptions/IsWorksheet shared helpers

- csharp: drop unused gen/messagerName params from genMessage/genMapGetters/generateFileContent

- remove obsolete cmd/protoc-gen-go-tableau-loader/helper.go

- test(csharp): align shared fixtures (HubFixture/LoadTests)
Comment thread .github/workflows/testing-cpp.yml Outdated
Fd protoreflect.FieldDescriptor // the map field descriptor this key belongs to
}
// MapKey aliases the cross-language shared key descriptor; see genhelper.MapKey.
type MapKey = genhelper.MapKey

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct embeding is more appropriate (auto reuse methods).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapKey has no method.

@Kybxd Kybxd requested a review from wenchy June 17, 2026 09:03
…oss loaders

Define a single generic genhelper.MapKeySlice[F ParamFormatter] carrying all
shared methods (AddMapKey/GenGetParams/GenGetArguments/GenCustom/
GenOtherArguments) as members. Each loader only provides a zero-size
ParamFormatter and aliases the slice, removing duplicated per-language
definitions of MapKeySlice and its methods across the cpp/csharp/go loaders.
@Kybxd Kybxd merged commit ba8ba13 into master Jun 17, 2026
7 checks passed
@Kybxd Kybxd deleted the refactor/shared-genhelper branch June 17, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants