From 5be5d6ab38b765500ef3fd672bfb0a1caee6c022 Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Sun, 25 Feb 2024 19:36:40 +0100 Subject: [PATCH] Add go to definition support for directives --- .../crates/relay-lsp/src/completion/mod.rs | 2 +- .../goto_graphql_definition.rs | 6 + .../relay-lsp/src/goto_definition/mod.rs | 23 ++++ .../crates/schema-print/src/print_schema.rs | 2 +- compiler/crates/schema-validate/src/lib.rs | 9 +- compiler/crates/schema/src/definitions/mod.rs | 4 +- compiler/crates/schema/src/flatbuffer/mod.rs | 2 +- .../crates/schema/src/flatbuffer/serialize.rs | 2 +- compiler/crates/schema/src/in_memory/mod.rs | 15 ++- .../fixtures/directive-on-arg-def.expected | 72 ++++++++---- .../directives-for-external-types.expected | 108 ++++++++++++------ .../fixtures/field-descriptions.expected | 36 ++++-- .../interface-implements-interface.expected | 36 ++++-- .../fixtures/kitchen-sink.expected | 54 ++++++--- .../crates/schema/tests/build_schema/mod.rs | 2 +- 15 files changed, 256 insertions(+), 117 deletions(-) diff --git a/compiler/crates/relay-lsp/src/completion/mod.rs b/compiler/crates/relay-lsp/src/completion/mod.rs index fc629c3e51183..c34d9eb3f4169 100644 --- a/compiler/crates/relay-lsp/src/completion/mod.rs +++ b/compiler/crates/relay-lsp/src/completion/mod.rs @@ -1239,7 +1239,7 @@ fn completion_item_from_directive( } = directive; // Always use the name of the directive as the label - let label = name.to_string(); + let label = name.item.to_string(); // We can return a snippet with the expected arguments of the directive let (insert_text, insert_text_format) = if arguments.is_empty() { diff --git a/compiler/crates/relay-lsp/src/goto_definition/goto_graphql_definition.rs b/compiler/crates/relay-lsp/src/goto_definition/goto_graphql_definition.rs index ffa142bd1f50b..b116bf612dcfd 100644 --- a/compiler/crates/relay-lsp/src/goto_definition/goto_graphql_definition.rs +++ b/compiler/crates/relay-lsp/src/goto_definition/goto_graphql_definition.rs @@ -64,6 +64,12 @@ pub fn get_graphql_definition_description( }) => Ok(DefinitionDescription::Type { type_name: type_condition.type_.value, }), + ResolutionPath::Ident(IdentPath { + inner: directive_name, + parent: IdentParent::DirectiveName(_), + }) => Ok(DefinitionDescription::Directive { + directive_name: directive_name.value, + }), _ => Err(LSPRuntimeError::ExpectedError), } } diff --git a/compiler/crates/relay-lsp/src/goto_definition/mod.rs b/compiler/crates/relay-lsp/src/goto_definition/mod.rs index 6e1d1610cc8c2..adff666f7d508 100644 --- a/compiler/crates/relay-lsp/src/goto_definition/mod.rs +++ b/compiler/crates/relay-lsp/src/goto_definition/mod.rs @@ -12,6 +12,7 @@ mod goto_graphql_definition; use std::str; use std::sync::Arc; +use common::DirectiveName; use graphql_ir::FragmentDefinitionName; use intern::string_key::Intern; use intern::string_key::StringKey; @@ -49,6 +50,9 @@ pub enum DefinitionDescription { Type { type_name: StringKey, }, + Directive { + directive_name: StringKey, + }, } /// Resolve a GotoDefinitionRequest to a GotoDefinitionResponse @@ -98,6 +102,9 @@ pub fn on_goto_definition( &schema, &root_dir, )?, + DefinitionDescription::Directive { directive_name } => { + locate_directive_definition(directive_name, &schema, &root_dir)? + } }; // For some lsp-clients, such as clients relying on org.eclipse.lsp4j, @@ -127,6 +134,22 @@ fn locate_fragment_definition( )) } +fn locate_directive_definition( + directive_name: StringKey, + schema: &Arc, + root_dir: &std::path::Path, +) -> Result { + let directive = schema.get_directive(DirectiveName(directive_name)); + + directive + .map(|directive| directive.name.location) + .map(|schema_location| { + transform_relay_location_to_lsp_location(root_dir, schema_location) + .map(GotoDefinitionResponse::Scalar) + }) + .ok_or(LSPRuntimeError::ExpectedError)? +} + fn locate_type_definition( extra_data_provider: &dyn LSPExtraDataProvider, project_name: StringKey, diff --git a/compiler/crates/schema-print/src/print_schema.rs b/compiler/crates/schema-print/src/print_schema.rs index 980672f1d3c0f..5d9fade3c5892 100644 --- a/compiler/crates/schema-print/src/print_schema.rs +++ b/compiler/crates/schema-print/src/print_schema.rs @@ -242,7 +242,7 @@ impl<'schema, 'writer, 'curent_writer> Printer<'schema, 'writer> { } fn print_directive(&mut self, directive: &Directive) -> FmtResult { - write!(self.writer(), "directive @{}", directive.name)?; + write!(self.writer(), "directive @{}", directive.name.item)?; self.print_args(&directive.arguments)?; write!( self.writer(), diff --git a/compiler/crates/schema-validate/src/lib.rs b/compiler/crates/schema-validate/src/lib.rs index dd77fd1527776..0bbc3a1253287 100644 --- a/compiler/crates/schema-validate/src/lib.rs +++ b/compiler/crates/schema-validate/src/lib.rs @@ -121,8 +121,8 @@ impl<'schema> ValidationContext<'schema> { fn validate_directives(&mut self) { for directive in self.schema.get_directives() { - let context = ValidationContextType::DirectiveNode(directive.name.0); - self.validate_name(directive.name.0, context); + let context = ValidationContextType::DirectiveNode(directive.name.item.0); + self.validate_name(directive.name.item.0, context); let mut arg_names = FnvHashSet::default(); for argument in directive.arguments.iter() { self.validate_name(argument.name.0, context); @@ -130,7 +130,10 @@ impl<'schema> ValidationContext<'schema> { // Ensure unique arguments per directive. if arg_names.contains(&argument.name) { self.report_error( - SchemaValidationError::DuplicateArgument(argument.name, directive.name.0), + SchemaValidationError::DuplicateArgument( + argument.name, + directive.name.item.0, + ), context, ); continue; diff --git a/compiler/crates/schema/src/definitions/mod.rs b/compiler/crates/schema/src/definitions/mod.rs index aecd3c79a27cf..ea4b014581f1f 100644 --- a/compiler/crates/schema/src/definitions/mod.rs +++ b/compiler/crates/schema/src/definitions/mod.rs @@ -361,7 +361,7 @@ impl TypeReference { #[derive(Clone, Debug, Eq, PartialEq)] pub struct Directive { - pub name: DirectiveName, + pub name: WithLocation, pub arguments: ArgumentDefinitions, pub locations: Vec, pub repeatable: bool, @@ -373,7 +373,7 @@ pub struct Directive { impl Named for Directive { type Name = DirectiveName; fn name(&self) -> DirectiveName { - self.name + self.name.item } } diff --git a/compiler/crates/schema/src/flatbuffer/mod.rs b/compiler/crates/schema/src/flatbuffer/mod.rs index 4d78f866343f7..1b7015b00c23d 100644 --- a/compiler/crates/schema/src/flatbuffer/mod.rs +++ b/compiler/crates/schema/src/flatbuffer/mod.rs @@ -170,7 +170,7 @@ impl<'fb> FlatBufferSchema<'fb> { .map(get_mapped_location) .collect::>(); let parsed_directive = Directive { - name: DirectiveName(directive.name()?.intern()), + name: WithLocation::generated(DirectiveName(directive.name()?.intern())), is_extension: directive.is_extension(), arguments: self.parse_arguments(directive.arguments()?)?, locations, diff --git a/compiler/crates/schema/src/flatbuffer/serialize.rs b/compiler/crates/schema/src/flatbuffer/serialize.rs index df8a51390492b..47cb9e35eea2a 100644 --- a/compiler/crates/schema/src/flatbuffer/serialize.rs +++ b/compiler/crates/schema/src/flatbuffer/serialize.rs @@ -141,7 +141,7 @@ impl<'fb, 'schema> Serializer<'fb, 'schema> { } fn serialize_directive(&mut self, directive: &Directive) { - let name = directive.name.0.lookup(); + let name = directive.name.item.0.lookup(); if self.directives.contains_key(name) { return; } diff --git a/compiler/crates/schema/src/in_memory/mod.rs b/compiler/crates/schema/src/in_memory/mod.rs index a334d058b574a..4f3debe1a349d 100644 --- a/compiler/crates/schema/src/in_memory/mod.rs +++ b/compiler/crates/schema/src/in_memory/mod.rs @@ -261,7 +261,7 @@ impl Schema for InMemorySchema { let ordered_type_map: BTreeMap<_, _> = type_map.iter().collect(); let mut ordered_directives = directives.values().collect::>(); - ordered_directives.sort_by_key(|dir| dir.name.0.lookup()); + ordered_directives.sort_by_key(|dir| dir.name.item.0.lookup()); format!( r#"Schema {{ @@ -372,10 +372,12 @@ impl InMemorySchema { } pub fn add_directive(&mut self, directive: Directive) -> DiagnosticsResult<()> { - if self.directives.contains_key(&directive.name) { - return todo_add_location(SchemaError::DuplicateDirectiveDefinition(directive.name.0)); + if self.directives.contains_key(&directive.name.item) { + return todo_add_location(SchemaError::DuplicateDirectiveDefinition( + directive.name.item.0, + )); } - self.directives.insert(directive.name, directive); + self.directives.insert(directive.name.item, directive); Ok(()) } @@ -1202,7 +1204,10 @@ impl InMemorySchema { self.directives.insert( DirectiveName(name.value), Directive { - name: DirectiveName(name.value), + name: WithLocation::new( + Location::new(*location_key, name.span), + DirectiveName(name.value), + ), arguments, locations: locations.clone(), repeatable: *repeatable, diff --git a/compiler/crates/schema/tests/build_schema/fixtures/directive-on-arg-def.expected b/compiler/crates/schema/tests/build_schema/fixtures/directive-on-arg-def.expected index a19f95f72df19..c1c6e96383e6a 100644 --- a/compiler/crates/schema/tests/build_schema/fixtures/directive-on-arg-def.expected +++ b/compiler/crates/schema/tests/build_schema/fixtures/directive-on-arg-def.expected @@ -21,9 +21,12 @@ Text Schema:Schema { subscription_type: None directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :255:262, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -50,9 +53,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "required", - ), + name: WithLocation { + location: :62:70, + item: DirectiveName( + "required", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -84,9 +90,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :334:338, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -113,9 +122,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "static", - ), + name: WithLocation { + location: :130:136, + item: DirectiveName( + "static", + ), + }, arguments: [], locations: [ ArgumentDefinition, @@ -371,9 +383,12 @@ Text Schema:Schema { FlatBuffer Schema:FB Schema { directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -400,9 +415,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "required", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "required", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -427,9 +445,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -456,9 +477,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "static", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "static", + ), + }, arguments: [], locations: [ ArgumentDefinition, diff --git a/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected b/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected index dc31f7f79a6c7..c91e51d7fefb0 100644 --- a/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected +++ b/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected @@ -53,9 +53,12 @@ Text Schema:Schema { subscription_type: None directives: [ Directive { - name: DirectiveName( - "extern_type", - ), + name: WithLocation { + location: :179:190, + item: DirectiveName( + "extern_type", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -89,9 +92,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "fetchable", - ), + name: WithLocation { + location: :245:254, + item: DirectiveName( + "fetchable", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -114,9 +120,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :255:262, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -143,9 +152,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "ref_type", - ), + name: WithLocation { + location: :109:117, + item: DirectiveName( + "ref_type", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -179,9 +191,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :334:338, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -208,9 +223,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "source", - ), + name: WithLocation { + location: :11:17, + item: DirectiveName( + "source", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1061,9 +1079,12 @@ Text Schema:Schema { FlatBuffer Schema:FB Schema { directives: [ Directive { - name: DirectiveName( - "extern_type", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "extern_type", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1097,9 +1118,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "fetchable", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "fetchable", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1122,9 +1146,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1151,9 +1178,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "ref_type", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "ref_type", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1187,9 +1217,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1216,9 +1249,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "source", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "source", + ), + }, arguments: [ Argument { name: ArgumentName( diff --git a/compiler/crates/schema/tests/build_schema/fixtures/field-descriptions.expected b/compiler/crates/schema/tests/build_schema/fixtures/field-descriptions.expected index 2e17be6d232ad..bf8326bfcba34 100644 --- a/compiler/crates/schema/tests/build_schema/fixtures/field-descriptions.expected +++ b/compiler/crates/schema/tests/build_schema/fixtures/field-descriptions.expected @@ -43,9 +43,12 @@ Text Schema:Schema { subscription_type: None directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :255:262, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -72,9 +75,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :334:338, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -454,9 +460,12 @@ Text Schema:Schema { FlatBuffer Schema:FB Schema { directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -483,9 +492,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( diff --git a/compiler/crates/schema/tests/build_schema/fixtures/interface-implements-interface.expected b/compiler/crates/schema/tests/build_schema/fixtures/interface-implements-interface.expected index 6df11fd059832..7302087743816 100644 --- a/compiler/crates/schema/tests/build_schema/fixtures/interface-implements-interface.expected +++ b/compiler/crates/schema/tests/build_schema/fixtures/interface-implements-interface.expected @@ -35,9 +35,12 @@ Text Schema:Schema { subscription_type: None directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :255:262, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -64,9 +67,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :334:338, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -551,9 +557,12 @@ Text Schema:Schema { FlatBuffer Schema:FB Schema { directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -580,9 +589,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( diff --git a/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected b/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected index f253722d6c3b2..159b45f2d1751 100644 --- a/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected +++ b/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected @@ -75,9 +75,12 @@ Text Schema:Schema { subscription_type: None directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :255:262, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -104,9 +107,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :334:338, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -133,9 +139,12 @@ Text Schema:Schema { hack_source: None, }, Directive { - name: DirectiveName( - "source", - ), + name: WithLocation { + location: :11:17, + item: DirectiveName( + "source", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -994,9 +1003,12 @@ Text Schema:Schema { FlatBuffer Schema:FB Schema { directives: [ Directive { - name: DirectiveName( - "include", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "include", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1023,9 +1035,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "skip", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "skip", + ), + }, arguments: [ Argument { name: ArgumentName( @@ -1052,9 +1067,12 @@ directives: [ hack_source: None, }, Directive { - name: DirectiveName( - "source", - ), + name: WithLocation { + location: :0:0, + item: DirectiveName( + "source", + ), + }, arguments: [ Argument { name: ArgumentName( diff --git a/compiler/crates/schema/tests/build_schema/mod.rs b/compiler/crates/schema/tests/build_schema/mod.rs index cc0fd326e72d0..7bb5268175bea 100644 --- a/compiler/crates/schema/tests/build_schema/mod.rs +++ b/compiler/crates/schema/tests/build_schema/mod.rs @@ -91,7 +91,7 @@ fn print_schema_and_flat_buffer_schema(schema: SDLSchema) -> String { let mut ordered_directives = schema.get_directives().collect::>(); ordered_directives.sort_by_key(|directive| directive.name); for directive in ordered_directives { - directives.push(fb_schema.get_directive(directive.name).unwrap()); + directives.push(fb_schema.get_directive(directive.name.item).unwrap()); } let fb_schema_snapshot = format!( r#"FB Schema {{