Skip to content

fix: ignore protobuf files without messages when generating code#210

Open
infastin wants to merge 1 commit intoCrowdStrike:mainfrom
infastin:fix/empty-files
Open

fix: ignore protobuf files without messages when generating code#210
infastin wants to merge 1 commit intoCrowdStrike:mainfrom
infastin:fix/empty-files

Conversation

@infastin
Copy link

@infastin infastin commented Feb 28, 2026

fastmarshal at the moment generates empty files if a protobuf file doesn't contain any messages.

So, for this file:

syntax = "proto3";

package nats;

import "google/protobuf/descriptor.proto";

option go_package = "natspb";

extend google.protobuf.FieldOptions {
  int32 subject = 123;
}

fastmarshal generates this:

// GENERATED CODE - DO NOT EDIT
// This file was generated by protoc-gen-fastmarshal

package natspb

import (
	"fmt"
	"sync/atomic"
	"github.com/CrowdStrike/csproto"
)

This PR makes it so fastmarshal first checks if there are any messages in a protobuf file before generating the code. grpc and drpc generators do the same thing.

Copy link
Contributor

@wmorgan6796 wmorgan6796 left a comment

Choose a reason for hiding this comment

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

Overall looks good. There are 2 things:

  1. It feels like there needs to be a test case added somewhere for this (I'm not entirely sure where though) to validate the new behavior.
  2. There is a typo (that was there from the moved function before) that should be fixed.

queue = queue[1:]
msgs = append(msgs, m)
for _, mm := range m.Messages {
// skip "messgaes" that represent map fields
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// skip "messgaes" that represent map fields
// skip "messages" that represent map fields

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