Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Varargs capitalization #6698

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Use "Varargs" instead of "VarArgs"
  • Loading branch information
mernst committed Jul 5, 2024
commit e940dad5898f374f63397342f97e1b67ef93db7b
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
* the {@code t} varargs argument before the method returns.
*
* <p>This annotation is not checked. An error will always be issued when it is used.
*
* @deprecated Use {@link EnsuresCalledMethodsVarargs}.
*/
@Deprecated // 2024-07-03
@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
public @interface EnsuresCalledMethodsVarArgs {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package org.checkerframework.checker.calledmethods.qual;

import java.lang.annotation.ElementType;
import java.lang.annotation.Target;

/**
* Indicates that the method, if it terminates successfully, always invokes the given methods on all
* of the arguments passed in the varargs position.
*
* <p>Consider the following method:
*
* <pre>
* &#64;EnsuresCalledMethodsVarargs("m")
* public void callMOnAll(S s, T t...) { ... }
* </pre>
*
* <p>This method guarantees that {@code m()} is always called on every {@code T} object passed in
* the {@code t} varargs argument before the method returns.
*
* <p>This annotation is not checked. An error will always be issued when it is used.
*/
@Target({ElementType.METHOD, ElementType.CONSTRUCTOR})
public @interface EnsuresCalledMethodsVarargs {

/**
* Returns the methods guaranteed to be invoked on the varargs parameters.
*
* @return the methods guaranteed to be invoked on the varargs parameters
*/
String[] value();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.checkerframework.checker.calledmethods.qual.CalledMethodsPredicate;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethods;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsOnException;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarargs;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.common.accumulation.AccumulationAnnotatedTypeFactory;
import org.checkerframework.common.basetype.BaseTypeChecker;
Expand Down Expand Up @@ -74,9 +74,18 @@ public class CalledMethodsAnnotatedTypeFactory extends AccumulationAnnotatedType
/*package-private*/ final ExecutableElement calledMethodsValueElement =
TreeUtils.getMethod(CalledMethods.class, "value", 0, processingEnv);

/** The {@link EnsuresCalledMethodsVarargs#value} element/argument. */
/*package-private*/ final ExecutableElement ensuresCalledMethodsVarargsValueElement =
TreeUtils.getMethod(EnsuresCalledMethodsVarargs.class, "value", 0, processingEnv);

/** The {@link EnsuresCalledMethodsVarArgs#value} element/argument. */
@SuppressWarnings("deprecation") // EnsuresCalledMethodsVarArgs
/*package-private*/ final ExecutableElement ensuresCalledMethodsVarArgsValueElement =
TreeUtils.getMethod(EnsuresCalledMethodsVarArgs.class, "value", 0, processingEnv);
TreeUtils.getMethod(
org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs.class,
"value",
0,
processingEnv);

/** The {@link EnsuresCalledMethodsOnException#value} element/argument. */
/*package-private*/ final ExecutableElement ensuresCalledMethodsOnExceptionValueElement =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.Types;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarargs;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.checker.resourceleak.ResourceLeakChecker;
import org.checkerframework.common.accumulation.AccumulationStore;
Expand Down Expand Up @@ -123,7 +123,7 @@ public TransferResult<AccumulationValue, AccumulationStore> visitMethodInvocatio
super.visitMethodInvocation(node, input);

ExecutableElement method = TreeUtils.elementFromUse(node.getTree());
handleEnsuresCalledMethodsVarArgs(node, method, superResult);
handleEnsuresCalledMethodsVarargs(node, method, superResult);
handleEnsuresCalledMethodsOnException(node, method, exceptionalStores);

Node receiver = node.getTarget().getReceiver();
Expand Down Expand Up @@ -208,27 +208,44 @@ private Map<TypeMirror, AccumulationStore> makeExceptionalStores(

/**
* Update the types of varargs parameters passed to a method with an {@link
* EnsuresCalledMethodsVarArgs} annotation. This method is a no-op if no such annotation is
* EnsuresCalledMethodsVarargs} annotation. This method is a no-op if no such annotation is
* present.
*
* @param node the method invocation node
* @param elt the method being invoked
* @param result the current result
*/
private void handleEnsuresCalledMethodsVarArgs(
@SuppressWarnings("deprecation") // EnsuresCalledMethodsVarArgs
private void handleEnsuresCalledMethodsVarargs(
MethodInvocationNode node,
ExecutableElement elt,
TransferResult<AccumulationValue, AccumulationStore> result) {
AnnotationMirror annot = atypeFactory.getDeclAnnotation(elt, EnsuresCalledMethodsVarArgs.class);
AnnotationMirror annot = atypeFactory.getDeclAnnotation(elt, EnsuresCalledMethodsVarargs.class);
// Temporary, for backward compatibility.
if (annot == null) {
annot =
atypeFactory.getDeclAnnotation(
elt,
org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs.class);
}
if (annot == null) {
return;
}
List<String> ensuredMethodNames =
AnnotationUtils.getElementValueArray(
annot,
((CalledMethodsAnnotatedTypeFactory) atypeFactory)
.ensuresCalledMethodsVarArgsValueElement,
.ensuresCalledMethodsVarargsValueElement,
String.class);
// Temporary, for backward compatibility.
if (ensuredMethodNames.isEmpty()) {
ensuredMethodNames =
AnnotationUtils.getElementValueArray(
annot,
((CalledMethodsAnnotatedTypeFactory) atypeFactory)
.ensuresCalledMethodsVarArgsValueElement,
String.class);
}
List<? extends VariableElement> parameters = elt.getParameters();
int varArgsPos = parameters.size() - 1;
Node varArgActual = node.getArguments().get(varArgsPos);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import javax.tools.Diagnostic;
import org.checkerframework.checker.calledmethods.builder.BuilderFrameworkSupport;
import org.checkerframework.checker.calledmethods.qual.CalledMethods;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs;
import org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarargs;
import org.checkerframework.common.accumulation.AccumulationVisitor;
import org.checkerframework.common.basetype.BaseTypeChecker;
import org.checkerframework.dataflow.expression.JavaExpression;
Expand Down Expand Up @@ -43,24 +43,35 @@ public CalledMethodsVisitor(BaseTypeChecker checker) {
}

/**
* Issue an error at every EnsuresCalledMethodsVarArgs annotation, because using it is unsound.
* Issue an error at every EnsuresCalledMethodsVarargs annotation, because using it is unsound.
*/
@Override
public Void visitAnnotation(AnnotationTree tree, Void p) {
AnnotationMirror anno = TreeUtils.annotationFromAnnotationTree(tree);
if (AnnotationUtils.areSameByName(
anno, "org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs")) {
anno, "org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarargs")
// Temporary, for backward compatibility.
|| AnnotationUtils.areSameByName(
anno, "org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs")) {
// We can't verify these yet. Emit an error (which will have to be suppressed) for now.
checker.report(tree, new DiagMessage(Diagnostic.Kind.ERROR, "ensuresvarargs.unverified"));
}
return super.visitAnnotation(tree, p);
}

@Override
@SuppressWarnings("deprecation") // EnsuresCalledMethodsVarArgs
public void processMethodTree(MethodTree tree) {
ExecutableElement elt = TreeUtils.elementFromDeclaration(tree);
AnnotationMirror ecmva = atypeFactory.getDeclAnnotation(elt, EnsuresCalledMethodsVarArgs.class);
if (ecmva != null) {
AnnotationMirror ecmv = atypeFactory.getDeclAnnotation(elt, EnsuresCalledMethodsVarargs.class);
// Temporary, for backward compatibility.
if (ecmv == null) {
ecmv =
atypeFactory.getDeclAnnotation(
elt,
org.checkerframework.checker.calledmethods.qual.EnsuresCalledMethodsVarArgs.class);
}
if (ecmv != null) {
if (!elt.isVarArgs()) {
checker.report(tree, new DiagMessage(Diagnostic.Kind.ERROR, "ensuresvarargs.invalid"));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
finalizer.invocation=This finalizer cannot be invoked, because the following methods have not been called: %s
ensuresvarargs.unverified=@EnsuresCalledMethodsVarArgs cannot be verified yet. Please check that the implementation of the method actually does call the given methods on the varargs parameters by hand, and then suppress this warning.
ensuresvarargs.invalid=@EnsuresCalledMethodsVarArgs cannot be written on a non-varargs method.
ensuresvarargs.unverified=@EnsuresCalledMethodsVarargs cannot be verified yet. Please check that the implementation of the method actually does call the given methods on the varargs parameters by hand, and then suppress this warning.
ensuresvarargs.invalid=@EnsuresCalledMethodsVarargs cannot be written on a non-varargs method.
contracts.exceptional.postcondition=on exception, postcondition of %s is not satisfied.%nfound : %s%nrequired: %s
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class IOUtils {
static void closeQuietly(Closeable arg0);

@SuppressWarnings("ensuresvarargs.unverified")
@EnsuresCalledMethodsVarArgs("close")
@EnsuresCalledMethodsVarargs("close")
static void closeQuietly(Closeable... arg0);

@EnsuresCalledMethods(value = "#1", methods = "close")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import org.checkerframework.checker.calledmethods.qual.*;
import org.checkerframework.checker.mustcall.qual.*;

class EnsuresCalledMethodsVarArgsTest {
class EnsuresCalledMethodsVarargsTest {

@InheritableMustCall("a")
static class Foo {
Expand All @@ -10,7 +10,7 @@ void a() {}

static class Utils {
@SuppressWarnings("ensuresvarargs.unverified")
@EnsuresCalledMethodsVarArgs("a")
@EnsuresCalledMethodsVarargs("a")
public static void close(Foo... foos) {
for (Foo f : foos) {
if (f != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// A simple test for the @EnsuresCalledMethodsVarArgs annotation.
// A simple test for the @EnsuresCalledMethodsVarargs annotation.

import java.io.IOException;
import java.net.Socket;
import java.util.List;
import org.checkerframework.checker.calledmethods.qual.*;

class EnsuresCalledMethodsVarArgsSimple {
class EnsuresCalledMethodsVarargsSimple {

// :: error: (ensuresvarargs.unverified)
@EnsuresCalledMethodsVarArgs("close")
@EnsuresCalledMethodsVarargs("close")
void closeAll(Socket... sockets) {
for (Socket s : sockets) {
try {
Expand All @@ -19,7 +19,7 @@ void closeAll(Socket... sockets) {
}

// :: error: (ensuresvarargs.unverified)
@EnsuresCalledMethodsVarArgs("close")
@EnsuresCalledMethodsVarargs("close")
// :: error: (ensuresvarargs.invalid)
void closeAllNotVA(List<Socket> sockets) {
for (Socket s : sockets) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
public class VarArgsIncompatible {
public class VarargsIncompatible {

public static void test(int[] arr) {
help(arr);
Expand Down
6 changes: 3 additions & 3 deletions checker/tests/nonempty/UnmodifiableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ void checkNonEmptyThenCopy(List<String> strs) {
}
}

void testVarArgsEmpty() {
void testVarargsEmpty() {
// :: error: (assignment)
@NonEmpty List<String> items = List.of();
}

void testVarArgsNonEmptyList() {
void testVarargsNonEmptyList() {
// Requires more than 10 elements to invoke the varargs version
@NonEmpty List<Integer> items = List.of(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); // OK
}

void testVarArgsNonEmptyMap() {
void testVarargsNonEmptyMap() {
// Requires more than 10 elements to invoke the varargs version
@NonEmpty
Map<String, Integer> map =
Expand Down
24 changes: 12 additions & 12 deletions checker/tests/nullness/Issue2171.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
public class Issue2171 {
static void varArgsMethod(@PolyNull Object... args) {}

static void callToVarArgsObject(@PolyNull Object pn, @NonNull Object nn, @Nullable Object nble) {
static void callToVarargsObject(@PolyNull Object pn, @NonNull Object nn, @Nullable Object nble) {
varArgsMethod(nble, nble);
varArgsMethod(nble, nn);
varArgsMethod(nble, pn);
Expand All @@ -17,19 +17,19 @@ static void callToVarArgsObject(@PolyNull Object pn, @NonNull Object nn, @Nullab
}

@SuppressWarnings("unchecked")
static void genVarArgsMethod(List<? extends @PolyNull Object>... args) {}
static void genVarargsMethod(List<? extends @PolyNull Object>... args) {}

@SuppressWarnings("unchecked")
static void genCallToVarArgsObject(
static void genCallToVarargsObject(
List<@PolyNull Object> pn, List<@NonNull Object> nn, List<@Nullable Object> nble) {
genVarArgsMethod(nble, nble);
genVarArgsMethod(nble, nn);
genVarArgsMethod(nble, pn);
genVarArgsMethod(nn, nble);
genVarArgsMethod(nn, nn);
genVarArgsMethod(nn, pn);
genVarArgsMethod(pn, nble);
genVarArgsMethod(pn, nn);
genVarArgsMethod(pn, pn);
genVarargsMethod(nble, nble);
genVarargsMethod(nble, nn);
genVarargsMethod(nble, pn);
genVarargsMethod(nn, nble);
genVarargsMethod(nn, nn);
genVarargsMethod(nn, pn);
genVarargsMethod(pn, nble);
genVarargsMethod(pn, nn);
genVarargsMethod(pn, pn);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import java.util.Arrays;
import java.util.Set;

public class VarArgsTest {
public class VarargsTest {
// :: warning: [unchecked] Possible heap pollution from parameterized vararg type
// java.util.Set<? super X>
<X> void test(Set<? super X>... args) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
import org.checkerframework.checker.tainting.qual.*;

class PolyVarArgs {
class PolyVarargs {

void testVarArgsNoFormals() {
void testVarargsNoFormals() {
@Tainted String tainted = varArgsNoFormals();
@Untainted String untainted = varArgsNoFormals("a");
@Untainted String untainted2 = varArgsNoFormals("b", "c");
}

void testVarArgsNoFormalsInvalid() {
void testVarargsNoFormalsInvalid() {
// :: error: (assignment)
@Untainted String tainted = varArgsNoFormals();
}

void testVarArgsWithFormals() {
void testVarargsWithFormals() {
@Tainted String tainted = varArgsWithFormals(1);
@Untainted String untainted = varArgsWithFormals(1, "a");
@Untainted String untainted2 = varArgsWithFormals(1, "a", "b");
}

void testVarArgsWithFormalsInvalid() {
void testVarargsWithFormalsInvalid() {
// :: error: (assignment)
@Untainted String tainted = varArgsWithFormals(1);
}

void testVarArgsWithPolyFormals() {
void testVarargsWithPolyFormals() {
@Tainted String tainted = varArgsWithPolyFormals(1);

// :: warning: (cast.unsafe)
@Untainted int safeInt = (@Untainted int) 1;
@Untainted String untainted = varArgsWithPolyFormals(safeInt, "a");
}

void testVarArgsWithPolyFormalsInvalid() {
void testVarargsWithPolyFormalsInvalid() {
// :: error: (assignment)
@Untainted String tainted = varArgsWithPolyFormals(1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ public JavaExpression atConstructorInvocation(NewClassTree newClassTree) {
*/
private static List<JavaExpression> argumentTreesToJavaExpressions(
ExecutableElement method, List<? extends ExpressionTree> argTrees) {
if (isVarArgsInvocation(method, argTrees)) {
if (isVarargsInvocation(method, argTrees)) {
List<JavaExpression> result = new ArrayList<>(method.getParameters().size());
for (int i = 0; i < method.getParameters().size() - 1; i++) {
result.add(JavaExpression.fromTree(argTrees.get(i)));
Expand Down Expand Up @@ -897,7 +897,7 @@ private static List<JavaExpression> argumentTreesToJavaExpressions(
* @param args the arguments at the call site
* @return true if method is a varargs method and its varargs arguments are not passed in an array
*/
private static boolean isVarArgsInvocation(
private static boolean isVarargsInvocation(
ExecutableElement method, List<? extends ExpressionTree> args) {
if (!method.isVarArgs()) {
return false;
Expand Down
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ Version 3.45.1 (August 1, 2024)

**User-visible changes:**

Deprecated `@EnsuresCalledMethodsVarArgs`; use `@EnsuresCalledMethodsVarargs` instead.

**Implementation details:**

**Closed issues:**
Expand Down
Loading