facebook / hhvm

A virtual machine for executing programs written in Hack.
https://hhvm.com
Other
18.21k stars 3k forks source link

Add quickfix for mixed functions that should return void #8980

Closed Wilfred closed 2 years ago

Wilfred commented 2 years ago

Example bad code:

function foo(): mixed {
  echo "hello world\n";
}

The quickfix should set the return type hint to void.

function foo(): void {
  echo "hello world\n";
}
Wilfred commented 2 years ago

The way I usually tackle quickfixes:

(1) Search for the error message and find where the error is emitted. In this example:

https://github.com/facebook/hhvm/blob/202003bda2d984cfb2acaf13f4afd7e0a1cd23ae/hphp/hack/src/errors/typing_error.ml#L1169-L1182

The last list should be a quickfix.

(2) Find the position where I want the position. You can make hh_single_type_check dump the full AAST ('annotated abstract syntax tree') available:

$ hh_single_type_check --nast ~/scratch/x.hack
[(Fun
    { fd_namespace =
      { Namespace_env.ns_ns_uses = <opaque>; ns_class_uses = <opaque>;
        ns_fun_uses = <opaque>; ns_const_uses = <opaque>; ns_name = None;
        ns_auto_ns_map = []; ns_is_codegen = false; ns_disable_xhp_element_mangling = false };
      fd_file_attributes = []; fd_mode = Mstrict;
      fd_fun =
      { f_span = [1:1-3:2]; f_readonly_this = None; f_annotation = ();
        f_readonly_ret = None; f_ret = ((), (Some ([1:27-32], Hmixed)));
        f_name = ([1:10-13], "\\foo"); f_tparams = []; f_where_constraints = [];
        f_variadic = FVnonVariadic;
        f_params =
        [{ param_annotation = (); param_type_hint = ((), (Some ([1:14-19], Hmixed)));
           param_is_variadic = false; param_pos = [1:20-24]; param_name = "$bar";
           param_expr = None; param_readonly = None; param_callconv = Pnormal;
           param_user_attributes = []; param_visibility = None }
          ];
        f_ctxs = None; f_unsafe_ctxs = None;
        f_body = { fb_ast = [([2:3-18], (Expr ((), [2:3-17], (Call (((), [2:3-11], (Id ([2:3-11], "\\var_dump"))), [], [(Pnormal, ((), [2:12-16], (Lvar ([2:12-16], $bar))))], None)))))] };
        f_fun_kind = FSync; f_user_attributes = []; f_external = false;
        f_doc_comment = None }
      })
  ]

(hh_single_type_check also has a --tast option if you want to see type information too.)

For this case, we want the position of the return type hint, which is this:

 f_ret = ((), (Some ([1:27-32], Hmixed)));

This data corresponds to this definition:

https://github.com/facebook/hhvm/blob/202003bda2d984cfb2acaf13f4afd7e0a1cd23ae/hphp/hack/src/annotated_ast/aast.ml#L738-L744

(3) This information is in the AAST, which makes this easier. Some of these tasks (e.g. #8963 and #8962) will require position information that isn't in the AAST.

Update the record to include the position we need:

      | Non_void_annotation_on_return_void_function of {
          pos: Pos.t;
          hint_pos: Pos.t;
          is_async: bool;
        }

(4) Update the original non_void_annotation_on_return_void_function to use this hint_pos to create a quickfix.

Wilfred commented 2 years ago

9000 has been merged.