Merge branch 'master' into yoo/refactor-methodValues

typetools · May 3, 2024 · ae7e557 · ae7e557
2 parents a06c390 + 5514dd2
commit ae7e557
Show file tree

Hide file tree

Showing 18 changed files with 124 additions and 65 deletions.
diff --git a/build.gradle b/build.gradle
@@ -59,7 +59,7 @@ ext {
     hashmapUtil : '0.0.1',
     reflectionUtil : '1.1.3',
     plumeUtil : '1.9.3',
-    errorprone : '2.27.0',
+    errorprone : '2.27.1',
   ]
 }
 
@@ -953,6 +953,9 @@ subprojects {
     tasks.withType(Test) {
       jvmArgs += compilerArgsForRunningCF
 
+      // maxParallelForks controls the parallelism of a single Test
+      // task. --parallel controls parallelism for the entire build.
+
       // Run tests in parallel, except on CI where it seems to lead to flaky failures.
       // The TF_BUILD environment variable is set to 'True' for jobs running on Azure Pipelines.
       if (!System.getenv('TF_BUILD')?.equals('True')) {
@@ -964,6 +967,9 @@ subprojects {
       } else {
         // Running under Azure Pipelines CI.
 
+        // Per Manu's comment above, on CI, parallelism seems to lead to flaky failures.
+        maxParallelForks = 1
+
         // Azure seems to time out when a task doesn't produce periodic output.
         if (project.name.is('checker')) {
           testLogging {

diff --git a/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanTransfer.java b/checker/src/main/java/org/checkerframework/checker/index/inequality/LessThanTransfer.java
@@ -50,7 +50,7 @@ protected void refineGT(
     // left > right so right < left
     // Refine right to @LessThan("left")
     JavaExpression leftJe = JavaExpression.fromNode(left);
-    if (leftJe != null && leftJe.isUnassignableByOtherCode()) {
+    if (leftJe != null && !leftJe.isAssignableByOtherCode()) {
       if (isDoubleOrFloatLiteral(leftJe)) {
         return;
       }
@@ -85,7 +85,7 @@ protected void refineGTE(
     // left > right so right is less than left
     // Refine right to @LessThan("left")
     JavaExpression leftJe = JavaExpression.fromNode(left);
-    if (leftJe != null && leftJe.isUnassignableByOtherCode()) {
+    if (leftJe != null && !leftJe.isAssignableByOtherCode()) {
       if (isDoubleOrFloatLiteral(leftJe)) {
         return;
       }
@@ -111,7 +111,7 @@ public TransferResult<CFValue, CFStore> visitNumericalSubtraction(
       NumericalSubtractionNode n, TransferInput<CFValue, CFStore> in) {
     LessThanAnnotatedTypeFactory factory = (LessThanAnnotatedTypeFactory) analysis.getTypeFactory();
     JavaExpression leftJe = JavaExpression.fromNode(n.getLeftOperand());
-    if (leftJe != null && leftJe.isUnassignableByOtherCode()) {
+    if (leftJe != null && !leftJe.isAssignableByOtherCode()) {
       ValueAnnotatedTypeFactory valueFactory = factory.getValueAnnotatedTypeFactory();
       Long right = ValueCheckerUtils.getMinValue(n.getRightOperand().getTree(), valueFactory);
       if (right != null && 0 < right) {

diff --git a/checker/src/main/java/org/checkerframework/checker/nullness/NullnessTransfer.java b/checker/src/main/java/org/checkerframework/checker/nullness/NullnessTransfer.java
@@ -397,7 +397,7 @@ public TransferResult<NullnessValue, NullnessStore> visitMethodInvocation(
     Node receiver = n.getTarget().getReceiver();
     if (nonNullAssumptionAfterInvocation
         || isMethodSideEffectFree
-        || JavaExpression.fromNode(receiver).isUnassignableByOtherCode()) {
+        || !JavaExpression.fromNode(receiver).isAssignableByOtherCode()) {
       // Make receiver non-null.
       makeNonNull(result, receiver);
     }
@@ -413,7 +413,7 @@ public TransferResult<NullnessValue, NullnessStore> visitMethodInvocation(
       if (methodParams.get(i).hasPrimaryAnnotation(NONNULL)
           && (nonNullAssumptionAfterInvocation
               || isMethodSideEffectFree
-              || JavaExpression.fromTree(methodArgs.get(i)).isUnassignableByOtherCode())) {
+              || !JavaExpression.fromTree(methodArgs.get(i)).isAssignableByOtherCode())) {
         makeNonNull(result, n.getArgument(i));
       }
     }

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayAccess.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayAccess.java
@@ -58,13 +58,13 @@ public JavaExpression getIndex() {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return false;
+  public boolean isAssignableByOtherCode() {
+    return true;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return false;
+  public boolean isModifiableByOtherCode() {
+    return true;
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayCreation.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/ArrayCreation.java
@@ -75,13 +75,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return false;
+  public boolean isAssignableByOtherCode() {
+    return true;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return false;
+  public boolean isModifiableByOtherCode() {
+    return true;
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/BinaryOperation.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/BinaryOperation.java
@@ -89,13 +89,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return left.isUnassignableByOtherCode() && right.isUnassignableByOtherCode();
+  public boolean isAssignableByOtherCode() {
+    return left.isAssignableByOtherCode() || right.isAssignableByOtherCode();
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return left.isUnmodifiableByOtherCode() && right.isUnmodifiableByOtherCode();
+  public boolean isModifiableByOtherCode() {
+    return left.isModifiableByOtherCode() || right.isModifiableByOtherCode();
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/ClassName.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/ClassName.java
@@ -61,13 +61,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return true;
+  public boolean isAssignableByOtherCode() {
+    return false;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return true;
+  public boolean isModifiableByOtherCode() {
+    return false;
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/FieldAccess.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/FieldAccess.java
@@ -159,13 +159,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return isFinal() && getReceiver().isUnassignableByOtherCode();
+  public boolean isAssignableByOtherCode() {
+    return !isFinal() || getReceiver().isAssignableByOtherCode();
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return isUnassignableByOtherCode() && TypesUtils.isImmutableTypeInJdk(getReceiver().type);
+  public boolean isModifiableByOtherCode() {
+    return isAssignableByOtherCode() || !TypesUtils.isImmutableTypeInJdk(getReceiver().type);
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/FormalParameter.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/FormalParameter.java
@@ -106,13 +106,13 @@ public boolean containsSyntacticEqualJavaExpression(JavaExpression other) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return true;
+  public boolean isAssignableByOtherCode() {
+    return false;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return true;
+  public boolean isModifiableByOtherCode() {
+    return false;
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/JavaExpression.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/JavaExpression.java
@@ -126,10 +126,34 @@ public static boolean listIsDeterministic(
    * final field accesses whose receiver is {@link #isUnassignableByOtherCode}, and operations whose
    * operands are all {@link #isUnmodifiableByOtherCode}.
    *
+   * @return true if no subexpression of this can be assigned to from outside the current method
+   *     body
    * @see #isUnmodifiableByOtherCode
+   * @deprecated use {@link #isAssignableByOtherCode}
    */
+  @Deprecated // 2024-04-30
   @Pure
-  public abstract boolean isUnassignableByOtherCode();
+  public boolean isUnassignableByOtherCode() {
+    return !isAssignableByOtherCode();
+  }
+
+  /**
+   * Returns true if some subexpression of this can be assigned to from outside the current method
+   * body.
+   *
+   * <p>This is false for local variables, the self reference, final field accesses whose receiver
+   * is {@link #isUnassignableByOtherCode}, and operations whose operands are all not {@link
+   * #isModifiableByOtherCode}.
+   *
+   * @return true if some subexpression of this can be assigned to from outside the current method
+   *     body
+   * @see #isModifiableByOtherCode
+   */
+  // TODO: Make abstract when isUnassignableByOtherCode is removed.
+  @Pure
+  public boolean isAssignableByOtherCode() {
+    return !isUnassignableByOtherCode();
+  }
 
   /**
    * Returns true if and only if the value this expression stands for cannot be changed by a method
@@ -138,10 +162,34 @@ public static boolean listIsDeterministic(
    * <p>Approximately, this returns true if the expression is {@link #isUnassignableByOtherCode} and
    * its type is immutable.
    *
+   * @return true if the value of this expression cannot be changed from outside the current method
+   *     body
    * @see #isUnassignableByOtherCode
+   * @deprecated use {@link #isModifiableByOtherCode}
    */
+  @Deprecated // 2024-04-30
   @Pure
-  public abstract boolean isUnmodifiableByOtherCode();
+  public boolean isUnmodifiableByOtherCode() {
+    return !isModifiableByOtherCode();
+  }
+
+  /**
+   * Returns true if the value this expression stands for can be changed by a method call;
+   * equivalently, if the value this expression evaluates to can be changed by a side effect from
+   * outside the containing method.
+   *
+   * <p>Approximately, this returns true if the expression is {@link #isAssignableByOtherCode} or
+   * its type is mutable. ({@code String} is an immutable type.)
+   *
+   * @return true if the value of this expression can be changed from outside the current method
+   *     body
+   * @see #isUnassignableByOtherCode
+   */
+  // TODO: Make abstract when isUnmodifiableByOtherCode is removed.
+  @Pure
+  public boolean isModifiableByOtherCode() {
+    return !isUnmodifiableByOtherCode();
+  }
 
   /**
    * Returns true if and only if the two Java expressions are syntactically identical.

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/LocalVariable.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/LocalVariable.java
@@ -117,13 +117,13 @@ public boolean containsSyntacticEqualJavaExpression(JavaExpression other) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return true;
+  public boolean isAssignableByOtherCode() {
+    return false;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return TypesUtils.isImmutableTypeInJdk(((VarSymbol) element).type);
+  public boolean isModifiableByOtherCode() {
+    return !TypesUtils.isImmutableTypeInJdk(((VarSymbol) element).type);
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/MethodCall.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/MethodCall.java
@@ -93,16 +93,18 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
+  public boolean isAssignableByOtherCode() {
+    // TODO: The following comment is no longer accurate.  It should be removed and the
+    // implementation changed.
     // There is no need to check that the method is deterministic, because a MethodCall is
     // only created for deterministic methods.
-    return receiver.isUnmodifiableByOtherCode()
-        && arguments.stream().allMatch(JavaExpression::isUnmodifiableByOtherCode);
+    return receiver.isModifiableByOtherCode()
+        || arguments.stream().anyMatch(JavaExpression::isModifiableByOtherCode);
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return isUnassignableByOtherCode();
+  public boolean isModifiableByOtherCode() {
+    return isAssignableByOtherCode();
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/ThisReference.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/ThisReference.java
@@ -43,13 +43,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return true;
+  public boolean isAssignableByOtherCode() {
+    return false;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return TypesUtils.isImmutableTypeInJdk(type);
+  public boolean isModifiableByOtherCode() {
+    return !TypesUtils.isImmutableTypeInJdk(type);
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/UnaryOperation.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/UnaryOperation.java
@@ -73,13 +73,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return operand.isUnassignableByOtherCode();
+  public boolean isAssignableByOtherCode() {
+    return operand.isAssignableByOtherCode();
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return operand.isUnmodifiableByOtherCode();
+  public boolean isModifiableByOtherCode() {
+    return operand.isModifiableByOtherCode();
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/Unknown.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/Unknown.java
@@ -82,13 +82,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return false;
+  public boolean isAssignableByOtherCode() {
+    return true;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return false;
+  public boolean isModifiableByOtherCode() {
+    return true;
   }
 
   @Override

diff --git a/dataflow/src/main/java/org/checkerframework/dataflow/expression/ValueLiteral.java b/dataflow/src/main/java/org/checkerframework/dataflow/expression/ValueLiteral.java
@@ -110,13 +110,13 @@ public boolean isDeterministic(AnnotationProvider provider) {
   }
 
   @Override
-  public boolean isUnassignableByOtherCode() {
-    return true;
+  public boolean isAssignableByOtherCode() {
+    return false;
   }
 
   @Override
-  public boolean isUnmodifiableByOtherCode() {
-    return true;
+  public boolean isModifiableByOtherCode() {
+    return false;
   }
 
   @Override

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -1,4 +1,4 @@
-Version 3.43.1 (June 1, 2024)
+Version 3.43.1 (June 3, 2024)
 -----------------------------
 
 **User-visible changes:**
@@ -8,6 +8,9 @@ Version 3.43.1 (June 1, 2024)
 Renamed `CFAbstractStore#methodValues` to
 `CFAbstractStore#methodCallExpressions`.
 
+Deprecated `isUnassignableByOtherCode()` in favor of `isAssignableByOtherCode()`.
+Deprecated `isUnmodifiableByOtherCode()` in favor of `isModifiableByOtherCode()`.
+
 **Closed issues:**
 
 Version 3.43.0 (May 1, 2024)