Fix schema-validation

facebook · Feb 25, 2024 · 35123da · 35123da
1 parent 987eb55
commit 35123da
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 71 deletions.
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
@@ -120,6 +120,7 @@ fn resolve_field(
     })
 }
 
+// TODO: Support for directive arguments
 fn resolve_field_argument(
     field_name: StringKey,
     argument_name: StringKey,

diff --git a/compiler/crates/relay-lsp/src/references/mod.rs b/compiler/crates/relay-lsp/src/references/mod.rs
@@ -32,22 +32,6 @@ use crate::node_resolution_info::NodeKind;
 use crate::server::GlobalState;
 use crate::FeatureResolutionInfo;
 
-pub fn on_references(
-    state: &impl GlobalState,
-    params: <References as Request>::Params,
-) -> LSPRuntimeResult<<References as Request>::Result> {
-    let node_resolution_info = state.resolve_node(&params.text_document_position)?;
-    let references_response = get_references_response(
-        node_resolution_info,
-        &state
-            .get_program(&state.extract_project_name_from_url(
-                &params.text_document_position.text_document.uri,
-            )?)?,
-        &state.root_dir(),
-    )?;
-    Ok(Some(references_response))
-}
-
 fn get_references_response(
     feature_resolution_info: FeatureResolutionInfo,
     program: &Program,
@@ -86,66 +70,42 @@ fn get_references_response(
                             .collect::<Result<Vec<_>, LSPRuntimeError>>()?;
                     Ok(lsp_locations)
                 }
-                NodeKind::FieldArgument(_field_name, _argument_name) => {
-                    // TODO: Implement
-
-                    Err(LSPRuntimeError::ExpectedError)
-                }
                 _ => Err(LSPRuntimeError::ExpectedError),
             }
         }
         FeatureResolutionInfo::DocblockNode(docblock_node) => {
-            match docblock_node.resolution_info {
-                DocblockResolutionInfo::FieldName(field_name) => {
-                    let type_name = get_parent_type_name(docblock_node.ir)?;
-
-                    let references = find_field_locations(program, field_name, type_name)
-                        .ok_or(LSPRuntimeError::ExpectedError)?
-                        .into_iter()
-                        .map(|location| {
-                            transform_relay_location_to_lsp_location(root_dir, location)
-                        })
-                        .collect::<Result<Vec<_>, LSPRuntimeError>>()?;
-
-                    Ok(references)
-                }
-                DocblockResolutionInfo::FieldArgumentName {
-                    field_name: _,
-                    argument_name: _,
-                } => {
-                    let _type_name = get_parent_type_name(docblock_node.ir)?;
-
-                    // TODO: Implement
-
-                    Err(LSPRuntimeError::ExpectedError)
-                }
-                _ => {
-                    // Go to reference not implemented for other parts of the docblocks yet.
-                    Err(LSPRuntimeError::ExpectedError)
-                }
+            if let DocblockResolutionInfo::FieldName(field_name) = docblock_node.resolution_info {
+                let type_name = match docblock_node.ir {
+                    DocblockIr::LegacyVerboseResolver(relay_resolver) => match relay_resolver.on {
+                        On::Type(type_) => type_.value.item,
+                        On::Interface(interface) => interface.value.item,
+                    },
+                    DocblockIr::TerseRelayResolver(terse_resolver) => terse_resolver.type_.item,
+                    DocblockIr::StrongObjectResolver(_) => {
+                        // TODO: Implement support for strong object.
+                        return Err(LSPRuntimeError::ExpectedError);
+                    }
+                    DocblockIr::WeakObjectType(_) => {
+                        // TODO: Implement support for weak object.
+                        return Err(LSPRuntimeError::ExpectedError);
+                    }
+                };
+
+                let references = find_field_locations(program, field_name, type_name)
+                    .ok_or(LSPRuntimeError::ExpectedError)?
+                    .into_iter()
+                    .map(|location| transform_relay_location_to_lsp_location(root_dir, location))
+                    .collect::<Result<Vec<_>, LSPRuntimeError>>()?;
+
+                Ok(references)
+            } else {
+                // Go to reference not implemented for other parts of the docblocks yet.
+                Err(LSPRuntimeError::ExpectedError)
             }
         }
     }
 }
 
-fn get_parent_type_name(docblock_ir: DocblockIr) -> LSPRuntimeResult<StringKey> {
-    Ok(match docblock_ir {
-        DocblockIr::LegacyVerboseResolver(relay_resolver) => match relay_resolver.on {
-            On::Type(type_) => type_.value.item,
-            On::Interface(interface) => interface.value.item,
-        },
-        DocblockIr::TerseRelayResolver(terse_resolver) => terse_resolver.type_.item,
-        DocblockIr::StrongObjectResolver(_) => {
-            // TODO: Implement support for strong object.
-            return Err(LSPRuntimeError::ExpectedError);
-        }
-        DocblockIr::WeakObjectType(_) => {
-            // TODO: Implement support for weak object.
-            return Err(LSPRuntimeError::ExpectedError);
-        }
-    })
-}
-
 #[derive(Debug, Clone)]
 struct ReferenceFinder {
     references: Vec<IRLocation>,
@@ -174,3 +134,19 @@ impl Visitor for ReferenceFinder {
         }
     }
 }
+
+pub fn on_references(
+    state: &impl GlobalState,
+    params: <References as Request>::Params,
+) -> LSPRuntimeResult<<References as Request>::Result> {
+    let node_resolution_info = state.resolve_node(&params.text_document_position)?;
+    let references_response = get_references_response(
+        node_resolution_info,
+        &state
+            .get_program(&state.extract_project_name_from_url(
+                &params.text_document_position.text_document.uri,
+            )?)?,
+        &state.root_dir(),
+    )?;
+    Ok(Some(references_response))
+}
diff --git a/compiler/crates/schema-validate/src/lib.rs b/compiler/crates/schema-validate/src/lib.rs
@@ -128,7 +128,7 @@ impl<'schema> ValidationContext<'schema> {
                 self.validate_name(argument.name.item.0, context);
 
                 // Ensure unique arguments per directive.
-                if arg_names.contains(&argument.name) {
+                if arg_names.contains(&argument.name.item) {
                     self.report_error(
                         SchemaValidationError::DuplicateArgument(
                             argument.name.item,
@@ -138,7 +138,7 @@ impl<'schema> ValidationContext<'schema> {
                     );
                     continue;
                 }
-                arg_names.insert(argument.name);
+                arg_names.insert(argument.name.item);
             }
         }
     }
@@ -228,7 +228,7 @@ impl<'schema> ValidationContext<'schema> {
 
                 // Ensure they are unique per field.
                 // Ensure unique arguments per directive.
-                if arg_names.contains(&argument.name) {
+                if arg_names.contains(&argument.name.item) {
                     self.report_error(
                         SchemaValidationError::DuplicateArgument(
                             argument.name.item,
@@ -238,7 +238,7 @@ impl<'schema> ValidationContext<'schema> {
                     );
                     continue;
                 }
-                arg_names.insert(argument.name);
+                arg_names.insert(argument.name.item);
 
                 // Ensure the type is an input type
                 if !is_input_type(&argument.type_) {

diff --git a/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_directives.expected b/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_directives.expected
@@ -38,9 +38,15 @@ had errors:
 Directive __fetchableOther with definition:
 	directive @__fetchableOther(field_name: String, field_name: Int) on OBJECT
 had errors:
+	* Duplicate argument 'field_name' found on field/directive '__fetchableOther'.
 	* Name '__fetchableOther' must not begin with '__', which is reserved by GraphQL introspection.
 
 Directive fetchable with definition:
 	directive @fetchable(__field_name: String) on OBJECT
 had errors:
 	* Name '__field_name' must not begin with '__', which is reserved by GraphQL introspection.
+
+Directive fetchableOther with definition:
+	directive @fetchableOther(field_name: String, field_name: Int) on OBJECT
+had errors:
+	* Duplicate argument 'field_name' found on field/directive 'fetchableOther'.
diff --git a/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_object.expected b/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_object.expected
@@ -78,6 +78,7 @@ Type Human with definition:
   other_friends(location: Location, location: Location): [Human]
 }
 had errors:
+	* Duplicate argument 'location' found on field/directive 'other_friends'.
 	* Duplicate field 'location' found.
 	* Interface field 'Hominid.pet' expects type 'Canine' but 'Human.pet' is of type 'OtherPet'.
 	* Interface field argument 'Hominid.friends(location:)' expected but 'Human.friends' does not provide it.