Skip to content

Commit

Permalink
Use EISOP CF sources (#118)
Browse files Browse the repository at this point in the history
Co-authored-by: David P. Baker <[email protected]>
  • Loading branch information
wmdietl and netdpb committed Feb 6, 2024
1 parent 9e57ea8 commit 33817d8
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 425 deletions.
11 changes: 8 additions & 3 deletions initialize-project
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# Set SHALLOW=1 to clone sibling projects at depth 1.
#
# This script automatically tries to download your fork of
# jspecify/checker-framework, jspecify/jspecify, or jspecify/jdk, if they exist.
# eisop/checker-framework, jspecify/jspecify, or jspecify/jdk, if they exist.
# It uses the URL of the origin remote (the default remote created when cloning
# a repo) to determine that.
#
Expand Down Expand Up @@ -55,12 +55,17 @@ git_clone() {

local forking_org
forking_org="$(forking_org)"
if [[ -n "${forking_org}" ]]; then
if [[ -n "${forking_org}" ]] && [[ "${forking_org}" != "https://github.com/jspecify" ]]; then
if run "${git[@]}" "${forking_org}/${repo}.git" "../${repo}"; then
return
fi
fi
run "${git[@]}" "https://github.com/jspecify/${repo}.git" "../${repo}"
if [[ "${repo}" == checker-framework ]]; then
forking_org=https://github.com/eisop
else
forking_org=https://github.com/jspecify
fi
run "${git[@]}" "${forking_org}/${repo}.git" "../${repo}"
}

git_clone jdk --depth 1 --single-branch
Expand Down
12 changes: 6 additions & 6 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ includeBuild("../checker-framework")
dependencyResolutionManagement {
versionCatalogs {
libs {
version("checkerFramework", "3.21.5-SNAPSHOT")
version("checkerFramework", "3.42.0-eisop2-SNAPSHOT")

library("checkerFramework-checker", "org.checkerframework", "checker").versionRef("checkerFramework")
library("checkerFramework-checker-qual", "org.checkerframework", "checker-qual").versionRef("checkerFramework")
library("checkerFramework-framework", "org.checkerframework", "framework").versionRef("checkerFramework")
library("checkerFramework-framework-test", "org.checkerframework", "framework-test").versionRef("checkerFramework")
library("checkerFramework-javacutil", "org.checkerframework", "javacutil").versionRef("checkerFramework")
library("checkerFramework-checker", "io.github.eisop", "checker").versionRef("checkerFramework")
library("checkerFramework-checker-qual", "io.github.eisop", "checker-qual").versionRef("checkerFramework")
library("checkerFramework-framework", "io.github.eisop", "framework").versionRef("checkerFramework")
library("checkerFramework-framework-test", "io.github.eisop", "framework-test").versionRef("checkerFramework")
library("checkerFramework-javacutil", "io.github.eisop", "javacutil").versionRef("checkerFramework")
library("errorProne-core", "com.google.errorprone:error_prone_core:2.18.0")
library("errorProne-javac", "com.google.errorprone:javac:9+181-r4173-1")
library("guava", "com.google.guava:guava:31.1-jre")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@

package com.google.jspecify.nullness;

import java.util.Set;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.type.TypeMirror;
import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.framework.flow.CFAbstractAnalysis;
import org.checkerframework.framework.flow.CFValue;
import org.checkerframework.javacutil.AnnotationMirrorSet;

final class NullSpecAnalysis extends CFAbstractAnalysis<CFValue, NullSpecStore, NullSpecTransfer> {
NullSpecAnalysis(BaseTypeChecker checker, NullSpecAnnotatedTypeFactory factory) {
Expand All @@ -37,7 +36,7 @@ public NullSpecStore createCopiedStore(NullSpecStore other) {
}

@Override
public CFValue createAbstractValue(Set<AnnotationMirror> annotations, TypeMirror underlyingType) {
public CFValue createAbstractValue(AnnotationMirrorSet annotations, TypeMirror underlyingType) {
return defaultCreateAbstractValue(this, annotations, underlyingType);
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.sun.source.util.TreePath;
import com.sun.tools.javac.processing.JavacProcessingEnvironment;
import com.sun.tools.javac.util.Log;
import java.util.SortedSet;
import java.util.NavigableSet;
import java.util.TreeSet;
import javax.lang.model.element.TypeElement;
import org.checkerframework.common.basetype.BaseTypeChecker;
Expand Down Expand Up @@ -62,8 +62,8 @@ public final class NullSpecChecker extends BaseTypeChecker {
public NullSpecChecker() {}

@Override
public SortedSet<String> getSuppressWarningsPrefixes() {
SortedSet<String> prefixes = new TreeSet<>();
public NavigableSet<String> getSuppressWarningsPrefixes() {
TreeSet<String> prefixes = new TreeSet<>();
prefixes.add("nullness");
return prefixes;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.Collections.singleton;
import static java.util.Collections.unmodifiableSet;
import static java.util.stream.Collectors.toList;
import static org.checkerframework.dataflow.expression.JavaExpression.fromNode;
Expand Down Expand Up @@ -71,6 +70,7 @@
import org.checkerframework.framework.flow.CFValue;
import org.checkerframework.framework.type.AnnotatedTypeMirror;
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType;
import org.checkerframework.javacutil.AnnotationMirrorSet;

final class NullSpecTransfer extends CFAbstractTransfer<CFValue, NullSpecStore, NullSpecTransfer> {
private final Util util;
Expand Down Expand Up @@ -1019,7 +1019,8 @@ private boolean valueIsAtLeastAsSpecificAs(CFValue value, CFValue targetDataflow
if (target == null) {
return false;
}
return atypeFactory.getQualifierHierarchy().greatestLowerBound(existing, target) == existing;
return atypeFactory.getQualifierHierarchy().greatestLowerBoundQualifiersOnly(existing, target)
== existing;
}

private static boolean isNullLiteral(Node node) {
Expand Down Expand Up @@ -1052,7 +1053,8 @@ private void setResultValue(
* if (clazz.cast(foo) != null) { return class.cast(foo); }
*/
result.setResultValue(
analysis.createAbstractValue(singleton(qual), result.getResultValue().getUnderlyingType()));
analysis.createAbstractValue(
AnnotationMirrorSet.singleton(qual), result.getResultValue().getUnderlyingType()));
}

private JavaExpression expressionToStoreFor(Node node) {
Expand Down
14 changes: 8 additions & 6 deletions src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@
import com.sun.source.tree.TypeParameterTree;
import com.sun.source.tree.VariableTree;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
Expand All @@ -75,6 +73,7 @@
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedDeclaredType;
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType;
import org.checkerframework.javacutil.AnnotationBuilder;
import org.checkerframework.javacutil.AnnotationMirrorSet;

final class NullSpecVisitor extends BaseTypeVisitor<NullSpecAnnotatedTypeFactory> {
private final boolean checkImpl;
Expand All @@ -100,9 +99,11 @@ private void ensureNonNull(Tree tree, String messageKey) {
}
}

/* TODO: implement feature to add extra args to return type errors.
*
@Override
protected String extraArgForReturnTypeError(Tree tree) {
/*
/
* We call originStringIfTernary, not originString:
*
* If the statement is `return foo.bar()`, then the problem is obvious, so we don't want our
Expand All @@ -115,10 +116,11 @@ protected String extraArgForReturnTypeError(Tree tree) {
* the possibly null value (possibly both!). However, this gets tricky: If the branches return
* `Foo?` and `Foo*`, then we ideally want to emphasize the `Foo?` branch *but*, at least in
* "strict mode," not altogether ignore the `Foo*` branch.
*/
/
String origin = originStringIfTernary(tree);
return origin.isEmpty() ? "" : (origin + "\n");
}
*/

private String originString(Tree tree) {
while (tree instanceof ParenthesizedTree) {
Expand Down Expand Up @@ -621,8 +623,8 @@ private boolean isClassCastAppliedToNonNullableType(MemberReferenceTree tree) {
}

@Override
protected Set<? extends AnnotationMirror> getExceptionParameterLowerBoundAnnotations() {
return new HashSet<>(asList(AnnotationBuilder.fromClass(elements, MinusNull.class)));
protected AnnotationMirrorSet getExceptionParameterLowerBoundAnnotations() {
return new AnnotationMirrorSet(asList(AnnotationBuilder.fromClass(elements, MinusNull.class)));
}

@Override
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/tests/ConformanceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,8 @@ static final class DetailMessageReportedFact extends ReportedFact {

private static final ImmutableSet<String> CANNOT_CONVERT_KEYS =
ImmutableSet.of(
"argument",
"assignment",
"argument.type.incompatible",
"assignment.type.incompatible",
"atomicreference.must.include.null",
"cast.unsafe",
"lambda.param",
Expand All @@ -164,9 +164,9 @@ static final class DetailMessageReportedFact extends ReportedFact {
"methodref.return",
"override.param",
"override.return",
"return",
"return.type.incompatible",
"threadlocal.must.include.null",
"type.argument");
"type.argument.type.incompatible");

private static final ImmutableSet<String> IRRELEVANT_ANNOTATION_KEYS =
ImmutableSet.of(
Expand Down
22 changes: 11 additions & 11 deletions src/test/java/tests/NullSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,20 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) {
|| missing.getMessage().contains("jspecify_nullness_mismatch")
|| missing.getMessage().contains("test:cannot-convert")) {
switch (unexpected.messageKey) {
case "argument":
case "assignment":
case "argument.type.incompatible":
case "assignment.type.incompatible":
case "atomicreference.must.include.null":
case "cast.unsafe":
case "dereference":
case "lambda.param":
case "methodref.receiver.bound":
case "methodref.receiver":
case "methodref.return":
case "override.param":
case "override.return":
case "return":
case "lambda.param.type.incompatible":
case "methodref.receiver.bound.invalid":
case "methodref.receiver.invalid":
case "methodref.return.invalid":
case "override.param.invalid":
case "override.return.invalid":
case "return.type.incompatible":
case "threadlocal.must.include.null":
case "type.argument":
case "type.argument.type.incompatible":
return true;
default:
return false;
Expand All @@ -196,7 +196,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) {
* custom `*.annotated` error. This test probably doesn't confirm that second thing
* anymore, but I did manually confirm that it is true as of this writing.
*/
case "bound":
case "bound.type.incompatible":
case "local.variable.annotated":
case "type.parameter.annotated":
case "wildcard.annotated":
Expand Down
Loading

0 comments on commit 33817d8

Please sign in to comment.