From b1206d50f05789958ea76d2d9d25be0fa6e252cc Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 22 Apr 2024 21:06:27 -0400 Subject: [PATCH 1/8] Introduce a NullSpecTypeValidator to ensure unspecified type variable bounds are ok --- .../nullness/NullSpecTypeValidator.java | 45 +++++++++++++++++++ .../jspecify/nullness/NullSpecVisitor.java | 6 +++ tests/regression/Issue177.java | 18 ++++++++ 3 files changed, 69 insertions(+) create mode 100644 src/main/java/com/google/jspecify/nullness/NullSpecTypeValidator.java create mode 100644 tests/regression/Issue177.java diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecTypeValidator.java b/src/main/java/com/google/jspecify/nullness/NullSpecTypeValidator.java new file mode 100644 index 0000000..bb29fc0 --- /dev/null +++ b/src/main/java/com/google/jspecify/nullness/NullSpecTypeValidator.java @@ -0,0 +1,45 @@ +// Copyright 2020 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.jspecify.nullness; + +import javax.lang.model.element.AnnotationMirror; +import org.checkerframework.common.basetype.BaseTypeChecker; +import org.checkerframework.common.basetype.BaseTypeValidator; +import org.checkerframework.framework.type.AnnotatedTypeMirror; + +final class NullSpecTypeValidator extends BaseTypeValidator { + private final AnnotationMirror nullnessOperatorUnspecified; + + /** Constructor. */ + NullSpecTypeValidator( + BaseTypeChecker checker, + NullSpecVisitor visitor, + NullSpecAnnotatedTypeFactory atypeFactory, + Util util) { + super(checker, visitor, atypeFactory); + + nullnessOperatorUnspecified = util.nullnessOperatorUnspecified; + } + + @Override + public boolean areBoundsValid(AnnotatedTypeMirror upperBound, AnnotatedTypeMirror lowerBound) { + if (upperBound.hasAnnotation(nullnessOperatorUnspecified) + || lowerBound.hasAnnotation(nullnessOperatorUnspecified)) { + return true; + } else { + return super.areBoundsValid(upperBound, lowerBound); + } + } +} diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java index 52000ef..bed3e45 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java @@ -71,6 +71,7 @@ import javax.lang.model.type.DeclaredType; import javax.lang.model.type.TypeMirror; import org.checkerframework.common.basetype.BaseTypeVisitor; +import org.checkerframework.common.basetype.TypeValidator; import org.checkerframework.framework.type.AnnotatedTypeMirror; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType; import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType; @@ -640,4 +641,9 @@ protected NullSpecAnnotatedTypeFactory createTypeFactory() { // Reading util this way is ugly but necessary. See discussion in NullSpecChecker. return new NullSpecAnnotatedTypeFactory(checker, ((NullSpecChecker) checker).util); } + + @Override + protected TypeValidator createTypeValidator() { + return new NullSpecTypeValidator(checker, this, atypeFactory, ((NullSpecChecker) checker).util); + } } diff --git a/tests/regression/Issue177.java b/tests/regression/Issue177.java new file mode 100644 index 0000000..c47c8a9 --- /dev/null +++ b/tests/regression/Issue177.java @@ -0,0 +1,18 @@ +// Copyright 2024 The JSpecify Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Test case for Issue 177: +// https://github.com/jspecify/jspecify-reference-checker/issues/177 + +class Issue177 {} From 533080de8d8df80b0132c49b9e1b9e62e85bfb11 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 22 Apr 2024 21:07:25 -0400 Subject: [PATCH 2/8] Change IMPLICIT_LOWER_BOUND default to minusNull. Depends on https://github.com/eisop/checker-framework/pull/746 --- .../jspecify/nullness/NullSpecAnnotatedTypeFactory.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index d37b533..ae92cee 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -141,6 +141,7 @@ final class NullSpecAnnotatedTypeFactory new TypeUseLocation[] { TypeUseLocation.CONSTRUCTOR_RESULT, TypeUseLocation.EXCEPTION_PARAMETER, + TypeUseLocation.IMPLICIT_LOWER_BOUND, TypeUseLocation.RECEIVER, }; @@ -153,10 +154,6 @@ final class NullSpecAnnotatedTypeFactory private static final TypeUseLocation[] defaultLocationsUnspecified = new TypeUseLocation[] { - // Lower bounds could be MinusNull, but all uses in unmarked code would become unspecified - // anyways. - // Revisit once https://github.com/eisop/checker-framework/issues/741 is fixed. - TypeUseLocation.IMPLICIT_LOWER_BOUND, TypeUseLocation.IMPLICIT_WILDCARD_UPPER_BOUND_NO_SUPER, TypeUseLocation.TYPE_VARIABLE_USE, TypeUseLocation.OTHERWISE From 20a41035b9d606bd1ff3e9fa60356d708a692241 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Mon, 22 Apr 2024 21:08:04 -0400 Subject: [PATCH 3/8] Parameters and return types should have same defaults --- tests/regression/Issue172.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/regression/Issue172.java b/tests/regression/Issue172.java index 5d30b31..b782b8d 100644 --- a/tests/regression/Issue172.java +++ b/tests/regression/Issue172.java @@ -21,6 +21,10 @@ class Issue172 { E e() { return null; } + + void p(E p) { + p = null; + } } class Issue172UnmarkedUse { From ffe4a0e40f42bcae63cace893416afc88d892913 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Wed, 10 Jul 2024 19:46:13 -0400 Subject: [PATCH 4/8] Use DetailedTestDiagnostic from EISOP instead of DetailMessage --- src/test/java/tests/ConformanceTest.java | 57 ++++---- src/test/java/tests/DetailMessage.java | 178 ----------------------- src/test/java/tests/NullSpecTest.java | 21 ++- 3 files changed, 42 insertions(+), 214 deletions(-) delete mode 100644 src/test/java/tests/DetailMessage.java diff --git a/src/test/java/tests/ConformanceTest.java b/src/test/java/tests/ConformanceTest.java index 93bd890..2745d05 100644 --- a/src/test/java/tests/ConformanceTest.java +++ b/src/test/java/tests/ConformanceTest.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Stream; @@ -37,7 +38,9 @@ import org.checkerframework.framework.test.TestUtilities; import org.checkerframework.framework.test.TypecheckExecutor; import org.checkerframework.framework.test.TypecheckResult; +import org.checkerframework.framework.test.diagnostics.DetailedTestDiagnostic; import org.checkerframework.framework.test.diagnostics.DiagnosticKind; +import org.checkerframework.framework.test.diagnostics.TestDiagnostic; import org.jspecify.annotations.Nullable; import org.jspecify.conformance.ConformanceTestRunner; import org.jspecify.conformance.ExpectedFact; @@ -141,10 +144,7 @@ private static ImmutableSet analyze( TestUtilities.getShouldEmitDebugInfo()); TypecheckResult result = new TypecheckExecutor().runTest(config); return result.getUnexpectedDiagnostics().stream() - .map(d -> DetailMessage.parse(d, testDirectory)) - // Do not filter out messages without details. - // .filter(DetailMessage::hasDetails) - .map(DetailMessageReportedFact::new) + .map(d -> new DetailMessageReportedFact(testDirectory, d)) .collect(toImmutableSet()); } @@ -177,60 +177,67 @@ static final class DetailMessageReportedFact extends ReportedFact { "type.parameter.annotated", "wildcard.annotated"); - private final DetailMessage detailMessage; + private final TestDiagnostic diagnostic; - DetailMessageReportedFact(DetailMessage detailMessage) { - super(detailMessage.getFile(), detailMessage.getLineNumber()); - this.detailMessage = detailMessage; + DetailMessageReportedFact(@Nullable Path testDirectory, TestDiagnostic diag) { + super( + (testDirectory != null && diag.getFile().startsWith(testDirectory)) + ? testDirectory.relativize(diag.getFile()) + : diag.getFile(), + diag.getLineNumber()); + this.diagnostic = diag; } @Override protected boolean matches(ExpectedFact expectedFact) { if (expectedFact.isNullnessMismatch()) { - return DEREFERENCE.equals(detailMessage.messageKey) - || CANNOT_CONVERT_KEYS.contains(detailMessage.messageKey); + return DEREFERENCE.equals(diagnostic.getMessageKey()) + || CANNOT_CONVERT_KEYS.contains(diagnostic.getMessageKey()); } return super.matches(expectedFact); } @Override protected boolean mustBeExpected() { - return detailMessage.getKind().equals(DiagnosticKind.Error); + return diagnostic.getKind().equals(DiagnosticKind.Error); } @Override protected String getFactText() { - if (CANNOT_CONVERT_KEYS.contains(detailMessage.messageKey)) { - if (detailMessage.messageArguments.size() < 2) { + if (!(diagnostic instanceof DetailedTestDiagnostic)) { + return toString(); + } + List args = ((DetailedTestDiagnostic) diagnostic).getAdditionalTokens(); + if (CANNOT_CONVERT_KEYS.contains(diagnostic.getMessageKey())) { + if (args.size() < 2) { // The arguments must end with sourceType and sinkType. return toString(); } - ImmutableList reversedArguments = detailMessage.messageArguments.reverse(); - String sourceType = fixType(reversedArguments.get(1)); // penultimate - String sinkType = fixType(reversedArguments.get(0)); // last + String sourceType = fixType(args.get(args.size() - 2)); // penultimate + String sinkType = fixType(args.get(args.size() - 1)); // last return cannotConvert(sourceType, sinkType); } - if (IRRELEVANT_ANNOTATION_KEYS.contains(detailMessage.messageKey)) { - if (detailMessage.messageArguments.isEmpty()) { + if (IRRELEVANT_ANNOTATION_KEYS.contains(diagnostic.getMessageKey())) { + if (args.isEmpty()) { // arguments must start with the annotation return toString(); } return irrelevantAnnotation( // Remove the package name (and any enclosing element name); emit just the simple name. - detailMessage.messageArguments.get(0).replaceFirst(".*\\.", "")); + args.get(0).replaceFirst(".*\\.", "")); } - switch (detailMessage.messageKey) { + switch (diagnostic.getMessageKey()) { case "sourceType": { - String expressionType = fixType(detailMessage.messageArguments.get(0)); - String expression = detailMessage.messageArguments.get(1); + String expressionType = fixType(args.get(0)); + String expression = args.get(1); return expressionType(expressionType, expression); } case "sinkType": { - String sinkType = fixType(detailMessage.messageArguments.get(0)); + String sinkType = fixType(args.get(0)); // Remove the simple name of the class and the dot before the method name. - String sink = detailMessage.messageArguments.get(1).replaceFirst("^[^.]+\\.", ""); + String sink = args.get(1).replaceFirst("^[^.]+\\.", ""); return sinkType(sinkType, sink); } } @@ -239,7 +246,7 @@ protected String getFactText() { @Override public String toString() { - return String.format("(%s) %s", detailMessage.messageKey, detailMessage.readableMessage); + return String.format("(%s) %s", diagnostic.getMessageKey(), diagnostic.getMessage()); } /** diff --git a/src/test/java/tests/DetailMessage.java b/src/test/java/tests/DetailMessage.java deleted file mode 100644 index cf77b02..0000000 --- a/src/test/java/tests/DetailMessage.java +++ /dev/null @@ -1,178 +0,0 @@ -package tests; - -import static com.google.common.base.MoreObjects.toStringHelper; -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.regex.Pattern.DOTALL; - -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.util.List; -import java.util.Objects; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import org.checkerframework.framework.test.diagnostics.DiagnosticKind; -import org.checkerframework.framework.test.diagnostics.TestDiagnostic; -import org.jspecify.annotations.Nullable; - -/** - * Information about a reported diagnostic. - * - *

Checker Framework uses a special format to put parseable information about a diagnostic into - * the message text. This object represents that information directly. - */ -final class DetailMessage extends TestDiagnostic { - - /** Parser for the output for -Adetailedmsgtext. */ - // Implemented here: org.checkerframework.framework.source.SourceChecker#detailedMsgTextPrefix - private static final Pattern DETAIL_MESSAGE_PATTERN = - Pattern.compile( - Joiner.on(" \\$\\$ ") - .join( - "\\((?[^)]+)\\)", "(?\\d+)", "(?.*)"), - DOTALL); - - private static final Pattern OFFSETS_PATTERN = - Pattern.compile("(\\( (?-?\\d+), (?-?\\d+) \\))?"); - - /** The message key for the user-visible text message that is emitted. */ - final String messageKey; - - /** The arguments to the text message format for the message key. */ - final ImmutableList messageArguments; - - /** The offset within the file of the start of the code covered by the diagnostic. */ - final Integer offsetStart; - - /** The offset within the file of the end (exclusive) of the code covered by the diagnostic. */ - final Integer offsetEnd; - - /** The user-visible message emitted for the diagnostic. */ - final String readableMessage; - - /** - * 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 DetailMessage parse(TestDiagnostic input, @Nullable Path rootDirectory) { - Path file = input.getFile(); - if (rootDirectory != null && file.startsWith(rootDirectory)) { - 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( - file, - input.getLineNumber(), - input.getKind(), - "", - ImmutableList.of(), - null, - null, - input.getMessage()); - } - - int messagePartCount = parseInt(detailsMatcher.group("messagePartCount")); - List messageParts = - Splitter.on("$$") - .trimResults() - .limit(messagePartCount + 2) - .splitToList(detailsMatcher.group("messageParts")); - ImmutableList messageArguments = - ImmutableList.copyOf(Iterables.limit(messageParts, messagePartCount)); - String readableMessage = getLast(messageParts); - - Matcher offsetsMatcher = OFFSETS_PATTERN.matcher(messageParts.get(messagePartCount)); - checkArgument(offsetsMatcher.matches(), "unparseable offsets: %s", input); - - return new DetailMessage( - file, - input.getLineNumber(), - input.getKind(), - detailsMatcher.group("messageKey"), - messageArguments, - intOrNull(offsetsMatcher.group("start")), - intOrNull(offsetsMatcher.group("end")), - readableMessage); - } - - private static Integer intOrNull(String input) { - return input == null ? null : parseInt(input); - } - - private DetailMessage( - Path file, - long lineNumber, - DiagnosticKind diagnosticKind, - String messageKey, - ImmutableList messageArguments, - Integer offsetStart, - Integer offsetEnd, - String readableMessage) { - super(file, lineNumber, diagnosticKind, readableMessage, false); - this.messageKey = messageKey; - this.messageArguments = messageArguments; - this.offsetStart = offsetStart; - this.offsetEnd = offsetEnd; - this.readableMessage = readableMessage; - } - - /** - * 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) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - DetailMessage that = (DetailMessage) o; - return lineNumber == that.lineNumber - && file.equals(that.file) - && messageKey.equals(that.messageKey) - && messageArguments.equals(that.messageArguments) - && Objects.equals(offsetStart, that.offsetStart) - && Objects.equals(offsetEnd, that.offsetEnd) - && readableMessage.equals(that.readableMessage); - } - - @Override - public int hashCode() { - return Objects.hash( - file, lineNumber, messageKey, messageArguments, offsetStart, offsetEnd, readableMessage); - } - - @Override - public String toString() { - return String.format("%s:%d:%s: (%s) %s", file, lineNumber, kind, messageKey, readableMessage); - } - - /** String format for debugging use. */ - String toDetailedString() { - return toStringHelper(this) - .add("file", file) - .add("lineNumber", lineNumber) - .add("kind", kind) - .add("messageKey", messageKey) - .add("messageArguments", messageArguments) - .add("offsetStart", offsetStart) - .add("offsetEnd", offsetEnd) - .add("readableMessage", readableMessage) - .toString(); - } -} diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index f5ca17e..3ca2f20 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -22,6 +22,7 @@ import java.util.ListIterator; import org.checkerframework.framework.test.CheckerFrameworkPerDirectoryTest; import org.checkerframework.framework.test.TypecheckResult; +import org.checkerframework.framework.test.diagnostics.DetailedTestDiagnostic; import org.checkerframework.framework.test.diagnostics.DiagnosticKind; import org.checkerframework.framework.test.diagnostics.TestDiagnostic; import org.checkerframework.javacutil.BugInCF; @@ -117,10 +118,8 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { for (ListIterator i = unexpected.listIterator(); i.hasNext(); ) { TestDiagnostic diagnostic = i.next(); - DetailMessage detailMessage = DetailMessage.parse(diagnostic, null); - if (detailMessage.hasDetails()) { - // Replace diagnostics that can be parsed with DetailMessage diagnostics. - i.set(detailMessage); + if (diagnostic instanceof DetailedTestDiagnostic) { + // Keep all detailed test diagnostics. } else if (diagnostic.getKind() != DiagnosticKind.Error) { // Remove warnings like explicit.annotation.ignored and deprecation. i.remove(); @@ -147,15 +146,15 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { * unexpected}, a reported diagnostic. */ private boolean corresponds(TestDiagnostic missing, TestDiagnostic unexpected) { - return unexpected instanceof DetailMessage - && corresponds(missing, ((DetailMessage) unexpected)); + return unexpected instanceof DetailedTestDiagnostic + && corresponds(missing, (DetailedTestDiagnostic) unexpected); } /** * Returns {@code true} if {@code missing} is a JSpecify directive that matches {@code * unexpected}, a reported diagnostic. */ - private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { + private boolean corresponds(TestDiagnostic missing, DetailedTestDiagnostic unexpected) { // First, make sure the two diagnostics are on the same file and line. if (!missing.getFilename().equals(unexpected.getFilename()) || missing.getLineNumber() != unexpected.getLineNumber()) { @@ -167,7 +166,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { || missing.getMessage().contains("jspecify_nullness_not_enough_information") || missing.getMessage().contains("jspecify_nullness_mismatch") || missing.getMessage().contains("test:cannot-convert")) { - switch (unexpected.messageKey) { + switch (unexpected.getMessageKey()) { case "argument.type.incompatible": case "assignment.type.incompatible": case "atomicreference.must.include.null": @@ -190,7 +189,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { switch (missing.getMessage()) { case "jspecify_nullness_intrinsically_not_nullable": - switch (unexpected.messageKey) { + switch (unexpected.getMessageKey()) { case "enum.constant.annotated": case "outer.annotated": case "primitive.annotated": @@ -199,7 +198,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { return false; } case "jspecify_unrecognized_location": - switch (unexpected.messageKey) { + switch (unexpected.getMessageKey()) { /* * We'd rather avoid this `bound` error (in part because it suggests that the annotation * is having some effect, which we don't want!), but the most important thing is that the @@ -217,7 +216,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) { return false; } case "jspecify_conflicting_annotations": - switch (unexpected.messageKey) { + switch (unexpected.getMessageKey()) { case "type.invalid.conflicting.annos": case "type.invalid.super.wildcard": return true; From 98337334e87142fa4a92926ea9b3006b36cc8746 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Jul 2024 17:26:11 -0400 Subject: [PATCH 5/8] No need to use `-Adetailedmsgtext` in tests --- src/test/java/tests/NullSpecTest.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index 3ca2f20..b98ec48 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -92,7 +92,7 @@ private static String[] getSamplesDirs() { private static String[] checkerOptions(boolean strict) { ImmutableList.Builder options = ImmutableList.builder(); options.add( - "-AassumePure", "-Adetailedmsgtext", "-AcheckImpl", "-AsuppressWarnings=conditional"); + "-AassumePure", "-AcheckImpl", "-AsuppressWarnings=conditional"); if (strict) { options.add("-Astrict"); } @@ -146,15 +146,6 @@ public TypecheckResult adjustTypecheckResult(TypecheckResult testResult) { * unexpected}, a reported diagnostic. */ private boolean corresponds(TestDiagnostic missing, TestDiagnostic unexpected) { - return unexpected instanceof DetailedTestDiagnostic - && corresponds(missing, (DetailedTestDiagnostic) unexpected); - } - - /** - * Returns {@code true} if {@code missing} is a JSpecify directive that matches {@code - * unexpected}, a reported diagnostic. - */ - private boolean corresponds(TestDiagnostic missing, DetailedTestDiagnostic unexpected) { // First, make sure the two diagnostics are on the same file and line. if (!missing.getFilename().equals(unexpected.getFilename()) || missing.getLineNumber() != unexpected.getLineNumber()) { From a4d555cb2f2fbabd71ef5011c13bd44ac9164c46 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Jul 2024 17:41:50 -0400 Subject: [PATCH 6/8] Add strict regression tests --- src/test/java/tests/NullSpecTest.java | 12 ++++++++++++ .../{regression => regression-strict}/Issue177.java | 0 2 files changed, 12 insertions(+) rename tests/{regression => regression-strict}/Issue177.java (100%) diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index b98ec48..e599daa 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -54,6 +54,18 @@ public static String[] getTestDirs() { } } + /** A small set of strict regression tests. */ + public static class RegressionStrict extends NullSpecTest { + public RegressionStrict(List testFiles) { + super(testFiles, true); + } + + @Parameters + public static String[] getTestDirs() { + return new String[] {"regression-strict"}; + } + } + /** A test that ignores cases where there is limited nullness information. */ public static class Lenient extends NullSpecTest { public Lenient(List testFiles) { diff --git a/tests/regression/Issue177.java b/tests/regression-strict/Issue177.java similarity index 100% rename from tests/regression/Issue177.java rename to tests/regression-strict/Issue177.java From a3e33e74584db188f0773bab6da3fdc4e5e99ab2 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Jul 2024 17:42:15 -0400 Subject: [PATCH 7/8] Adapt to https://github.com/jspecify/jspecify/pull/508 --- tests/ConformanceTest-report.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/ConformanceTest-report.txt b/tests/ConformanceTest-report.txt index 23c8a51..c8496ee 100644 --- a/tests/ConformanceTest-report.txt +++ b/tests/ConformanceTest-report.txt @@ -1,4 +1,4 @@ -# 90 pass; 24 fail; 114 total; 78.9% score +# 93 pass; 24 fail; 117 total; 79.5% score PASS: Basic.java:28:test:expression-type:Object?:nullable PASS: Basic.java:28:test:sink-type:Object!:return PASS: Basic.java:28:test:cannot-convert:Object? to Object! @@ -44,6 +44,7 @@ PASS: irrelevantannotations/notnullmarked/Other.java:Nullable local variable arr PASS: irrelevantannotations/notnullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull PASS: irrelevantannotations/notnullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable PASS: irrelevantannotations/notnullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/notnullmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException! PASS: irrelevantannotations/notnullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable FAIL: irrelevantannotations/notnullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull FAIL: irrelevantannotations/notnullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable @@ -75,6 +76,7 @@ PASS: irrelevantannotations/nullmarked/Other.java:Nullable local variable array: PASS: irrelevantannotations/nullmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull PASS: irrelevantannotations/nullmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable PASS: irrelevantannotations/nullmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException! PASS: irrelevantannotations/nullmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable FAIL: irrelevantannotations/nullmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull FAIL: irrelevantannotations/nullmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable @@ -107,6 +109,7 @@ PASS: irrelevantannotations/nullunmarked/Other.java:Nullable local variable arra PASS: irrelevantannotations/nullunmarked/Other.java:NonNull local variable array:test:irrelevant-annotation:NonNull PASS: irrelevantannotations/nullunmarked/Other.java:Nullable exception parameter:test:irrelevant-annotation:Nullable PASS: irrelevantannotations/nullunmarked/Other.java:NonNull exception parameter:test:irrelevant-annotation:NonNull +PASS: irrelevantannotations/nullunmarked/Other.java:Intrinsically NonNull exception parameter cannot be assigned null:test:cannot-convert:null? to RuntimeException! PASS: irrelevantannotations/nullunmarked/Other.java:Nullable try-with-resources:test:irrelevant-annotation:Nullable FAIL: irrelevantannotations/nullunmarked/Other.java:NonNull try-with-resources:test:irrelevant-annotation:NonNull FAIL: irrelevantannotations/nullunmarked/Other.java:Nullable exception type:test:irrelevant-annotation:Nullable From 1aadcab07c7e573069314cbcb8ad0a76bc420f94 Mon Sep 17 00:00:00 2001 From: Werner Dietl Date: Sun, 14 Jul 2024 17:48:50 -0400 Subject: [PATCH 8/8] Fix formatting --- src/test/java/tests/NullSpecTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index b98ec48..7584547 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -91,8 +91,7 @@ private static String[] getSamplesDirs() { private static String[] checkerOptions(boolean strict) { ImmutableList.Builder options = ImmutableList.builder(); - options.add( - "-AassumePure", "-AcheckImpl", "-AsuppressWarnings=conditional"); + options.add("-AassumePure", "-AcheckImpl", "-AsuppressWarnings=conditional"); if (strict) { options.add("-Astrict"); }