DEPRECATED_API_SHOULD_NOT_BE_USED has false positive if a user doesn't actually use deprecated features

[eirnym]

Hi,

I've found interesting false-positive example, where DEPRECATED_API_SHOULD_NOT_BE_USED trows an exception, while I don't actually use a deprecated feature.

Minimal example is below, it's in Kotlin.
There archunit found that JsonSerialize annotation has a deprecated field (include), but I don't use it directly, so this code should not violate the check.

Jackson version is 2.17.2
archunit version is 1.3.0
Java version: 17

import com.fasterxml.jackson.databind.annotation.JsonSerialize
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer

data class Response(
    @get:JsonSerialize(using = ToStringSerializer::class)
    val field: Int? = null,
)

[hankem]

I can reproduce your finding.

The generated bytecode only has the annotation `JsonSerialize` using `ToStringSerializer`

  public final java.lang.Integer getField();
    descriptor: ()Ljava/lang/Integer;
    flags: (0x0011) ACC_PUBLIC, ACC_FINAL
    Code:
      stack=1, locals=1, args_size=1
         0: aload_0
         1: getfield      #14                 // Field field:Ljava/lang/Integer;
         4: areturn
      LineNumberTable:
        line 23: 0
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0       5     0  this   Lcom/tngtech/archunit/issues/issue1333/Response;
    RuntimeVisibleAnnotations:
      0: #22(#23=c#24)
        com.fasterxml.jackson.databind.annotation.JsonSerialize(
          using=class Lcom/fasterxml/jackson/databind/ser/std/ToStringSerializer;
        )
    RuntimeInvisibleAnnotations:
      0: #7()
        org.jetbrains.annotations.Nullable

but the rule fails with:

Method <Response.getField()> has annotation member of type <com.fasterxml.jackson.databind.annotation.JsonSerialize$Inclusion>

I think this is because DEPRECATED_API_SHOULD_NOT_BE_USED forbids that classes dependOnClassesThat are annotatedWith Deprecated.class, and parameters of annotations are automatically considered as dependencies (whether you access them or not).

[eirnym]

I expect, that it would check actual usage, as name suggests.

Jackson is a very stable library, where deprecation can be for years before actually removed. This renders ArchUnit to conflict with quite popular Jackson library.

Is there's a way to ignore this particular field without adding archunit to production code and without changing code generation process?

[hankem]

Don't get me wrong: I see your point and agree that Jackson and JsonSerialize should be fine to use.
I just couldn't come up with a general solution for the DEPRECATED_API_SHOULD_NOT_BE_USED rule yet. 🙂

As a workaround, you can "fork" the predefined rule in your own code and whitelist that @Deprecated class JsonSerialize.Inclusion:

     val deprecatedApiShouldNotBeUsed = noClasses()
         .should(accessTargetWhere(target(annotatedWith(Deprecated::class.java))).`as`("access @Deprecated members"))
         .orShould(
             dependOnClassesThat(
+                not(equivalentTo(JsonSerialize.Inclusion::class.java)).and( // workaround for ArchUnit#1333
                     annotatedWith(Deprecated::class.java)
+                )
                     .`as`("depend on @Deprecated classes")
             )
         )

FYI: I'm using the following imports:

import com.tngtech.archunit.base.DescribedPredicate.not
import com.tngtech.archunit.core.domain.JavaAccess.Predicates.target
import com.tngtech.archunit.core.domain.JavaClass.Predicates.equivalentTo
import com.tngtech.archunit.core.domain.properties.CanBeAnnotated.Predicates.annotatedWith
import com.tngtech.archunit.lang.conditions.ArchConditions.accessTargetWhere
import com.tngtech.archunit.lang.conditions.ArchConditions.dependOnClassesThat
import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.noClasses

[hankem]

FYI: Only the additional condition "should not depend on @Deprecated classes" catches 6 violations in the following example from 3d48c64 where the directly accessed target (field or code unit) is actually not deprecated, but the owner class is:

        @Deprecated
        class DeprecatedClass {
            int target;

            void target() {
            }
        }

        class Origin {
            @DeprecatedAnnotation
            void origin() {
                DeprecatedClass instanceOfDeprecatedClass = new DeprecatedClass();
                instanceOfDeprecatedClass.target++;
                instanceOfDeprecatedClass.target();
                Class<?> deprecatedClass = DeprecatedClass.class;
            }
        }

We need to wrap our head around the question whether it's fair to say that we depend on a deprecated class when we use an annotation that has deprecated members. Probably the dependencies from annotations needs to be revised here.

[eirnym]

Thank you for your response.