diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 63c141144a..b722fee63c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -370,6 +370,22 @@ test-pyramid:legacy-integration-instrumented-median-api: - !reference [.snippets, install-android-sdk] - !reference [.snippets, run-legacy-integration-instrumented] +test-pyramid:detekt-api-coverage: + tags: [ "arch:amd64" ] + image: $CI_IMAGE_DOCKER + stage: test-pyramid + timeout: 1h + script: + - mkdir -p ./config/ + - aws ssm get-parameter --region us-east-1 --name ci.dd-sdk-android.gradle-properties --with-decryption --query "Parameter.Value" --out text >> ./gradle.properties + - GRADLE_OPTS="-Xmx4096M" ./gradlew assembleLibrariesDebug --stacktrace --no-daemon + - GRADLE_OPTS="-Xmx4096M" ./gradlew printSdkDebugRuntimeClasspath --stacktrace --no-daemon + - GRADLE_OPTS="-Xmx4096M" ./gradlew :tools:detekt:jar --stacktrace --no-daemon + - curl -sSLO https://github.com/detekt/detekt/releases/download/v1.23.4/detekt-cli-1.23.4-all.jar + - java -jar detekt-cli-1.23.4-all.jar --config detekt_test_pyramid.yml --plugins tools/detekt/build/libs/detekt.jar -ex "**/*.kts" --jvm-target 11 -cp $(cat sdk_classpath) + # For now we just print the uncovered apis, eventually we will fail if it's not empty + - grep -v -f apiUsage.log apiSurface.log + test-pyramid:publish-e2e-synthetics: tags: [ "arch:amd64" ] image: $CI_IMAGE_DOCKER @@ -424,7 +440,6 @@ test-pyramid:publish-webview-synthetics: paths: - sample/kotlin/build/outputs/apk/us1/release/kotlin-us1-release.apk - test-pyramid:publish-staging-synthetics: tags: [ "arch:amd64" ] image: $CI_IMAGE_DOCKER diff --git a/detekt_test_pyramid.yml b/detekt_test_pyramid.yml index 3ea20126ef..856e596651 100644 --- a/detekt_test_pyramid.yml +++ b/detekt_test_pyramid.yml @@ -82,9 +82,13 @@ datadog-test-pyramid: active: true ApiUsage: active: true - includes: ['**/reliability/**'] + includes: [ '**/reliability/**' ] ApiSurface: active: true - includes: ['**/dd-sdk-android-*/**'] - excludes: ['**/build/**', '**/test/**', '**/testDebug/**','**/testRelease/**', '**/androidTest/**', '**/testFixtures/**', '**/buildSrc/**', '**/*.kts', '**/instrumented/**', '**/sample/**', '**/tools/**'] - + includes: [ '**/dd-sdk-android-*/**' ] + excludes: [ '**/build/**', '**/test/**', '**/testDebug/**','**/testRelease/**', '**/androidTest/**', '**/testFixtures/**', '**/buildSrc/**', '**/*.kts', '**/instrumented/**', '**/sample/**', '**/tools/**' ] + ignoredAnnotations: + - "com.datadog.android.lint.InternalApi" + ignoredClasses: + - "com.datadog.android._InternalProxy" + - "com.datadog.android.rum._RumInternalProxy" diff --git a/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/pyramid/ApiSurface.kt b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/pyramid/ApiSurface.kt index c11afc3683..64a9d13446 100644 --- a/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/pyramid/ApiSurface.kt +++ b/tools/detekt/src/main/kotlin/com/datadog/tools/detekt/rules/pyramid/ApiSurface.kt @@ -13,12 +13,12 @@ import io.gitlab.arturbosch.detekt.api.Issue import io.gitlab.arturbosch.detekt.api.Severity import io.gitlab.arturbosch.detekt.api.config import io.gitlab.arturbosch.detekt.api.internal.RequiresTypeResolution +import io.gitlab.arturbosch.detekt.rules.isOverride import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtClass import org.jetbrains.kotlin.psi.KtFunctionType import org.jetbrains.kotlin.psi.KtNamedFunction import org.jetbrains.kotlin.psi.KtNullableType -import org.jetbrains.kotlin.psi.KtObjectLiteralExpression import org.jetbrains.kotlin.psi.KtParameter import org.jetbrains.kotlin.psi.KtParameterList import org.jetbrains.kotlin.psi.KtPrimaryConstructor @@ -26,7 +26,6 @@ import org.jetbrains.kotlin.psi.KtTypeElement import org.jetbrains.kotlin.psi.KtUserType import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject import org.jetbrains.kotlin.psi.psiUtil.getChildrenOfType -import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.psi.psiUtil.isExtensionDeclaration import java.io.File @@ -40,6 +39,8 @@ class ApiSurface( private val outputFileName: String by config(defaultValue = "apiSurface.log") private val outputFile: File by lazy { File(outputFileName) } + private val ignoredAnnotations: List by config(defaultValue = emptyList()) + private val ignoredClasses: List by config(defaultValue = emptyList()) // region Rule @@ -51,19 +52,29 @@ class ApiSurface( ) override fun visitClass(klass: KtClass) { - if (klass.hasModifier(KtTokens.PRIVATE_KEYWORD) || klass.hasModifier(KtTokens.INTERNAL_KEYWORD)) { - return + val hasIgnoredKeyword = ignoredKeywords.any { klass.hasModifier(it) } + val isInterface = klass.isInterface() + val isEnum = klass.isEnum() + val isIgnoredClass = klass.fqName?.asString() in ignoredClasses + val hasIgnoredAnnotation = klass.annotationEntries.any { + it.shortName?.asString()?.resolveFullType() in ignoredAnnotations } - if (klass.isInterface()) { + + @Suppress("ComplexCondition") + if (isIgnoredClass || hasIgnoredKeyword || isInterface || isEnum || hasIgnoredAnnotation) { return } + super.visitClass(klass) } override fun visitPrimaryConstructor(constructor: KtPrimaryConstructor) { - if (constructor.hasModifier(KtTokens.PRIVATE_KEYWORD) || constructor.hasModifier(KtTokens.INTERNAL_KEYWORD)) { + val hasIgnoredKeyword = ignoredKeywords.any { constructor.hasModifier(it) } + + if (hasIgnoredKeyword) { return } + val parentName = constructor.containingClassOrObject?.fqName ?: constructor.containingKtFile.packageFqName outputFile.appendText("$parentName.constructor(") @@ -78,20 +89,19 @@ class ApiSurface( outputFile.appendText(")\n") } - @Suppress("ReturnCount") override fun visitNamedFunction(function: KtNamedFunction) { - if (function.hasModifier(KtTokens.PRIVATE_KEYWORD) || function.hasModifier(KtTokens.INTERNAL_KEYWORD)) { - return + val hasIgnoredKeyword = ignoredKeywords.any { function.hasModifier(it) } + val hasIgnoredName = function.name in ignoredFunctionNames + val hasIgnoredAnnotation = function.annotationEntries.any { + it.shortName?.asString()?.resolveFullType() in ignoredAnnotations } - val parameterList = function.getChildrenOfType().firstOrNull() - if (function.name == "toString" && parameterList?.children.isNullOrEmpty()) { - return - } - if (function.getStrictParentOfType() != null) { - // Function is overriding something in an anonymous object - // e.g.: val x = object : Interface { override fun foo() {} } + val isOverride = function.isOverride() + + @Suppress("ComplexCondition") + if (hasIgnoredKeyword || hasIgnoredName || hasIgnoredAnnotation || isOverride) { return } + if (function.isExtensionDeclaration()) { val target = function.receiverTypeReference?.nameForReceiverLabel() val fqName = target?.resolveFullType() @@ -102,6 +112,7 @@ class ApiSurface( outputFile.appendText("$parentName.${function.nameAsSafeName}(") } + val parameterList = function.getChildrenOfType().firstOrNull() parameterList?.children?.filterIsInstance()?.forEachIndexed { idx, p -> val typeElement = p.typeReference?.typeElement if (idx > 0) outputFile.appendText(", ") @@ -137,4 +148,13 @@ class ApiSurface( } // endregion + + companion object { + private val ignoredFunctionNames = setOf("toString", "equals", "hashCode") + + private val ignoredKeywords = setOf( + KtTokens.PRIVATE_KEYWORD, + KtTokens.INTERNAL_KEYWORD + ) + } }