[nullsafe] Introduce NullsafeRules and start consolidating checks

Summary:
In nutshell, Nullsafe is driven by relatively simple set of rules.
It is currently not well reflected in code: we are duplicating the same logic in different places, which is:
- error prone (we need to adjust ensure all places are addressed if a new feature is introduced)
- complicates understanding of nullsafe

Consolidating checks will simplify introducing Unknown Nullability and
strict/partial check modes.

## this diff
This diff does it for one particular check.
See follow up diffs re that proceed consolidation.

## future diffs
Future diffs will:
- consolidate other checks that use 'assignment rule'
- introduce other rules, most notably 'dereference rule' and
'inheritance rule'

Reviewed By: artempyanykh

Differential Revision: D17498630

fbshipit-source-id: 079d36518
master
Mitya Lyubarskiy 5 years ago committed by Facebook Github Bot
parent 74c8629d13
commit e24cf9f155

@ -0,0 +1,19 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
let is_nonnull nullsafe_nullability =
match nullsafe_nullability with
| NullsafeType.Nullable _ ->
false
| NullsafeType.Nonnull _ ->
true
let passes_assignment_rule ~lhs ~rhs =
(not (is_nonnull lhs)) || not (InferredNullability.is_nullable rhs)

@ -0,0 +1,24 @@
(*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*)
open! IStd
(** This is a single place consolidating core rules driving Nullsafe type checking.
Nullsafe enforces similar rules in different places (e.g. places dealing with fields,
function calls, assignments, local variables etc.).
Those places might have additional specifics, but core checks should be done through this class.
If you are writing a new or modifying an existing check, ask yourself if you can directly
use already existng rules from this module.
If you feel you need a rule of a completely new nature, add it to this module.
As a rule of thumb, every different "check" that is responsible for detecting issues, should query
this module instead of doing things on their own.
*)
val passes_assignment_rule : lhs:NullsafeType.nullability -> rhs:InferredNullability.t -> bool
(** Assignment rule: No expression of nullable type is ever assigned to a location
of non-nullable type.
*)

@ -326,7 +326,7 @@ let check_call_receiver tenv find_canonical_duplicate curr_pdesc node typestate
type resolved_param =
{ num: int
; formal: Mangled.t * InferredNullability.t * Typ.t
; formal: AnnotatedSignature.param_signature
; actual: Exp.t * InferredNullability.t
; is_formal_propagates_nullable: bool }
@ -334,15 +334,14 @@ type resolved_param =
let check_call_parameters tenv find_canonical_duplicate curr_pdesc node callee_attributes
resolved_params loc instr_ref : unit =
let callee_pname = callee_attributes.ProcAttributes.proc_name in
let check
{num= param_num; formal= s1, nullability_formal, t1; actual= orig_e2, nullability_actual} =
let check {num= param_num; formal; actual= orig_e2, nullability_actual} =
let report () =
let description =
match explain_expr tenv node orig_e2 with
| Some descr ->
descr
| None ->
"formal parameter " ^ Mangled.to_string s1
"formal parameter " ^ Mangled.to_string formal.mangled
in
let origin_descr = InferredNullability.descr_origin nullability_actual in
let callee_loc = callee_attributes.ProcAttributes.loc in
@ -351,10 +350,12 @@ let check_call_parameters tenv find_canonical_duplicate curr_pdesc node callee_a
(description, param_num, callee_pname, callee_loc, origin_descr))
(Some instr_ref) loc curr_pdesc
in
if PatternMatch.type_is_class t1 then
let is_nullable_formal = InferredNullability.is_nullable nullability_formal in
let is_nullable_actual = InferredNullability.is_nullable nullability_actual in
if (not is_nullable_formal) && is_nullable_actual then report ()
if PatternMatch.type_is_class formal.param_nullsafe_type.typ then
if
not
(NullsafeRules.passes_assignment_rule ~lhs:formal.param_nullsafe_type.nullability
~rhs:nullability_actual)
then report ()
in
let should_check_parameters =
match callee_pname with

@ -688,26 +688,20 @@ let typecheck_instr tenv calls_this checks (node : Procdesc.Node.t) idenv curr_p
in
let typestate_after_call, finally_resolved_ret =
let resolve_param i (formal_param, actual_param) =
let AnnotatedSignature.{mangled; param_annotation_deprecated; param_nullsafe_type} =
formal_param
in
let (orig_e2, e2), t2 = actual_param in
let inferred_nullability_formal =
InferredNullability.from_nullsafe_type param_nullsafe_type (TypeOrigin.Formal mangled)
in
let _, inferred_nullability_actual, _ =
typecheck_expr find_canonical_duplicate calls_this checks tenv node instr_ref
curr_pdesc typestate e2
(t2, InferredNullability.create ~is_nullable:false TypeOrigin.ONone, [])
loc
in
let formal = (mangled, inferred_nullability_formal, param_nullsafe_type.typ) in
let actual = (orig_e2, inferred_nullability_actual) in
let num = i + 1 in
let is_formal_propagates_nullable =
Annotations.ia_is_propagates_nullable param_annotation_deprecated
Annotations.ia_is_propagates_nullable
formal_param.AnnotatedSignature.param_annotation_deprecated
in
EradicateChecks.{num; formal; actual; is_formal_propagates_nullable}
EradicateChecks.{num; formal= formal_param; actual; is_formal_propagates_nullable}
in
(* If the function has @PropagatesNullable params, and inferred type for each of
corresponding actual param is non-nullable, inferred type for the whole result

Loading…
Cancel
Save