From d10cd9450e785f59f8cec4ea2c345e5e0666bf69 Mon Sep 17 00:00:00 2001 From: Panos Vekris Date: Sat, 13 Apr 2024 15:35:52 -0700 Subject: [PATCH] [flow] generalize matching-prop check to complex expressions 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 --- src/analysis/env_builder/eq_test.ml | 6 -- src/analysis/env_builder/eq_test.mli | 4 - src/analysis/env_builder/refinement_key.ml | 14 ++-- src/typing/statement.ml | 6 +- .../literal_subtype_checking.exp | 74 ++++++++++++++++++- tests/literal_subtype_checking/test.js | 18 +++++ 6 files changed, 98 insertions(+), 24 deletions(-) diff --git a/src/analysis/env_builder/eq_test.ml b/src/analysis/env_builder/eq_test.ml index ddda86a733d..f2176a91d3a 100644 --- a/src/analysis/env_builder/eq_test.ml +++ b/src/analysis/env_builder/eq_test.ml @@ -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 -> @@ -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 diff --git a/src/analysis/env_builder/eq_test.mli b/src/analysis/env_builder/eq_test.mli index 592bc646fe2..82599d80720 100644 --- a/src/analysis/env_builder/eq_test.mli +++ b/src/analysis/env_builder/eq_test.mli @@ -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 -> diff --git a/src/analysis/env_builder/refinement_key.ml b/src/analysis/env_builder/refinement_key.ml index ac666d517d8..80ec30bc63e 100644 --- a/src/analysis/env_builder/refinement_key.ml +++ b/src/analysis/env_builder/refinement_key.ml @@ -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 @@ -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. *) diff --git a/src/typing/statement.ml b/src/typing/statement.ml index 7b8d1b51878..03844aa7a3f 100644 --- a/src/typing/statement.ml +++ b/src/typing/statement.ml @@ -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; _ }) @@ -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 *) diff --git a/tests/literal_subtype_checking/literal_subtype_checking.exp b/tests/literal_subtype_checking/literal_subtype_checking.exp index 2dd31d24a64..45067d5dc32 100644 --- a/tests/literal_subtype_checking/literal_subtype_checking.exp +++ b/tests/literal_subtype_checking/literal_subtype_checking.exp @@ -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 diff --git a/tests/literal_subtype_checking/test.js b/tests/literal_subtype_checking/test.js index 47ec3e38ac9..73b998e3988 100644 --- a/tests/literal_subtype_checking/test.js +++ b/tests/literal_subtype_checking/test.js @@ -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 +}