Skip to content

Commit

Permalink
Don't give quickfixes for object-object casts; only member expressions
Browse files Browse the repository at this point in the history 
Summary:
The "did you mean?" quickfix is meant to apply to member expressions, but erroneously they used to also be given for object subtyping errors where it's less clear how to apply a fix. This diff filters out those situations so that the quickfix is only suggested where intended. It turns off suggesting fixes for `LookupT`s that were spawned from object-object subtyping checks.

Some React-related tests had to be updated because they involve error messages which used to have "did you mean?" suggestions but now don't. We could avoid this by letting some error messages say "did you mean?" while not emitting quickfixes. But upon inspecting these test cases, the changes seem for the better; the "did you mean?" suggestions that were in the error messages seemed backwards, telling the user to change the upper bound their code was being checked against rather than the lower bound coming from the incorrect code.

Reviewed By: dsainati1

Differential Revision: D20609278

fbshipit-source-id: 969021db5ceb613ffb0c2c5f5de6e255cc010841
  • Loading branch information
@facebook-github-bot
Vijay Ramamurthy authored and facebook-github-bot committed Mar 30, 2020
1 parent 91515e7 commit 598b7bc
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 27 deletions.
4 changes: 4 additions & 0 deletions newtests/lsp/code-action/object-cast.js.ignored
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// @flow

type T = {| foo: string |};
({floo: '123'}: T);
58 changes: 58 additions & 0 deletions newtests/lsp/code-action/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,5 +243,63 @@ export default suite(
['textDocument/publishDiagnostics', ...lspIgnoreStatusAndCancellation],
),
]),
test("don't provide quickfixes for object subtyping errors", [
addFile('object-cast.js.ignored', 'object-cast.js'),
lspStartAndConnect(),
lspRequestAndWaitUntilResponse('textDocument/codeAction', {
textDocument: {uri: '<PLACEHOLDER_PROJECT_URL>/object-cast.js'},
range: {
start: {
line: 3,
character: 1,
},
end: {
line: 3,
character: 14,
},
},
context: {
diagnostics: [
{
range: {
start: {
line: 3,
character: 1,
},
end: {
line: 3,
character: 14,
},
},
message:
'Cannot cast object literal to `T` because property `floo` (did you mean `foo`?) is missing in `T` [1] but exists in object literal [2].',
severity: 1,
code: 'InferError',
source: 'Flow',
},
{
range: {
start: {
line: 3,
character: 1,
},
end: {
line: 3,
character: 14,
},
},
message:
'Cannot cast object literal to `T` because property `foo` (did you mean `floo`?) is missing in object literal [1] but exists in `T` [2].',
severity: 1,
code: 'InferError',
source: 'Flow',
},
],
},
}).verifyAllLSPMessagesInStep(
[['textDocument/codeAction', '[]']],
['textDocument/publishDiagnostics', ...lspIgnoreStatusAndCancellation],
),
]),
],
);
12 changes: 3 additions & 9 deletions src/typing/flow_js.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7196,13 +7196,7 @@ struct
let reason_prop = replace_desc_reason (RProperty (Some s)) lreason in
let err =
Error_message.EPropNotFound
{
prop_name = Some s;
reason_prop;
reason_obj = ureason;
use_op;
suggestion = prop_typo_suggestion cx [uflds] s;
}
{ prop_name = Some s; reason_prop; reason_obj = ureason; use_op; suggestion = None }
in
add_output cx ~trace err);
Base.Option.iter lcall ~f:(fun _ ->
Expand Down Expand Up @@ -7317,7 +7311,7 @@ struct
ts = [];
propref;
lookup_action = LookupProp (use_op, up);
ids = Some (Properties.Set.singleton lflds);
ids = None;
} )
| _ ->
(* When an object type is unsealed, typing it as another object type should add properties
Expand Down Expand Up @@ -7345,7 +7339,7 @@ struct
ts = [];
propref;
lookup_action = LookupProp (use_op, up);
ids = Some (Properties.Set.singleton lflds);
ids = None;
} )));

(* Any properties in l but not u must match indexer *)
Expand Down
4 changes: 2 additions & 2 deletions tests/jsx_intrinsics.builtin/jsx_intrinsics.builtin.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ References:

Error ----------------------------------------------------------------------------------------------------- main.js:14:3

Cannot assign `<CustomComponent />` to `c` because property `prop1` (did you mean `prop`?) is missing in object type [1]
but exists in object type [2] in property `props`.
Cannot assign `<CustomComponent />` to `c` because property `prop1` is missing in object type [1] but exists in object
type [2] in property `props`.

main.js:14:3
14| <CustomComponent prop="asdf" />; // Error: Props<{prop}> ~> Props<{prop1}>
Expand Down
7 changes: 3 additions & 4 deletions tests/new_react/new_react.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,7 @@ References:

Error ---------------------------------------------------------------------------------------- object_component.js:17:13

Cannot create `Component` element because property `baz` (did you mean `bar`?) is missing in object type [1] but exists
in props [2].
Cannot create `Component` element because property `baz` is missing in object type [1] but exists in props [2].

object_component.js:17:13
17| const _d = <Component foo={3} bar={3} baz={3} />; // Error, baz is not in the config
Expand Down Expand Up @@ -677,8 +676,8 @@ References:

Error ---------------------------------------------------------------------------------------- object_component.js:24:57

Cannot assign object literal to `_badProps2` because property `baz` (did you mean `bar`?) is missing in object type [1]
but exists in object literal [2].
Cannot assign object literal to `_badProps2` because property `baz` is missing in object type [1] but exists in object
literal [2].

object_component.js:24:57
24| const _badProps2: React.ElementProps<ObjectComponent> = {bar: 3, foo: 3, baz: 3}; // Error extra baz
Expand Down
21 changes: 9 additions & 12 deletions tests/react_abstract_component/react_abstract_component.exp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ References:

Error --------------------------------------------------------------------------------------------------- config.js:13:2

Cannot cast `y` to `NotTheRightConfig` because property `baz` (did you mean `bar`?) is missing in `Props` [1] but exists
in `NotTheRightConfig` [2].
Cannot cast `y` to `NotTheRightConfig` because property `baz` is missing in `Props` [1] but exists in
`NotTheRightConfig` [2].

config.js:13:2
13| (y: NotTheRightConfig); // Error, configs don't match
Expand Down Expand Up @@ -211,8 +211,8 @@ References:

Error --------------------------------------------------------------------------------------------------- config.js:16:2

Cannot cast `z` to config of React component because property `bar` (did you mean `baz`?) is missing in
`NotTheRightConfig` [1] but exists in `Props` [2].
Cannot cast `z` to config of React component because property `bar` is missing in `NotTheRightConfig` [1] but exists in
`Props` [2].

config.js:16:2
16| (z: React$Config<Props, DefaultProps>); // Error, configs don't match
Expand Down Expand Up @@ -264,8 +264,7 @@ References:

Error ------------------------------------------------------------------------------------------- create_element.js:9:13

Cannot create `C` element because property `bar` (did you mean `baz`?) is missing in props [1] but exists in object
type [2].
Cannot create `C` element because property `bar` is missing in props [1] but exists in object type [2].

create_element.js:9:13
9| const _c = <C baz={4} />; // Error missing bar
Expand All @@ -282,8 +281,7 @@ References:

Error ------------------------------------------------------------------------------------------ create_element.js:10:13

Cannot create `C` element because property `baz` (did you mean `bar`?) is missing in props [1] but exists in object
type [2].
Cannot create `C` element because property `baz` is missing in props [1] but exists in object type [2].

create_element.js:10:13
10| const _d = <C bar={3} />; // Error missing baz
Expand Down Expand Up @@ -345,8 +343,7 @@ References:

Error ---------------------------------------------------------------------------------------------- destructors.js:9:13

Cannot create `C` element because property `bar` (did you mean `baz`?) is missing in props [1] but exists in object
type [2].
Cannot create `C` element because property `bar` is missing in props [1] but exists in object type [2].

destructors.js:9:13
9| const _b = <C baz={3} />; // Error, bar missing
Expand Down Expand Up @@ -377,8 +374,8 @@ References:

Error ---------------------------------------------------------------------------------------------- destructors.js:21:2

Cannot cast object literal to `React.ElementConfig` because property `bar` (did you mean `baz`?) is missing in object
literal [1] but exists in object type [2].
Cannot cast object literal to `React.ElementConfig` because property `bar` is missing in object literal [1] but exists
in object type [2].

destructors.js:21:2
21| ({baz: 3}: React$ElementConfig<typeof C>); // Error, bar missing
Expand Down

0 comments on commit 598b7bc

Please sign in to comment.