twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 408 forks source link

build errors with JDK 9 #444

Closed christhalinger closed 6 years ago

christhalinger commented 6 years ago

Some method usages are ambiguous with 9.

Expected behavior

No build failures.

Actual behavior

[error] /home/opc/finatra/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/ByteBufferUtils.scala:39:50: ambiguous reference to overloaded definition,
[error] both method position in class ByteBuffer of type (x$1: Int)java.nio.ByteBuffer
[error] and  method position in class Buffer of type ()Int
[error] match expected type Any
[error]       debug(s"byteBuffer: $str pos: ${byteBuffer.position}")
[error]                                                  ^
[error] /home/opc/finatra/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayChunker.scala:75:26: ambiguous reference to overloaded definition,
[error] both method position in class ByteBuffer of type (x$1: Int)java.nio.ByteBuffer
[error] and  method position in class Buffer of type ()Int
[error] match expected type ?
[error]       else if (in.get(in.position - 2) != '\\') {
[error]                          ^
[error] /home/opc/finatra/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayChunker.scala:86:27: ambiguous reference to overloaded definition,
[error] both method position in class ByteBuffer of type (x$1: Int)java.nio.ByteBuffer
[error] and  method position in class Buffer of type ()Int
[error] match expected type ?
[error]     copy.limit(byteBuffer.position - 1)
[error]                           ^

Steps to reproduce the behavior

Run sbt with JDK 9, e.g.:

./sbt helloWorld/assembly
christhalinger commented 6 years ago

I think the fix is just this:

diff --git a/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/ByteBufferUtils.scala b/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/ByteBufferU
index 59ccaff..de61053 100644
--- a/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/ByteBufferUtils.scala
+++ b/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/ByteBufferUtils.scala
@@ -36,7 +36,7 @@ private[finatra] object ByteBufferUtils extends Logging {
       val buf = Buf.ByteBuffer.Shared(copy)
       val str = buf.utf8str

-      debug(s"byteBuffer: $str pos: ${byteBuffer.position}")
+      debug(s"byteBuffer: $str pos: ${byteBuffer.position()}")
     }
   }
 }
diff --git a/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayChunker.scala b/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayC
index 35f3ded..4ad4526 100644
--- a/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayChunker.scala
+++ b/jackson/src/main/scala/com/twitter/finatra/json/internal/streaming/JsonArrayChunker.scala
@@ -72,7 +72,7 @@ private[finatra] class JsonArrayChunker extends Logging {
         parsingState = InsideString
       }
       // If the double quote wasn't escaped then this is the end of a string.
-      else if (in.get(in.position - 2) != '\\') {
+      else if (in.get(in.position() - 2) != '\\') {
         debug("State = Parsing")
         parsingState = Normal
       }
@@ -83,7 +83,7 @@ private[finatra] class JsonArrayChunker extends Logging {
   private def extractBuf(): Buf = {
     val copy = byteBuffer.duplicate()
     copy.position(0)
-    copy.limit(byteBuffer.position - 1)
+    copy.limit(byteBuffer.position() - 1)
     val copyBuf = Buf.ByteBuffer.Shared(copy)

     byteBuffer = byteBuffer.slice()

but I'm by no means an expert.

dschobel commented 6 years ago

@christhalinger it's not obvious to me why java9 overriding ByteBuffer#position​(int newPosition) would confuse usage of the non-parameterized version but your fix looks semantically correct so if that resolves it, let's go with that.

want to post a PR?

christhalinger commented 6 years ago

I have no idea. Yes, I'll post a PR.

cacoco commented 6 years ago

Fixed merged in 65561afe9d6695c9e0d0b06bc59efca1ed4d68c8.