Skip to content

precision.json cannot express a package-provided storage type, and nothing reads the StorageTypes it generates #236

Description

@matt-edmondson

Background

#232 added Semantics.Quantities.Precise, the fourth storage-type alias package. The plan for it included adding PreciseNumber to precision.json alongside float, double and decimal. That step was deliberately skipped, and this issue records why, so the reasoning is not buried in a merged PR.

Why it was skipped

precision.json feeds PrecisionGenerator, which emits a StorageTypes class into the core ktsu.Semantics.Quantities namespace (Semantics.SourceGenerators/Generators/PrecisionGenerator.cs:31):

Namespace = "ktsu.Semantics.Quantities",

with one field per entry (:53-60):

foreach (string storageType in metadata.StorageTypes.OrderBy(t => t))
{
    storageClass.Members.Add(new FieldTemplate()
    {
        Name = storageType.ToUpperInvariant(),
        DefaultValue = $"typeof({storageType})",
    });
}

Adding PreciseNumber there would emit typeof(PreciseNumber) into the core assembly, which would force Semantics.Quantities to reference ktsu.PreciseNumber — undoing the separation the alias package exists to keep. PreciseNumber support is meant to be opt-in through the satellite alone.

The fully qualified form does not help either: ToUpperInvariant() on ktsu.PreciseNumber.PreciseNumber yields KTSU.PRECISENUMBER.PRECISENUMBER, which is not a valid identifier. The generator assumes the storage type is a C# keyword — which was true for all three entries until now.

The other half

StorageTypes has no reader anywhere in the repository. Grepping outside Generated/, the only mentions of it are PrecisionGenerator and PrecisionMetadata themselves. So the entry would have bought nothing even setting the dependency aside:

// Semantics.Quantities/Generated/…/StorageTypes.g.cs
public static class StorageTypes
{
    public static readonly Type DECIMAL = typeof(decimal);
    public static readonly Type DOUBLE = typeof(double);
    public static readonly Type FLOAT = typeof(float);
    public static readonly IReadOnlyList<Type> All = new List<Type> { DECIMAL, DOUBLE, FLOAT };
    public static readonly IReadOnlyList<string> Names = new List<string> { "decimal", "double", "float" };
}

It is public surface, so it is not free — it is a list that claims to enumerate the library's storage types and is now incomplete, since a fourth one ships as a supported alias package.

The decision to make

It is one of:

  1. Remove precision.json, PrecisionMetadata, PrecisionGenerator and StorageTypes. Nothing reads it; the storage type is a generic parameter, and the set of supported ones is the set of alias packages. This is the tidiest outcome if nobody has a use for the list. It removes public API, so it is a major-version change.
  2. Keep it and make it complete. Requires the generator to accept a fully qualified type name (separate the identifier from the field name rather than deriving one from the other), and requires StorageTypes to move out of the core into somewhere that may depend on ktsu.PreciseNumber — otherwise the dependency problem above stands.
  3. Leave as is, accepting that StorageTypes enumerates three of the four.

Option 3 is the current state and is the one that quietly goes stale.

Suggested next step

Decide whether anything is meant to consume StorageTypes. If yes, option 2 and the list becomes meaningful; if no, option 1 at the next major. Either way precision.json's relationship to the alias packages should be stated in CLAUDE.md, since "add the storage type to precision.json" is the natural assumption when adding the fifth one.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions