kphp icon indicating copy to clipboard operation
kphp copied to clipboard

Need to fix design of shapes - they must allow to be created with empty array [], must accept [] as default value of parameter, must not require to be wrapped in shape() call when created

Open hrissan opened this issue 4 years ago • 0 comments

Shapes are good, but they are not as useful as they could be, because they force to change code, and changed code to the worse.

An example, we had a function in PHP accepting options, with empty options as default

function withOptions($options = []) {
  $timestamp = $options['timestamp'] ?? 0;
  $exotic = $options['exotic'] ?? false;
  echo 'timestamp='.$timestamp.' exotic='.(int)$exotic.'<br/>';
}

withOptions(['timestamp'=>15]);
withOptions(['exotic'=>true]);
withOptions(['timestamp'=>10, 'exotic'=>true]);
withOptions([]);
withOptions();

Everything was good and terse, except we wanted KPHP compiler to check that only allowed options are passed, and generate much more efficient code using static typing.

So what we wanted is that our code does not change at all, we only wanted annotation that forces static type checks, like this.

/**
 * @param shape(exotic:?bool, timestamp:?int) $options
 */
function withOptions($options = []) {
  $timestamp = $options['timestamp'] ?? 0;
  $exotic = $options['exotic'] ?? false;
  echo 'timestamp='.$timestamp.' exotic='.(int)$exotic.'<br/>';
}

withOptions(['timestamp'=>15]);
withOptions(['exotic'=>true]);
withOptions(['timestamp'=>10, 'exotic'=>true]);
withOptions([]);
withOptions();

Now KPHP compiler has enough information to static check and generate efficient code here.

Instead, with current shapes design, we need to change our code to be

/**
 * @param ?shape(exotic:?bool, timestamp:?int) $options
 */
function withOptions($options = null) {
  $timestamp = $options ? ($options['timestamp'] ?? 0) : 0;
  $exotic = $options ? ($options['exotic'] ?? false) : false;
  echo 'timestamp='.$timestamp.' exotic='.(int)$exotic.'<br/>';
}

withOptions(shape(['timestamp'=>15]));
withOptions(shape(['exotic'=>true]));
withOptions(shape(['timestamp'=>10, 'exotic'=>true]));
withOptions();

This make shapes much less usable than they could be.

hrissan avatar Oct 02 '21 22:10 hrissan