In onLine(n), produce a sensible exception message if n is out of range. Currently you get an ArrayIndexOutOfBoundsException.

eamonnmcmanus · nick-someone · commit b4eaa92f272e · 2020-04-01T12:48:43.000-04:00
RELNOTES=Better exception in onLine(n) when n is out of range. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=304030110
diff --git a/pom.xml b/pom.xml
@@ -50,7 +50,7 @@
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
-      <version>4.12</version>
+      <version>4.13</version>
     </dependency>
     <dependency>
       <groupId>com.google.truth</groupId>
diff --git a/src/main/java/com/google/testing/compile/CompilationSubject.java b/src/main/java/com/google/testing/compile/CompilationSubject.java
@@ -466,9 +466,12 @@ ImmutableList<String> linesInFile() {
 
     /** Lists the line at a line number (1-based). */
     String listLine(long lineNumber) {
-      return lineNumber == Diagnostic.NOPOS
-          ? "(no associated line)"
-          : String.format("%4d: %s", lineNumber, linesInFile().get((int) (lineNumber - 1)));
+      if (lineNumber == Diagnostic.NOPOS) {
+        return "(no associated line)";
+      }
+      checkArgument(lineNumber > 0 && lineNumber <= linesInFile().size(),
+          "Invalid line number %s; number of lines is only %s", lineNumber, linesInFile().size());
+      return String.format("%4d: %s", lineNumber, linesInFile().get((int) (lineNumber - 1)));
     }
   }
 
diff --git a/src/test/java/com/google/testing/compile/CompilationSubjectTest.java b/src/test/java/com/google/testing/compile/CompilationSubjectTest.java
@@ -24,12 +24,15 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.stream.Collectors.joining;
 import static javax.tools.StandardLocation.CLASS_OUTPUT;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.ByteSource;
 import com.google.common.truth.ExpectFailure;
 import com.google.common.truth.Truth;
+import java.io.BufferedReader;
+import java.io.IOException;
 import java.util.regex.Pattern;
 import java.util.stream.Stream;
 import javax.tools.Diagnostic;
@@ -347,6 +350,22 @@ public void hadWarningContainingInFileOnLine_wrongLine() {
       assertThat(expected.getMessage()).contains("" + actualErrorLine);
     }
 
+    @Test
+    public void hadWarningContainingInFileOnLine_lineTooBig() throws IOException {
+      long lineCount = new BufferedReader(sourceFile.openReader(false)).lines().count();
+      IllegalArgumentException exception =
+          assertThrows(
+              IllegalArgumentException.class,
+              () ->
+                  assertThat(compilerWithWarning().compile(sourceFile))
+                      .hadWarningContainingMatch("this is a+ message")
+                      .inFile(sourceFile)
+                      .onLine(100));
+      assertThat(exception)
+          .hasMessageThat()
+          .isEqualTo("Invalid line number 100; number of lines is only " + lineCount);
+    }
+
     /* TODO(dpb): Negative cases for onLineContaining for
      * (warning, error, note) x
      * (containing(String), containingMatch(String), containingMatch(Pattern)). */

Original file line number	Diff line number	Diff line change
`@@ -466,9 +466,12 @@ ImmutableList<String> linesInFile() {`
`466`	`466`
`467`	`467`	`/** Lists the line at a line number (1-based). */`
`468`	`468`	`String listLine(long lineNumber) {`
`469`		`- return lineNumber == Diagnostic.NOPOS`
`470`		`- ? "(no associated line)"`
`471`		`- : String.format("%4d: %s", lineNumber, linesInFile().get((int) (lineNumber - 1)));`
	`469`	`+ if (lineNumber == Diagnostic.NOPOS) {`
	`470`	`+ return "(no associated line)";`
	`471`	`+ }`
	`472`	`+ checkArgument(lineNumber > 0 && lineNumber <= linesInFile().size(),`
	`473`	`+ "Invalid line number %s; number of lines is only %s", lineNumber, linesInFile().size());`
	`474`	`+ return String.format("%4d: %s", lineNumber, linesInFile().get((int) (lineNumber - 1)));`
`472`	`475`	`}`
`473`	`476`	`}`
`474`	`477`