Kotlin / kotlinx-kover

Apache License 2.0
1.37k stars 53 forks source link

Difficulty excluding some generated code #690

Open ansman opened 1 month ago

ansman commented 1 month ago

Describe the bug Dagger generates code that looks like this:

@ScopeMetadata
@QualifierMetadata
@DaggerGenerated
@Generated(
    value = "dagger.internal.codegen.ComponentProcessor",
    comments = "https://dagger.dev"
)
@SuppressWarnings({
    "unchecked",
    "rawtypes",
    "KotlinInternal",
    "KotlinInternalInJava",
    "cast",
    "deprecation"
})
public final class ElapsedRealTimeClockImpl_Factory implements Factory<ElapsedRealTimeClockImpl> {
  @Override
  public ElapsedRealTimeClockImpl get() {
    return newInstance();
  }

  public static ElapsedRealTimeClockImpl_Factory create() {
    return InstanceHolder.INSTANCE;
  }

  public static ElapsedRealTimeClockImpl newInstance() {
    return new ElapsedRealTimeClockImpl();
  }

  private static final class InstanceHolder {
    private static final ElapsedRealTimeClockImpl_Factory INSTANCE = new ElapsedRealTimeClockImpl_Factory();
  }
}

We exclude this by excluding classes annotated by @Generated and @DaggerGenerated. The issue is the inner InstanceHolder class which isn't annotated.

Expected behavior I'm not sure I'd honestly expect it to exclude this class since it's not annotated, but most code generation frameworks will only annotate the top class and most people will want to exclude the inner class too so there should be some mechanism for excluding these.

Reports

image

Environment

NinoDLC commented 1 month ago

Unfortunately they don't want to spread the exclusion of annotated classes to their inner classes.

https://github.com/Kotlin/kotlinx-kover/issues/331

That being said, I have exactly the same issue, and even more "conventional" way of excluding this nasty inner class InstanceHolder won't work. I tried everything I could think of, this ain't moving from the report.

kover {
    reports {
        filters {
            excludes {
                ...
                packages(
                    "InstanceHolder",
                    "*InstanceHolder",
                    "\$*InstanceHolder",
                    "*\$*InstanceHolder",
                    "*\$InstanceHolder",
                    "*ProvideFactory\$InstanceHolder",
                )
                ...
            }
        }
    }
}

Coverage:
image

JADX bytecode inspection: image

ansman commented 1 month ago

You can exclude them by name:

excludedClasses.addAll(
    "*\$InstanceHolder",
    "ComposableSingletons*",
    "Hilt_*",
    "*\$DefaultImpls"
)
ansman commented 1 month ago

Also, like I mentioned in my description I agree that excluding annotated classes probably shouldn't exclude inner classes if they're not annotated but I do think kover need to add a mechanism for excluding based on parent annotation to allow excluding these types of files

NinoDLC commented 1 month ago

Oh yes I was confused between classes and packages 🙈

shanshin commented 1 month ago

Hi, the problem with the additional filter will be that it is needed in very rare cases, it will confuse users who do not need it, and when you add it, report generation will slow down significantly - because it will be necessary for any class to scan its containing class for annotations.

Could you give an example of nested classes other than InstanceHolder that are not solved by specifying the excludes.classes("*\$NestedName") filter?

beigirad commented 3 weeks ago

I ignored all hilt classes by these lines (until better solution):

packages(
    "dagger.hilt.internal.aggregatedroot.codegen",
    "hilt_aggregated_deps",
)
annotatedBy(
    "javax.annotation.processing.Generated",
    "dagger.internal.DaggerGenerated",
    "dagger.hilt.android.internal.lifecycle.HiltViewModelMap\$KeySet",
)
classes(
    "*\$InstanceHolder",
    "*Hilt_*",
    "*BindsModule*",
    "*\$DefaultImpls"
)