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:

```kt 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 https://github.com/TNG/ArchUnit/pull/970/commits/3d48c64200c1e10620fa81c794862e98a157f353 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.