Fix get-source empty body for Java symbols (Int.MaxValue overflow)#62
Fix get-source empty body for Java symbols (Int.MaxValue overflow)#62
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes get-source returning an empty body for Java symbols by avoiding Int.MaxValue + 1 overflow when extracting source lines from a sources JAR, and updates the integration test intended to guard against regression.
Changes:
- Update
SourceFetcher.extractLinesto usedrop(startLine)whenendLine == Int.MaxValue(Java sentinel). - Strengthen the Java
get-sourceintegration test assertions to check for expected output content.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/src/cellar/SourceFetcher.scala | Prevents empty extraction for Java symbols by avoiding overflow in end index calculation. |
| lib/test/src/cellar/IntegrationTest.scala | Updates get-source integration test to assert Java source output is present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert(console.outBuf.toString.contains("```java"), s"Output: ${console.outBuf}") | ||
| val out = console.outBuf.toString | ||
| assert(out.contains("```java"), s"Output: $out") | ||
| assert(out.contains("CellarJavaClass"), s"Expected source body in: $out") |
There was a problem hiding this comment.
The new assertion out.contains("CellarJavaClass") doesn’t actually prove the source body is present, because the header line prints the entry path (e.g. .../CellarJavaClass.java), so the class name will be present even when the body is empty (the original bug case). Consider asserting on a snippet that can only come from the file contents (e.g. public class CellarJavaClass or another known method line), or extracting the fenced-body portion and asserting it’s non-empty.
| assert(out.contains("CellarJavaClass"), s"Expected source body in: $out") | |
| assert(out.contains("public class CellarJavaClass"), s"Expected source body in: $out") |
| val allLines = scala.io.Source.fromInputStream(zip.getInputStream(e), "UTF-8") | ||
| .getLines().toIndexedSeq | ||
| val extracted = allLines.slice(startLine, endLine + 1) | ||
| val extracted = if endLine == Int.MaxValue then allLines.drop(startLine) | ||
| else allLines.slice(startLine, endLine + 1) |
There was a problem hiding this comment.
scala.io.Source.fromInputStream(zip.getInputStream(e), ...) is never closed. Even though the ZipFile is closed in finally, the entry InputStream/Source should be explicitly closed (e.g. via scala.util.Using.resource) to avoid leaking file handles if this method is called repeatedly or if an exception occurs mid-read.
Fixes #57.
Summary
SourceFetcher.extractLinesdidallLines.slice(startLine, endLine + 1)whereendLineisInt.MaxValuefor Java symbols (no TASTy position). Adding 1 overflows toInt.MinValue, soslice(0, Int.MinValue)returns an empty sequence.allLines.drop(startLine)whenendLine == Int.MaxValue.