GlassBricks / typed-factorio

Complete and featureful Typescript defintions for the Factorio modding API
MIT License
33 stars 3 forks source link

Request: Provide strong typing of numeric Builtin Types #12

Closed aSemy closed 1 year ago

aSemy commented 2 years ago

At the moment all numeric types are Type Aliases of number. This means that there is no type-checking, so accidentally using the wrong type doesn't raise an compilation error.

function test(
    player: LuaPlayer
) {
  const position: MapEntityPosition = player.position
  const x1: double = position.x // correct
  const x2: uint = position.x // incorrect, x is not a uint

  const playerIndex1: uint = player.index // correct
  const playerIndex2: double = player.index // incorrect, index is not a double
}

Is it possible to make all numeric types type-safe, so const x2: uint = position.x creates a compilation error?

I've done a bit of research and I think a couple of the approaches described here Nominal Typing in Typescript might be the most intuitive and least intrusive, but I don't know if they are technically compatible with TypeScriptToLua.

type BuiltInNumeric<T extends "uint" | "double"> = number & { __precision: T }

type uint2 = BuiltInNumeric<"uint">
type double2 = BuiltInNumeric<"double">

function double2(value: number) { // util function
  return value as double2;
}

function test(
    playerIndex: uint2,
    playerXPos: double2
) {
  const x1: double2 = playerXPos // compiles
  const x2: uint2 = playerXPos // error, Type 'double2' is not assignable to type 'uint2'.

  const playerIndex1: uint2 = playerIndex // compiles
  const playerIndex2: double2 = playerIndex // error, Type 'uint2' is not assignable to type 'double2'.

  const fooX: double2 = 123 as double2
  const barX: double2 = double2(123)

  const bazX: uint2 = double2(123) // error
}
GlassBricks commented 2 years ago

Typescript still just use number to represent all numeric types, regardless of precision, as its all the same representation in the runtime. The typescript nominal typing you described would technically work, however it will likely cause more frustration than benefit: The result of any math operation (like a + b) with "branded" numbers is a plain number, so simple code would be full of casts and hard to use. Another example, uint should be assignable to double, but it doesn't in the example you provided.

Another idea, perhaps, is to "brand" numbers which weren't intended to have arithmetic done to them, like player ids, entity ids, etc. This would run into less of the above problems. However, this would be stricter than many people would like, and could be confusing to newer users of typescript, so I'm hesitant to implement this.

aSemy commented 2 years ago

It would be a very impactful change, so I think it's good to think thoroughly about if it's worth implementing.

Casting

Can you give an example of some code that would be full of casts? I tried it out and from what I can see, casting is only required when assigning a value with a specific numeric type.

This basically makes it 'opt-in', so I don't think it's too intrusive.

function example(myData: MyData) {
  // no type specified, so no casting needed
  const someCount = myData.count
  const result = ((someCount * 100) + 123) / 654
  // now a cast is needed, because `printCount` needs a specific type
  printCount(result as uint2)

  // a cast is needed because 'result2' is explicitly typed
  const result2: uint2 = ((someCount * 100) + 123) / 654 as uint2
  // no casting needed, because `result2` is the correct type
  printCount(result2)
}

interface MyData {
  count: uint2
}

function printCount(count: uint2) {
  print(`count is ${count}`)
}

The biggest problem I see is that as uint implies, but doesn't ensure, the value is an unsigned integer. This post creates a roundToInt(...) function to do this, as well as casting.

Explicit conversions

Personally I'd really, really prefer having explicit casting. Automatic conversion causes problems in Java, and this is fixed in Kotlin, which requires explicitness.

It's explained here: https://kotlinlang.org/docs/basic-types.html#explicit-conversions

But I don't know how applicable my preference is to the world of TypeScript/Lua/C :)

GlassBricks commented 2 years ago

Thanks for your comments. However I still see several major problems with explicit int types in Typescript:

Explicit/implicit conversions aside: something as simple as (a + 1) would require a cast; literally any math operation will require a cast. Additionally, there is no integer division, numeric conversion, specifying the type of a numeric literal, integer overflow, etc.. So, using this would require tons of casts that aren't checked by the compiler, easily leading to wrong code that is type annotated otherwise. This is unlike other statically typed languages that have actual int types represented in the runtime environment. I don't want separately typed integers unless typescript itself supports them.

Some examples:

const pos = ...
const north: uint = (pos.y + 1) as uint // more boilerplate than arithmetic

let counter = 0 as int
let id = (++counter) as int // this is nessecary

const x = 3 as int
const y = (x / 2) as int // no compiler complaints, evaluates to 1.5

Numbers that represent "ids" which are usually not meant to have arithmetic done to them, such as player_number, surface_number, etc., still may benefit from being strongly typed in this way. This probably addresses a good portion of the use cases this issue wished to address. The above can be considered in a separate issue. Unless there are additional comments I'll close this issue for now.

aSemy commented 2 years ago

I've put your examples into the tstl playground.

It doesn't look like casting is required for ++counter?

I also added example util functions. This would help "more boilerplate than arithmetic" and produces a correct result.

const x = 3
const y: int = to_int(x / 2)
print("y = " + y) // y = 2

wrong code that is type annotated otherwise

That describes the current situation too. At worst 'brand typing' would make the problems visible.

To demonstrate, I monkey-patched my local project, it highlights a LOT of mistakes I've made, that weren't previously visible. Here are some of them

schema definitions

I made a mistake in my schema definition for MapTilePosition

  export function mapTilePosition(mapPos: MapPositionTable): MapTilePosition {
    return {
      x: mapPos.x, // TS2322: Type 'double' is not assignable to type 'int'.
      y: mapPos.y, TS2322: Type 'double' is not assignable to type 'int'.
    }
  }

I made a mistake in my schema definition for EntityData

export function entityToTable(entity: LuaEntity): EntityData {

    let player: LuaPlayer | null = entity.is_player() ? entity.player!! : null

    return {
      objectName: entity.object_name,

      name: entity.name,
      type: entity.type,
      active: entity.active,
      health: entity.health ?? null, // TS2322: Type 'float | null' is not assignable to type 'double | null'.
      healthRatio: entity.get_health_ratio(), // TS2322: Type 'float' is not assignable to type 'double'.
      surface: entity.surface.index,
      unitNumber: entity.unit_number ?? null,
      position: mapEntityPosition(entity.position),
      force: entity.force.index,
    }
  }

athematic mistakes

A mistake when converting a map position to a chunk position.

let chunkPosition: ChunkPosition = {
  x: (event.position.x / 32), // TS2322: Type 'number' is not assignable to type 'int'.
  y: (event.position.y / 32), // TS2322: Type 'number' is not assignable to type 'int'.
}

// this should be 
let chunkPosition: ChunkPosition = {
  x: to_int(floor(player.position.x / 32)),
  y: to_int(floor(player.position.y / 32)),
}

And a bug which would have tried to have create an event in the past. I wanted to create an event in the future, and when the trigger is nearer the origin the event should be triggered sooner), but because x + y is an int, this creates events in the past, when the trigger is in some parts of the map.

  SpecialEventHandler.createEvent(key, key.position.y + key.position.y) // TS2322: Type 'int' is not assignable to type 'uint'.

pain points

There were a couple of pain points, like registering nth_tick events

// annoying...
script.on_nth_tick(6, (nthTick: NthTickEventData) => { // TS2345: Argument of type '6' is not assignable to parameter of type 'uint | readonly uint[] | undefined'.
}

But then I don't know what should happen if someone sends a decimal tick

// no warning at the moment... 
script.on_nth_tick(100 * math.random() + 10, (nthTick: NthTickEventData) => {
// (random() is an example, it could also be based on some other double, like an entity position)

Numbers that represent "ids" which are usually not meant to have arithmetic done to them, such as player_number, surface_number, etc., still may benefit from being strongly typed in this way. This probably addresses a good portion of the use cases this issue wished to address.

Yeah, that would help for sure.


I'm thinking out load a bit, but I wonder if instead of specifically locking each number to a specific type, would only marking if a number is unsigned and/or an integer (e.g. uint, int, ...)? (number covers signed && decimal, which is float and double).

It's a little more abstract, but it's also less strict. The scales of numbers aren't as important as the signedness or whether they are decimal (for me at least).

type numeric_signing = "signed" | "unsigned"

type Integer<S extends numeric_signing> =
    number & {__signing: S }

type float = number
type double = number

type int = Integer<"signed">
type int8 = Integer<"signed">

type uint = Integer<"unsigned">
type uint8 = Integer<"unsigned">
type uint16 = Integer<"unsigned">
type uint64 = Integer<"unsigned">
GlassBricks commented 2 years ago

What I meant was that (a + 1), if a is an int, gives number instead of int. even (a + b), which are both ints, yields number.

Support for integral numbers simply doesn't exist in Typescript, and it's not worth trying to patch it in like this.

GlassBricks commented 2 years ago

UPDATE:

Since version 0.20.0, Strict index types have been added, which addresses some cases this issue covers. This is an opt-in feature, see the readme doc here for more details.

Additionally, custom-index-template.d.ts has been added (at the root of this repo). You can use this to create "branded" numeric types in your own project as you see fit without as much monkey-patching. However, I still don't want to add this officially to this project (yet).

Reopening this to be visible to others, and in case strong builtin numeric types are actually implemented in the future.

GlassBricks commented 1 year ago

After further consideration, this will likely not be implemented. Typescript (and lua) has no separate integer type and typescript did not decide to implement integers; it's not worth attempting to work around this.