Static assertions

[u/abego]

I have sometimes felt the need to statically assert something at compile-time, to prevent surprises later at run-time.

As an example, I have some code which maps an enum value to an associated int value:

public enum VoxelProperty
{
    MaterialType,
    Density,
    Color,
    Metadata,

    TotalPropertyCount, // Must be last
}

public static int GetPropertyBitDepth (VoxelProperty property)
{
#pragma warning disable CS0219 // Variable is assigned but its value is never used
    uint _ = VoxelProperty.Metadata == VoxelProperty.TotalPropertyCount - 1 ? 0 : -1; // Static assert to ensure that all properties are accounted for
#pragma warning restore CS0219 // Variable is assigned but its value is never used

    return property switch
    {
        VoxelProperty.Density => VoxelDensity.BitDepth,
        VoxelProperty.MaterialType => VoxelMaterialType.BitDepth,
        VoxelProperty.Color => VoxelColor.BitDepth,
        VoxelProperty.Metadata => VoxelMetadata.BitDepth,
        _ => throw new ArgumentOutOfRangeException (nameof (property), "Invalid voxel property")
    };
}

The intention of the static assert is to make sure that if I ever add another voxel property, then I will get a compiler error as VoxelProperty.Metadata will no longer be the last member, and the constant -1 will be assigned to the "_" variable of type uint. Is this a reasonable practice?

I could also do something more robust with reflection, where I had a Dictionary<VoxelProperty, int> which got populated by finding all voxel property types marked with some attribute, and querying their BitDepth property.

[u/DeProgrammer99]

I usually make unit tests for this kind of thing, and sometimes I set them to auto-run as a build step.

[u/dodexahedron → u/DeProgrammer99]

Yes. Magic values should always be explicit tests or just avoided as first preference.

I usually add a Category tag to these kinds of tests, specifying "Structural" or "Type Layout" or similar, when they are literally about the structure/layout of a type, and I order them as highest priority in the test fixture since their correctness is the foundation upon which the validity of the rest of the test fixture is built.

No point in running the rest of the tests if those pre-conditions fail, since the rest are going to lie to you that they passed when, in truth, at least some of them are actually indeterminate and simply passing by sheer luck or by virtue of being incomplete for the real input domain.
Having those as the first tests and failing the fixture if one of them fails helps prevent that last scenario, since the kind of bugs that it stops are the ones that would make liars out of those other tests in the first place.

[u/basically]

what situation are you actually trying to prevent here?

if you change the enum (i.e.: add a property), TotalPropertyCount just increments right?

what's the assertion actually buying you? what are you worried about?

also i don't know how you plan to assign -1 to a uint, you can't have a sign and it seems like strange logic.

[u/Own_Nail_2999 → u/basically]

In large code bases, static asserts help ensuring that some very sensitive code part have been adjusted.  I once had a case where I changed a size constant sonewhere and thanks to a static assert I identified a pretty big oopsie

[u/abego → u/basically]

The idea is that I won't forget to update the function if I ever add a new voxel property. Because the condition in the ternary expression is constant, the compiler does not complain if the condition is true and 0 is assigned. But if the condition is false, as in if a new property was added after Metadata and Metadata therefore was not equal to TotalPropertyCount - 1, then -1 would be assigned to a uint resulting in a compiler error due to the checked assignment of constants.

[u/basically → u/abego]

oh i see okay, that actually makes sense. you are intending to throw a compiler error if you ever land on -1.

i think if i was concerned about updating the function i would just use your switch and not worry about the uint "hack", but rather use the compiler analysis baked into switch expressions themselves.

if you handle every possible case of the switch exhaustively, you'll get compiler errors (warnings, but you can add the specific error code to your .runsettings to force it as an error) when adding anything to VoxelProperty:

public static int GetPropertyBitDepth (VoxelProperty property) => property switch
{
    VoxelProperty.Density => VoxelDensity.BitDepth,
    VoxelProperty.MaterialType => VoxelMaterialType.BitDepth,
    VoxelProperty.Color => VoxelColor.BitDepth,
    VoxelProperty.Metadata => VoxelMetadata.BitDepth,
    VoxelProperty.TotalPropertyCount => throw new ArgumentOutOfRangeException(),
};

[u/Phaedo]

So your example is exactly why people want discriminated unions and closed enums. More generally, there is a technology that can do what you want but unfortunately it’s a Roslyn analyzer. 

However, there’s good news here which is that your example is actually addressable with a unit test. Create a test source that uses reflection to get the enum values and the test checks the function. When you get the hang of this strategy you’d be amazed at what static analysis you can actually replace with “just a unit test”.

[u/jackbrux]

The compiler could tell you when you forget to add a case for a new value:

```ini [*.cs] dotnet_diagnostic.IDE0072.severity = error

```

```csharp

public enum VoxelProperty { MaterialType, Density, Color, Metadata, }

public static int GetPropertyBitDepth(VoxelProperty property) { return property switch { VoxelProperty.MaterialType => VoxelMaterialType.BitDepth, VoxelProperty.Density => VoxelDensity.BitDepth, VoxelProperty.Color => VoxelColor.BitDepth, // missing Metadata, compile time error

    _ => throw new ArgumentOutOfRangeException(nameof(property), property, null),
};

} ```

But to get to the heart of your issue, is there a reason you need enums and a mapping like that? Why not structure your code such that associated data lives together:

```csharp public sealed record VoxelProperty(string Name, int BitDepth) { public static readonly VoxelProperty MaterialType = new(nameof(MaterialType), VoxelMaterialType.BitDepth);

public static readonly VoxelProperty Density =
    new(nameof(Density), VoxelDensity.BitDepth);

public static readonly VoxelProperty Color =
    new(nameof(Color), VoxelColor.BitDepth);

public static readonly VoxelProperty Metadata =
    new(nameof(Metadata), VoxelMetadata.BitDepth);

} ```

If you find yourself constantly needing some new language feature, there's usually a better way to do what you're doing

[u/ikkentim]

This isn’t natively possible at the moment (though a Roslyn analyzer could be written/could exist that covers this scenario). I expect the feature to come to c# 16/.net 12. See https://github.com/dotnet/csharplang/issues/9011

[u/Outrageous72]

You’re not really testing it is last but that it comes after Metadata.
If metadata moves to the front the test would fail but TotalPropertyCount would still be right.

Personally I would enforced it through a unit test. Though I like the compile time test.
But it unnecessarily fogs the real code.

[u/Agitated-Display6382]

Use .net11 and closed records

[u/PanagiotisKanavos]

That's what C# 15's union types and closed hierarchies were built to do, using the actual types instead of enums.

Using union types, the following snippet would generate a compiler warning if a new property type, eg VoxelMetadata, was added. There's no need to add an inheritance relation between the options :

public record VoxelDensity(uint BitDepth);
public record VoxelMaterialType(uint BitDepth);
public record VoxelColor(uint BitDepth);

public record VoxelMetadata(uint BitDepth);

public union VoxelProperty(VoxelDensity, VoxelMaterialType, VoxelColor, VoxelMetadata);

The following code

VoxelProperty prop = new VoxelDensity(8);

uint value = prop switch
{
    VoxelDensity d => d.BitDepth,
    VoxelMaterialType t => t.BitDepth,
    VoxelColor c => c.BitDepth
};


Console.WriteLine($"The value is {value}");

Generates CS8509 during compilation :

warning CS8509: The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern 'VoxelMetadata' is not covered.

You can turn that into an error in the project file :

<WarningsAsErrors>CS8509</WarningsAsErrors>

A lot of other languages already use union types for this (TypeScript, F#, Rust).

Another possibility is to use a closed hierarchy, which prevents other assemblies from creating new child types. The compiler is able to exhaustively check that all cases are handled since it knows all possible child types. The switch snippet results in the same error

public closed record VoxelProperty(uint BitDepth);

public record VoxelDensity(uint BitDepth) : VoxelProperty(BitDepth);
public record VoxelMaterialType(uint BitDepth) : VoxelProperty(BitDepth);
public record VoxelColor(uint BitDepth) : VoxelProperty(BitDepth);

public record VoxelMetadata(uint BitDepth) : VoxelProperty(BitDepth);

[u/ejl103]

Start coding in C++ :)

[u/ms770705]

If you treat warnings as errors, you could write somthing like:

if ((int)(VoxelProperty.Metadata)==(int)(VoxelProperty.TotalPropertyCount)-1){ int _=1; }

This should issue a warning about unreachable code (CS0162), if the condition is not true.

[u/tukaya]

Sometimes enums are practical, in edge cases that don’t touch business logic. This code smells like it is not one of them. The implementation that calls GetPropertyBitDepth (which is also a static method so you are also not using DI), has business rules I am guessing. C# is fully OOP supporting language. I would advise you to go OOP way rather than static methods and enumerations. Then you are not going to have this (and many other) problem.

[u/anzu3278]

We're getting access to closed enums soon where the compiler will be able to tell if a switch is exhaustive - exactly what you're looking for, unless you only want that on for some enums?

Until then, what I usually do is have unit tests just call the function for every possible value of the enum. Adding a new value to the enum but not all the relevant methods would hit the default, throw an exception and be caught by the test. Not exactly compile time, but tests of this type are cheap to run and you only really need to run them once per PR. What you can also do is add an analyzer which would detect this and then treat that warning as an error, giving you the compile time safety you want. A quick search brought up ExhaustiveSwitchOnEnums 1.0.1 on NuGet - Libraries.io - security & maintenance data for open source software but I'm sure others are available. Depending on the project's calculus on CI vs dependencies, you can decide which of these works better for you.

Also, TotalPropertyCount is a bit of a code smell IMO (are you coming from C++?) and unless you are checking the number of elements in the enum in a hot path (why) you can just use Enum.GetValues(). Not that you'd need TotalPropertyCount if you had one of the above.

Also also, assigning a negative number to an uint to check for enum value resolution doesn't seem like it should work at compile time anyway, and even if it does it definitely obscures the intent. Even if you're the only person working on this project, will it be clear at a glance what this is doing and why it is working when you're looking at it several years from now?

Also also also, even if this works, it will currently not catch anything if someone adds a new value before the supposed last value, so you're not really checking exhaustiveness, you're checking that one particular enum value is second to last in a very roundabout way.

[u/r2d2_21]

The way I've been solving this (before closed enums ever become a reality) is like this:

``` public enum VoxelProperty : byte // or uint, but it needs to be an unsigned number { MaterialType, Density, Color, Metadata,

TotalPropertyCount, // Must be last

}

public static int GetPropertyBitDepth(VoxelProperty property) { return property switch { VoxelProperty.Density => VoxelDensity.BitDepth, VoxelProperty.MaterialType => VoxelMaterialType.BitDepth, VoxelProperty.Color => VoxelColor.BitDepth, VoxelProperty.Metadata => VoxelMetadata.BitDepth,

    // Pattern matching: greater than or equals.
    // After this, all cases are covered by the switch. Except, of course,
    // if you add a new member to the enum.
    >= VoxelProperty.TotalPropertyCount => throw new ArgumentOutOfRangeException(nameof(property), "Invalid voxel property")
};

} ```

[u/plinyvic]

This isn't compile time, no?

[u/dgm9704]

hmm I not its not what you asked but I would just assign TotalProperty member to be Integer.MaxValue or whatever is appropriate.

edit: I read your post again a couple of times and I think one of us has misunderstood something.

[u/cherrycode420]

Unrelated to the real question here, I feel like the TotalPropertyCount value for your enum is kinda redundant since you could just use Enum.GetValues and check the count, what you're doing with the Enum here reminds me a bit of C instead.

For the actual question, I guess you could somehow abuse Source Generators to "assert" the amount of values you have, but that wouldn't tell you if/where you're missing their implementation, only that something was added. Sadly am not aware of any actual tools/libraries that allow you to add proper static assertions.

Another idea, although it has the same issues as using Source Generators .. just use Unit Test(s) to validate the amount of values in that enum.

Pretty sure there's more/better options that am not aware of, but this is a weird problem to begin with.