From 9fef70b579803adb4d1e2ee0c5f8adc5dec83a25 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Apr 2024 19:32:59 -0400 Subject: [PATCH 01/10] Adapt to changes in https://github.com/eisop/checker-framework/pull/739 --- src/test/java/tests/DetailMessage.java | 73 ++++++++++---------------- 1 file changed, 28 insertions(+), 45 deletions(-) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index c7deeedf..3d84bcda 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -4,16 +4,13 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Iterables.getLast; import static java.lang.Integer.parseInt; -import static java.util.Arrays.stream; import static java.util.regex.Pattern.DOTALL; -import static java.util.stream.Collectors.joining; import com.google.common.base.Joiner; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.List; import java.util.Objects; import java.util.regex.Matcher; @@ -30,13 +27,6 @@ */ final class DetailMessage extends TestDiagnostic { - private static final Pattern MESSAGE_PATTERN = - Pattern.compile( - "(?\\S+):(?\\d+): (?" - + stream(DiagnosticKind.values()).map(k -> k.parseString).collect(joining("|")) - + "): (?.*)", - DOTALL); - /** Parser for the output for -Adetailedmsgtext. */ // Implemented here: org.checkerframework.framework.source.SourceChecker#detailedMsgTextPrefix private static final Pattern DETAIL_MESSAGE_PATTERN = @@ -49,12 +39,6 @@ final class DetailMessage extends TestDiagnostic { private static final Pattern OFFSETS_PATTERN = Pattern.compile("(\\( (?-?\\d+), (?-?\\d+) \\))?"); - /** The path to the source file containing the diagnostic. */ - final Path file; - - /** The line number (1-based) of the diagnostic in the {@link #file}. */ - final int lineNumber; - /** The message key for the user-visible text message that is emitted. */ final String messageKey; @@ -77,25 +61,26 @@ final class DetailMessage extends TestDiagnostic { * @param rootDirectory if not null, a root directory prefix to remove from the file part of the * message */ - static @Nullable DetailMessage parse(String input, @Nullable Path rootDirectory) { - Matcher messageMatcher = MESSAGE_PATTERN.matcher(input); - if (!messageMatcher.matches()) { - return null; + static @Nullable DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { + if (input.getFile().toString().contains("CurrentThreadThreadGroup")) + System.out.println("looking at: " + input); + Matcher detailsMatcher = DETAIL_MESSAGE_PATTERN.matcher(input.getMessage()); + if (!detailsMatcher.matches()) { + // Return a message with no key or parts. + return new DetailMessage( + input.getFile(), + input.getLineNumber(), + input.getKind(), + "", + ImmutableList.of(), + null, + null, + input.getMessage()); } - - Path file = Paths.get(messageMatcher.group("file")); + Path file = input.getFile(); if (rootDirectory != null) { file = rootDirectory.relativize(file); } - int lineNumber = parseInt(messageMatcher.group("lineNumber")); - DiagnosticKind kind = DiagnosticKind.fromParseString(messageMatcher.group("kind")); - - String message = messageMatcher.group("message"); - Matcher detailsMatcher = DETAIL_MESSAGE_PATTERN.matcher(message); - if (!detailsMatcher.matches()) { - // Return a message with no key or parts. - return new DetailMessage(file, lineNumber, kind, "", ImmutableList.of(), null, null, message); - } int messagePartCount = parseInt(detailsMatcher.group("messagePartCount")); List messageParts = @@ -112,8 +97,8 @@ final class DetailMessage extends TestDiagnostic { return new DetailMessage( file, - lineNumber, - kind, + input.getLineNumber(), + input.getKind(), detailsMatcher.group("messageKey"), messageArguments, intOrNull(offsetsMatcher.group("start")), @@ -127,16 +112,14 @@ private static Integer intOrNull(String input) { private DetailMessage( Path file, - int lineNumber, + long lineNumber, DiagnosticKind diagnosticKind, String messageKey, ImmutableList messageArguments, Integer offsetStart, Integer offsetEnd, String readableMessage) { - super(file.toString(), lineNumber, diagnosticKind, readableMessage, false, true); - this.file = file; - this.lineNumber = lineNumber; + super(file, lineNumber, diagnosticKind, readableMessage, false); this.messageKey = messageKey; this.messageArguments = messageArguments; this.offsetStart = offsetStart; @@ -144,11 +127,6 @@ private DetailMessage( this.readableMessage = readableMessage; } - /** The last part of the {@link #file}. */ - String getFileName() { - return file.getFileName().toString(); - } - /** * True if this was parsed from an actual {@code -Adetailedmsgtext} message; false if this was * some other diagnostic. @@ -166,8 +144,7 @@ public boolean equals(Object o) { return false; } DetailMessage that = (DetailMessage) o; - return lineNumber == that.lineNumber - && file.equals(that.file) + return super.equals(that) && messageKey.equals(that.messageKey) && messageArguments.equals(that.messageArguments) && Objects.equals(offsetStart, that.offsetStart) @@ -178,7 +155,13 @@ public boolean equals(Object o) { @Override public int hashCode() { return Objects.hash( - file, lineNumber, messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); + filename, + lineNumber, + messageKey, + messageArguments, + offsetStart, + offsetEnd, + readableMessage); } @Override From d3198a359b3c7c57ff32c4fb3e6742baa74adf97 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Apr 2024 19:36:55 -0400 Subject: [PATCH 02/10] Add regression test case; adapt to API changes --- src/test/java/tests/ConformanceTest.java | 4 ++-- src/test/java/tests/NullSpecTest.java | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index c7e2cb7d..cc1ce901 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -142,7 +142,7 @@ private static ImmutableSet analyze( TestUtilities.getShouldEmitDebugInfo()); TypecheckResult result = new TypecheckExecutor().runTest(config); return result.getUnexpectedDiagnostics().stream() - .map(d -> DetailMessage.parse(d.getMessage(), testDirectory)) + .map(d -> DetailMessage.parse(d, testDirectory)) .filter(Objects::nonNull) // Do not filter out messages without details. // .filter(DetailMessage::hasDetails) @@ -182,7 +182,7 @@ static final class DetailMessageReportedFact extends ReportedFact { private final DetailMessage detailMessage; DetailMessageReportedFact(DetailMessage detailMessage) { - super(detailMessage.file, detailMessage.lineNumber); + super(detailMessage.getFile(), detailMessage.getLineNumber()); this.detailMessage = detailMessage; } diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index 500e6f4b..e5116c05 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -41,6 +41,18 @@ public static String[] getTestDirs() { } } + /** A small set of regression tests. */ + public static class Regression extends NullSpecTest { + public Regression(List testFiles) { + super(testFiles, false); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"regression"}; + } + } + /** A test that ignores cases where there is limited nullness information. */ public static class Lenient extends NullSpecTest { public Lenient(List testFiles) { @@ -105,7 +117,7 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { for (ListIterator i = unexpected.listIterator(); i.hasNext(); ) { TestDiagnostic diagnostic = i.next(); - DetailMessage detailMessage = DetailMessage.parse(diagnostic.getMessage(), null); + DetailMessage detailMessage = DetailMessage.parse(diagnostic, null); if (detailMessage != null && detailMessage.hasDetails()) { // Replace diagnostics that can be parsed with DetailMessage diagnostics. i.set(detailMessage); @@ -145,8 +157,8 @@ private boolean corresponds(TestDiagnostic missing, TestDiagnostic unexpected) { */ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { // First, make sure the two diagnostics are on the same file and line. - if (!missing.getFilename().equals(unexpected.getFileName()) - || missing.getLineNumber() != unexpected.lineNumber) { + if (!missing.getFilename().equals(unexpected.getFilename()) + || missing.getLineNumber() != unexpected.getLineNumber()) { return false; } From 9c70df23781c0ffef072c9cb9af8e1e4631b938e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Apr 2024 19:37:25 -0400 Subject: [PATCH 03/10] Remove accesses and assertions that are no longer needed. (The assertion violation was silently ignored in a few test cases. As it is no longer needed, remove it.) --- .../nullness/ConformanceTypeInformationPresenter.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java b/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java index 83d4da63..9bffb729 100644 --- a/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java +++ b/src/main/java/com/google/jspecify/nullness/ConformanceTypeInformationPresenter.java @@ -16,7 +16,6 @@ import com.sun.source.tree.AssignmentTree; import com.sun.source.tree.ClassTree; -import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; import java.util.List; @@ -85,9 +84,6 @@ protected void reportTreeType( String methodName = calledElem.getSimpleName().toString(); AnnotatedExecutableType calledType = (AnnotatedExecutableType) type; List params = calledType.getParameterTypes(); - MethodInvocationTree mit = (MethodInvocationTree) tree; - List args = mit.getArguments(); - assert params.size() == args.size(); for (int i = 0; i < params.size(); ++i) { String paramName = calledElem.getParameters().get(i).getSimpleName().toString(); From 40111435a3eb7990dc239ab628d87bbfd28bf54e Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Apr 2024 19:40:05 -0400 Subject: [PATCH 04/10] Run the regression tests --- build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 947bb6f4..509b0f4e 100644 --- a/build.gradle +++ b/build.gradle @@ -138,8 +138,10 @@ tasks.withType(Test).configureEach { test { include '**/NullSpecTest$Minimal.class' - inputs.files("${rootDir}/tests/minimal") + + include '**/NullSpecTest$Regression.class' + inputs.files("${rootDir}/tests/regression") } tasks.register('jspecifySamplesTest', Test) { From c8a671ed905163516fcbdc987a97d742cca8c1d1 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Apr 2024 21:59:58 -0400 Subject: [PATCH 05/10] Include "kind" in output; remove debugging output --- src/test/java/tests/DetailMessage.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index 3d84bcda..7280e205 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -62,8 +62,6 @@ final class DetailMessage extends TestDiagnostic { * message */ static @Nullable DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { - if (input.getFile().toString().contains("CurrentThreadThreadGroup")) - System.out.println("looking at: " + input); Matcher detailsMatcher = DETAIL_MESSAGE_PATTERN.matcher(input.getMessage()); if (!detailsMatcher.matches()) { // Return a message with no key or parts. @@ -155,18 +153,12 @@ public boolean equals(Object o) { @Override public int hashCode() { return Objects.hash( - filename, - lineNumber, - messageKey, - messageArguments, - offsetStart, - offsetEnd, - readableMessage); + super.hashCode(), messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); } @Override public String toString() { - return String.format("%s:%d: (%s) %s", file, lineNumber, messageKey, readableMessage); + return String.format("%s:%d:%s: (%s) %s", file, lineNumber, kind, messageKey, readableMessage); } /** String format for debugging use. */ @@ -174,6 +166,7 @@ String toDetailedString() { return toStringHelper(this) .add("file", file) .add("lineNumber", lineNumber) + .add("kind", kind) .add("messageKey", messageKey) .add("messageArguments", messageArguments) .add("offsetStart", offsetStart) From f44acb07d8d44749f611c42eeba0f3d676576cbf Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 16 Apr 2024 09:47:15 -0400 Subject: [PATCH 06/10] Adapt nullness annotation/comment; undo changes to hashCode/equals. --- src/test/java/tests/ConformanceTest.java | 2 -- src/test/java/tests/DetailMessage.java | 10 +++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index cc1ce901..93bd890e 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -29,7 +29,6 @@ import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Objects; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; @@ -143,7 +142,6 @@ private static ImmutableSet analyze( TypecheckResult result = new TypecheckExecutor().runTest(config); return result.getUnexpectedDiagnostics().stream() .map(d -> DetailMessage.parse(d, testDirectory)) - .filter(Objects::nonNull) // Do not filter out messages without details. // .filter(DetailMessage::hasDetails) .map(DetailMessageReportedFact::new) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index 7280e205..e5de3c8e 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -55,13 +55,12 @@ final class DetailMessage extends TestDiagnostic { final String readableMessage; /** - * Returns an object parsed from a diagnostic message, or {@code null} if the message doesn't - * match the expected format. + * Returns an object parsed from a diagnostic message. * * @param rootDirectory if not null, a root directory prefix to remove from the file part of the * message */ - static @Nullable DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { + static DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { Matcher detailsMatcher = DETAIL_MESSAGE_PATTERN.matcher(input.getMessage()); if (!detailsMatcher.matches()) { // Return a message with no key or parts. @@ -142,7 +141,8 @@ public boolean equals(Object o) { return false; } DetailMessage that = (DetailMessage) o; - return super.equals(that) + return lineNumber == that.lineNumber + && file.equals(that.file) && messageKey.equals(that.messageKey) && messageArguments.equals(that.messageArguments) && Objects.equals(offsetStart, that.offsetStart) @@ -153,7 +153,7 @@ public boolean equals(Object o) { @Override public int hashCode() { return Objects.hash( - super.hashCode(), messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); + file, lineNumber, messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); } @Override From 92a46589075ac3df22dad8cb4c8a7b767288d5bc Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Tue, 16 Apr 2024 14:21:54 -0400 Subject: [PATCH 07/10] Relativize earlier. --- src/test/java/tests/DetailMessage.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index e5de3c8e..e225688f 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -61,11 +61,17 @@ final class DetailMessage extends TestDiagnostic { * message */ static DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { + Path file = input.getFile(); + if (rootDirectory != null && !file.toString().equals("")) { + // Empty file strings cannot be relativized. + file = rootDirectory.relativize(file); + } + Matcher detailsMatcher = DETAIL_MESSAGE_PATTERN.matcher(input.getMessage()); if (!detailsMatcher.matches()) { // Return a message with no key or parts. return new DetailMessage( - input.getFile(), + file, input.getLineNumber(), input.getKind(), "", @@ -74,10 +80,6 @@ static DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { null, input.getMessage()); } - Path file = input.getFile(); - if (rootDirectory != null) { - file = rootDirectory.relativize(file); - } int messagePartCount = parseInt(detailsMatcher.group("messagePartCount")); List messageParts = From 87b803840316142dbd50fce9b9d9826c4c1f0413 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 17 Apr 2024 16:00:25 -0400 Subject: [PATCH 08/10] More robust check Co-authored-by: David P. Baker --- src/test/java/tests/DetailMessage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index e225688f..8d00f9d8 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -62,7 +62,7 @@ final class DetailMessage extends TestDiagnostic { */ static DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { Path file = input.getFile(); - if (rootDirectory != null && !file.toString().equals("")) { + if (rootDirectory != null && file.startsWith(rootDirectory)) { // Empty file strings cannot be relativized. file = rootDirectory.relativize(file); } From df2dcd144b2bd014ab1e237c03e6800c3b2c09ae Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 17 Apr 2024 16:04:32 -0400 Subject: [PATCH 09/10] Remove redundant null check --- src/test/java/tests/NullSpecTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index e5116c05..f5ca17e9 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -118,7 +118,7 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { for (ListIterator i = unexpected.listIterator(); i.hasNext(); ) { TestDiagnostic diagnostic = i.next(); DetailMessage detailMessage = DetailMessage.parse(diagnostic, null); - if (detailMessage != null && detailMessage.hasDetails()) { + if (detailMessage.hasDetails()) { // Replace diagnostics that can be parsed with DetailMessage diagnostics. i.set(detailMessage); } else if (diagnostic.getKind() != DiagnosticKind.Error) { From 04eeebb9f3fad51aabad4da042caae3f3da18cc0 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 17 Apr 2024 16:10:57 -0400 Subject: [PATCH 10/10] Remove old comment --- src/test/java/tests/DetailMessage.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index 8d00f9d8..cf77b024 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -63,7 +63,6 @@ final class DetailMessage extends TestDiagnostic { static DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { Path file = input.getFile(); if (rootDirectory != null && file.startsWith(rootDirectory)) { - // Empty file strings cannot be relativized. file = rootDirectory.relativize(file); }