From 5cadb4368f69e468e3b39a887110622d07a45efd Mon Sep 17 00:00:00 2001 From: Tobias Tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Tue, 17 Oct 2023 10:01:43 -0700 Subject: [PATCH] Improve inline fragment suggestions for abstract types (#4453) Summary: Previously the LSP would suggest the abstract type itself (why?) and either the members of a union or the object types implementing an interface when writing an inline fragment for an abstract type. I've removed the suggestion of the abstract type itself and am instead including all the interfaces that are implemented by the object types that are primarily suggested. This is great for patterns like the Stage 6a mutation error pattern, where you have a union where each member implicitly implements a shared interface. With this change this shared interface would be suggested by the LSP as well, instead of just the concrete "errors". Pull Request resolved: https://github.com/facebook/relay/pull/4453 Reviewed By: captbaritone Differential Revision: D49956467 Pulled By: alunyov fbshipit-source-id: db4581532f2dd74d1262eb8f9a0c4952480ccee8 --- .../crates/relay-lsp/src/completion/mod.rs | 63 +++++++++++++------ .../crates/relay-lsp/src/completion/test.rs | 44 ++++++++++--- 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/compiler/crates/relay-lsp/src/completion/mod.rs b/compiler/crates/relay-lsp/src/completion/mod.rs index 0ced431fc7bf5..fc629c3e51183 100644 --- a/compiler/crates/relay-lsp/src/completion/mod.rs +++ b/compiler/crates/relay-lsp/src/completion/mod.rs @@ -6,7 +6,6 @@ */ //! Utilities for providing the completion language feature -use std::iter::once; use common::ArgumentName; use common::DirectiveName; @@ -51,6 +50,8 @@ use lsp_types::MarkupKind; use schema::Argument as SchemaArgument; use schema::Directive as SchemaDirective; use schema::InputObject; +use schema::InterfaceID; +use schema::ObjectID; use schema::SDLSchema; use schema::Schema; use schema::Type; @@ -671,7 +672,6 @@ fn completion_items_for_request( program: &Program, ) -> Option> { let kind = request.kind; - debug!("completion_items_for_request: {:?}", kind); match kind { CompletionKind::FragmentSpread => { let leaf_type = request.type_path.resolve_leaf_type(schema)?; @@ -971,25 +971,13 @@ fn resolve_completion_items_for_inline_fragment( match type_ { Type::Interface(id) => { let interface = schema.interface(id); - once(type_) - .chain( - interface - .implementing_objects - .iter() - .filter_map(|id| schema.get_type(schema.object(*id).name.item.0)), - ) - .collect() + + get_abstract_type_suggestions(schema, &interface.implementing_objects, Some(&id)) } Type::Union(id) => { let union = schema.union(id); - once(type_) - .chain( - union - .members - .iter() - .filter_map(|id| schema.get_type(schema.object(*id).name.item.0)), - ) - .collect() + + get_abstract_type_suggestions(schema, &union.members, None) } Type::Enum(_) | Type::Object(_) | Type::InputObject(_) | Type::Scalar(_) => vec![], } @@ -1404,5 +1392,44 @@ fn make_markdown_table_documentation( }) } +fn get_abstract_type_suggestions( + schema: &SDLSchema, + objects: &[ObjectID], + base_interface_id: Option<&InterfaceID>, +) -> Vec { + let object_types: Vec<_> = objects.iter().map(|id| schema.object(*id)).collect(); + + let mut interfaces = Vec::new(); + let mut types = Vec::new(); + + for object_type in &object_types { + if let Some(t) = schema.get_type(object_type.name.item.0) { + types.push(t); + } + + for interface_id in &object_type.interfaces { + let interface_type = schema.interface(*interface_id); + + if let Some(base_id) = base_interface_id { + if interface_id == base_id || !interface_type.interfaces.contains(base_id) { + continue; + } + } + + if let Some(t) = schema.get_type(interface_type.name.item.0) { + if interfaces.contains(&t) { + continue; + } + + interfaces.push(t); + } + } + } + + types.extend(interfaces); + + types +} + #[cfg(test)] mod test; diff --git a/compiler/crates/relay-lsp/src/completion/test.rs b/compiler/crates/relay-lsp/src/completion/test.rs index 74a06c1baaf89..23a6345a445a3 100644 --- a/compiler/crates/relay-lsp/src/completion/test.rs +++ b/compiler/crates/relay-lsp/src/completion/test.rs @@ -244,7 +244,6 @@ fn whitespace_in_interface() { "source", "node", "__typename", - "... on CommentsEdgeInterface", "... on CommentsEdge", "...ImplementingFragment", "...InterfaceFragment", @@ -280,11 +279,10 @@ fn whitespace_in_union() { items.unwrap(), vec![ "__typename", - "... on CommentBody", - "... on PlainCommentBody", "... on MarkdownCommentBody", - "...UnionFragment", + "... on PlainCommentBody", "...UnionVariantFragment", + "...UnionFragment", ], ) } @@ -312,9 +310,28 @@ fn inline_fragment_on_interface() { "#, None, ); + assert_labels(items.unwrap(), vec!["... on SimpleNamed", "... on User"]); +} + +#[test] +fn inline_fragment_on_interface_objects_implement_interface_implementing_base_interface() { + let items = parse_and_resolve_completion_items( + r#" + fragment Test on UserNameRenderable { + ... on |a + } + "#, + None, + ); + assert_labels( items.unwrap(), - vec!["... on Named", "... on User", "... on SimpleNamed"], + vec![ + "... on PlainUserNameRenderer", + "... on ImplementsImplementsUserNameRenderableAndUserNameRenderable", + "... on MarkdownUserNameRenderer", + "... on ImplementsUserNameRenderable", + ], ); } @@ -328,7 +345,7 @@ fn inline_fragment_on_interface_with_existing_inline_fragment() { "#, None, ); - assert_labels(items.unwrap(), vec!["Named", "User", "SimpleNamed"]); + assert_labels(items.unwrap(), vec!["User", "SimpleNamed"]); } #[test] @@ -344,10 +361,12 @@ fn inline_fragment_on_union() { assert_labels( items.unwrap(), vec![ - "... on MaybeNode", - "... on Story", "... on FakeNode", + "... on FeedUnit", + "... on Node", "... on NonNode", + "... on Story", + "... on MaybeNodeInterface", ], ); } @@ -364,7 +383,14 @@ fn inline_fragment_on_union_with_existing_inline_fragment() { ); assert_labels( items.unwrap(), - vec!["MaybeNode", "Story", "FakeNode", "NonNode"], + vec![ + "Node", + "Story", + "FakeNode", + "NonNode", + "MaybeNodeInterface", + "FeedUnit", + ], ); }