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

Consider re-introducing lints to apollo-rs #751

Open
SimonSapin opened this issue Nov 17, 2023 · 3 comments
Open

Consider re-introducing lints to apollo-rs #751

SimonSapin opened this issue Nov 17, 2023 · 3 comments

Comments

@SimonSapin
Copy link
Contributor

In apollo-compiler 0.11 validation returns a Vec that can contain:

  • Any number of error-level diagnostics
  • A warning for enum value definitions with non-uppercase names
  • An "advice" for custom scalar definitions without an @specifiedBy directive

To find out if a document is valid, callers need to iterate the vec and find if any of the diagnostic return true for .is_error().

In apollo-compiler 1.0 (in beta as of this writing) the API changes so that the library does that filtering and returns Result::Err for invalid documents. However as discussed starting at #709 (comment) returning potential warnings and advice for a valid document makes the API awkward. Since they arguably don’t belong in validate() in the first place, they could be moved to a separate lint() method.

But with only two rules, this linter would not be very valuable and we don’t expect many users to go out of their way to call it explicitly.

An upcoming will therefore remove these two non-error rules from apollo-compiler entirely. This issue keeps a record of this regression of functionality. These two rule could be restored one day if we want to seriously work on having a GraphQL linter as part of apollo-rs.

@goto-bus-stop
Copy link
Member

goto-bus-stop commented Aug 21, 2024

Here is something that would be a useful "warning" lint while it's not a spec validation error: graphql/graphql-spec#1053

Defining an interface without defining any implementers could also be a warning.

@SimonSapin
Copy link
Contributor Author

Isn't the linked PR changing the spec to make it a validation error? It would only be a warning until the spec change is adopted (assuming it is)

@goto-bus-stop
Copy link
Member

goto-bus-stop commented Aug 22, 2024

yes! i meant "while" as in, "during the time that" it's not an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants