apollographql / apollo-rs

Spec compliant GraphQL Tools in Rust.
Apache License 2.0
566 stars 43 forks source link

fix(parser): add missing location information for some lexer errors #886

Closed PhoebeSzmucer closed 1 month ago

PhoebeSzmucer commented 1 month ago

Fixes https://github.com/apollographql/apollo-rs/issues/731

I think these are all the missing locations, but there are a few more places that do Error::new - as far as I can tell though these errors are getting location information filled in at a later stage (via self.done()).

I saw the original issue proposed removing Error::new completely - I'm happy to do it, but it would be a break, and I'm not sure what the policy on those is in this repo.

apollo-cla commented 1 month ago

@PhoebeSzmucer: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

goto-bus-stop commented 1 month ago

Also, totally fine to remove Error::new entirely. We are still working on the apollo-rs v1 release and we are regularly making breaking changes in apollo-compiler until then.

PhoebeSzmucer commented 1 month ago

@goto-bus-stop I removed Error::new entirely. There is only Error::with_loc left - I wonder if it should become the new Error::new 🤔