fix(protoc-gen-openapiv2): fix naming cache for multi-file generation

[franchb]

Summary

Properly fixes #6274 (panic when processing multiple proto files) without causing #6308 (naming inconsistency regression).

This replaces the approach from PR #6275 which was reverted.

Root Cause

The fundamental issue was execution order in applyTemplate():

• renderServices() was called first, using/building the cache for $ref generation
• Cache was then rebuilt with filtered names
• renderMessagesAsDefinition() used the NEW filtered cache for definitions
• Result: $ref and definition names could mismatch

Solution

Move cache initialization BEFORE renderServices():

1. Pre-scan services to collect referenced names WITHOUT using the cache
2. Build filtered names and initialize cache upfront
3. All subsequent lookups use the same consistent cache

Changes

• Add collectReferencedNamesForCache() - scans services/messages without using cache
• Add collectNestedTypeFQNs() - recursively collects nested type FQNs
• Restructure applyTemplate() to initialize cache before renderServices()

Test Plan

• TestGenerateMergeFilesWithBodyAndPathParams - tests protoc-gen-openapiv2 panics when generating OpenAPI for services with messages in separate files (Protobuf Edition 2023) #6274 scenario (no panic)
• TestGenerateMultiFileNamingConsistency - tests $ref and definition name mismatch for google.rpc.Status when processing multiple proto files with nested Status type #6308 scenario ($ref matches definition)
• All existing tests pass

🤖 Generated with Claude Code

[franchb]

Code Review

Overview

This PR fixes a race condition/ordering bug in the OpenAPI v2 generator when processing multiple proto files. The fix restructures applyTemplate() to initialize the naming cache before renderServices() is called, ensuring all subsequent lookups use consistent naming.

Strengths

✅ Root cause fix - Addresses execution order rather than patching symptoms
✅ Clear architecture - Pre-scanning to build cache before usage is sound
✅ Excellent test coverage - Both regression scenarios are tested
✅ Well-documented - Good explanatory comments throughout
✅ Follows conventions - Matches existing code patterns and style

Code Quality Observations

1. Edge case verification (template.go:572-594)
The cycle detection in collectNestedTypeFQNs looks correct - it only recurses into messages since enums don't have children. Good pattern.

2. Error handling consistency (template.go:563-566)

if statusMsg, err := reg.LookupMsg("google.rpc", "Status"); err == nil {
    collectNestedTypeFQNs(statusMsg, reg, refs)
}

The error is silently ignored, which matches the existing pattern at lines 2225-2230. Consider adding a debug log for troubleshooting, but not blocking.

3. Test helper duplication (generator_test.go:2191-2227)
The new requireGenerateWithNamingStrategy helper is nearly identical to requireGenerate. Minor suggestion: could refactor to use optional parameters or a config struct to reduce duplication.

Performance

• Adds one additional O(n) scan pass over services/messages
• For typical protobuf definitions, overhead is negligible
• Actually improves performance by avoiding mid-execution cache rebuilds

Test Coverage

Excellent coverage with realistic scenarios:

• TestGenerateMergeFilesWithBodyAndPathParams - validates #6274 panic fix
• TestGenerateMultiFileNamingConsistency - validates #6308 $ref consistency
• Tests verify correctness, not just "no panic"

Recommendation

✅ Approve - Well-crafted fix that properly addresses the root cause. Ready to merge once CI passes.

---

🤖 Reviewed with Claude Code

[johanbrandhorst]

Thanks for your PR, @codeout-ssol, any chance you could try and build the plugin from this PR to confirm that this is not causing your panic? Thanks!

[franchb]

Note: I have successfully executed the script from the GitHub issue link here and observed no errors during the process.

[johanbrandhorst]

Thanks @franchb, sounds good enough to me!