buf
buf copied to clipboard
Buf linter should allow unprefixed values in nested enums
It's a common practice to nest an enum within a dummy message to give it an isolated naming space. For example:
message ProcessStatus {
enum Value {
UNKNOWN = 0;
SUCCESS = 1;
FAILURE = 2;
}
}
You can see many examples of this in Google-maintained libraries, like their FHIR library. See here: https://github.com/google/fhir/blob/bc3fe21af307a0a8c4cca00bb24995137ee9a683/proto/google/fhir/proto/stu3/codes.proto#L868-L886
It would be great if the ENUM_VALUE_PREFIX lint rule would allow this (or if there were another rule that allowed this, or there were a configuration option for this). I'm not sure what the heuristic ought to be, but here are a few possibilities:
- Any enum named
Value
that is the only item nested within a message is allowed to omit prefixes- The name could be a configurable option if Value seems too opinionated
- Any enum has an
isolated_namespace
option set totrue
is allowed to omit prefixes - Any enum that is the only item nested within a message is allowed to omit prefixes
- Any enum nested within a message is allowed to omit prefixes
I wish there were a more obvious way to do this, but adding lint suppression every time we do this seems less than ideal.
Thanks for the issue - this is an interesting use case. The options you've suggested seem reasonable overall, but I have a few thoughts:
- Any enum named Value that is the only item nested within a message is allowed to omit prefixes
- The name could be a configurable option if Value seems too opinionated
This feels a little too implicit in practice. Even if we expose configuration for it, we might eventually be asked to support multiple nested enum
values in single message
, so configuring this can get awkward.
- Any enum has an isolated_namespace option set to true is allowed to omit prefixes
Given that there isn't a standard Enum option for isolated_namespace
, we would need to support a custom option for this. For this to work, we would need to compile the option definition into the linter implementation, and so would everyone that wants to use the option, which is not ideal.
- Any enum that is the only item nested within a message is allowed to omit prefixes
- Any enum nested within a message is allowed to omit prefixes
These seem reasonable in general, but we can't change the behavior of ENUM_VALUE_PREFIX
without breaking compatibility. We would need to roll out a new linter that supports this behavior and ENUM_VALUE_PREFIX
would need to be removed via lint.except.
I know that you mentioned suppressing this every time is less than ideal, but have you been using allow_comment_ignores for such use cases?
If you enable this in your buf.yaml
like so:
version: v1
lint:
allow_comment_ignores: true
You could adapt your enum definition like so (I tested this locally):
message ProcessStatus {
// buf:lint:ignore ENUM_VALUE_PREFIX
enum Value {
UNKNOWN = 0;
SUCCESS = 1;
FAILURE = 2;
}
}
Let us know if you think this is sufficient for your use case!
Thanks for prompt response Alex! Requiring a customized lint policy to avoid breaking existing usages is exactly what I would expect, and would be super happy with that option!
Regarding comment ignores, we are currently using them as you suggest, it's just that the uninitiated can see this as a code smell that we're suppressing rather than a very valid pattern. It would be nice to have a solution that clarifies the practice (why I mentioned the option
methodology), or at least doesn't confuse the issue more by making it look like we're following a discouraged pattern.