diff --git a/services/core/Android.bp b/services/core/Android.bp index 8b13cc8e8d88d..cbd3f6087df25 100644 --- a/services/core/Android.bp +++ b/services/core/Android.bp @@ -134,6 +134,7 @@ java_library_static { "service-permission.stubs.system_server", "service-sdksandbox.stubs.system_server", ], + plugins: ["ImmutabilityAnnotationProcessor"], required: [ "default_television.xml", @@ -172,6 +173,7 @@ java_library_static { "overlayable_policy_aidl-java", "SurfaceFlingerProperties", "com.android.sysprop.watchdog", + "ImmutabilityAnnotation", ], javac_shard_size: 50, } diff --git a/services/core/java/com/android/server/pm/pkg/AndroidPackage.java b/services/core/java/com/android/server/pm/pkg/AndroidPackage.java index 6078d4a3a3484..8a82d6e414645 100644 --- a/services/core/java/com/android/server/pm/pkg/AndroidPackage.java +++ b/services/core/java/com/android/server/pm/pkg/AndroidPackage.java @@ -34,6 +34,7 @@ import android.content.pm.ProviderInfo; import android.content.pm.ServiceInfo; import android.content.pm.SigningDetails; import android.os.Bundle; +import android.processor.immutability.Immutable; import android.util.ArraySet; import android.util.Pair; import android.util.SparseArray; @@ -65,6 +66,7 @@ import java.util.Set; * @hide */ //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER) +@Immutable public interface AndroidPackage { /** @@ -86,6 +88,7 @@ public interface AndroidPackage { * @see ActivityInfo * @see PackageInfo#activities */ + @Immutable.Ignore @NonNull List getActivities(); @@ -101,6 +104,7 @@ public interface AndroidPackage { /** * @see R.styleable#AndroidManifestApexSystemService */ + @Immutable.Ignore @NonNull List getApexSystemServices(); @@ -111,6 +115,7 @@ public interface AndroidPackage { @Nullable String getAppComponentFactory(); + @Immutable.Ignore @NonNull List getAttributions(); @@ -189,6 +194,7 @@ public interface AndroidPackage { * @see PackageInfo#configPreferences * @see R.styleable#AndroidManifestUsesConfiguration */ + @Immutable.Ignore @NonNull List getConfigPreferences(); @@ -208,6 +214,7 @@ public interface AndroidPackage { * @see PackageInfo#featureGroups * @see R.styleable#AndroidManifestUsesFeature */ + @Immutable.Ignore @NonNull List getFeatureGroups(); @@ -247,6 +254,7 @@ public interface AndroidPackage { * @see InstrumentationInfo * @see PackageInfo#instrumentation */ + @Immutable.Ignore @NonNull List getInstrumentations(); @@ -257,6 +265,7 @@ public interface AndroidPackage { * @see R.styleable#AndroidManifestKeySet * @see R.styleable#AndroidManifestPublicKey */ + @Immutable.Ignore @NonNull Map> getKeySetMapping(); @@ -341,6 +350,7 @@ public interface AndroidPackage { /** * TODO(b/135203078): Make all the Bundles immutable (and non-null by shared empty reference?) */ + @Immutable.Ignore @Nullable Bundle getMetaData(); @@ -356,6 +366,7 @@ public interface AndroidPackage { /** * @see R.styleable#AndroidManifestExtensionSdk */ + @Immutable.Ignore @Nullable SparseIntArray getMinExtensionVersions(); @@ -464,6 +475,7 @@ public interface AndroidPackage { /** * @see android.content.pm.PermissionGroupInfo */ + @Immutable.Ignore @NonNull List getPermissionGroups(); @@ -471,6 +483,7 @@ public interface AndroidPackage { * @see PermissionInfo * @see PackageInfo#permissions */ + @Immutable.Ignore @NonNull List getPermissions(); @@ -480,6 +493,7 @@ public interface AndroidPackage { * Map of component className to intent info inside that component. TODO(b/135203078): Is this * actually used/working? */ + @Immutable.Ignore @NonNull List> getPreferredActivityFilters(); @@ -493,12 +507,14 @@ public interface AndroidPackage { /** * @see android.content.pm.ProcessInfo */ + @Immutable.Ignore @NonNull Map getProcesses(); /** * Returns the properties set on the application */ + @Immutable.Ignore @NonNull Map getProperties(); @@ -522,6 +538,7 @@ public interface AndroidPackage { * @see ProviderInfo * @see PackageInfo#providers */ + @Immutable.Ignore @NonNull List getProviders(); @@ -530,6 +547,7 @@ public interface AndroidPackage { * * @see R.styleable#AndroidManifestQueriesIntent */ + @Immutable.Ignore @NonNull List getQueriesIntents(); @@ -566,6 +584,7 @@ public interface AndroidPackage { * @see ActivityInfo * @see PackageInfo#receivers */ + @Immutable.Ignore @NonNull List getReceivers(); @@ -573,6 +592,7 @@ public interface AndroidPackage { * @see PackageInfo#reqFeatures * @see R.styleable#AndroidManifestUsesFeature */ + @Immutable.Ignore @NonNull List getRequestedFeatures(); @@ -615,6 +635,7 @@ public interface AndroidPackage { * * @see R.styleable#AndroidManifestRestrictUpdate */ + @Immutable.Ignore @Nullable byte[] getRestrictUpdateHash(); @@ -662,6 +683,7 @@ public interface AndroidPackage { * @see ServiceInfo * @see PackageInfo#services */ + @Immutable.Ignore @NonNull List getServices(); @@ -682,6 +704,7 @@ public interface AndroidPackage { * The signature data of all APKs in this package, which must be exactly the same across the * base and splits. */ + @Immutable.Ignore @NonNull SigningDetails getSigningDetails(); @@ -689,6 +712,7 @@ public interface AndroidPackage { * @see ApplicationInfo#splitClassLoaderNames * @see R.styleable#AndroidManifestApplication_classLoader */ + @Immutable.Ignore @Nullable String[] getSplitClassLoaderNames(); @@ -696,18 +720,21 @@ public interface AndroidPackage { * @see ApplicationInfo#splitSourceDirs * @see ApplicationInfo#getSplitCodePaths */ + @Immutable.Ignore @NonNull String[] getSplitCodePaths(); /** * @see ApplicationInfo#splitDependencies */ + @Immutable.Ignore @NonNull SparseArray getSplitDependencies(); /** * Flags of any split APKs; ordered by parsed splitName */ + @Immutable.Ignore @Nullable int[] getSplitFlags(); @@ -717,12 +744,14 @@ public interface AndroidPackage { * @see ApplicationInfo#splitNames * @see PackageInfo#splitNames */ + @Immutable.Ignore @NonNull String[] getSplitNames(); /** * @see PackageInfo#splitRevisionCodes */ + @Immutable.Ignore @NonNull int[] getSplitRevisionCodes(); @@ -818,6 +847,7 @@ public interface AndroidPackage { @NonNull List getUsesOptionalNativeLibraries(); + @Immutable.Ignore @NonNull List getUsesPermissions(); @@ -832,12 +862,14 @@ public interface AndroidPackage { /** * @see R.styleable#AndroidManifestUsesSdkLibrary_certDigest */ + @Immutable.Ignore @Nullable String[][] getUsesSdkLibrariesCertDigests(); /** * @see R.styleable#AndroidManifestUsesSdkLibrary_versionMajor */ + @Immutable.Ignore @Nullable long[] getUsesSdkLibrariesVersionsMajor(); @@ -852,12 +884,14 @@ public interface AndroidPackage { /** * @see R.styleable#AndroidManifestUsesStaticLibrary_certDigest */ + @Immutable.Ignore @Nullable String[][] getUsesStaticLibrariesCertDigests(); /** * @see R.styleable#AndroidManifestUsesStaticLibrary_version */ + @Immutable.Ignore @Nullable long[] getUsesStaticLibrariesVersions(); diff --git a/services/core/java/com/android/server/pm/pkg/PackageState.java b/services/core/java/com/android/server/pm/pkg/PackageState.java index f0e386ce1d293..12e9671f301a6 100644 --- a/services/core/java/com/android/server/pm/pkg/PackageState.java +++ b/services/core/java/com/android/server/pm/pkg/PackageState.java @@ -25,6 +25,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.SharedLibraryInfo; import android.content.pm.SigningInfo; +import android.processor.immutability.Immutable; import android.util.SparseArray; import com.android.server.pm.PackageSetting; @@ -55,6 +56,7 @@ import java.util.Set; * @hide */ //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER) +@Immutable public interface PackageState { /** @@ -109,6 +111,7 @@ public interface PackageState { * Keys are indexes into the array represented by {@link PackageManager.NotifyReason}, values * are in epoch milliseconds. */ + @Immutable.Ignore @Size(PackageManager.NOTIFY_PACKAGE_USE_REASONS_COUNT) @NonNull long[] getLastPackageUsageTime(); @@ -172,9 +175,11 @@ public interface PackageState { */ int getSharedUserAppId(); + @Immutable.Ignore @NonNull SigningInfo getSigningInfo(); + @Immutable.Ignore @NonNull SparseArray getUserStates(); @@ -199,30 +204,35 @@ public interface PackageState { /** * @see R.styleable#AndroidManifestUsesLibrary */ + @Immutable.Ignore @NonNull List getUsesLibraryInfos(); /** * @see R.styleable#AndroidManifestUsesSdkLibrary */ + @Immutable.Ignore @NonNull String[] getUsesSdkLibraries(); /** * @see R.styleable#AndroidManifestUsesSdkLibrary_versionMajor */ + @Immutable.Ignore @NonNull long[] getUsesSdkLibrariesVersionsMajor(); /** * @see R.styleable#AndroidManifestUsesStaticLibrary */ + @Immutable.Ignore @NonNull String[] getUsesStaticLibraries(); /** * @see R.styleable#AndroidManifestUsesStaticLibrary_version */ + @Immutable.Ignore @NonNull long[] getUsesStaticLibrariesVersions(); diff --git a/services/core/java/com/android/server/pm/pkg/PackageUserState.java b/services/core/java/com/android/server/pm/pkg/PackageUserState.java index e19e555b5fcfe..2d2538553bb6f 100644 --- a/services/core/java/com/android/server/pm/pkg/PackageUserState.java +++ b/services/core/java/com/android/server/pm/pkg/PackageUserState.java @@ -21,6 +21,7 @@ import android.annotation.Nullable; import android.content.pm.PackageManager; import android.content.pm.overlay.OverlayPaths; import android.os.UserHandle; +import android.processor.immutability.Immutable; import android.util.ArraySet; import java.util.Map; @@ -35,6 +36,7 @@ import java.util.Map; */ // TODO(b/173807334): Expose API //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER) +@Immutable.Ignore(reason = "Exposed through PackageState pending refactor") public interface PackageUserState { /** @hide */ diff --git a/services/core/java/com/android/server/pm/pkg/component/ParsedProvider.java b/services/core/java/com/android/server/pm/pkg/component/ParsedProvider.java index ba85e0795c709..c66a5c1b6c92c 100644 --- a/services/core/java/com/android/server/pm/pkg/component/ParsedProvider.java +++ b/services/core/java/com/android/server/pm/pkg/component/ParsedProvider.java @@ -20,6 +20,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.pm.PathPermission; import android.os.PatternMatcher; +import android.processor.immutability.Immutable; import java.util.List; @@ -34,12 +35,14 @@ public interface ParsedProvider extends ParsedMainComponent { boolean isMultiProcess(); + @Immutable.Ignore @NonNull List getPathPermissions(); @Nullable String getReadPermission(); + @Immutable.Ignore @NonNull List getUriPermissionPatterns(); diff --git a/tools/processors/immutability/Android.bp b/tools/processors/immutability/Android.bp new file mode 100644 index 0000000000000..01e7a31887eb6 --- /dev/null +++ b/tools/processors/immutability/Android.bp @@ -0,0 +1,58 @@ +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_base_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_base_license"], +} + +java_library_host { + name: "ImmutabilityAnnotationProcessorHostLibrary", + srcs: [ + "src/**/*.kt", + "src/**/*.java", + ], + use_tools_jar: true, + // The --add-modules/exports flags below don't work for kotlinc yet, so pin this module to Java + // language level 8 (see b/139342589): + java_version: "1.8", + openjdk9: { + javacflags: [ + "--add-modules=jdk.compiler", + "--add-exports jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "--add-exports jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED", + "--add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "--add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + ], + }, +} + +java_plugin { + name: "ImmutabilityAnnotationProcessor", + processor_class: "android.processor.immutability.ImmutabilityProcessor", + static_libs: ["ImmutabilityAnnotationProcessorHostLibrary"], +} + +java_library { + name: "ImmutabilityAnnotation", + srcs: ["src/**/Immutable.java"], + sdk_version: "core_current", + host_supported: true, +} + +java_test_host { + name: "ImmutabilityAnnotationProcessorUnitTests", + + srcs: ["test/**/*.kt"], + + static_libs: [ + "compile-testing-prebuilt", + "truth-prebuilt", + "junit", + "kotlin-reflect", + "ImmutabilityAnnotationProcessorHostLibrary", + ], + + test_suites: ["general-tests"], +} diff --git a/tools/processors/immutability/OWNERS b/tools/processors/immutability/OWNERS new file mode 100644 index 0000000000000..86ae5818e91c3 --- /dev/null +++ b/tools/processors/immutability/OWNERS @@ -0,0 +1 @@ +include /PACKAGE_MANAGER_OWNERS diff --git a/tools/processors/immutability/TEST_MAPPING b/tools/processors/immutability/TEST_MAPPING new file mode 100644 index 0000000000000..4e8e2386ad8db --- /dev/null +++ b/tools/processors/immutability/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "ImmutabilityAnnotationProcessorUnitTests" + } + ] +} diff --git a/tools/processors/immutability/src/android/processor/immutability/ImmutabilityProcessor.kt b/tools/processors/immutability/src/android/processor/immutability/ImmutabilityProcessor.kt new file mode 100644 index 0000000000000..f7690d2b66d1c --- /dev/null +++ b/tools/processors/immutability/src/android/processor/immutability/ImmutabilityProcessor.kt @@ -0,0 +1,253 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.processor.immutability + +import com.sun.tools.javac.code.Symbol +import com.sun.tools.javac.code.Type +import javax.annotation.processing.AbstractProcessor +import javax.annotation.processing.ProcessingEnvironment +import javax.annotation.processing.RoundEnvironment +import javax.lang.model.SourceVersion +import javax.lang.model.element.Element +import javax.lang.model.element.ElementKind +import javax.lang.model.element.Modifier +import javax.lang.model.element.TypeElement +import javax.lang.model.type.TypeKind +import javax.lang.model.type.TypeMirror +import javax.tools.Diagnostic + +val IMMUTABLE_ANNOTATION_NAME = Immutable::class.qualifiedName + +class ImmutabilityProcessor : AbstractProcessor() { + + companion object { + /** + * Types that are already immutable. + */ + private val IGNORED_TYPES = listOf( + "java.io.File", + "java.lang.Boolean", + "java.lang.Byte", + "java.lang.CharSequence", + "java.lang.Character", + "java.lang.Double", + "java.lang.Float", + "java.lang.Integer", + "java.lang.Long", + "java.lang.Short", + "java.lang.String", + "java.lang.Void", + ) + } + + private lateinit var collectionType: TypeMirror + private lateinit var mapType: TypeMirror + + private lateinit var ignoredTypes: List + + private val seenTypes = mutableSetOf() + + override fun getSupportedSourceVersion() = SourceVersion.latest()!! + + override fun getSupportedAnnotationTypes() = setOf(Immutable::class.qualifiedName) + + override fun init(processingEnv: ProcessingEnvironment) { + super.init(processingEnv) + collectionType = processingEnv.erasedType("java.util.Collection") + mapType = processingEnv.erasedType("java.util.Map") + ignoredTypes = IGNORED_TYPES.map { processingEnv.elementUtils.getTypeElement(it).asType() } + } + + override fun process( + annotations: MutableSet, + roundEnvironment: RoundEnvironment + ): Boolean { + annotations.find { + it.qualifiedName.toString() == IMMUTABLE_ANNOTATION_NAME + } ?: return false + roundEnvironment.getElementsAnnotatedWith(Immutable::class.java) + .forEach { visitClass(emptyList(), seenTypes, it, it as Symbol.TypeSymbol) } + return true + } + + private fun visitClass( + parentChain: List, + seenTypes: MutableSet, + elementToPrint: Element, + classType: Symbol.TypeSymbol, + ) { + if (!seenTypes.add(classType.asType())) return + if (classType.getAnnotation(Immutable.Ignore::class.java) != null) return + + if (classType.getAnnotation(Immutable::class.java) == null) { + printError(parentChain, elementToPrint, + MessageUtils.classNotImmutableFailure(classType.simpleName.toString())) + } + + if (classType.getKind() != ElementKind.INTERFACE) { + printError(parentChain, elementToPrint, MessageUtils.nonInterfaceClassFailure()) + } + + val filteredElements = classType.enclosedElements + .filterNot(::isIgnored) + + filteredElements + .filter { it.getKind() == ElementKind.FIELD } + .forEach { + if (it.isStatic) { + if (!it.isPrivate) { + if (!it.modifiers.contains(Modifier.FINAL)) { + printError(parentChain, it, MessageUtils.staticNonFinalFailure()) + } + + visitType(parentChain, seenTypes, it, it.type) + } + } else { + printError(parentChain, it, MessageUtils.memberNotMethodFailure()) + } + } + + // Scan inner classes before methods so that any violations isolated to the file prints + // the error on the class declaration rather than on the method that returns the type. + // Although it doesn't matter too much either way. + filteredElements + .filter { it.getKind() == ElementKind.CLASS } + .map { it as Symbol.ClassSymbol } + .forEach { + visitClass(parentChain, seenTypes, it, it) + } + + val newChain = parentChain + "$classType" + + filteredElements + .filter { it.getKind() == ElementKind.METHOD } + .map { it as Symbol.MethodSymbol } + .forEach { + visitMethod(newChain, seenTypes, it) + } + } + + private fun visitMethod( + parentChain: List, + seenTypes: MutableSet, + method: Symbol.MethodSymbol, + ) { + val returnType = method.returnType + val typeName = returnType.toString() + when (returnType.kind) { + TypeKind.BOOLEAN, + TypeKind.BYTE, + TypeKind.SHORT, + TypeKind.INT, + TypeKind.LONG, + TypeKind.CHAR, + TypeKind.FLOAT, + TypeKind.DOUBLE, + TypeKind.NONE, + TypeKind.NULL -> { + // Do nothing + } + TypeKind.VOID -> { + if (!method.isConstructor) { + printError(parentChain, method, MessageUtils.voidReturnFailure()) + } + } + TypeKind.ARRAY -> { + printError(parentChain, method, MessageUtils.arrayFailure()) + } + TypeKind.DECLARED -> { + visitType(parentChain, seenTypes, method, method.returnType) + } + TypeKind.ERROR, + TypeKind.TYPEVAR, + TypeKind.WILDCARD, + TypeKind.PACKAGE, + TypeKind.EXECUTABLE, + TypeKind.OTHER, + TypeKind.UNION, + TypeKind.INTERSECTION, + // Java 9+ + // TypeKind.MODULE, + null -> printError(parentChain, method, + MessageUtils.genericTypeKindFailure(typeName = typeName)) + else -> printError(parentChain, method, + MessageUtils.genericTypeKindFailure(typeName = typeName)) + } + } + + private fun visitType( + parentChain: List, + seenTypes: MutableSet, + symbol: Symbol, + type: Type, + nonInterfaceClassFailure: () -> String = { MessageUtils.nonInterfaceReturnFailure() }, + ) { + if (type.isPrimitive) return + if (type.isPrimitiveOrVoid) { + printError(parentChain, symbol, MessageUtils.voidReturnFailure()) + return + } + + if (ignoredTypes.any { processingEnv.typeUtils.isSameType(it, type) }) { + return + } + + // Collection (and Map) types are ignored for the interface check as they have immutability + // enforced through a runtime exception which must be verified in a separate runtime test + val isMap = processingEnv.typeUtils.isAssignable(type, mapType) + if (!processingEnv.typeUtils.isAssignable(type, collectionType) && !isMap) { + if (type.isInterface) { + visitClass(parentChain, seenTypes, symbol, + processingEnv.typeUtils.asElement(type) as Symbol.TypeSymbol) + } else { + printError(parentChain, symbol, nonInterfaceClassFailure()) + // If the type already isn't an interface, don't scan deeper children + // to avoid printing an excess amount of errors for a known bad type. + return + } + } + + type.typeArguments.forEachIndexed { index, typeArg -> + visitType(parentChain, seenTypes, symbol, typeArg) { + MessageUtils.nonInterfaceReturnFailure(prefix = when { + !isMap -> "" + index == 0 -> "Key " + typeArg.asElement().simpleName + else -> "Value " + typeArg.asElement().simpleName + }, index = index) + } + } + } + + private fun printError( + parentChain: List, + element: Element, + message: String, + ) = processingEnv.messager.printMessage( + Diagnostic.Kind.ERROR, + // Drop one from the parent chain so that the directly enclosing class isn't logged. + // It exists in the list at this point in the traversal so that further children can + // include the right reference. + parentChain.dropLast(1).joinToString() + "\n\t" + message, + element, + ) + + private fun ProcessingEnvironment.erasedType(typeName: String) = + typeUtils.erasure(elementUtils.getTypeElement(typeName).asType()) + + private fun isIgnored(symbol: Symbol) = + symbol.getAnnotation(Immutable.Ignore::class.java) != null +} \ No newline at end of file diff --git a/tools/processors/immutability/src/android/processor/immutability/Immutable.java b/tools/processors/immutability/src/android/processor/immutability/Immutable.java new file mode 100644 index 0000000000000..0470fafac5dd9 --- /dev/null +++ b/tools/processors/immutability/src/android/processor/immutability/Immutable.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.processor.immutability; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.Collection; +import java.util.Map; + +/** + * Marks a class as immutable. When used with the Immutability processor, verifies at compile that + * the class is truly immutable. Immutable is defined as: + *
    + *
  • Only exposes methods and/or static final constants
    • + *
  • Every exposed type is an @Immutable interface or otherwise immutable class
    • + *
      + *
    • Implicitly immutable types like {@link String} are ignored
      • + *
    • {@link Collection} and {@link Map} and their subclasses where immutability is + * enforced at runtime are ignored
      • + *
    + *
  • Every method must return a type (no void methods allowed)
    • + *
  • All inner classes must be @Immutable interfaces
    • + *
+ */ +public @interface Immutable { + + /** + * Marks a specific class, field, or method as ignored for immutability validation. + */ + @Retention(RetentionPolicy.CLASS) // Not SOURCE as that isn't retained for some reason + @Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD}) + @interface Ignore { + String reason() default ""; + } +} diff --git a/tools/processors/immutability/src/android/processor/immutability/MessageUtils.kt b/tools/processors/immutability/src/android/processor/immutability/MessageUtils.kt new file mode 100644 index 0000000000000..63f5641995925 --- /dev/null +++ b/tools/processors/immutability/src/android/processor/immutability/MessageUtils.kt @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.processor.immutability + +object MessageUtils { + + fun classNotImmutableFailure(className: String) = "$className should be marked @Immutable" + + fun memberNotMethodFailure() = "Member must be a method" + + fun nonInterfaceClassFailure() = "Class was not an interface" + + fun nonInterfaceReturnFailure(prefix: String, index: Int = -1) = + if (prefix.isEmpty()) { + "Type at index $index was not an interface" + } else { + "$prefix was not an interface" + } + + fun genericTypeKindFailure(typeName: CharSequence) = "TypeKind $typeName unsupported" + + fun arrayFailure() = "Array types are not supported as they can be mutated by callers" + + fun nonInterfaceReturnFailure() = "Must return an interface" + + fun voidReturnFailure() = "Cannot return void" + + fun staticNonFinalFailure() = "Static member must be final" +} \ No newline at end of file diff --git a/tools/processors/immutability/test/android/processor/ImmutabilityProcessorTest.kt b/tools/processors/immutability/test/android/processor/ImmutabilityProcessorTest.kt new file mode 100644 index 0000000000000..7e1e8e15cd055 --- /dev/null +++ b/tools/processors/immutability/test/android/processor/ImmutabilityProcessorTest.kt @@ -0,0 +1,217 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.processor + +import android.processor.immutability.IMMUTABLE_ANNOTATION_NAME +import android.processor.immutability.ImmutabilityProcessor +import android.processor.immutability.MessageUtils +import com.google.common.truth.Expect +import com.google.testing.compile.CompilationSubject.assertThat +import com.google.testing.compile.Compiler.javac +import com.google.testing.compile.JavaFileObjects +import org.junit.Rule +import org.junit.Test +import javax.tools.JavaFileObject + +class ImmutabilityProcessorTest { + + companion object { + private const val PACKAGE_PREFIX = "android.processor.immutability" + private const val DATA_CLASS_NAME = "DataClass" + private val ANNOTATION = JavaFileObjects.forSourceString(IMMUTABLE_ANNOTATION_NAME, + /* language=JAVA */ """ + package $PACKAGE_PREFIX; + + import java.lang.annotation.Retention; + import java.lang.annotation.RetentionPolicy; + + @Retention(RetentionPolicy.SOURCE) + public @interface Immutable { + @Retention(RetentionPolicy.SOURCE) + @interface Ignore {} + } + """.trimIndent() + ) + } + + @get:Rule + val expect = Expect.create() + + @Test + fun validInterface() = test( + JavaFileObjects.forSourceString("$PACKAGE_PREFIX.$DATA_CLASS_NAME", + /* language=JAVA */ """ + package $PACKAGE_PREFIX; + + import $IMMUTABLE_ANNOTATION_NAME; + import java.util.ArrayList; + import java.util.Collections; + import java.util.List; + + @Immutable + public interface $DATA_CLASS_NAME { + InnerInterface DEFAULT = new InnerInterface() { + @Override + public String getValue() { + return ""; + } + @Override + public List getArray() { + return Collections.emptyList(); + } + }; + + String getValue(); + ArrayList getArray(); + InnerInterface getInnerInterface(); + + @Immutable + interface InnerInterface { + String getValue(); + List getArray(); + } + } + """.trimIndent() + ), errors = emptyList()) + + @Test + fun abstractClass() = test( + JavaFileObjects.forSourceString("$PACKAGE_PREFIX.$DATA_CLASS_NAME", + /* language=JAVA */ """ + package $PACKAGE_PREFIX; + + import $IMMUTABLE_ANNOTATION_NAME; + import java.util.Map; + + @Immutable + public abstract class $DATA_CLASS_NAME { + public static final String IMMUTABLE = ""; + public static final InnerClass NOT_IMMUTABLE = null; + public static InnerClass NOT_FINAL = null; + + // Field finality doesn't matter, methods are always enforced so that future + // field compaction or deprecation is possible + private final String fieldFinal = ""; + private String fieldNonFinal; + public abstract void sideEffect(); + public abstract String[] getArray(); + public abstract InnerClass getInnerClassOne(); + public abstract InnerClass getInnerClassTwo(); + @Immutable.Ignore + public abstract InnerClass getIgnored(); + public abstract InnerInterface getInnerInterface(); + + public abstract Map getValidMap(); + public abstract Map getInvalidMap(); + + public static final class InnerClass { + public String innerField; + public String[] getArray() { return null; } + } + + public interface InnerInterface { + String[] getArray(); + InnerClass getInnerClass(); + } + } + """.trimIndent() + ), errors = listOf( + nonInterfaceClassFailure(line = 7), + nonInterfaceReturnFailure(line = 9), + staticNonFinalFailure(line = 10), + nonInterfaceReturnFailure(line = 10), + memberNotMethodFailure(line = 14), + memberNotMethodFailure(line = 15), + voidReturnFailure(line = 16), + arrayFailure(line = 17), + nonInterfaceReturnFailure(line = 18), + nonInterfaceReturnFailure(line = 19), + nonInterfaceReturnFailure(line = 25, prefix = "Key InnerClass"), + nonInterfaceReturnFailure(line = 25, prefix = "Value InnerClass"), + classNotImmutableFailure(line = 27, className = "InnerClass"), + nonInterfaceClassFailure(line = 27), + memberNotMethodFailure(line = 28), + arrayFailure(line = 29), + classNotImmutableFailure(line = 22, className = "InnerInterface"), + arrayFailure(line = 33), + nonInterfaceReturnFailure(line = 34), + )) + + private fun test(source: JavaFileObject, errors: List) { + val compilation = javac() + .withProcessors(ImmutabilityProcessor()) + .compile(listOf(source) + ANNOTATION) + errors.forEach { + try { + assertThat(compilation) + .hadErrorContaining(it.message) + .inFile(source) + .onLine(it.line) + } catch (e: AssertionError) { + // Wrap the exception so that the line number is logged + val wrapped = AssertionError("Expected $it, ${e.message}").apply { + stackTrace = e.stackTrace + } + + // Wrap again with Expect so that all errors are reported. This is very bad code + // but can only be fixed by updating compile-testing with a better Truth Subject + // implementation. + expect.that(wrapped).isNull() + } + } + + try { + assertThat(compilation).hadErrorCount(errors.size) + } catch (e: AssertionError) { + if (expect.hasFailures()) { + expect.that(e).isNull() + } else throw e + } + } + + private fun classNotImmutableFailure(line: Long, className: String) = + CompilationError(line = line, message = MessageUtils.classNotImmutableFailure(className)) + + private fun nonInterfaceClassFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.nonInterfaceClassFailure()) + + private fun nonInterfaceReturnFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.nonInterfaceReturnFailure()) + + private fun nonInterfaceReturnFailure(line: Long, prefix: String, index: Int = -1) = + CompilationError( + line = line, + message = MessageUtils.nonInterfaceReturnFailure(prefix = prefix, index = index) + ) + + private fun memberNotMethodFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.memberNotMethodFailure()) + + private fun voidReturnFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.voidReturnFailure()) + + private fun staticNonFinalFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.staticNonFinalFailure()) + + private fun arrayFailure(line: Long) = + CompilationError(line = line, message = MessageUtils.arrayFailure()) + + data class CompilationError( + val line: Long, + val message: String, + ) +} \ No newline at end of file