Skip to content

Commit

Permalink
[flow] generalize matching-prop check to complex expressions
Browse files Browse the repository at this point in the history
Summary:
Lifts the restriction that the object part in the matching-prop test needs to be an identifier. This allows us to error in the following cases:
```
type Literal = 'foo';

function complex_expression (
  o: () => {p: Literal},
  m: {p: {q: Literal}},
) {
  if (o().p === 'bar') {} // now errors
  if (m.p.q === 'bar') {} // now errors
}
```
This diff also cleanups after the previous diff.

Changelog: [fix] We now error more reliably in the matching property tests, for example when the object part is a complex expression ([try-Flow](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+IFkolXpUCWewUEAwhCQgRDH8wEH4hMnwROHlsNnw4KHwwSLAAC3wANyo4LFxscWQuHgMNZmwsiRSAWglaY1cq-hIAa2wJXNpG4Vxcdvdu3v7B0RxKUYMhKDBSqmbWwIq3eagoOrKSKgH0wtMMPznY7d2SfcoBiEZ-aG5G3Ix085AF-ZhsRoRehqUEiNMgSQHlSruBZxJrMcJwMhzAC+-EgGiCLWMAAIADJwQHcLEAXixAHIYMRSQBuAA6UDpMAWyjg0CxkBEjFK8nwClq1xZUCxAAo6VisfJkMKAJTEgB8uPxJm4vFFWIgkqFMqJ8uAjEleIJuBRqpEkt1ZoAjvrFTQjcaoDLgKq4DBhfJNcSiSTyZTHUjna6he6tV6yZhKKS-ViAPTRrFQCAAdyxJkoECBAbdHq9JIAjFHY-GkynqOmSHTM0KIJqrIxPd6KRBI1jgP7BViXcLq1La-Ww1Rm62Y3GE8nU2XK93ezmsfmW0jh0Wx6WM-T252hSJa1YLX2fU2-ZWt4wd3vw4OF4XRyW06uxRvj6eZ3Oh1fi+PV0iYiB8iYSAKgnyAAGKwACYAGZwKsICQCRIA))

Reviewed By: SamChou19815

Differential Revision: D56038177

fbshipit-source-id: bc0f7778e20d477473f0d2991ad075e97974efb7
  • Loading branch information
panagosg7 authored and facebook-github-bot committed Apr 13, 2024
1 parent 1a587a0 commit d10cd94
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 24 deletions.
6 changes: 0 additions & 6 deletions src/analysis/env_builder/eq_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,12 @@ let extract_bigint_literal node =
module type S = sig
module Env_api : Env_api.S with module L = Loc_sig.ALocS

module RefinementKey : Refinement_key.REFINEMENT_KEY with module L = Loc_sig.ALocS

val jsx_attributes_possible_sentinel_refinements :
(ALoc.t, ALoc.t) Ast.JSX.Opening.attribute list -> Hint.sentinel_refinement SMap.t

val object_properties_possible_sentinel_refinements :
(ALoc.t, ALoc.t) Ast.Expression.Object.property list -> Hint.sentinel_refinement SMap.t

val refinement_of_expr : ('a, 'b) Ast.Expression.t -> 'b RefinementKey.t_ option

val visit_eq_test :
on_type_of_test:
(ALoc.t ->
Expand Down Expand Up @@ -183,8 +179,6 @@ module Make
| Ast.Expression.Object.SpreadProperty _ -> acc
)

let refinement_of_expr = RefinementKey.of_expression

let visit_eq_test
~on_type_of_test
~on_literal_test
Expand Down
4 changes: 0 additions & 4 deletions src/analysis/env_builder/eq_test.mli
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@
module type S = sig
module Env_api : Env_api.S with module L = Loc_sig.ALocS

module RefinementKey : Refinement_key.REFINEMENT_KEY with module L = Loc_sig.ALocS

val jsx_attributes_possible_sentinel_refinements :
(ALoc.t, ALoc.t) Flow_ast.JSX.Opening.attribute list -> Hint.sentinel_refinement SMap.t

val object_properties_possible_sentinel_refinements :
(ALoc.t, ALoc.t) Flow_ast.Expression.Object.property list -> Hint.sentinel_refinement SMap.t

val refinement_of_expr : ('a, 'b) Flow_ast.Expression.t -> 'b RefinementKey.t_ option

val visit_eq_test :
on_type_of_test:
(ALoc.t ->
Expand Down
14 changes: 5 additions & 9 deletions src/analysis/env_builder/refinement_key.ml
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,16 @@ module type REFINEMENT_KEY = sig
projections: proj list;
}

type 'b t_ = {
loc: 'b;
type t = {
loc: L.t;
lookup: lookup;
}

type t = L.t t_

val debug_string_of_t : t -> string

val of_optional_chain : ('a, L.t) Ast.Expression.t -> t option

val of_expression : ('a, 'b) Ast.Expression.t -> 'b t_ option
val of_expression : ('a, L.t) Ast.Expression.t -> t option

val of_argument : ('a, L.t) Ast.Expression.expression_or_spread -> t option

Expand Down Expand Up @@ -64,13 +62,11 @@ module Make (L : Loc_sig.S) : REFINEMENT_KEY with module L = L = struct
projections: proj list;
}

type 'b t_ = {
loc: 'b;
and t = {
loc: L.t;
lookup: lookup;
}

type t = L.t t_

(* These functions are all either slightly modified or directly copied from src/typing/refinement.ml.
* Eventually this module will replace that one. *)

Expand Down
6 changes: 2 additions & 4 deletions src/typing/statement.ml
Original file line number Diff line number Diff line change
Expand Up @@ -4651,7 +4651,7 @@ module Make
| ( _,
Member
{
Member._object = ((_, obj_t), _) as obj;
Member._object = ((_, obj_t), _);
property =
( Member.PropertyIdentifier (_, { Ast.Identifier.name = pname; _ })
| Member.PropertyExpression (_, StringLiteral { Ast.StringLiteral.value = pname; _ })
Expand All @@ -4660,9 +4660,7 @@ module Make
}
) ->
let ((_, other_t), _) = right in
Base.Option.iter (Eq_test.refinement_of_expr obj) ~f:(fun _ ->
Context.add_matching_props cx (pname, other_t, obj_t)
)
Context.add_matching_props cx (pname, other_t, obj_t)
| _ -> ()

(* traverse a binary expression, return result type *)
Expand Down
74 changes: 73 additions & 1 deletion tests/literal_subtype_checking/literal_subtype_checking.exp
Original file line number Diff line number Diff line change
Expand Up @@ -538,5 +538,77 @@ References:
^^^^^ [1]


Error --------------------------------------------------------------------------------------------------- test.js:109:17

Found 32 errors
Cannot compare string [1] with property `p` of call of `o` [2] because string [1] is incompatible with string literal
`foo` [3]. [incompatible-type]

test.js:109:17
109| if (o().p === 'bar') {} // Error
^^^^^ [1]

References:
test.js:109:7
109| if (o().p === 'bar') {} // Error
^^^ [2]
test.js:101:16
101| o: () => {p: Literal},
^^^^^^^ [3]


Error --------------------------------------------------------------------------------------------------- test.js:110:17

Cannot compare number [1] with property `p` of call of `o` [2] because number [1] is incompatible with string literal
`foo` [3]. [incompatible-type]

test.js:110:17
110| if (o().p === 1) {} // Error
^ [1]

References:
test.js:110:7
110| if (o().p === 1) {} // Error
^^^ [2]
test.js:101:16
101| o: () => {p: Literal},
^^^^^^^ [3]


Error --------------------------------------------------------------------------------------------------- test.js:113:17

Cannot compare string [1] with property `q` of `m.p` [2] because string [1] is incompatible with string literal
`foo` [3]. [incompatible-type]

test.js:113:17
113| if (m.p.q === 'bar') {} // Error
^^^^^ [1]

References:
test.js:113:7
113| if (m.p.q === 'bar') {} // Error
^^^ [2]
test.js:102:14
102| m: {p: {q: Literal}},
^^^^^^^ [3]


Error --------------------------------------------------------------------------------------------------- test.js:114:17

Cannot compare number [1] with property `q` of `m.p` [2] because number [1] is incompatible with string literal
`foo` [3]. [incompatible-type]

test.js:114:17
114| if (m.p.q === 1) {} // Error
^ [1]

References:
test.js:114:7
114| if (m.p.q === 1) {} // Error
^^^ [2]
test.js:102:14
102| m: {p: {q: Literal}},
^^^^^^^ [3]



Found 36 errors
18 changes: 18 additions & 0 deletions tests/literal_subtype_checking/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,21 @@ function bool_literal_subtyping_check(b1: true, b2: false, b3: boolean) {
if (b3 !== true) {} // ok
if (b3 !== false) {} // ok
}

function complex_expression (
x: () => Literal,
o: () => {p: Literal},
m: {p: {q: Literal}},
) {
if (x() === 'foo') {}
if (x() === 'bar') {} // TODO Error
if (x() === 1) {} // TODO Error

if (o().p === 'foo') {}
if (o().p === 'bar') {} // Error
if (o().p === 1) {} // Error

if (m.p.q === 'foo') {}
if (m.p.q === 'bar') {} // Error
if (m.p.q === 1) {} // Error
}

0 comments on commit d10cd94

Please sign in to comment.