Skip to content

Commit

Permalink
Use the new TypeInformationPresenter to output more type information (
Browse files Browse the repository at this point in the history
  • Loading branch information
wmdietl committed Apr 10, 2024
1 parent 570d80e commit c8d5933
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 10 deletions.
2 changes: 1 addition & 1 deletion settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ includeBuild("../checker-framework")
dependencyResolutionManagement {
versionCatalogs {
libs {
version("checkerFramework", "3.42.0-eisop2-SNAPSHOT")
version("checkerFramework", "3.42.0-eisop3-SNAPSHOT")

library("checkerFramework-checker", "io.github.eisop", "checker").versionRef("checkerFramework")
library("checkerFramework-checker-qual", "io.github.eisop", "checker-qual").versionRef("checkerFramework")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// 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.

package com.google.jspecify.nullness;

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;
import javax.lang.model.element.ExecutableElement;
import org.checkerframework.framework.type.AnnotatedTypeFactory;
import org.checkerframework.framework.type.AnnotatedTypeFormatter;
import org.checkerframework.framework.type.AnnotatedTypeMirror;
import org.checkerframework.framework.type.AnnotatedTypeMirror.AnnotatedExecutableType;
import org.checkerframework.framework.util.visualize.AbstractTypeInformationPresenter;
import org.checkerframework.framework.util.visualize.TypeOccurrenceKind;
import org.checkerframework.javacutil.TreeUtils;

/**
* Output "sinkType" and "sourceType" diagnostic warning messages so the conformance tests can look
* for (a subset of) them.
*/
public final class ConformanceTypeInformationPresenter extends AbstractTypeInformationPresenter {

/**
* Constructs a presenter for the given factory.
*
* @param atypeFactory the AnnotatedTypeFactory for the current analysis
*/
public ConformanceTypeInformationPresenter(AnnotatedTypeFactory atypeFactory) {
super(atypeFactory);
}

@Override
protected AnnotatedTypeFormatter createTypeFormatter() {
// Use the same type formatter as normal error messages. Look into whether a different format
// would be better here.
return atypeFactory.getAnnotatedTypeFormatter();
}

@Override
protected TypeInformationReporter createTypeInformationReporter(ClassTree tree) {
return new ConformanceTypeInformationReporter(tree);
}

class ConformanceTypeInformationReporter extends TypeInformationReporter {
ConformanceTypeInformationReporter(ClassTree tree) {
super(tree);
}

@Override
protected void reportTreeType(
Tree tree, AnnotatedTypeMirror type, TypeOccurrenceKind occurrenceKind) {
switch (tree.getKind()) {
case ASSIGNMENT:
AssignmentTree asgn = (AssignmentTree) tree;
AnnotatedTypeMirror varType =
genFactory != null
? genFactory.getAnnotatedTypeLhs(asgn.getVariable())
: atypeFactory.getAnnotatedType(asgn.getVariable());
checker.reportWarning(
asgn.getVariable(),
"sinkType",
typeFormatter.format(varType),
asgn.getVariable().toString());
break;
case RETURN:
checker.reportWarning(tree, "sinkType", typeFormatter.format(type), "return");
break;
case METHOD_INVOCATION:
ExecutableElement calledElem = TreeUtils.elementFromUse((MethodInvocationTree) tree);
String methodName = calledElem.getSimpleName().toString();
AnnotatedExecutableType calledType = (AnnotatedExecutableType) type;
List<? extends AnnotatedTypeMirror> params = calledType.getParameterTypes();
MethodInvocationTree mit = (MethodInvocationTree) tree;
List<? extends ExpressionTree> args = mit.getArguments();
assert params.size() == args.size();

for (int i = 0; i < params.size(); ++i) {
String paramName = calledElem.getParameters().get(i).getSimpleName().toString();
String paramLocation = String.format("%s#%s", methodName, paramName);
checker.reportWarning(
tree, "sinkType", typeFormatter.format(params.get(i)), paramLocation);
}
break;
default:
// Nothing special for other trees.
}

if (TreeUtils.isExpressionTree(tree)) {
checker.reportWarning(tree, "sourceType", typeFormatter.format(type), tree.toString());
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ final class NullSpecAnnotatedTypeFactory

private final AnnotatedDeclaredType javaUtilCollection;

private final ConformanceTypeInformationPresenter conformanceInformationPresenter;

final AnnotatedDeclaredType javaLangClass;
final AnnotatedDeclaredType javaLangThreadLocal;
final AnnotatedDeclaredType javaUtilMap;
Expand Down Expand Up @@ -305,6 +307,9 @@ private NullSpecAnnotatedTypeFactory(
withMostConvenientWorld = this;
}

conformanceInformationPresenter =
checker.hasOption("showTypes") ? new ConformanceTypeInformationPresenter(this) : null;

if (!givenOtherWorld) {
/*
* Now the withLeastConvenientWorld and withMostConvenientWorld fields of both `this` and
Expand Down Expand Up @@ -1707,6 +1712,10 @@ public void postProcessClassTree(ClassTree tree) {
* type.invalid.conflicting.annos error, which I have described more in
* https://github.com/jspecify/jspecify-reference-checker/commit/d16a0231487e239bc94145177de464b5f77c8b19
*/

if (conformanceInformationPresenter != null) {
conformanceInformationPresenter.process(tree, getPath(tree));
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@
* <li>"strict": Whether the checker should be a sound, strict type system. Does not imply that
* implementation code is checked.
* <li>"checkImpl": Whether implementation code should be checked.
* <li>"showTypes": Whether to output type information for the conformance test suite.
* </ol>
*/
@SupportedOptions({"strict", "checkImpl"})
@SupportedOptions({"strict", "checkImpl", "showTypes"})
public final class NullSpecChecker extends BaseTypeChecker {
/*
* A non-final field is ugly, but we can't create our Util instance in the constructor because the
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/tests/NullSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private boolean corresponds(TestDiagnostic missing, DetailMessage unexpected) {
}
case "jspecify_conflicting_annotations":
switch (unexpected.messageKey) {
case "conflicting.annos":
case "type.invalid.conflicting.annos":
return true;
default:
return false;
Expand Down
14 changes: 7 additions & 7 deletions tests/ConformanceTest-report.txt
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# 84 pass; 30 fail; 114 total; 73.7% score
FAIL: Basic.java:28:test:expression-type:Object?:nullable
FAIL: Basic.java:28:test:sink-type:Object!:return
# 90 pass; 24 fail; 114 total; 78.9% 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!
FAIL: Basic.java:34:test:expression-type:Object!:nonNull
FAIL: Basic.java:34:test:sink-type:Object?:return
FAIL: Basic.java:41:test:sink-type:Object?:nullableObject
FAIL: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString
PASS: Basic.java:34:test:expression-type:Object!:nonNull
PASS: Basic.java:34:test:sink-type:Object?:return
PASS: Basic.java:41:test:sink-type:Object?:nullableObject
PASS: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString
FAIL: Basic.java:49:test:expression-type:List!<capture of ? extends String?>:nullableStrings
PASS: Basic.java: no unexpected facts
PASS: Irrelevant.java:28:test:irrelevant-annotation:Nullable
Expand Down

0 comments on commit c8d5933

Please sign in to comment.