Skip to content

Commit

Permalink
[react] deprecate $Supertype and $Subtype
Browse files Browse the repository at this point in the history
Summary:
This bans the utility types `$Supertype` and `$Subtype` from Flow. These utilities are unsound and can usually be modeled using shapes or bounded generics.

This also changes the types of the proptypes from `lib/react.js` to `any`, since these relied on `$Subtype`.

Reviewed By: jbrown215

Differential Revision: D13244153

fbshipit-source-id: cd82d603b4ada59b792832ce0bcfbc14bc667914
  • Loading branch information
dsainati1 authored and facebook-github-bot committed Dec 5, 2018
1 parent 5f25cfb commit d935071
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 38 deletions.
6 changes: 3 additions & 3 deletions lib/react.js
Expand Up @@ -89,7 +89,7 @@ declare class React$Component<Props, State = void> {
static displayName?: ?string; static displayName?: ?string;
static childContextTypes: any; static childContextTypes: any;
static contextTypes: any; static contextTypes: any;
static propTypes: $Subtype<{[_: $Keys<Props>]: any}>; static propTypes: any;


// We don't add a type for `defaultProps` so that its type may be entirely // We don't add a type for `defaultProps` so that its type may be entirely
// inferred when we diff the type for `defaultProps` with `Props`. Otherwise // inferred when we diff the type for `defaultProps` with `Props`. Otherwise
Expand Down Expand Up @@ -145,7 +145,7 @@ declare type React$AbstractComponentStatics<DefaultProps> = {
declare type React$StatelessFunctionalComponent<Props> = { declare type React$StatelessFunctionalComponent<Props> = {
(props: Props, context: any): React$Node, (props: Props, context: any): React$Node,
displayName?: ?string, displayName?: ?string,
propTypes?: $Subtype<{[_: $Keys<Props>]: any}>, propTypes?: any,
contextTypes?: any contextTypes?: any
}; };


Expand Down Expand Up @@ -217,7 +217,7 @@ declare module react {
declare export var version: string; declare export var version: string;


declare export function checkPropTypes<V>( declare export function checkPropTypes<V>(
propTypes: $Subtype<{[_: $Keys<V>]: ReactPropsCheckType}>, propTypes : any,
values: V, values: V,
location: string, location: string,
componentName: string, componentName: string,
Expand Down
3 changes: 3 additions & 0 deletions src/common/lints/lints.ml
Expand Up @@ -22,6 +22,7 @@ type lint_kind =
| NonstrictImport | NonstrictImport
| UnclearType | UnclearType
| DeprecatedType | DeprecatedType
| DeprecatedUtility
| UnsafeGettersSetters | UnsafeGettersSetters
| InexactSpread | InexactSpread
| UnnecessaryOptionalChain | UnnecessaryOptionalChain
Expand All @@ -46,6 +47,7 @@ let string_of_kind = function
| NonstrictImport -> "nonstrict-import" | NonstrictImport -> "nonstrict-import"
| UnclearType -> "unclear-type" | UnclearType -> "unclear-type"
| DeprecatedType -> "deprecated-type" | DeprecatedType -> "deprecated-type"
| DeprecatedUtility -> "deprecated-utility"
| UnsafeGettersSetters -> "unsafe-getters-setters" | UnsafeGettersSetters -> "unsafe-getters-setters"
| InexactSpread -> "inexact-spread" | InexactSpread -> "inexact-spread"
| UnnecessaryOptionalChain -> "unnecessary-optional-chain" | UnnecessaryOptionalChain -> "unnecessary-optional-chain"
Expand Down Expand Up @@ -73,6 +75,7 @@ let kinds_of_string = function
| "untyped-import" -> Some [UntypedImport] | "untyped-import" -> Some [UntypedImport]
| "unclear-type" -> Some [UnclearType] | "unclear-type" -> Some [UnclearType]
| "deprecated-type" -> Some [DeprecatedType] | "deprecated-type" -> Some [DeprecatedType]
| "deprecated-utility" -> Some [DeprecatedUtility]
| "unsafe-getters-setters" -> Some [UnsafeGettersSetters] | "unsafe-getters-setters" -> Some [UnsafeGettersSetters]
| "inexact-spread" -> Some [InexactSpread] | "inexact-spread" -> Some [InexactSpread]
| "unnecessary-optional-chain" -> Some [UnnecessaryOptionalChain] | "unnecessary-optional-chain" -> Some [UnnecessaryOptionalChain]
Expand Down
1 change: 1 addition & 0 deletions src/common/lints/lints.mli
Expand Up @@ -22,6 +22,7 @@ type lint_kind =
| NonstrictImport | NonstrictImport
| UnclearType | UnclearType
| DeprecatedType | DeprecatedType
| DeprecatedUtility
| UnsafeGettersSetters | UnsafeGettersSetters
| InexactSpread | InexactSpread
| UnnecessaryOptionalChain | UnnecessaryOptionalChain
Expand Down
2 changes: 2 additions & 0 deletions src/typing/debug_js.ml
Expand Up @@ -2773,6 +2773,8 @@ let dump_flow_error =
spf "ENonstrictImport (%s)" (string_of_aloc loc) spf "ENonstrictImport (%s)" (string_of_aloc loc)
| EUnclearType loc -> | EUnclearType loc ->
spf "EUnclearType (%s)" (string_of_aloc loc) spf "EUnclearType (%s)" (string_of_aloc loc)
| EDeprecatedUtility (loc, name) ->
spf "EDeprecatedUtility (%s, %s)" (string_of_aloc loc) name
| EDeprecatedType loc -> | EDeprecatedType loc ->
spf "EDeprecatedType (%s)" (string_of_aloc loc) spf "EDeprecatedType (%s)" (string_of_aloc loc)
| EUnsafeGettersSetters loc -> | EUnsafeGettersSetters loc ->
Expand Down
14 changes: 11 additions & 3 deletions src/typing/flow_error.ml
Expand Up @@ -152,6 +152,7 @@ type error_message =
| ENonstrictImport of ALoc.t | ENonstrictImport of ALoc.t
| EUnclearType of ALoc.t | EUnclearType of ALoc.t
| EDeprecatedType of ALoc.t | EDeprecatedType of ALoc.t
| EDeprecatedUtility of ALoc.t * string
| EUnsafeGettersSetters of ALoc.t | EUnsafeGettersSetters of ALoc.t
| EUnusedSuppression of ALoc.t | EUnusedSuppression of ALoc.t
| ELintSetting of LintSettings.lint_parse_error | ELintSetting of LintSettings.lint_parse_error
Expand Down Expand Up @@ -384,7 +385,8 @@ let util_use_op_of_msg nope util = function
| EUntypedImport (_, _) | EUntypedImport (_, _)
| ENonstrictImport (_) | ENonstrictImport (_)
| EUnclearType (_) | EUnclearType (_)
| EDeprecatedType (_) | EDeprecatedType _
| EDeprecatedUtility _
| EUnsafeGettersSetters (_) | EUnsafeGettersSetters (_)
| EUnusedSuppression (_) | EUnusedSuppression (_)
| ELintSetting (_) | ELintSetting (_)
Expand Down Expand Up @@ -2072,15 +2074,20 @@ let rec error_of_msg ~trace_reasons ~source_file =
| EUnclearType loc -> | EUnclearType loc ->
mk_error ~trace_infos ~kind:(LintError Lints.UnclearType) loc [ mk_error ~trace_infos ~kind:(LintError Lints.UnclearType) loc [
text "Unclear type. Using "; code "any"; text ", "; text "Unclear type. Using "; code "any"; text ", ";
code "Object"; text ", "; code "Function"; text ", "; code "Object"; text ", or "; code "Function";
code "$Subtype<...>"; text ", or "; code "$Supertype<...>"; text " types is not safe!" text " types is not safe!"
] ]


| EDeprecatedType loc -> | EDeprecatedType loc ->
mk_error ~trace_infos ~kind:(LintError Lints.DeprecatedType) loc [ mk_error ~trace_infos ~kind:(LintError Lints.DeprecatedType) loc [
text "Deprecated type. Using "; code "*"; text " types is not recommended!" text "Deprecated type. Using "; code "*"; text " types is not recommended!"
] ]


| EDeprecatedUtility (loc, name) ->
mk_error ~trace_infos ~kind:(LintError Lints.DeprecatedUtility) loc [
text "Deprecated utility. Using "; code name; text " types is not recommended!"
]

| EUnsafeGettersSetters loc -> | EUnsafeGettersSetters loc ->
mk_error ~trace_infos ~kind:(LintError Lints.UnsafeGettersSetters) loc mk_error ~trace_infos ~kind:(LintError Lints.UnsafeGettersSetters) loc
[text "Getters and setters can have side effects and are unsafe."] [text "Getters and setters can have side effects and are unsafe."]
Expand Down Expand Up @@ -2187,6 +2194,7 @@ let is_lint_error = function
| ENonstrictImport _ | ENonstrictImport _
| EUnclearType _ | EUnclearType _
| EDeprecatedType _ | EDeprecatedType _
| EDeprecatedUtility _
| EUnsafeGettersSetters _ | EUnsafeGettersSetters _
| ESketchyNullLint _ | ESketchyNullLint _
| ESketchyNumberLint _ | ESketchyNumberLint _
Expand Down
5 changes: 3 additions & 2 deletions src/typing/type_annotation.ml
Expand Up @@ -302,9 +302,10 @@ let rec convert cx tparams_map = Ast.Type.(function
targs targs
) )


(* These utilities are no longer supported *)
(* $Supertype<T> acts as any over supertypes of T *) (* $Supertype<T> acts as any over supertypes of T *)
| "$Supertype" -> | "$Supertype" ->
add_unclear_type_error_if_not_lib_file cx loc; FlowError.EDeprecatedUtility (loc, name) |> Flow_js.add_output cx;
check_type_arg_arity cx loc targs 1 (fun () -> check_type_arg_arity cx loc targs 1 (fun () ->
let ts, targs = convert_type_params () in let ts, targs = convert_type_params () in
let t = List.hd ts in let t = List.hd ts in
Expand All @@ -313,7 +314,7 @@ let rec convert cx tparams_map = Ast.Type.(function


(* $Subtype<T> acts as any over subtypes of T *) (* $Subtype<T> acts as any over subtypes of T *)
| "$Subtype" -> | "$Subtype" ->
add_unclear_type_error_if_not_lib_file cx loc; FlowError.EDeprecatedUtility (loc, name) |> Flow_js.add_output cx;
check_type_arg_arity cx loc targs 1 (fun () -> check_type_arg_arity cx loc targs 1 (fun () ->
let ts, targs = convert_type_params () in let ts, targs = convert_type_params () in
let t = List.hd ts in let t = List.hd ts in
Expand Down
15 changes: 3 additions & 12 deletions tests/more_react/more_react.exp
Expand Up @@ -61,7 +61,7 @@ References:
<BUILTINS>/react.js:219:41 <BUILTINS>/react.js:219:41
v--- v---
219| declare export function checkPropTypes<V>( 219| declare export function checkPropTypes<V>(
220| propTypes: $Subtype<{[_: $Keys<V>]: ReactPropsCheckType}>, 220| propTypes : any,
221| values: V, 221| values: V,
222| location: string, 222| location: string,
223| componentName: string, 223| componentName: string,
Expand All @@ -82,7 +82,7 @@ References:
<BUILTINS>/react.js:219:41 <BUILTINS>/react.js:219:41
v--- v---
219| declare export function checkPropTypes<V>( 219| declare export function checkPropTypes<V>(
220| propTypes: $Subtype<{[_: $Keys<V>]: ReactPropsCheckType}>, 220| propTypes : any,
221| values: V, 221| values: V,
222| location: string, 222| location: string,
223| componentName: string, 223| componentName: string,
Expand All @@ -91,15 +91,6 @@ References:
-------^ [1] -------^ [1]




Error ------------------------------------------------------------------------------------------ checkPropTypes.js:10:43

Cannot call `checkPropTypes` with object literal bound to `values` because property `bar` is missing in object
literal [1].

10| checkPropTypes({ bar: PropTypes.string }, { foo: 'foo' }, 'value', 'TestComponent'); // error: property not found
^^^^^^^^^^^^^^ [1]


Error ------------------------------------------------------------------------------------------ checkPropTypes.js:12:85 Error ------------------------------------------------------------------------------------------ checkPropTypes.js:12:85


Cannot call `checkPropTypes` with function bound to `getStack` because number [1] is incompatible with string [2] in the Cannot call `checkPropTypes` with function bound to `getStack` because number [1] is incompatible with string [2] in the
Expand Down Expand Up @@ -139,4 +130,4 @@ References:






Found 9 errors Found 8 errors
3 changes: 3 additions & 0 deletions tests/record/.flowconfig
@@ -1,2 +1,5 @@
[options] [options]
no_flowlib=false no_flowlib=false

[lints]
deprecated-utility=error
10 changes: 9 additions & 1 deletion tests/record/record.exp
Expand Up @@ -47,6 +47,14 @@ References:
^^^^ [1] ^^^^ [1]




Error ----------------------------------------------------------------------------------------------------- test.js:17:6

Deprecated utility. Using `$Subtype` types is not recommended! (`deprecated-utility`)

17| x: $Subtype<{[key: $Keys<X>]: any}>; // object with larger key set than X's
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^


Error ----------------------------------------------------------------------------------------------------- test.js:20:6 Error ----------------------------------------------------------------------------------------------------- test.js:20:6


Property `qux` is missing in object type [1]. Property `qux` is missing in object type [1].
Expand All @@ -62,4 +70,4 @@ References:






Found 4 errors Found 5 errors
3 changes: 1 addition & 2 deletions tests/strict_local_mode/strict_local_mode.exp
Expand Up @@ -23,8 +23,7 @@ References:


Error ---------------------------------------------------------------------------------------------------- test.js:12:10 Error ---------------------------------------------------------------------------------------------------- test.js:12:10


Unclear type. Using `any`, `Object`, `Function`, `$Subtype<...>`, or `$Supertype<...>` types is not safe! Unclear type. Using `any`, `Object`, or `Function` types is not safe! (`unclear-type`)
(`unclear-type`)


12| const x: any = {}; // Error: unclear-type 12| const x: any = {}; // Error: unclear-type
^^^ ^^^
Expand Down
10 changes: 10 additions & 0 deletions tests/supertype_subtype/.flowconfig
@@ -0,0 +1,10 @@
[ignore]

[include]

[libs]

[options]

[lints]
deprecated-utility=error
18 changes: 18 additions & 0 deletions tests/supertype_subtype/supertype_subtype.exp
@@ -0,0 +1,18 @@
Error ------------------------------------------------------------------------------------------------------ test.js:3:9

Deprecated utility. Using `$Supertype` types is not recommended! (`deprecated-utility`)

3| let x : $Supertype<number> = 3;
^^^^^^^^^^^^^^^^^^


Error ------------------------------------------------------------------------------------------------------ test.js:4:9

Deprecated utility. Using `$Subtype` types is not recommended! (`deprecated-utility`)

4| let y : $Subtype<number> = 3;
^^^^^^^^^^^^^^^^



Found 2 errors
4 changes: 4 additions & 0 deletions tests/supertype_subtype/test.js
@@ -0,0 +1,4 @@
// @flow

let x : $Supertype<number> = 3;
let y : $Subtype<number> = 3;

0 comments on commit d935071

Please sign in to comment.