hhvm icon indicating copy to clipboard operation
hhvm copied to clipboard

[ TypeChecker ] Shapes::at() does not generate an error when a field is known to not exist if the given key is a class constant

Open lexidor opened this issue 4 years ago • 0 comments

Describe the bug Indexing into a shape using Shapes::at($shape, SomeClass::SOME_CONSTANT) silently returns nothing instead of emitting a typechecker error.

Standalone code, or other way to reproduce the problem

final class MyClass {
    const string A = 'A';
    const string B = 'B';
}

function shape_with_class_const_fields(): shape(MyClass::A => string) {
    return shape(MyClass::A => 'A');
}

function shape_with_plain_string_fields(): shape('A' => string) {
    return shape('A' => 'A');
}

function indexing_into_shape(): void {
    $_strange_since_myclass_b_is_known_to_not_exist = Shapes::at(shape_with_class_const_fields(), MyClass::B);
    $_even_stranger_since_myclass_is_not_related_to_this_shape = Shapes::at(shape_with_plain_string_fields(), MyClass::B);

    // Typing[4251] You are calling Shapes::at() on a field known to not exist Hack(4251)
    // file.hack(6, 43): The field A is not defined in this shape
    $_expected_this_shape_has_no_plain_string_keys = Shapes::at(shape_with_class_const_fields(), 'A');
    // Typing[4251] You are calling Shapes::at() on a field known to not exist Hack(4251)
    // file.hack(5, 44): The field B is not defined in this shape
    $_expected_this_shape_has_string_a_but_not_b = Shapes::at(shape_with_plain_string_fields(), 'B');
}

Steps to reproduce the behavior:

  1. hh_client

Expected behavior

All these cases should result in an error.

Actual behavior

Indexing with a class constant is not a typechecker error. At runtime this throws.

Copy-paste output, or add a screenshot to illustrate what actually happens. Copy-pasted text output (e.g. from hhvm or hh_client) is preferred to screenshots.

Typing[4251] You are calling Shapes::at() on a field known to not exist [1]
-> The field A is not defined in this shape [2]

file.hack:20:54
     4 | }
     5 | 
[2]  6 | function shape_with_class_const_fields(): shape(MyClass::A => string) {
     7 |     return shape(MyClass::A => 'A');
     8 | }
       :
    18 |     // Typing[4251] You are calling Shapes::at() on a field known to not exist Hack(4251)
    19 |     // file.hack(6, 43): The field A is not defined in this shape
[1] 20 |     $_expected_this_shape_has_no_plain_string_keys = Shapes::at(shape_with_class_const_fields(), 'A');
    21 |     // Typing[4251] You are calling Shapes::at() on a field known to not exist Hack(4251)
    22 |     // file.hack(5, 44): The field B is not defined in this shape

Typing[4251] You are calling Shapes::at() on a field known to not exist [1]
-> The field B is not defined in this shape [2]

file.hack:23:52
     8 | }
     9 | 
[2] 10 | function shape_with_plain_string_fields(): shape('A' => string) {
    11 |     return shape('A' => 'A');
    12 | }
       :
    21 |     // Typing[4251] You are calling Shapes::at() on a field known to not exist Hack(4251)
    22 |     // file.hack(5, 44): The field B is not defined in this shape
[1] 23 |     $_expected_this_shape_has_string_a_but_not_b = Shapes::at(shape_with_plain_string_fields(), 'B');
    24 | }

2 errors found.

Environment

  • Operating system

'Ubuntu 18.04'

  • Installation method

'hhvm/hhvm on dockerhub'

  • HHVM Version
HipHop VM 4.121.0 (rel) (non-lowptr)
Compiler: 1627924236_151424549
Repo schema: 10a34637ad661d98ba3344717656fcc76209c2f8
hackc-d5e69b1378e833da7a831b4ced2e3f28824d6fad-4.121.0

Additional context We use class constant shapes if the field name needs to be changeable in the future. Mostly at API boundaries. These kinds of shapes have a lot of optional fields, which invites the use of Shapes::at(). This makes not having the typechecker correct us here extra difficult to deal with.

lexidor avatar Aug 16 '21 06:08 lexidor