ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.32k stars 2.51k forks source link

style guide: make enum and union fields snake_case the same as struct fields #2101

Open andrewrk opened 5 years ago

andrewrk commented 5 years ago

Currently we have this problem with union(enum):

https://github.com/ziglang/zig/blob/aff7b38838628a18f384c1f625d71c085c8eee1f/std/zig/ast.zig#L110-L130

Now, if you use Error.InvalidToken are you referring to the type, or the enum value? It's ambiguous. This causes switch expressions to resort to awkward uses of @TagType:

https://github.com/ziglang/zig/blob/aff7b38838628a18f384c1f625d71c085c8eee1f/std/zig/ast.zig#L133-L141

However if we had a different style guide, the enum value would be Error.invalid_token and the type would be Error.InvalidToken.

This is less of a problem now that #683 is mostly solved, but there is still the ambiguity.

This sort of conflicts with #995.

Akuli commented 5 years ago

How about UPPER_CASE for enum fields? Many other languages do that too.

andrewrk commented 5 years ago

I'll note that @thejoshwolfe even before this proposal already broke from the style guide to do this: https://github.com/thejoshwolfe/legend-of-swarkland/blob/ace37864bf4754851bfe0c5256077171c2db695f/src-core/protocol.zig#L8-L28

I'll also note that the enums in @import("builtin") adhere to this, but that's more for consistency with the LLVM names.

thejoshwolfe commented 5 years ago

for unions specifically, the options are very similar to fields, so matching the style of field names makes more sense than matching the style of type names. For enums, the options are most similar to global constants, and i believe the style there is snake case as well. For union enums, we have to choose one style for both.

In no way does it make sense for the options to match the style of a type name.

andrewrk commented 4 years ago

After months of silent deliberation and sleeping on it, this is now accepted.

I'm in no hurry to break everybody's code, but this is the new style for the standard library.

Exceptions:

hryx commented 4 years ago

Will this style be applied to errors too? (Tangentially related: #2647)

How about UPPER_CASE for enum fields?

@Akuli I think that would be the right thing to do if the compiler style guide already had that e.g. for constants, but as of now it doesn't use that style for naming anywhere.

andrewrk commented 4 years ago

Will this style be applied to errors too? (Tangentially related: #2647)

No, I think errors can remain as is.

SebastianKeller commented 4 years ago

One thing to consider is the null keyword.

The following example wont compile:

pub const Value = union(enum) {
    null: void,
    bool: bool,
    int: i64,
    uint: u64,
    float: f64,
    string: []const u8,
    object: std.StringHashMap(Value),
};
./types.zig:174:5: error: expected token '}', found 'null'
    null: void,
    ^

If i escape null it works just fine. But then again, the access is quite odd.

pub const Value = union(enum) {
    @"null": void,
    bool: bool,
    int: i64,
    uint: u64,
    float: f64,
    string: []const u8,
    object: std.StringHashMap(Value),
};

This issue applies to std.json.Value.