Fix MultiSourceReader compatibility with Java 16+ - #3670
Conversation
Sorry, something went wrong.
| } | ||
| LINE_NUMBER_READER_EOS_IS_TERMINATOR = reader.getLineNumber() > 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
Clever!
Sorry, something went wrong.
bbakerman
left a comment
There was a problem hiding this comment.
Thanks for this contribution
Sorry, something went wrong.
| throw new UncheckedIOException(e); | ||
| } | ||
| LINE_NUMBER_READER_EOS_IS_TERMINATOR = reader.getLineNumber() > 0; | ||
| } |
There was a problem hiding this comment.
Minor pick - I would encapsulate this into a static method
So you can do
private static final boolean LINE_NUMBER_READER_EOS_IS_TERMINATOR = determineStuff();
Sorry, something went wrong.
|
Thanks for this PR! Leaving a link for later, this issue was reported a while back #2709 with a related suggestion to run tests on different Java versions to catch these issues earlier #2789. Might be time to look into different Java versions for our tests again. Thanks! |
Sorry, something went wrong.
This is a fix for #2709
LineNumberReader was updated in Java 16 to consider EOS a line terminator, so if a source doesn't end in newline, getLineNumber() will return 1 + the value in older Java versions. We ran into this issue at Airbnb where we use MultiSourceReader to read graphqls files. If a schema file didn't end in newline, the SourceLocation for some types would be incorrect. This PR attempts to address this issue.
Testing:
I ran the unit tests with Java 11 and Java 20 (this required bumping the language version in build.gradle) with and without this fix. Without this fix, LineNumberingTest, MultiSourceReaderTest, ParserExceptionTest, TypeDefinitionRegistryTest fail with Java 20 as described in the open issue. With this fix, they succeed.
This also fixed the bug we ran into at Airbnb.