From 6346be07103fab234f41fb2f75093f22c6b7dd4c Mon Sep 17 00:00:00 2001 From: "David P. Baker" Date: Mon, 8 Jan 2024 17:43:09 -0500 Subject: [PATCH] Handle unexpected diagnostics that aren't in "detail message" format. (#130) Otherwise they were getting swallowed and never emitted even in details mode. --- src/test/java/tests/ConformanceTest.java | 2 +- src/test/java/tests/DetailMessage.java | 90 +++++++++++++++--------- src/test/java/tests/NullSpecTest.java | 2 +- 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index 11a5f13..11b6ef3 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -228,7 +228,7 @@ protected boolean mustBeExpected() { @Override public String toString() { - return String.format("(%s) %s", detailMessage.messageKey, detailMessage.message); + return String.format("(%s) %s", detailMessage.messageKey, detailMessage.readableMessage); } /** diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java index 2b26b59..c7deeed 100644 --- a/src/test/java/tests/DetailMessage.java +++ b/src/test/java/tests/DetailMessage.java @@ -30,22 +30,23 @@ */ 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 - static final Pattern DETAIL_MESSAGE_PATTERN = + private static final Pattern DETAIL_MESSAGE_PATTERN = Pattern.compile( Joiner.on(" \\$\\$ ") .join( - "(?\\S+):(?\\d+): (?" - + stream(DiagnosticKind.values()) - .map(k -> k.parseString) - .collect(joining("|")) - + "): \\((?[^)]+)\\)", - "(?\\d+)", - "(?.*)"), + "\\((?[^)]+)\\)", "(?\\d+)", "(?.*)"), DOTALL); - static final Pattern OFFSETS_PATTERN = + private static final Pattern OFFSETS_PATTERN = Pattern.compile("(\\( (?-?\\d+), (?-?\\d+) \\))?"); /** The path to the source file containing the diagnostic. */ @@ -67,7 +68,7 @@ final class DetailMessage extends TestDiagnostic { final Integer offsetEnd; /** The user-visible message emitted for the diagnostic. */ - final String message; + final String readableMessage; /** * Returns an object parsed from a diagnostic message, or {@code null} if the message doesn't @@ -77,41 +78,54 @@ final class DetailMessage extends TestDiagnostic { * message */ static @Nullable DetailMessage parse(String input, @Nullable Path rootDirectory) { - Matcher matcher = DETAIL_MESSAGE_PATTERN.matcher(input); - if (!matcher.matches()) { + Matcher messageMatcher = MESSAGE_PATTERN.matcher(input); + if (!messageMatcher.matches()) { return null; } - int messagePartCount = parseInt(matcher.group("messagePartCount")); + Path file = Paths.get(messageMatcher.group("file")); + 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 = Splitter.on("$$") .trimResults() .limit(messagePartCount + 2) - .splitToList(matcher.group("messageParts")); + .splitToList(detailsMatcher.group("messageParts")); ImmutableList messageArguments = ImmutableList.copyOf(Iterables.limit(messageParts, messagePartCount)); - String message = getLast(messageParts); + String readableMessage = getLast(messageParts); - Matcher offsets = OFFSETS_PATTERN.matcher(messageParts.get(messagePartCount)); - checkArgument(offsets.matches(), "unparseable offsets: %s", input); + Matcher offsetsMatcher = OFFSETS_PATTERN.matcher(messageParts.get(messagePartCount)); + checkArgument(offsetsMatcher.matches(), "unparseable offsets: %s", input); - Path file = Paths.get(matcher.group("file")); return new DetailMessage( - (rootDirectory != null) ? rootDirectory.relativize(file) : file, - parseInt(matcher.group("lineNumber")), - DiagnosticKind.fromParseString(matcher.group("kind")), - matcher.group("messageKey"), + file, + lineNumber, + kind, + detailsMatcher.group("messageKey"), messageArguments, - intOrNull(offsets.group("start")), - intOrNull(offsets.group("end")), - message); + intOrNull(offsetsMatcher.group("start")), + intOrNull(offsetsMatcher.group("end")), + readableMessage); } - static Integer intOrNull(String input) { + private static Integer intOrNull(String input) { return input == null ? null : parseInt(input); } - DetailMessage( + private DetailMessage( Path file, int lineNumber, DiagnosticKind diagnosticKind, @@ -119,15 +133,15 @@ static Integer intOrNull(String input) { ImmutableList messageArguments, Integer offsetStart, Integer offsetEnd, - String message) { - super(file.toString(), lineNumber, diagnosticKind, message, false, true); + String readableMessage) { + super(file.toString(), lineNumber, diagnosticKind, readableMessage, false, true); this.file = file; this.lineNumber = lineNumber; this.messageKey = messageKey; this.messageArguments = messageArguments; this.offsetStart = offsetStart; this.offsetEnd = offsetEnd; - this.message = message; + this.readableMessage = readableMessage; } /** The last part of the {@link #file}. */ @@ -135,6 +149,14 @@ String getFileName() { return file.getFileName().toString(); } + /** + * True if this was parsed from an actual {@code -Adetailedmsgtext} message; false if this was + * some other diagnostic. + */ + boolean hasDetails() { + return !messageKey.equals(""); + } + @Override public boolean equals(Object o) { if (this == o) { @@ -150,18 +172,18 @@ public boolean equals(Object o) { && messageArguments.equals(that.messageArguments) && Objects.equals(offsetStart, that.offsetStart) && Objects.equals(offsetEnd, that.offsetEnd) - && message.equals(that.message); + && readableMessage.equals(that.readableMessage); } @Override public int hashCode() { return Objects.hash( - file, lineNumber, messageKey, messageArguments, offsetStart, offsetEnd, message); + file, lineNumber, messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); } @Override public String toString() { - return String.format("%s:%d: (%s) %s", file, lineNumber, messageKey, message); + return String.format("%s:%d: (%s) %s", file, lineNumber, messageKey, readableMessage); } /** String format for debugging use. */ @@ -173,7 +195,7 @@ String toDetailedString() { .add("messageArguments", messageArguments) .add("offsetStart", offsetStart) .add("offsetEnd", offsetEnd) - .add("message", message) + .add("readableMessage", readableMessage) .toString(); } } diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index d5ed847..fd19818 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -106,7 +106,7 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { for (ListIterator i = unexpected.listIterator(); i.hasNext(); ) { TestDiagnostic diagnostic = i.next(); DetailMessage detailMessage = DetailMessage.parse(diagnostic.getMessage(), null); - if (detailMessage != null) { + if (detailMessage != null && detailMessage.hasDetails()) { // Replace diagnostics that can be parsed with DetailMessage diagnostics. i.set(detailMessage); } else if (diagnostic.getKind() != DiagnosticKind.Error) {