diff --git a/compiler/crates/common/src/diagnostic.rs b/compiler/crates/common/src/diagnostic.rs index 8f8daa7a88ba9..b9e9894b18bc8 100644 --- a/compiler/crates/common/src/diagnostic.rs +++ b/compiler/crates/common/src/diagnostic.rs @@ -12,6 +12,7 @@ use std::fmt::Write; use lsp_types::DiagnosticSeverity; use lsp_types::DiagnosticTag; +use lsp_types::NumberOrString; use serde::ser::SerializeMap; use serde_json::Value; @@ -64,6 +65,7 @@ impl Diagnostic { tags: Vec, ) -> Self { Self(Box::new(DiagnosticData { + code: None, message: Box::new(message), location, related_information: Vec::new(), @@ -88,6 +90,27 @@ impl Diagnostic { ) -> Self { let data = message.get_data(); Self(Box::new(DiagnosticData { + code: None, + message: Box::new(message), + location, + tags: Vec::new(), + severity: DiagnosticSeverity::ERROR, + related_information: Vec::new(), + data, + machine_readable: BTreeMap::new(), + })) + } + + /// Creates a new error Diagnostic with additional data and code + /// that can be used in IDE code actions + pub fn error_with_data_and_code( + code: i32, + message: T, + location: Location, + ) -> Self { + let data = message.get_data(); + Self(Box::new(DiagnosticData { + code: Some(NumberOrString::Number(code)), message: Box::new(message), location, tags: Vec::new(), @@ -167,6 +190,10 @@ impl Diagnostic { self } + pub fn code(&self) -> Option { + self.0.code.to_owned() + } + pub fn message(&self) -> &impl DiagnosticDisplay { &self.0.message } @@ -276,6 +303,9 @@ where #[derive(fmt::Debug)] struct DiagnosticData { + /// The diagnostic's code. Can be omitted. + code: Option, + /// Human readable error message. message: Box, diff --git a/compiler/crates/graphql-ir/src/build.rs b/compiler/crates/graphql-ir/src/build.rs index f9b2b501f14b4..0e47b58dd8c9d 100644 --- a/compiler/crates/graphql-ir/src/build.rs +++ b/compiler/crates/graphql-ir/src/build.rs @@ -49,6 +49,7 @@ use schema::Type; use schema::TypeReference; use crate::constants::ARGUMENT_DEFINITION; +use crate::errors::ValidationDiagnosticCode; use crate::errors::ValidationMessage; use crate::errors::ValidationMessageWithData; use crate::ir::*; @@ -423,10 +424,12 @@ impl<'schema, 'signatures, 'options> Builder<'schema, 'signatures, 'options> { .used_variables .iter() .map(|(undefined_variable, usage)| { - Diagnostic::error( - ValidationMessage::ExpectedOperationVariableToBeDefined( - *undefined_variable, - ), + Diagnostic::error_with_data_and_code( + ValidationDiagnosticCode::EXPECTED_OPERATION_VARIABLE_TO_BE_DEFINED, + ValidationMessageWithData::ExpectedOperationVariableToBeDefined { + variable_name: *undefined_variable, + variable_type: self.schema.get_type_string(&usage.type_), + }, self.location.with_span(usage.span), ) }) diff --git a/compiler/crates/graphql-ir/src/errors.rs b/compiler/crates/graphql-ir/src/errors.rs index cd8db2ee4dbe7..811905459571b 100644 --- a/compiler/crates/graphql-ir/src/errors.rs +++ b/compiler/crates/graphql-ir/src/errors.rs @@ -149,9 +149,6 @@ pub enum ValidationMessage { next_type: String, }, - #[error("Expected variable `${0}` to be defined on the operation")] - ExpectedOperationVariableToBeDefined(VariableName), - #[error( "Expected argument definition to have an input type (scalar, enum, or input object), found type '{0}'" )] @@ -423,12 +420,6 @@ pub enum ValidationMessage { )] UnusedIgnoreUnusedVariablesDirective { operation_name: StringKey }, - #[error("Operation '{operation_name}' references undefined variable{variables_string}.")] - GlobalVariables { - operation_name: StringKey, - variables_string: String, - }, - #[error("Subscription '{subscription_name}' must have a single selection")] GenerateSubscriptionNameSingleSelectionItem { subscription_name: StringKey }, @@ -539,6 +530,14 @@ pub enum ValidationMessage { ResolverInMutation, } +#[derive(Clone, Debug)] +pub struct ValidationDiagnosticCode; + +impl ValidationDiagnosticCode { + pub const EXPECTED_OPERATION_VARIABLE_TO_BE_DEFINED: i32 = 1; + pub const UNDEFINED_VARIABLE_REFERENCED: i32 = 2; +} + #[derive( Clone, Debug, @@ -584,6 +583,19 @@ pub enum ValidationMessageWithData { argument_name: StringKey, suggestions: Vec, }, + + #[error("Expected variable `${variable_name}` to be defined on the operation")] + ExpectedOperationVariableToBeDefined { + variable_name: VariableName, + variable_type: String, + }, + + #[error("Operation '{operation_name}' references undefined variable '${variable_name}'.")] + UndefinedVariableReferenced { + operation_name: StringKey, + variable_name: VariableName, + variable_type: String, + }, } impl WithDiagnosticData for ValidationMessageWithData { @@ -599,6 +611,21 @@ impl WithDiagnosticData for ValidationMessageWithData { ValidationMessageWithData::ExpectedSelectionsOnObjectField { field_name, .. } => { vec![Box::new(format!("{} {{ }}", field_name))] } + ValidationMessageWithData::ExpectedOperationVariableToBeDefined { + variable_name, + variable_type, + } => vec![ + Box::new(variable_name.to_owned()), + Box::new(variable_type.to_owned()), + ], + ValidationMessageWithData::UndefinedVariableReferenced { + variable_name, + variable_type, + .. + } => vec![ + Box::new(variable_name.to_owned()), + Box::new(variable_type.to_owned()), + ], } } } diff --git a/compiler/crates/graphql-ir/src/lib.rs b/compiler/crates/graphql-ir/src/lib.rs index 5c81c271e0b38..ec741dd7ce5de 100644 --- a/compiler/crates/graphql-ir/src/lib.rs +++ b/compiler/crates/graphql-ir/src/lib.rs @@ -47,7 +47,9 @@ pub use transform::Transformer; pub use validator::Validator; pub use visitor::Visitor; +pub use crate::errors::ValidationDiagnosticCode; pub use crate::errors::ValidationMessage; +pub use crate::errors::ValidationMessageWithData; /// Re-exported values to be used by the `associated_data_impl!` macro. pub mod reexport { diff --git a/compiler/crates/relay-lsp/src/code_action.rs b/compiler/crates/relay-lsp/src/code_action.rs index 497b0827f38b0..1751c5e837850 100644 --- a/compiler/crates/relay-lsp/src/code_action.rs +++ b/compiler/crates/relay-lsp/src/code_action.rs @@ -5,24 +5,31 @@ * LICENSE file in the root directory of this source tree. */ +mod create_fragment_argument; mod create_name_suggestion; +mod create_operation_variable; use std::collections::HashMap; use std::collections::HashSet; +use common::Location; use common::Span; use create_name_suggestion::create_default_name; use create_name_suggestion::create_default_name_with_index; use create_name_suggestion::create_impactful_name; use create_name_suggestion::create_name_wrapper; use create_name_suggestion::DefinitionNameSuffix; +use graphql_ir::ValidationDiagnosticCode; use graphql_syntax::ExecutableDefinition; +use graphql_syntax::ExecutableDocument; use intern::Lookup; +use itertools::Itertools; use lsp_types::request::CodeActionRequest; use lsp_types::request::Request; use lsp_types::CodeAction; use lsp_types::CodeActionOrCommand; use lsp_types::Diagnostic; +use lsp_types::NumberOrString; use lsp_types::Position; use lsp_types::Range; use lsp_types::TextDocumentPositionParams; @@ -33,9 +40,12 @@ use resolution_path::IdentParent; use resolution_path::IdentPath; use resolution_path::OperationDefinitionPath; use resolution_path::ResolutionPath; +use resolution_path::ResolveDefinition; use resolution_path::ResolvePosition; use serde_json::Value; +use self::create_fragment_argument::create_fragment_argument_code_action; +use self::create_operation_variable::create_operation_variable_code_action; use crate::lsp_runtime_error::LSPRuntimeError; use crate::lsp_runtime_error::LSPRuntimeResult; use crate::server::GlobalState; @@ -51,54 +61,95 @@ pub(crate) fn on_code_action( return Err(LSPRuntimeError::ExpectedError); } - if let Some(diagnostic) = state.get_diagnostic_for_range(&uri, params.range) { - let code_actions = get_code_actions_from_diagnostics(&uri, diagnostic); - if code_actions.is_some() { - return Ok(code_actions); - } - } - - let definitions = state.resolve_executable_definitions(¶ms.text_document.uri)?; - let text_document_position_params = TextDocumentPositionParams { text_document: params.text_document, position: params.range.start, }; - let (document, position_span) = + let (document, location) = state.extract_executable_document_from_text(&text_document_position_params, 1)?; - let path = document.resolve((), position_span); + if let Some(diagnostic) = state.get_diagnostic_for_range(&uri, params.range) { + let code_actions = + get_code_actions_from_diagnostic(&uri, diagnostic, &document, &location, state); + if code_actions.is_some() { + return Ok(code_actions); + } + } + let path = document.resolve((), location.span()); + let definitions = state.resolve_executable_definitions(&uri)?; let used_definition_names = get_definition_names(&definitions); - let result = get_code_actions(path, used_definition_names, uri, params.range) - .ok_or(LSPRuntimeError::ExpectedError)?; - Ok(Some(result)) + + get_code_actions(path, used_definition_names, uri, params.range) + .map(|code_actions| Some(code_actions)) + .ok_or(LSPRuntimeError::ExpectedError) } -fn get_code_actions_from_diagnostics( +fn get_code_actions_from_diagnostic( url: &Url, diagnostic: Diagnostic, + document: &ExecutableDocument, + location: &Location, + state: &impl GlobalState, ) -> Option> { - let code_actions = if let Some(Value::Array(data)) = &diagnostic.data { - data.iter() - .filter_map(|item| match item { - Value::String(suggestion) => Some(create_code_action( - "Fix Error", - suggestion.to_string(), - url, - diagnostic.range, + match diagnostic { + Diagnostic { + code: + Some(NumberOrString::Number( + ValidationDiagnosticCode::EXPECTED_OPERATION_VARIABLE_TO_BE_DEFINED + | ValidationDiagnosticCode::UNDEFINED_VARIABLE_REFERENCED, )), - _ => None, - }) - .collect::<_>() - } else { - vec![] - }; + data: Some(Value::Array(array_data)), + .. + } => { + let definition = document.find_definition(location.span())?; - if !code_actions.is_empty() { - Some(code_actions) - } else { - None + match &array_data[..] { + [Value::String(variable_name), Value::String(variable_type)] => match definition { + ExecutableDefinition::Operation(operation) => { + create_operation_variable_code_action( + operation, + variable_name, + variable_type, + location, + state, + url, + ) + .and_then(|code_action| Some(vec![code_action])) + } + ExecutableDefinition::Fragment(fragment) => { + create_fragment_argument_code_action( + fragment, + variable_name, + variable_type, + location, + state, + url, + ) + .and_then(|code_action| Some(vec![code_action])) + } + }, + _ => None, + } + } + Diagnostic { + data: Some(Value::Array(array_data)), + .. + } => Some( + array_data + .iter() + .filter_map(|item| match item { + Value::String(suggestion) => Some(create_code_action( + "Fix Error", + suggestion.to_string(), + url, + diagnostic.range, + )), + _ => None, + }) + .collect_vec(), + ), + _ => None, } } @@ -244,53 +295,3 @@ fn create_code_action( ..Default::default() }) } - -#[cfg(test)] -mod tests { - use lsp_types::CodeActionOrCommand; - use lsp_types::Diagnostic; - use lsp_types::Position; - use lsp_types::Range; - use lsp_types::Url; - use serde_json::json; - - use crate::code_action::get_code_actions_from_diagnostics; - - #[test] - fn test_get_code_actions_from_diagnostics() { - let diagnostic = Diagnostic { - range: Range { - start: Position { - line: 0, - character: 0, - }, - end: Position { - line: 0, - character: 0, - }, - }, - message: "Error Message".to_string(), - data: Some(json!(vec!["item1", "item2"])), - ..Default::default() - }; - let url = Url::parse("file://relay.js").unwrap(); - let code_actions = get_code_actions_from_diagnostics(&url, diagnostic); - - assert_eq!( - code_actions - .unwrap() - .iter() - .map(|item| { - match item { - CodeActionOrCommand::CodeAction(action) => action.title.clone(), - _ => panic!("unexpected case"), - } - }) - .collect::>(), - vec![ - "Fix Error: 'item1'".to_string(), - "Fix Error: 'item2'".to_string(), - ] - ); - } -} diff --git a/compiler/crates/relay-lsp/src/code_action/create_fragment_argument.rs b/compiler/crates/relay-lsp/src/code_action/create_fragment_argument.rs new file mode 100644 index 0000000000000..3add01c482827 --- /dev/null +++ b/compiler/crates/relay-lsp/src/code_action/create_fragment_argument.rs @@ -0,0 +1,93 @@ +use std::collections::HashMap; + +use common::Location; +use common::Span; +use docblock_shared::ARGUMENT_DEFINITIONS; +use lsp_types::CodeAction; +use lsp_types::CodeActionOrCommand; +use lsp_types::TextEdit; +use lsp_types::Url; +use lsp_types::WorkspaceEdit; + +use crate::GlobalState; + +pub fn create_fragment_argument_code_action( + fragment: &graphql_syntax::FragmentDefinition, + variable_name: &str, + variable_type: &str, + location: &Location, + state: &impl GlobalState, + url: &Url, +) -> Option { + create_fragment_argument_text_edit(fragment, variable_name, variable_type, location, state).map( + |text_edit| { + let mut changes = HashMap::new(); + changes.insert(url.to_owned(), vec![text_edit]); + + CodeActionOrCommand::CodeAction(CodeAction { + title: format!("Create fragment argument '${}'", variable_name), + kind: Some(lsp_types::CodeActionKind::QUICKFIX), + diagnostics: None, + edit: Some(WorkspaceEdit { + changes: Some(changes), + document_changes: None, + ..Default::default() + }), + command: None, + is_preferred: Some(true), + ..Default::default() + }) + }, + ) +} + +fn create_fragment_argument_text_edit( + fragment: &graphql_syntax::FragmentDefinition, + variable_name: &str, + variable_type: &str, + location: &Location, + state: &impl GlobalState, +) -> Option { + if let Some(argument_definitions_directive) = fragment + .directives + .iter() + .find(|directive| directive.name.value == ARGUMENT_DEFINITIONS.0) + { + argument_definitions_directive + .arguments + .as_ref() + .and_then(|arguments| arguments.items.last()) + .and_then(|argument| { + state + .get_lsp_location(location.with_span(Span { + start: argument.span.end, + end: argument.span.end, + })) + .ok() + }) + .map(|lsp_location| TextEdit { + range: lsp_location.range, + new_text: format!( + ", {name}: {{ type: \"{type}\" }}", + name = variable_name, + type = variable_type + ), + }) + } else { + state + .get_lsp_location(location.with_span(Span { + start: fragment.type_condition.span.end, + end: fragment.type_condition.span.end, + })) + .ok() + .map(|lsp_location| TextEdit { + range: lsp_location.range, + new_text: format!( + " @{directive_name}({name}: {{ type: \"{type}\" }})", + directive_name = ARGUMENT_DEFINITIONS.0, + name = variable_name, + type = variable_type + ), + }) + } +} diff --git a/compiler/crates/relay-lsp/src/code_action/create_operation_variable.rs b/compiler/crates/relay-lsp/src/code_action/create_operation_variable.rs new file mode 100644 index 0000000000000..3df7bd16bcc4e --- /dev/null +++ b/compiler/crates/relay-lsp/src/code_action/create_operation_variable.rs @@ -0,0 +1,90 @@ +use std::collections::HashMap; + +use common::Location; +use common::Span; +use lsp_types::CodeAction; +use lsp_types::CodeActionOrCommand; +use lsp_types::TextEdit; +use lsp_types::Url; +use lsp_types::WorkspaceEdit; + +use crate::GlobalState; + +pub fn create_operation_variable_code_action( + operation: &graphql_syntax::OperationDefinition, + variable_name: &str, + variable_type: &str, + location: &Location, + state: &impl GlobalState, + url: &Url, +) -> Option { + create_operation_variable_text_edit(operation, variable_name, variable_type, location, state) + .map(|text_edit| { + let mut changes = HashMap::new(); + changes.insert(url.to_owned(), vec![text_edit]); + + CodeActionOrCommand::CodeAction(CodeAction { + title: format!("Create operation variable '${}'", variable_name), + kind: Some(lsp_types::CodeActionKind::QUICKFIX), + diagnostics: None, + edit: Some(WorkspaceEdit { + changes: Some(changes), + document_changes: None, + ..Default::default() + }), + command: None, + is_preferred: Some(true), + ..Default::default() + }) + }) +} + +fn create_operation_variable_text_edit( + operation: &graphql_syntax::OperationDefinition, + variable_name: &str, + variable_type: &str, + location: &Location, + state: &impl GlobalState, +) -> Option { + if operation.variable_definitions.is_none() { + operation + .name + .and_then(|operation_name| { + state + .get_lsp_location(location.with_span(Span { + start: operation_name.span.end, + end: operation_name.span.end, + })) + .ok() + }) + .map(|lsp_location| TextEdit { + range: lsp_location.range, + new_text: format!( + "(${name}: {type})", + name = variable_name, + type = variable_type + ), + }) + } else { + operation + .variable_definitions + .as_ref() + .and_then(|variable_definitions| variable_definitions.items.last()) + .and_then(|last_variable| { + state + .get_lsp_location(location.with_span(Span { + start: last_variable.span.end, + end: last_variable.span.end, + })) + .ok() + }) + .map(|lsp_location| TextEdit { + range: lsp_location.range, + new_text: format!( + ", ${name}: {type}", + name = variable_name, + type = variable_type + ), + }) + } +} diff --git a/compiler/crates/relay-lsp/src/completion.rs b/compiler/crates/relay-lsp/src/completion.rs index 3f09db9a8c1b1..981244e891bb7 100644 --- a/compiler/crates/relay-lsp/src/completion.rs +++ b/compiler/crates/relay-lsp/src/completion.rs @@ -1312,13 +1312,13 @@ pub fn on_completion( params: ::Params, ) -> LSPRuntimeResult<::Result> { match state.extract_executable_document_from_text(¶ms.text_document_position, 0) { - Ok((document, position_span)) => { + Ok((document, location)) => { let project_name = state .extract_project_name_from_url(¶ms.text_document_position.text_document.uri)?; let schema = &state.get_schema(&project_name)?; let items = resolve_completion_items( document, - position_span, + location.span(), project_name, schema, state.get_schema_documentation(project_name.lookup()), diff --git a/compiler/crates/relay-lsp/src/diagnostic_reporter.rs b/compiler/crates/relay-lsp/src/diagnostic_reporter.rs index 9cb87c3b041ed..7b948dde597d3 100644 --- a/compiler/crates/relay-lsp/src/diagnostic_reporter.rs +++ b/compiler/crates/relay-lsp/src/diagnostic_reporter.rs @@ -210,7 +210,7 @@ impl DiagnosticReporter { .collect::>(); Diagnostic { - code: None, + code: diagnostic.code(), data: get_diagnostics_data(diagnostic), message: diagnostic.message().to_string(), range: text_source.to_span_range(diagnostic.location().span()), diff --git a/compiler/crates/relay-lsp/src/find_field_usages.rs b/compiler/crates/relay-lsp/src/find_field_usages.rs index 70af7ad77d10d..292bf42c57ca8 100644 --- a/compiler/crates/relay-lsp/src/find_field_usages.rs +++ b/compiler/crates/relay-lsp/src/find_field_usages.rs @@ -31,7 +31,6 @@ use schema::Type; use serde::Deserialize; use serde::Serialize; -use crate::location::transform_relay_location_to_lsp_location; use crate::server::GlobalState; use crate::LSPRuntimeError; use crate::LSPRuntimeResult; @@ -80,13 +79,12 @@ pub fn on_find_field_usages( let schema = state.get_schema(&schema_name)?; let program = state.get_program(&schema_name)?; - let root_dir = &state.root_dir(); let ir_locations = get_usages(&program, &schema, type_name, field_name)?; let lsp_locations = ir_locations .into_iter() .map(|(label, ir_location)| { - let lsp_location = transform_relay_location_to_lsp_location(root_dir, ir_location)?; + let lsp_location = state.get_lsp_location(ir_location)?; Ok(FindFieldUsageResultItem { location_uri: lsp_location.uri.to_string(), location_range: lsp_location.range, diff --git a/compiler/crates/relay-lsp/src/goto_definition.rs b/compiler/crates/relay-lsp/src/goto_definition.rs index ddab94daba668..04b67260f3646 100644 --- a/compiler/crates/relay-lsp/src/goto_definition.rs +++ b/compiler/crates/relay-lsp/src/goto_definition.rs @@ -31,7 +31,6 @@ use serde::Serialize; use self::goto_docblock_definition::get_docblock_definition_description; use self::goto_graphql_definition::get_graphql_definition_description; -use crate::location::transform_relay_location_to_lsp_location; use crate::lsp_runtime_error::LSPRuntimeError; use crate::lsp_runtime_error::LSPRuntimeResult; use crate::server::GlobalState; @@ -70,7 +69,7 @@ pub fn on_goto_definition( state: &impl GlobalState, params: ::Params, ) -> LSPRuntimeResult<::Result> { - let (feature, position_span) = + let (feature, location) = state.extract_feature_from_text(¶ms.text_document_position_params, 1)?; let project_name = state @@ -80,15 +79,14 @@ pub fn on_goto_definition( let definition_description = match feature { crate::Feature::GraphQLDocument(document) => { - get_graphql_definition_description(document, position_span, &schema)? + get_graphql_definition_description(document, location.span(), &schema)? } crate::Feature::DocblockIr(docblock_ir) => { - get_docblock_definition_description(&docblock_ir, position_span)? + get_docblock_definition_description(&docblock_ir, location.span())? } }; let extra_data_provider = state.get_extra_data_provider(); - let root_dir = state.root_dir(); let goto_definition_response: GotoDefinitionResponse = match definition_description { DefinitionDescription::FieldArgument { @@ -100,14 +98,12 @@ pub fn on_goto_definition( parent_type, field_name, argument_name, - &root_dir, + state, )?, DefinitionDescription::DirectiveArgument { directive_name, argument_name, - } => { - locate_directive_argument_definition(&schema, directive_name, argument_name, &root_dir)? - } + } => locate_directive_argument_definition(&schema, directive_name, argument_name, state)?, DefinitionDescription::Field { parent_type, field_name, @@ -117,20 +113,16 @@ pub fn on_goto_definition( field_name, extra_data_provider, project_name, - &root_dir, + state, )?, DefinitionDescription::Fragment { fragment_name } => { - locate_fragment_definition(program, fragment_name, &root_dir)? + locate_fragment_definition(program, fragment_name, state)? + } + DefinitionDescription::Type { type_name } => { + locate_type_definition(extra_data_provider, project_name, type_name, &schema, state)? } - DefinitionDescription::Type { type_name } => locate_type_definition( - extra_data_provider, - project_name, - type_name, - &schema, - &root_dir, - )?, DefinitionDescription::Directive { directive_name } => { - locate_directive_definition(directive_name, &schema, &root_dir)? + locate_directive_definition(directive_name, &schema, state)? } }; @@ -148,7 +140,7 @@ pub fn on_goto_definition( fn locate_fragment_definition( program: graphql_ir::Program, fragment_name: FragmentDefinitionName, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> Result { let fragment = program.fragment(fragment_name).ok_or_else(|| { LSPRuntimeError::UnexpectedError(format!( @@ -156,22 +148,24 @@ fn locate_fragment_definition( fragment_name )) })?; - Ok(GotoDefinitionResponse::Scalar( - transform_relay_location_to_lsp_location(root_dir, fragment.name.location)?, - )) + + state + .get_lsp_location(fragment.name.location) + .map(GotoDefinitionResponse::Scalar) } fn locate_directive_definition( directive_name: DirectiveName, schema: &Arc, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> Result { let directive = schema.get_directive(directive_name); directive .map(|directive| directive.name.location) .map(|schema_location| { - transform_relay_location_to_lsp_location(root_dir, schema_location) + state + .get_lsp_location(schema_location) .map(GotoDefinitionResponse::Scalar) }) .ok_or(LSPRuntimeError::ExpectedError)? @@ -182,7 +176,7 @@ fn locate_type_definition( project_name: StringKey, type_name: StringKey, schema: &Arc, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> Result { let provider_response = extra_data_provider.resolve_field_definition( project_name.to_string(), @@ -222,7 +216,8 @@ fn locate_type_definition( Type::Object(object_id) => schema.object(object_id).name.location, }) .map(|schema_location| { - transform_relay_location_to_lsp_location(root_dir, schema_location) + state + .get_lsp_location(schema_location) .map(GotoDefinitionResponse::Scalar) }) .ok_or(LSPRuntimeError::ExpectedError)? @@ -235,7 +230,7 @@ fn locate_field_argument_definition( parent_type: Type, field_name: StringKey, argument_name: ArgumentName, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> Result { let field = schema.field(schema.named_field(parent_type, field_name).ok_or_else(|| { LSPRuntimeError::UnexpectedError(format!("Could not find field with name {}", field_name)) @@ -252,15 +247,16 @@ fn locate_field_argument_definition( )) })?; - transform_relay_location_to_lsp_location(root_dir, argument.name.location) - .map(|location| Ok(GotoDefinitionResponse::Scalar(location)))? + state + .get_lsp_location(argument.name.location) + .map(GotoDefinitionResponse::Scalar) } fn locate_directive_argument_definition( schema: &SDLSchema, directive_name: DirectiveName, argument_name: ArgumentName, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> LSPRuntimeResult { let directive = schema @@ -281,8 +277,9 @@ fn locate_directive_argument_definition( )) })?; - transform_relay_location_to_lsp_location(root_dir, argument.name.location) - .map(|location| Ok(GotoDefinitionResponse::Scalar(location)))? + state + .get_lsp_location(argument.name.location) + .map(GotoDefinitionResponse::Scalar) } fn locate_field_definition( @@ -291,7 +288,7 @@ fn locate_field_definition( field_name: StringKey, extra_data_provider: &dyn LSPExtraDataProvider, project_name: StringKey, - root_dir: &std::path::Path, + state: &impl GlobalState, ) -> Result { let field = schema.field(schema.named_field(parent_type, field_name).ok_or_else(|| { LSPRuntimeError::UnexpectedError(format!("Could not find field with name {}", field_name,)) @@ -334,10 +331,9 @@ fn locate_field_definition( } } - transform_relay_location_to_lsp_location(root_dir, field.name.location) + state + .get_lsp_location(field.name.location) .map(GotoDefinitionResponse::Scalar) - // If the field does not exist in the schema, that's fine - .map_err(|_| LSPRuntimeError::ExpectedError) } fn get_location(path: &str, line: u64) -> Result { diff --git a/compiler/crates/relay-lsp/src/hover.rs b/compiler/crates/relay-lsp/src/hover.rs index c0ea6d36ed953..46b42419f7535 100644 --- a/compiler/crates/relay-lsp/src/hover.rs +++ b/compiler/crates/relay-lsp/src/hover.rs @@ -67,10 +67,10 @@ pub fn on_hover( state: &impl GlobalState, params: ::Params, ) -> LSPRuntimeResult<::Result> { - let (document, position_span) = + let (document, location) = state.extract_executable_document_from_text(¶ms.text_document_position_params, 1)?; - let resolution_path = document.resolve((), position_span); + let resolution_path = document.resolve((), location.span()); let project_name = state .extract_project_name_from_url(¶ms.text_document_position_params.text_document.uri)?; diff --git a/compiler/crates/relay-lsp/src/location.rs b/compiler/crates/relay-lsp/src/location.rs index 4b741b980b61b..20e0f84d5bd39 100644 --- a/compiler/crates/relay-lsp/src/location.rs +++ b/compiler/crates/relay-lsp/src/location.rs @@ -11,6 +11,8 @@ use std::path::PathBuf; use common::Location; use common::SourceLocationKey; use common::TextSource; +use dashmap::DashMap; +use extract_graphql::JavaScriptSourceFeature; use intern::Lookup; use lsp_types::Url; @@ -22,6 +24,7 @@ use crate::lsp_runtime_error::LSPRuntimeResult; pub fn transform_relay_location_to_lsp_location( root_dir: &Path, location: Location, + synced_javascript_features: &DashMap>, ) -> LSPRuntimeResult { match location.source_location() { SourceLocationKey::Standalone { path } => { @@ -36,19 +39,28 @@ pub fn transform_relay_location_to_lsp_location( Ok(lsp_types::Location { uri, range }) } SourceLocationKey::Embedded { path, index } => { - let path_to_fragment = root_dir.join(PathBuf::from(path.lookup())); - let uri = get_uri(&path_to_fragment)?; + let file_path = root_dir.join(PathBuf::from(path.lookup())); + let uri = get_uri(&file_path)?; - let file_contents = get_file_contents(&path_to_fragment)?; + let features = match synced_javascript_features.get(&uri) { + Some(features) => Ok(features.clone()), + None => { + let file_contents = get_file_contents(&file_path)?; - let response = extract_graphql::extract(&file_contents); - let response_length = response.len(); - let embedded_source = response.into_iter().nth(index.into()).ok_or_else(|| { - LSPRuntimeError::UnexpectedError(format!( - "File {:?} does not contain enough graphql literals: {} needed; {} found", - path_to_fragment, index, response_length - )) - })?; + Ok(extract_graphql::extract(&file_contents)) + } + }?; + + let features_length = features.len(); + let embedded_source = features + .into_iter() + .nth(index.try_into().unwrap()) + .ok_or_else(|| { + LSPRuntimeError::UnexpectedError(format!( + "File {:?} does not contain enough graphql literals: {} needed; {} found", + file_path, index, features_length + )) + })?; let text_source = embedded_source.text_source(); let range = text_source.to_span_range(location.span()); diff --git a/compiler/crates/relay-lsp/src/references.rs b/compiler/crates/relay-lsp/src/references.rs index 0bc4bd35dbf63..4b81a731fdda1 100644 --- a/compiler/crates/relay-lsp/src/references.rs +++ b/compiler/crates/relay-lsp/src/references.rs @@ -7,8 +7,6 @@ //! Utilities for providing the goto definition feature -use std::path::Path; - use common::Location as IRLocation; use graphql_ir::FragmentDefinitionName; use graphql_ir::FragmentSpread; @@ -25,7 +23,6 @@ use schema::Schema; use crate::docblock_resolution_info::DocblockResolutionInfo; use crate::find_field_usages::find_field_locations; use crate::find_field_usages::get_usages; -use crate::location::transform_relay_location_to_lsp_location; use crate::lsp_runtime_error::LSPRuntimeError; use crate::lsp_runtime_error::LSPRuntimeResult; use crate::node_resolution_info::NodeKind; @@ -35,7 +32,7 @@ use crate::FeatureResolutionInfo; fn get_references_response( feature_resolution_info: FeatureResolutionInfo, program: &Program, - root_dir: &Path, + state: &impl GlobalState, ) -> LSPRuntimeResult> { match feature_resolution_info { FeatureResolutionInfo::GraphqlNode(node_resolution_info) => { @@ -44,9 +41,7 @@ fn get_references_response( let references = ReferenceFinder::get_references_to_fragment(program, fragment.name.value) .into_iter() - .map(|location| { - transform_relay_location_to_lsp_location(root_dir, location) - }) + .map(|location| state.get_lsp_location(location)) .collect::, LSPRuntimeError>>()?; Ok(references) @@ -64,9 +59,7 @@ fn get_references_response( let lsp_locations = get_usages(program, &program.schema, type_name, field_name)? .into_iter() - .map(|(_, ir_location)| { - transform_relay_location_to_lsp_location(root_dir, ir_location) - }) + .map(|(_, ir_location)| state.get_lsp_location(ir_location)) .collect::, LSPRuntimeError>>()?; Ok(lsp_locations) } @@ -94,7 +87,7 @@ fn get_references_response( 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)) + .map(|location| state.get_lsp_location(location)) .collect::, LSPRuntimeError>>()?; Ok(references) @@ -135,6 +128,7 @@ impl Visitor for ReferenceFinder { } } +/// Resolve a [`References`] request to a list of locations pub fn on_references( state: &impl GlobalState, params: ::Params, @@ -146,7 +140,7 @@ pub fn on_references( .get_program(&state.extract_project_name_from_url( ¶ms.text_document_position.text_document.uri, )?)?, - &state.root_dir(), + state, )?; Ok(Some(references_response)) } diff --git a/compiler/crates/relay-lsp/src/server/lsp_state.rs b/compiler/crates/relay-lsp/src/server/lsp_state.rs index 73273b75d088a..b0ce5de13192c 100644 --- a/compiler/crates/relay-lsp/src/server/lsp_state.rs +++ b/compiler/crates/relay-lsp/src/server/lsp_state.rs @@ -8,9 +8,9 @@ use std::path::PathBuf; use std::sync::Arc; +use common::Location; use common::PerfLogger; use common::SourceLocationKey; -use common::Span; use crossbeam::channel::SendError; use crossbeam::channel::Sender; use dashmap::mapref::entry::Entry; @@ -52,6 +52,7 @@ use super::task_queue::TaskScheduler; use crate::diagnostic_reporter::DiagnosticReporter; use crate::docblock_resolution_info::create_docblock_resolution_info; use crate::graphql_tools::get_query_text; +use crate::location::transform_relay_location_to_lsp_location; use crate::lsp_runtime_error::LSPRuntimeResult; use crate::node_resolution_info::create_node_resolution_info; use crate::utils::extract_executable_definitions_from_text_document; @@ -93,13 +94,13 @@ pub trait GlobalState { &self, position: &TextDocumentPositionParams, index_offset: usize, - ) -> LSPRuntimeResult<(ExecutableDocument, Span)>; + ) -> LSPRuntimeResult<(ExecutableDocument, Location)>; fn extract_feature_from_text( &self, position: &TextDocumentPositionParams, index_offset: usize, - ) -> LSPRuntimeResult<(Feature, Span)>; + ) -> LSPRuntimeResult<(Feature, Location)>; fn get_schema_documentation(&self, schema_name: &str) -> Self::TSchemaDocumentation; @@ -135,6 +136,8 @@ pub trait GlobalState { /// we may need to know who's our current consumer. /// This is mostly for hover handler (where we render markup) fn get_content_consumer_type(&self) -> ContentConsumerType; + + fn get_lsp_location(&self, location: Location) -> LSPRuntimeResult; } /// This structure contains all available resources that we may use in the Relay LSP message/notification @@ -399,7 +402,7 @@ impl LSPRuntimeResult { - let (feature, position_span) = self.extract_feature_from_text( + let (feature, location) = self.extract_feature_from_text( text_document_position, // For hovering, offset the index by 1 // ``` @@ -413,10 +416,10 @@ impl FeatureResolutionInfo::GraphqlNode( - create_node_resolution_info(executable_document, position_span)?, + create_node_resolution_info(executable_document, location.span())?, ), Feature::DocblockIr(docblock_ir) => FeatureResolutionInfo::DocblockNode(DocblockNode { - resolution_info: create_docblock_resolution_info(&docblock_ir, position_span) + resolution_info: create_docblock_resolution_info(&docblock_ir, location.span()) .ok_or(LSPRuntimeError::ExpectedError)?, ir: docblock_ir, }), @@ -430,10 +433,10 @@ impl LSPRuntimeResult<(ExecutableDocument, Span)> { - let (feature, span) = self.extract_feature_from_text(position, index_offset)?; + ) -> LSPRuntimeResult<(ExecutableDocument, Location)> { + let (feature, location) = self.extract_feature_from_text(position, index_offset)?; match feature { - Feature::GraphQLDocument(document) => Ok((document, span)), + Feature::GraphQLDocument(document) => Ok((document, location)), Feature::DocblockIr(_) => Err(LSPRuntimeError::ExpectedError), } } @@ -444,7 +447,7 @@ impl LSPRuntimeResult<(Feature, Span)> { + ) -> LSPRuntimeResult<(Feature, Location)> { let project_name: ProjectName = self .extract_project_name_from_url(&position.text_document.uri)? .into(); @@ -566,6 +569,16 @@ impl ContentConsumerType { ContentConsumerType::Relay } + + fn get_lsp_location(&self, location: Location) -> LSPRuntimeResult { + let root_dir = &self.root_dir(); + + transform_relay_location_to_lsp_location( + root_dir, + location, + &self.synced_javascript_features, + ) + } } #[derive(Debug)] diff --git a/compiler/crates/relay-lsp/src/utils.rs b/compiler/crates/relay-lsp/src/utils.rs index 0d45499a7cbc2..3d1fac9d90ff7 100644 --- a/compiler/crates/relay-lsp/src/utils.rs +++ b/compiler/crates/relay-lsp/src/utils.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; +use common::Location; use common::SourceLocationKey; use common::Span; use common::TextSource; @@ -119,7 +120,7 @@ pub fn extract_feature_from_text( source_feature_cache: &DashMap>, text_document_position: &TextDocumentPositionParams, index_offset: usize, -) -> LSPRuntimeResult<(Feature, Span)> { +) -> LSPRuntimeResult<(Feature, Location)> { let uri = &text_document_position.text_document.uri; let position = text_document_position.position; @@ -136,7 +137,7 @@ pub fn extract_feature_from_text( }) .ok_or(LSPRuntimeError::ExpectedError)?; - let source_location_key = SourceLocationKey::embedded(uri.as_ref(), index); + let source_location_key = SourceLocationKey::embedded(uri.path(), index); let parser_features = get_parser_features(project_config); @@ -167,7 +168,10 @@ pub fn extract_feature_from_text( // since the change event fires before completion. debug!("position_span: {:?}", position_span); - Ok((Feature::GraphQLDocument(document), position_span)) + Ok(( + Feature::GraphQLDocument(document), + Location::new(source_location_key, position_span), + )) } JavaScriptSourceFeature::Docblock(docblock_source) => { let executable_definitions_in_file = extract_executable_definitions_from_text_document( @@ -213,7 +217,10 @@ pub fn extract_feature_from_text( ) })?; - Ok((Feature::DocblockIr(docblock_ir), position_span)) + Ok(( + Feature::DocblockIr(docblock_ir), + Location::new(source_location_key, position_span), + )) } } } diff --git a/compiler/crates/relay-transforms/src/validations/validate_global_variables.rs b/compiler/crates/relay-transforms/src/validations/validate_global_variables.rs index 977323ec34ac8..d70b5f3efbcad 100644 --- a/compiler/crates/relay-transforms/src/validations/validate_global_variables.rs +++ b/compiler/crates/relay-transforms/src/validations/validate_global_variables.rs @@ -11,9 +11,11 @@ use common::NamedItem; use graphql_ir::FragmentDefinition; use graphql_ir::OperationDefinition; use graphql_ir::Program; -use graphql_ir::ValidationMessage; +use graphql_ir::ValidationDiagnosticCode; +use graphql_ir::ValidationMessageWithData; use graphql_ir::Validator; -use intern::Lookup; +use itertools::Itertools; +use schema::Schema; use crate::root_variables::InferVariablesVisitor; use crate::DIRECTIVE_SPLIT_OPERATION; @@ -24,12 +26,14 @@ pub fn validate_global_variables(program: &Program) -> DiagnosticsResult<()> { pub struct ValidateGlobalVariables<'program> { visitor: InferVariablesVisitor<'program>, + program: &'program Program, } impl<'program> ValidateGlobalVariables<'program> { fn new(program: &'program Program) -> Self { Self { visitor: InferVariablesVisitor::new(program), + program, } } } @@ -62,29 +66,20 @@ impl Validator for ValidateGlobalVariables<'_> { .collect(); undefined_variables.sort_by(|a, b| a.name.cmp(&b.name)); if !undefined_variables.is_empty() { - let is_plural = undefined_variables.len() > 1; - let mut locations = undefined_variables + return Err(undefined_variables .iter() - .map(|arg_def| arg_def.name.location); - let mut error = Diagnostic::error( - ValidationMessage::GlobalVariables { - operation_name: operation.name.item.0, - variables_string: format!( - "{}: '${}'", - if is_plural { "s" } else { "" }, - undefined_variables - .iter() - .map(|var| var.name.item.0.lookup()) - .collect::>() - .join("', '$"), - ), - }, - locations.next().unwrap(), - ); - for related_location in locations { - error = error.annotate("related location", related_location); - } - return Err(vec![error]); + .map(|variable| { + Diagnostic::error_with_data_and_code( + ValidationDiagnosticCode::UNDEFINED_VARIABLE_REFERENCED, + ValidationMessageWithData::UndefinedVariableReferenced { + operation_name: operation.name.item.0, + variable_name: variable.name.item, + variable_type: self.program.schema.get_type_string(&variable.type_), + }, + variable.name.location, + ) + }) + .collect_vec()); } Ok(()) } diff --git a/compiler/crates/resolution-path/src/lib.rs b/compiler/crates/resolution-path/src/lib.rs index 05c3b9743368c..180e7723ca7e9 100644 --- a/compiler/crates/resolution-path/src/lib.rs +++ b/compiler/crates/resolution-path/src/lib.rs @@ -196,6 +196,18 @@ pub trait ResolvePosition<'a>: Sized { } } +pub trait ResolveDefinition { + fn find_definition(&self, position_span: Span) -> Option<&ExecutableDefinition>; +} + +impl ResolveDefinition for ExecutableDocument { + fn find_definition(&self, position_span: Span) -> Option<&ExecutableDefinition> { + self.definitions + .iter() + .find(|def| def.contains(position_span)) + } +} + pub type ExecutableDocumentPath<'a> = Path<&'a ExecutableDocument, ()>; // Clippy gets grumpy about us passing around `()`, but we need it for consistency with other implementations of `ResolvePosition`.