-
Notifications
You must be signed in to change notification settings - Fork 6
Properly Handle Diagnostics From Plugins #776
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5a5ec35
f774231
9aaaf28
28f5564
4564c97
2e57578
981e0eb
343d967
be01c42
3280340
f359a61
6bf2002
71225fc
6cfca7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,13 +3,25 @@ | |
| [cs::identifier("ZeroC.Slice.Symbols.Compiler")] | ||
| module Compiler | ||
|
|
||
| /// Represents the source of a {@link Diagnostic}, which can be used to provide context and snippets during emission. | ||
| /// | ||
| /// The source of a diagnostic can be either: | ||
| /// - A reference to a Slice entity, written as its fully-scoped identifier; ex: 'MyModule::MyInterface::myOp'. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you give a example where a pluhin might want to emit an error for a identifier? In my view the generator would only validate attributes which can be specific to a plug-in, other diagnostics are handled by slicec and are plugin agnostic. I guess if we can need it if we end-up not supporting some Slice features in a generator.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yeah, this is more about future proofing. But, I think there is already one possible use case. Typealias is a first class Slice feature, and we'll generate code for it in Rust, Swift, and Python. |
||
| /// - A reference to a Slice file, written as that file's relative path prefixed with '#'; ex: '#dir1/dir2/file.slice'. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can use file URIs here. For example: EDIT: see other comment.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could, but I'd rather avoid using URI-like syntax, unless the thing is an actual URI, and here it isn't. I'm open to replacements for '#', but I don't like using 'file://' for something which isn't actually a URI. |
||
| /// | ||
| /// To reference an attribute on a Slice entity or file, you can use '$attributes' after the source, followed by the | ||
| /// index of the attribute to reference, and optionally, the index of an argument to that attribute. For example: | ||
| /// 'MyMod::MyEnum::$attributes::0' references the first attribute of 'MyEnum', and '#MyFile.slice::$attributes::0::1' | ||
| /// references the second argument of the first attribute on 'MyFile.slice'. | ||
| typealias DiagnosticSource = string | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you reference an attribute for an operation parameter, or return, does this work?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this already works. This doesn't have to be a For example, if you wanted to report the 4th attribute of a parameter, you'd do: This also works for return values, with one special case. |
||
|
|
||
| /// A message that is reported to provide information and context for errors, warnings, or other issues. | ||
| struct Diagnostic { | ||
| /// The exact kind of diagnostic. | ||
| kind: DiagnosticKind | ||
|
|
||
| /// The fully-scoped Slice identifier of the element that this diagnostic is in reference to (if there is one). | ||
| source: string? | ||
| source: DiagnosticSource? | ||
|
|
||
| /// Any additional information that should be reported alongside this diagnostic's main message. | ||
| notes: Sequence<DiagnosticNote> | ||
|
|
@@ -21,7 +33,7 @@ struct DiagnosticNote { | |
| message: string | ||
|
|
||
| /// The fully-scoped Slice identifier of the element that this note is in reference to (if there is one). | ||
| source: string? | ||
| source: DiagnosticSource? | ||
| } | ||
|
|
||
| unchecked enum DiagnosticKind { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,3 +366,20 @@ pub enum LookupError { | |
| is_concrete: bool, | ||
| }, | ||
| } | ||
|
|
||
| impl From<LookupError> for crate::diagnostics::Error { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a helper to make it easier to convert from |
||
| fn from(error: LookupError) -> Self { | ||
| match error { | ||
| LookupError::DoesNotExist { identifier } => Self::DoesNotExist { identifier }, | ||
| LookupError::TypeMismatch { | ||
| expected, | ||
| actual, | ||
| is_concrete, | ||
| } => Self::TypeMismatch { | ||
| expected, | ||
| actual, | ||
| is_concrete, | ||
| }, | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,34 @@ impl<'a> TryFrom<&'a Node> for WeakPtr<dyn Type> { | |
| } | ||
| } | ||
|
|
||
| impl<'a> TryFrom<&'a Node> for &'a dyn NamedSymbol { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This allows us to write functions for, and search the AST for (entities + Module), since modules are a little different and special. We used to have this function a while ago, but then it was unused, so we removed it, but we once again need it to look up diagnostic sources in the AST (since modules can be referenced by sources). |
||
| type Error = LookupError; | ||
|
|
||
| /// Attempts to unwrap a node to a dynamically typed reference of a Slice [NamedSymbol]. | ||
| /// | ||
| /// If the Slice element held by the node implements [NamedSymbol], this succeeds and returns a typed reference, | ||
| /// otherwise this fails and returns an error message. | ||
| fn try_from(node: &'a Node) -> Result<&'a dyn NamedSymbol, Self::Error> { | ||
| match node { | ||
| Node::Module(module_ptr) => Ok(module_ptr.borrow()), | ||
| Node::Struct(struct_ptr) => Ok(struct_ptr.borrow()), | ||
| Node::Field(field_ptr) => Ok(field_ptr.borrow()), | ||
| Node::Interface(interface_ptr) => Ok(interface_ptr.borrow()), | ||
| Node::Operation(operation_ptr) => Ok(operation_ptr.borrow()), | ||
| Node::Parameter(parameter_ptr) => Ok(parameter_ptr.borrow()), | ||
| Node::Enum(enum_ptr) => Ok(enum_ptr.borrow()), | ||
| Node::Enumerator(enumerator_ptr) => Ok(enumerator_ptr.borrow()), | ||
| Node::CustomType(custom_type_ptr) => Ok(custom_type_ptr.borrow()), | ||
| Node::TypeAlias(type_alias_ptr) => Ok(type_alias_ptr.borrow()), | ||
| _ => Err(LookupError::TypeMismatch { | ||
| expected: "named symbol".to_owned(), | ||
| actual: ccase!(lower, node.to_string()), | ||
| is_concrete: false, | ||
| }), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<'a> TryFrom<&'a Node> for &'a dyn Entity { | ||
| type Error = LookupError; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ impl CompilationState { | |
| /// | ||
| /// # Safety | ||
| /// | ||
| /// The caller of this function must ensure that no (`WeakPtr`s)[crate::utils::ptr_util::WeakPtr] exist that point | ||
| /// The caller of this function must ensure that no [`WeakPtr`](crate::utils::ptr_util::WeakPtr)s exist that point | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got the Markdown |
||
| /// to the contents of this `CompilationState`. Even if they're not being actively used, their existence causes UB. | ||
| pub unsafe fn apply_unsafe(&mut self, function: unsafe fn(&mut Self)) { | ||
| if !self.diagnostics.has_errors() { | ||
|
|
@@ -52,7 +52,7 @@ impl CompilationState { | |
| pub fn get_annotated_diagnostics(&self, options: &SliceOptions) -> Vec<AnnotatedDiagnostic> { | ||
| let mut annotated_diagnostics = Vec::new(); | ||
| for diagnostic in &*self.diagnostics { | ||
| let converted = crate::diagnostics::convert_diagnostic(diagnostic, options, self); | ||
| let converted = crate::diagnostics::convert_diagnostic(diagnostic, options, &self.ast, &self.files); | ||
| if !annotated_diagnostics.contains(&converted) { | ||
| annotated_diagnostics.push(converted); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -405,6 +405,8 @@ pub enum DiagnosticKind { | |
| impl DecodeFrom for DiagnosticKind { | ||
| fn decode_from(decoder: &mut Decoder<impl InputSource>) -> Result<Self> { | ||
| let value: usize = decoder.decode_varint()?; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a straight-up bug in my implementation. Now we always decode this size, after decoding the discriminant. Even if it's only used in the case of an |
||
| let payload_size = decoder.decode_size()?; | ||
|
|
||
|
InsertCreativityHere marked this conversation as resolved.
|
||
| let variant = match value { | ||
| 0 => Self::Info { | ||
| message: decoder.decode()?, | ||
|
|
@@ -441,7 +443,6 @@ impl DecodeFrom for DiagnosticKind { | |
| n => { | ||
| // Read the unknown variant's fields payload. | ||
| // We don't know what they are, but we at least know the length of the payload. | ||
| let payload_size = decoder.decode_size()?; | ||
| let mut fields_payload = vec![0; payload_size]; | ||
| decoder.read_bytes_into_exact(&mut fields_payload)?; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| // Copyright (c) ZeroC, Inc. | ||
|
|
||
| use crate::compilation_state::CompilationState; | ||
| use crate::ast::Ast; | ||
| use crate::diagnostics::{Diagnostic, DiagnosticKind, DiagnosticLevel, Lint}; | ||
| use crate::grammar::{attributes, Attributable, Entity}; | ||
| use crate::slice_file::{SliceFile, Span}; | ||
|
|
@@ -34,18 +34,19 @@ pub struct Snippet { | |
| pub fn convert_diagnostic( | ||
| diagnostic: &Diagnostic, | ||
| options: &SliceOptions, | ||
| compilation_state: &CompilationState, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of passing the entire |
||
| ast: &Ast, | ||
| files: &[SliceFile], | ||
| ) -> AnnotatedDiagnostic { | ||
| let notes = diagnostic.notes.iter().map(|n| AnnotatedNote { | ||
| message: n.message.clone(), | ||
| snippet: get_snippet(&n.span, &compilation_state.files), | ||
| snippet: get_snippet(&n.span, files), | ||
| }); | ||
|
|
||
| AnnotatedDiagnostic { | ||
| message: diagnostic.message(), | ||
| level: get_diagnostic_level_for(diagnostic, options, compilation_state), | ||
| level: get_diagnostic_level_for(diagnostic, options, ast, files), | ||
| code: diagnostic.code().to_owned(), | ||
| snippet: get_snippet(&diagnostic.span, &compilation_state.files), | ||
| snippet: get_snippet(&diagnostic.span, files), | ||
| notes: notes.collect(), | ||
| } | ||
| } | ||
|
|
@@ -54,7 +55,8 @@ pub fn convert_diagnostic( | |
| fn get_diagnostic_level_for( | ||
| diagnostic: &Diagnostic, | ||
| options: &SliceOptions, | ||
| compilation_state: &CompilationState, | ||
| ast: &Ast, | ||
| files: &[SliceFile], | ||
| ) -> DiagnosticLevel { | ||
| // Only lints can have their diagnostic levels changed (through attributes or command-line options). | ||
| // For other kinds of diagnostics, we can immediately return their levels. | ||
|
|
@@ -83,15 +85,15 @@ fn get_diagnostic_level_for( | |
|
|
||
| // If the diagnostic has a span, check if it's affected by an `allow` attribute on its file. | ||
| if let Some(span) = &diagnostic.span { | ||
| let file = compilation_state.files.iter().find(|f| f.relative_path == span.file); | ||
| let file = files.iter().find(|f| f.relative_path == span.file); | ||
| if is_lint_allowed_by_attributes(file.unwrap(), lint) { | ||
| return DiagnosticLevel::Allowed; | ||
| } | ||
| } | ||
|
|
||
| // If the diagnostic has a scope, check if it's affected by an `allow` attribute in that scope. | ||
| if let Some(scope) = &diagnostic.scope { | ||
| if let Ok(entity) = compilation_state.ast.find_element::<dyn Entity>(scope) { | ||
| if let Ok(entity) = ast.find_element::<dyn Entity>(scope) { | ||
| if is_lint_allowed_by_attributes(entity, lint) { | ||
| return DiagnosticLevel::Allowed; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -387,9 +387,8 @@ impl Error { | |
| if expected == 0 { | ||
| format!("attribute '{directive}' does not take any arguments, but {args_provided}") | ||
| } else { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This caused us to emit messages like: |
||
| let only = if expected > *actual_count { "only " } else { "" }; | ||
| let args = if expected == 1 { "argument" } else { "arguments" }; | ||
| format!("attribute '{directive}' takes exactly {expected} {args}, but {only}{args_provided}") | ||
| format!("attribute '{directive}' takes exactly {expected} {args}, but {args_provided}") | ||
| } | ||
| } else if expected_count.end == usize::MAX { | ||
| // If the range has no upper bound, then this attribute only requires a minimum number of arguments. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ pub trait ScopedSymbol: Symbol { | |
| fn get_raw_scope(&self) -> &Scope; | ||
| } | ||
|
|
||
| pub trait NamedSymbol: Symbol { | ||
| pub trait NamedSymbol: Symbol + Attributable { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It has always been the case that all Perhaps some future refactoring around modules, or how module attributes work can let us once again remove this coupling. |
||
| fn identifier(&self) -> &str; | ||
| fn raw_identifier(&self) -> &Identifier; | ||
| fn module_scoped_identifier(&self) -> String; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,8 +10,11 @@ use clap::Parser; | |
| use slice_codec::decoder::Decoder; | ||
| use slice_codec::encoder::Encoder; | ||
|
|
||
| use slicec::ast::Ast; | ||
| use slicec::compilation_state::CompilationState; | ||
| use slicec::diagnostic_emitter::DiagnosticEmitter; | ||
| use slicec::diagnostics::Diagnostics; | ||
| use slicec::slice_file::SliceFile; | ||
| use slicec::slice_options::{DiagnosticFormat, Plugin, SliceOptions}; | ||
|
|
||
| mod definition_types; | ||
|
|
@@ -50,6 +53,8 @@ fn encode_generate_code_request(parsed_files: &[slicec::slice_file::SliceFile]) | |
| Ok(encoding_buffer) | ||
| } | ||
|
|
||
| /// Spawns (and starts) a subprocess to run the provided plugin, and writes the provided payload to its 'stdin', | ||
| /// followed by any plugin arguments. | ||
| fn spawn_plugin_process(plugin: &Plugin, slice_payload: &[u8]) -> std::io::Result<Child> { | ||
| // Spawn a new subprocess and set up pipes for all of its streams. | ||
| let mut subprocess = Command::new(&plugin.path) | ||
|
|
@@ -75,21 +80,21 @@ fn spawn_plugin_process(plugin: &Plugin, slice_payload: &[u8]) -> std::io::Resul | |
| Ok(subprocess) | ||
| } | ||
|
|
||
| /// Runs the provided subprocess to completion, handling any failures that may occur during its execution. | ||
| /// | ||
| /// If the subprocess completes successfully, this returns `Ok` with its response payload. | ||
| /// Otherwise, if the subprocess fails to complete, completed with a non-zero status code, or wrote to 'stderr', | ||
| /// this returns an `Err` describing the failure. | ||
| fn collect_plugin_output(subprocess: Child) -> std::io::Result<Vec<u8>> { | ||
|
InsertCreativityHere marked this conversation as resolved.
|
||
| // Wait until the subprocess finishes, then retrieve its output. | ||
| let output = subprocess.wait_with_output()?; | ||
|
|
||
| // If the subprocess wrote anything to its 'stderr', we consider this a failure and don't generate any code. | ||
| if !output.stderr.is_empty() { | ||
| // Obtain an exclusive handle to this process's 'stderr'. | ||
| let mut stderr = std::io::stderr().lock(); | ||
|
|
||
| // Pipe the output from the subprocess's 'stderr' to this process's 'stderr', and then return. | ||
| let error = match stderr.write_all(&output.stderr) { | ||
| Ok(_) => Error::other("errors reported on 'stderr'"), | ||
| Err(err) => Error::new(ErrorKind::BrokenPipe, err), | ||
| }; | ||
| return Err(error); | ||
| // TODO: switch to 'from_utf8_lossy_owned' when stabilized (https://github.com/rust-lang/rust/issues/129436). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better move to GitHub issue
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'll definitely open an issue, but there's no point opening it unless/until this PR is merged. |
||
| let mut error_string = String::from_utf8_lossy(&output.stderr).into_owned(); | ||
| error_string.insert_str(0, "errors reported on 'stderr':\n"); | ||
| return Err(Error::other(error_string)); | ||
| } | ||
|
|
||
| // Otherwise, check the subprocess's status code to determine success. | ||
|
|
@@ -105,39 +110,33 @@ fn collect_plugin_output(subprocess: Child) -> std::io::Result<Vec<u8>> { | |
| } | ||
| } | ||
|
|
||
| fn handle_generator_response(response_payload: Vec<u8>, output_dir: &Option<String>) -> std::io::Result<Diagnostics> { | ||
| /// Decodes a response from a code-generator plugin. | ||
| /// | ||
| /// If the response could be successfully decoded, this returns the generated files and diagnostics contained in the | ||
| /// response, converted to a form `slicec` can utilize. Otherwise this returns an `Err` describing the failure. | ||
| fn decode_generator_response( | ||
| response_payload: Vec<u8>, | ||
| ast: &Ast, | ||
| files: &[SliceFile], | ||
| ) -> std::io::Result<(Vec<definition_types::GeneratedFile>, Diagnostics)> { | ||
| // Decode the generator's response. It consists of 2 sequences, one of generated files and one of diagnostics. | ||
| let mut slice_decoder = Decoder::from(&response_payload); | ||
| let generated_files: Vec<definition_types::GeneratedFile> = slice_decoder.decode()?; | ||
| let generator_diagnostics: Vec<definition_types::Diagnostic> = slice_decoder.decode()?; | ||
|
|
||
| // TODO: Convert the diagnostics we decode from the generator, into diagnostics that slicec can handle. | ||
| // To do this requires re-working the diagnostic API fairly substantially. | ||
| // Take all of the decoded diagnostics from the generator, and convert them into diagnostics that slicec can handle. | ||
| let mut converted_diagnostics = Diagnostics::new(); | ||
| for generator_diagnostic in generator_diagnostics { | ||
| println!("{generator_diagnostic:?}"); | ||
| } | ||
| let mut diagnostics = Diagnostics::new(); | ||
|
|
||
| // If no errors were reported, attempt to generate the files in the response. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function was renamed from |
||
| if !diagnostics.has_errors() { | ||
| for generated_file in &generated_files { | ||
| // Try to write the generated file to disk. | ||
| if let Err(io_error) = write_generated_file(generated_file, output_dir) { | ||
| // If an error occurred during writing the file, create a diagnostic that slicec can emit. | ||
| let diagnostic = slicec::diagnostics::Error::IO { | ||
| action: "write generated file", | ||
| path: generated_file.path.to_owned(), | ||
| error: io_error, | ||
| }; | ||
| slicec::diagnostics::Diagnostic::from_error(diagnostic).push_into(&mut diagnostics); | ||
| } | ||
| } | ||
| slice_file_converter::convert_diagnostic(generator_diagnostic, ast, files, &mut converted_diagnostics); | ||
| } | ||
|
|
||
| // Return any decoded diagnostics, so slicec can emit them at the end. | ||
| Ok(diagnostics) | ||
| Ok((generated_files, converted_diagnostics)) | ||
| } | ||
|
|
||
| /// Attempts to write a generated file to disk. | ||
| /// | ||
| /// If an output directory was specified, the generated file will be written relative to it; Otherwise, the generated | ||
| /// file will be written relative to the current working directory. | ||
| fn write_generated_file( | ||
| generated_file: &definition_types::GeneratedFile, | ||
| output_dir: &Option<String>, | ||
|
|
@@ -164,11 +163,24 @@ fn write_generated_file( | |
| Ok(()) | ||
| } | ||
|
|
||
| fn convert_generator_error_to_diagnostic(generator: &Plugin, generator_error: std::io::Error) -> Diagnostics { | ||
| /// Converts an [`std::io::Error`] that occurred while trying to write a generated file into a | ||
| /// [`Diagnostic`](slicec::diagnostics::Diagnostic), and pushes it into the provided [`Diagnostics`] collection. | ||
| fn report_file_writing_error(file_path: &String, io_error: std::io::Error, diagnostics: &mut Diagnostics) { | ||
| let diagnostic = slicec::diagnostics::Error::IO { | ||
| action: "write generated file", | ||
| path: file_path.to_owned(), | ||
| error: io_error, | ||
| }; | ||
| slicec::diagnostics::Diagnostic::from_error(diagnostic).push_into(diagnostics); | ||
| } | ||
|
|
||
| /// Converts any [`std::io::Error`]s that occurred while trying to run a plugin into | ||
| /// [`Diagnostic`](slicec::diagnostics::Diagnostic)s which can be emitted by the compiler. | ||
| fn convert_generator_errors_to_diagnostics(generator: &Plugin, io_error: std::io::Error) -> Diagnostics { | ||
| let mapped_io_error = slicec::diagnostics::Error::IO { | ||
| action: "run code-generator", | ||
| path: generator.path.clone(), | ||
| error: generator_error, | ||
| error: io_error, | ||
| }; | ||
|
|
||
| let mut diagnostics = Diagnostics::new(); | ||
|
|
@@ -180,14 +192,18 @@ fn main() -> ExitCode { | |
| // Parse the command-line input. | ||
| let slice_options = SliceOptions::parse(); | ||
|
|
||
| // Perform the compilation. | ||
| // Compile the provided Slice files. | ||
| let mut compilation_state = slicec::compile_from_options(&slice_options); | ||
| let diagnostics = &mut compilation_state.diagnostics; | ||
| let CompilationState { | ||
| ref ast, | ||
| ref mut diagnostics, | ||
| ref files, | ||
| } = compilation_state; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Destructuring assignment, now that we need the |
||
|
|
||
| // Only invoke the plugins if there were no errors in the Slice files. | ||
| if !diagnostics.has_errors() { | ||
| // Encode the request which will be sent to each of the code-generation plugins. | ||
| let encoded_request = match encode_generate_code_request(&compilation_state.files) { | ||
| let encoded_request = match encode_generate_code_request(files) { | ||
| Ok(result) => result, | ||
| Err(error) => { | ||
| eprintln!("Critical error: failed to encode request payload!\n{error:?}"); | ||
|
|
@@ -205,12 +221,26 @@ fn main() -> ExitCode { | |
| // we get the response payload from it, write any generated files in the payload, and store any diagnostics | ||
| // the generator reported so we can emit them at the end along with all the others. | ||
| for (generator, generator_process) in generator_processes { | ||
| let generator_diagnostics = generator_process | ||
| .and_then(collect_plugin_output) // Returns the response payload if the generator ran successfully. | ||
| .and_then(|payload| handle_generator_response(payload, &slice_options.output_dir)) // Returns any diagnostics if the payload successfully decoded. | ||
| .unwrap_or_else(|err| convert_generator_error_to_diagnostic(generator, err)); | ||
| let (generated_files, mut generator_diagnostics) = generator_process | ||
| // 'collect_plugin_output' returns the response payload if the generator ran successfully. | ||
| .and_then(collect_plugin_output) | ||
| // 'decode_generator_response' decodes the payload into a Vec<GeneratedFile> and a Vec<Diagnostic>. | ||
| .and_then(|payload| decode_generator_response(payload, ast, files)) | ||
| // Convert any unexpected errors into reportable diagnostics. | ||
| .unwrap_or_else(|err| (Vec::new(), convert_generator_errors_to_diagnostics(generator, err))); | ||
|
|
||
| // If the generator didn't report any errors, write the generated files. | ||
| if !generator_diagnostics.has_errors() { | ||
| for generated_file in &generated_files { | ||
| if let Err(io_error) = write_generated_file(generated_file, &slice_options.output_dir) { | ||
| // If an error occurred while writing the file, report the error as a slicec diagnostic. | ||
| report_file_writing_error(&generated_file.path, io_error, &mut generator_diagnostics); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| diagnostics.extend(generator_diagnostics.into_inner()); // Store generator's diagnostics for later emission. | ||
| // Store generator's diagnostics for later emission. | ||
| diagnostics.extend(generator_diagnostics.into_inner()); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a new
Sourcetype to communicate "this can't be any random string", and so we have a place to document the structure of it.