Add go to definition support for directives (#4625)

Summary: Pull Request resolved: #4625 Reviewed By: tyao1 Differential Revision: D54207213 Pulled By: captbaritone fbshipit-source-id: c9921e23104285b2e1400870ab3d69cd889e3c16
facebook · Feb 28, 2024 · 2ba726b · 2ba726b
1 parent 2d20c09
commit 2ba726b
Show file tree

Hide file tree

Showing 15 changed files with 256 additions and 117 deletions.
diff --git 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
@@ -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
@@ -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<SDLSchema>,
+    root_dir: &std::path::Path,
+) -> Result<GotoDefinitionResponse, LSPRuntimeError> {
+    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
@@ -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
@@ -121,16 +121,19 @@ 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);
 
                 // 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
@@ -361,7 +361,7 @@ impl<T> TypeReference<T> {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Directive {
-    pub name: DirectiveName,
+    pub name: WithLocation<DirectiveName>,
     pub arguments: ArgumentDefinitions,
     pub locations: Vec<DirectiveLocation>,
     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
@@ -170,7 +170,7 @@ impl<'fb> FlatBufferSchema<'fb> {
             .map(get_mapped_location)
             .collect::<Vec<_>>();
         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
@@ -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
@@ -261,7 +261,7 @@ impl Schema for InMemorySchema {
         let ordered_type_map: BTreeMap<_, _> = type_map.iter().collect();
 
         let mut ordered_directives = directives.values().collect::<Vec<&Directive>>();
-        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
@@ -21,9 +21,12 @@ Text Schema:Schema {
   subscription_type: None
   directives: [
     Directive {
-        name: DirectiveName(
-            "include",
-        ),
+        name: WithLocation {
+            location: <generated>: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: <generated>: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: <generated>: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: <generated>: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: <generated>:0:0,
+            item: DirectiveName(
+                "include",
+            ),
+        },
         arguments: [
             Argument {
                 name: ArgumentName(
@@ -400,9 +415,12 @@ directives: [
         hack_source: None,
     },
     Directive {
-        name: DirectiveName(
-            "required",
-        ),
+        name: WithLocation {
+            location: <generated>:0:0,
+            item: DirectiveName(
+                "required",
+            ),
+        },
         arguments: [
             Argument {
                 name: ArgumentName(
@@ -427,9 +445,12 @@ directives: [
         hack_source: None,
     },
     Directive {
-        name: DirectiveName(
-            "skip",
-        ),
+        name: WithLocation {
+            location: <generated>:0:0,
+            item: DirectiveName(
+                "skip",
+            ),
+        },
         arguments: [
             Argument {
                 name: ArgumentName(
@@ -456,9 +477,12 @@ directives: [
         hack_source: None,
     },
     Directive {
-        name: DirectiveName(
-            "static",
-        ),
+        name: WithLocation {
+            location: <generated>:0:0,
+            item: DirectiveName(
+                "static",
+            ),
+        },
         arguments: [],
         locations: [
             ArgumentDefinition,