Skip to content
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

Emit YAML processing errors that Xcode can display #81

Conversation

macblazer
Copy link
Contributor

Motivation

If there are any errors in the openapi.yaml file currently the errors show up in the build transcript but are not shown in the Xcode issue sidebar and are also not linked to the openapi.yaml file in the sidebar. This makes it hard to know what has failed, and what to do to fix the file.

Modifications

The YAML parser now catches decoding errors, and if they are common parsing or scanning errors it will emit a diagnostic message then throw a failure exit code. This required importing the swift-argument-parser module into the main _OpenAPIGeneratorCore.

Added absoluteFilePath and lineNumber optional properties to the Diagnostics struct to capture more info. Modified the Diagnostics.description method to build a properly formatted message that Xcode can parse to point out the file and line causing issues.

Result

In consuming projects, invalid yaml in the openapi.yaml file is now highlighted in the Xcode Issue Navigator after a build with this plugin.

Test Plan

Added unit tests to verify emitted diagnostic and thrown error. Also tested manually with an example project.

Resolves

macblazer and others added 2 commits June 20, 2023 00:05
### Motivation

If there are any errors in the openapi.yaml file currently the errors show up in the build transcript
but are not shown in the Xcode issue sidebar and are also not linked to the openapi.yaml file in the sidebar.
This makes it hard to know what has failed, and what to do to fix the file.

### Modifications

The YAML parser now catches decoding errors, and if they are common parsing or scanning errors it will emit
a diagnostic message then throw a failure exit code.  This required importing the swift-argument-parser module into
the main _OpenAPIGeneratorCore.

Added `absoluteFilePath` and `lineNumber` optional properties to the `Diagnostics` struct to capture more info.
Modified the `Diagnostics.description` method to build a properly formatted message that Xcode can parse to point
out the file and line causing issues.

### Result

In consuming projects, invalid yaml in the openapi.yaml file is now highlighted in the Xcode Issue Navigator
after a build with this plugin.

### Test Plan

Added unit tests to verify emitted diagnostic and thrown error.  Also tested manually with an example project.

### Resolves

- Resolves apple#65.

Signed-off-by: Kyle Hammond <[email protected]>
@macblazer
Copy link
Contributor Author

I'm not particularly happy with adding ArgumentParser as a dependency into the _OpenAPIGeneratorCore.

Without it, any thrown error is caught by ArgumentParser and the thrown error's localizedDescription is used with a prefix of "Error: " which completely blows away any other diagnostic that I was able to emit.

The code now throws an ArgumentParser.ExitCode.failure after emitting a diagnostic with the proper information for Xcode to pick up the filepath and line number. Using ArgumentParser.ExitCode was the only way I found to have the correct diagnostic message.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for working on it, @macblazer! I think we can do this without the swift-argument-parser dependency, so I added a few comments.

Package.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Diagnostics.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Diagnostics.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift Outdated Show resolved Hide resolved
Sources/_OpenAPIGeneratorCore/Parser/YamsParser.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Just added a few requests for missing documentation comments, otherwise it's almost ready to go.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@czechboy0 czechboy0 requested a review from simonjbeaumont June 22, 2023 05:32
@czechboy0
Copy link
Contributor

@swift-server-bot add to allowlist

@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jun 22, 2023
@simonjbeaumont simonjbeaumont changed the title Emit yaml processing errors that Xcode can display (#65) Emit YAML processing errors that Xcode can display Jun 22, 2023
Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @macblazer. This is a great step forward, here ❤️

@simonjbeaumont simonjbeaumont merged commit a7b8348 into apple:main Jun 22, 2023
@macblazer macblazer deleted the 65-emit-xcode-errors-while-parsing-openapi-yaml branch June 22, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emit errors processing .yaml files that Xcode can display
4 participants