From 4ef2d156c478aa5599225a4e5346010c3d000137 Mon Sep 17 00:00:00 2001
From: Marcono1234 <Marcono1234@users.noreply.github.com>
Date: Mon, 28 Mar 2022 00:22:18 +0200
Subject: [PATCH] Java: Deprecate error-prone and rarely used annotation
 predicates

---
 java/ql/lib/semmle/code/java/Annotation.qll   |  8 +++++++-
 java/ql/lib/semmle/code/java/Dependency.qll   |  2 +-
 .../lib/semmle/code/java/DependencyCounts.qll |  2 +-
 .../lib/semmle/code/java/JDKAnnotations.qll   | 19 ++++++++++---------
 4 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/Annotation.qll b/java/ql/lib/semmle/code/java/Annotation.qll
index c8af982b81f..237c2166f94 100644
--- a/java/ql/lib/semmle/code/java/Annotation.qll
+++ b/java/ql/lib/semmle/code/java/Annotation.qll
@@ -45,12 +45,18 @@ class Annotation extends @annotation, Expr {
   }
 
   /**
+   * DEPRECATED: Getting the value of _any_ annotation element is error-prone because
+   * it could lead to selecting the value of the wrong element by accident (for example
+   * when an annotation type is extended in the future). Prefer the predicate `getValue(string)`
+   * and explicitly specify the element name. Use `getValue(_)` if it is really desired to
+   * get the value of any element.
+   *
    * Gets a value of an annotation element. This includes default values in case
    * no explicit value is specified. For elements with an array value type this
    * might have an `ArrayInit` as result. To properly handle array values, prefer
    * the predicate `getAnArrayValue`.
    */
-  Expr getAValue() { filteredAnnotValue(this, _, result) }
+  deprecated Expr getAValue() { filteredAnnotValue(this, _, result) }
 
   /**
    * Gets the value of the annotation element with the specified `name`.
diff --git a/java/ql/lib/semmle/code/java/Dependency.qll b/java/ql/lib/semmle/code/java/Dependency.qll
index f69dbe6efc8..17236c6d05e 100644
--- a/java/ql/lib/semmle/code/java/Dependency.qll
+++ b/java/ql/lib/semmle/code/java/Dependency.qll
@@ -71,7 +71,7 @@ predicate depends(RefType t, RefType dep) {
       a.getAnnotatedElement().(Member).getDeclaringType() = t
     |
       usesType(a.getType(), dep) or
-      usesType(a.getAValue().getType(), dep) or
+      usesType(a.getValue(_).getType(), dep) or
       usesType(a.getAnArrayValue(_).getType(), dep)
     )
     or
diff --git a/java/ql/lib/semmle/code/java/DependencyCounts.qll b/java/ql/lib/semmle/code/java/DependencyCounts.qll
index dca8c44e18d..1010be48055 100644
--- a/java/ql/lib/semmle/code/java/DependencyCounts.qll
+++ b/java/ql/lib/semmle/code/java/DependencyCounts.qll
@@ -90,7 +90,7 @@ predicate numDepends(RefType t, RefType dep, int value) {
       |
         elem = a and usesType(a.getType(), dep)
         or
-        elem = [a.getAValue(), a.getAnArrayValue(_)] and
+        elem = [a.getValue(_), a.getAnArrayValue(_)] and
         elem.getFile().getExtension() = "java" and
         usesType(elem.(Expr).getType(), dep)
       )
diff --git a/java/ql/lib/semmle/code/java/JDKAnnotations.qll b/java/ql/lib/semmle/code/java/JDKAnnotations.qll
index 91dfae2da83..d3207f41659 100644
--- a/java/ql/lib/semmle/code/java/JDKAnnotations.qll
+++ b/java/ql/lib/semmle/code/java/JDKAnnotations.qll
@@ -19,17 +19,15 @@ class SuppressWarningsAnnotation extends Annotation {
   SuppressWarningsAnnotation() { this.getType().hasQualifiedName("java.lang", "SuppressWarnings") }
 
   /**
-   * Gets the `StringLiteral` of a warning suppressed by this annotation. To get the name of a suppressed
-   * warning, prefer `getASuppressedWarning()`. That predicate considers more cases because it does not
-   * restrict results to `StringLiteral`.
+   * DEPRECATED: This predicate restricts the results to `StringLiteral`; prefer `getASuppressedWarning()`
+   * to get the name of a suppressed warning.
+   *
+   * Gets the `StringLiteral` of a warning suppressed by this annotation.
    */
-  StringLiteral getASuppressedWarningLiteral() { result = this.getAnArrayValue("value") }
+  deprecated StringLiteral getASuppressedWarningLiteral() { result = this.getAnArrayValue("value") }
 
   /** Gets the name of a warning suppressed by this annotation. */
-  string getASuppressedWarning() {
-    // Don't use getASuppressedWarningLiteral() because that restricts results to StringLiteral
-    result = this.getAStringArrayValue("value")
-  }
+  string getASuppressedWarning() { result = this.getAStringArrayValue("value") }
 }
 
 /** A `@Target` annotation. */
@@ -37,12 +35,15 @@ class TargetAnnotation extends Annotation {
   TargetAnnotation() { this.getType().hasQualifiedName("java.lang.annotation", "Target") }
 
   /**
+   * DEPRECATED: Getting the field access expression is rarely useful. Use `getATargetElementType()`
+   * to get the name of the target element.
+   *
    * Gets a target expression within this annotation.
    *
    * For example, the field access `ElementType.FIELD` is a target expression in
    * `@Target({ElementType.FIELD, ElementType.METHOD})`.
    */
-  Expr getATargetExpression() { result = this.getAnArrayValue("value") }
+  deprecated Expr getATargetExpression() { result = this.getAnArrayValue("value") }
 
   /**
    * Gets the name of a target element type.