Skip to content

Commit

Permalink
Report schema parsing errors for synced schema sources (#4719)
Browse files Browse the repository at this point in the history
Summary:
This adds support for diagnostics while typing (without saving) within schema documents.

Currently it's only reporting diagnostics for parsing errors, but it can be extended with actual validation of schema semantics in the future.

Pull Request resolved: #4719

Reviewed By: gordyf

Differential Revision: D60786428

Pulled By: captbaritone

fbshipit-source-id: 8543f0d8d2abbfa385fea263c5c5fbcd6036ef43
  • Loading branch information
tobias-tengler authored and facebook-github-bot committed Aug 29, 2024
1 parent 51af829 commit eec9602
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 9 deletions.
7 changes: 7 additions & 0 deletions compiler/crates/common/src/text_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,13 @@ impl TextSource {
character += 1;
}
}

if start_position != lsp_types::Position::default()
&& end_position == lsp_types::Position::default()
{
end_position = lsp_types::Position::new(line as u32, character as u32);
}

lsp_types::Range::new(start_position, end_position)
}
}
Expand Down
68 changes: 61 additions & 7 deletions compiler/crates/relay-lsp/src/server/lsp_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,45 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
Ok(())
}

fn validate_synced_schema_source(&self, url: &Url) -> LSPRuntimeResult<()> {
let schema_source = self.synced_schema_sources.get(url).ok_or_else(|| {
LSPRuntimeError::UnexpectedError(format!("Expected schema source for URL {}", url))
})?;
let project_name = self.extract_project_name_from_url(url)?;
let project_config = self
.config
.projects
.get(&ProjectName::from(project_name))
.unwrap();

if project_config.feature_flags.disable_schema_validation {
return Ok(());
}

let source_location_key = SourceLocationKey::standalone(url.as_ref());

let mut diagnostics = vec![];
let text_source = schema_source.text_source();
let result = graphql_syntax::parse_schema_document(&text_source.text, source_location_key);

match result {
// In the future you could run additional validations based on the new schema document here
Ok(_) => (),
Err(raw_diagnostics) => {
diagnostics.extend(raw_diagnostics.iter().map(|diagnostic| {
// TODO: The very last character is problematic for some reason
self.diagnostic_reporter
.convert_diagnostic(text_source, diagnostic)
}));
}
}

self.diagnostic_reporter
.update_quick_diagnostics_for_url(url, diagnostics);

Ok(())
}

fn preload_documentation(&self) {
for project_config in self.config.enabled_projects() {
self.get_schema_documentation(&project_config.name.to_string());
Expand Down Expand Up @@ -355,8 +394,15 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
.insert(url.clone(), graphql_source);
}

fn process_synced_schema_sources(&self, uri: &Url, graphql_source: GraphQLSource) {
self.insert_synced_schema_source(uri, graphql_source);
self.schedule_task(Task::ValidateSchemaSource(uri.clone()));
}

fn remove_synced_schema_source(&self, url: &Url) {
self.synced_schema_sources.remove(url);
self.diagnostic_reporter
.clear_quick_diagnostics_for_url(url);
}

fn initialize_lsp_state_resources(&self, project_name: StringKey) {
Expand Down Expand Up @@ -540,7 +586,7 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
match file_group {
FileGroup::Schema { project_set: _ } | FileGroup::Extension { project_set: _ } => {
self.initialize_lsp_state_resources(project_name);
self.insert_synced_schema_source(uri, GraphQLSource::new(text, 0, 0));
self.process_synced_schema_sources(uri, GraphQLSource::new(text, 0, 0));

Ok(())
}
Expand All @@ -567,7 +613,7 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio

match file_group {
FileGroup::Schema { project_set: _ } | FileGroup::Extension { project_set: _ } => {
self.insert_synced_schema_source(uri, GraphQLSource::new(text, 0, 0));
self.process_synced_schema_sources(uri, GraphQLSource::new(text, 0, 0));

Ok(())
}
Expand Down Expand Up @@ -631,8 +677,9 @@ pub fn build_ir_for_lsp(

#[derive(Debug)]
pub enum Task {
ValidateSyncedDocuments,
ValidateSyncedSource(Url),
ValidateSyncedSources,
ValidateSchemaSource(Url),
}

pub(crate) fn handle_lsp_state_tasks<
Expand All @@ -643,13 +690,20 @@ pub(crate) fn handle_lsp_state_tasks<
task: Task,
) {
match task {
Task::ValidateSyncedSource(url) => {
state.validate_synced_js_sources(&url).ok();
}
Task::ValidateSyncedSources => {
Task::ValidateSyncedDocuments => {
for item in &state.synced_javascript_sources {
state.schedule_task(Task::ValidateSyncedSource(item.key().clone()));
}

for item in &state.synced_schema_sources {
state.schedule_task(Task::ValidateSchemaSource(item.key().clone()));
}
}
Task::ValidateSyncedSource(url) => {
state.validate_synced_js_sources(&url).ok();
}
Task::ValidateSchemaSource(url) => {
state.validate_synced_schema_source(&url).ok();
}
}
}
4 changes: 2 additions & 2 deletions compiler/crates/relay-lsp/src/server/lsp_state_resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,8 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
base_fragment_names,
} = get_project_asts(&schema, graphql_asts_map, project_config)?;

// This will kick-off the validation for all synced sources
self.lsp_state.schedule_task(Task::ValidateSyncedSources);
// This will kick-off the validation of all synced documents
self.lsp_state.schedule_task(Task::ValidateSyncedDocuments);

self.build_programs(
project_config,
Expand Down

0 comments on commit eec9602

Please sign in to comment.