Add API lint to metalava
This CL implements the various checks from
frameworks/base/tools/apilint, around 80 rules for
good API design. It also runs the existing Kotlin
interoperability checks as part of API lint.
The checks are only applied if you invoke metalava
with the --api-lint flag.
Test: Unit tests included
Change-Id: I5eb6693bdad4a0144dc0a7b2b09308edb3e7cb59
diff --git a/API-LINT.md b/API-LINT.md
new file mode 100644
index 0000000..f0ab779
--- /dev/null
+++ b/API-LINT.md
@@ -0,0 +1,149 @@
+# API Lint
+
+Metalava can optionally run warn about various API problems based on the Android
+API Council's guidelines, described in go/android-api-guidelines.
+
+These rules are not always exact. For example, you should avoid using acronyms
+in names; e.g. a method should be named handleUri, not handleURI. But what about
+getZOrder? This is probably better than getZorder, but metalava can't tell
+without consulting a dictionary.
+
+Therefore, there are cases where you want to say "ok, that's good advice in
+general, but wrong here". In order to avoid having this warningshow up again
+and again, there are two ways to mark an issue such that it is no longer
+flagged.
+
+(Note that metalava will not report issues on classes, methods and fields that
+are deprecated since these are presumably already known to be bad and are already
+discouraged.)
+
+## Suppressing with @Suppress
+
+Next to an error message, metalava will include the issue id. For example,
+here's a sample error message:
+
+ src/android/pkg/MyStringImpl.java:3: error: Don't expose your implementation details: MyStringImpl ends with Impl [EndsWithImpl]
+
+Here the id is "EndsWithImpl". You can suppress this with the @SuppressLint
+annotation:
+
+ ...
+ import android.annotation.SuppressLint;
+ ...
+
+ @SuppressLint("EndsWithImpl")
+ public class MyStringImpl {
+ ...
+
+## Suppressing with baselines
+
+Metalava also has support for "baselines", which are files which record all the
+current warnings and errors in the codebase. When metalava runs, it looks up the
+baseline to see if a given issue is already listed in the baseline, and if so,
+it is silently ignored.
+
+You can pass a flag to metalava ("--update-baseline") to tell it to update the
+baseline files with any new errors it comes across instead of reporting
+them. With soong, you'd do something like this:
+
+ --- a/Android.bp
+ +++ b/Android.bp
+ @@ -1678,6 +1678,7 @@ droidstubs {
+ },
+ api_lint: true,
+ baseline_filename: "baseline.txt",
+ + update_baseline: true,
+ jdiff_enabled: true,
+ }
+
+Then re-run the build and you should now see diffs to the baseline file; git
+diff to make sure you're really only marking the issues you intended to include.
+
+ $ git status
+ ...
+ Untracked files:
+ (use "git add <file>..." to include in what will be committed)
+
+ baseline.txt
+
+## Existing Issues
+
+You can view the exact set of existing issues (current APIs that get flagged by
+API lint) by inspecting the baseline files in the source tree, but to get a
+sense of the types of issues that are more likely to have a false positive,
+here's the existing distribution as of early 2019:
+
+ Count Issue Id Rule Severity
+ --------------------------------------------------
+
+ 932 ProtectedMember M7 error
+ 321 OnNameExpected warning
+ 288 ArrayReturn warning
+ 269 ActionValue C4 error
+ 266 NoByteOrShort FW12 warning
+ 221 ExecutorRegistration L1 warning
+ 211 AllUpper C2 error
+ 206 GetterSetterNames M6 error
+ 185 BannedThrow error
+ 172 SamShouldBeLast warning
+ 159 NoClone error
+ 159 ParcelNotFinal FW8 error
+ 119 NotCloseable warning
+ 105 ListenerLast M3 warning
+ 81 ConcreteCollection CL2 error
+ 78 StaticUtils error
+ 76 IntentName C3 error
+ 74 VisiblySynchronized M5 error
+ 72 GenericException S1 error
+ 65 KotlinOperator warning
+ 57 AcronymName S1 warning
+ 55 ParcelCreator FW3 error
+ 54 ParcelConstructor FW3 error
+ 53 UseIcu warning
+ 48 Enum F5 error
+ 41 RethrowRemoteException FW9 error
+ 37 AutoBoxing M11 error
+ 35 StreamFiles M10 warning
+ 28 IntentBuilderName FW1 warning
+ 27 ServiceName C4 error
+ 26 ListenerInterface L1 error
+ 25 ContextFirst M3 error
+ 25 InterfaceConstant C4 error
+ 24 CallbackInterface CL3 error
+ 24 RegistrationName L3 error
+ 23 IllegalStateException S1 warning
+ 22 EqualsAndHashCode M8 error
+ 22 PackageLayering FW6 warning
+ 18 MinMaxConstant C8 warning
+ 18 SingletonConstructor error
+ 17 MethodNameUnits error
+ 15 MissingBuild warning
+ 15 UserHandleName warning
+ 14 UserHandle warning
+ 13 ResourceFieldName error
+ 12 ManagerLookup error
+ 11 ManagerConstructor error
+ 9 CallbackMethodName L1 error
+ 9 ParcelableList warning
+ 8 CallbackName L1 warning
+ 7 HeavyBitSet error
+ 7 ResourceValueFieldName C7 error
+ 6 CompileTimeConstant error
+ 6 SetterReturnsThis M4 warning
+ 4 EndsWithImpl error
+ 4 TopLevelBuilder warning
+ 4 UseParcelFileDescriptor FW11 error
+ 3 MentionsGoogle error
+ 3 StartWithLower S1 error
+ 2 AbstractInner warning
+ 2 BuilderSetStyle warning
+ 2 OverlappingConstants C1 warning
+ 2 PairedRegistration L2 error
+ 2 SingularCallback L1 error
+ 2 StartWithUpper S1 error
+ 1 ContextNameSuffix C4 error
+ 1 KotlinKeyword error
+ --------------------------------------------------
+ 4902
+
+(This is generated when metalava is invoked with both --verbose and --update-baseline.)
\ No newline at end of file
diff --git a/README.md b/README.md
index 608e7a9..ed88779 100644
--- a/README.md
+++ b/README.md
@@ -177,6 +177,19 @@
hooked up to the build it immediately became apparent that it was missing
important methods that should really be part of the API.)
+* API Lint: Metalava can optionally (with --api-lint) run a series of additional
+ checks on the public API in the codebase and flag issues that are discouraged
+ or forbidden by the Android API Council; there are currently around 80 checks.
+
+* Baselines: Metalava can report all of its issues into a "baseline" file, which
+ records the current set of issues. From that point forward, when metalava
+ finds a problem, it will only be reported if it is not already in the
+ baseline. This lets you enforce new issues going forward without having to
+ fix all existing violations. Periodically, as older issues are fixed, you can
+ regenerate the baseline. For issues with some false positives, such as API
+ Lint, being able to check in the set of accepted or verified false positives
+ is quite important.
+
* Metalava can generate reports about nullness annotation coverage (which helps
target efforts since we plan to annotate the entire API). First, it can
generate a raw count:
diff --git a/src/main/java/com/android/resources/ResourceType.java b/src/main/java/com/android/resources/ResourceType.java
new file mode 100644
index 0000000..9277059
--- /dev/null
+++ b/src/main/java/com/android/resources/ResourceType.java
@@ -0,0 +1,295 @@
+/*
+ * Copyright (C) 2007 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.
+ */
+
+// Copied from tools/base/laoyutlib-api
+
+package com.android.resources;
+
+import com.android.SdkConstants;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+
+import java.util.Arrays;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+import static com.google.common.base.MoreObjects.firstNonNull;
+
+/**
+ * Enum representing a type of compiled resource.
+ *
+ * <p>See {@code ResourceType} in aapt2/Resource.h.
+ */
+public enum ResourceType {
+ ANIM("anim", "Animation"),
+ ANIMATOR("animator", "Animator"),
+ ARRAY("array", "Array", "string-array", "integer-array"),
+ ATTR("attr", "Attr"),
+ BOOL("bool", "Boolean"),
+ COLOR("color", "Color"),
+ DIMEN("dimen", "Dimension"),
+ DRAWABLE("drawable", "Drawable"),
+ FONT("font", "Font"),
+ FRACTION("fraction", "Fraction"),
+ ID("id", "ID"),
+ INTEGER("integer", "Integer"),
+ INTERPOLATOR("interpolator", "Interpolator"),
+ LAYOUT("layout", "Layout"),
+ MENU("menu", "Menu"),
+ MIPMAP("mipmap", "Mip Map"),
+ NAVIGATION("navigation", "Navigation"),
+ PLURALS("plurals", "Plurals"),
+ RAW("raw", "Raw"),
+ STRING("string", "String"),
+ STYLE("style", "Style"),
+ STYLEABLE("styleable", "Styleable", Kind.STYLEABLE),
+ TRANSITION("transition", "Transition"),
+ XML("xml", "XML"),
+
+ /**
+ * This is not actually used. Only there because they get parsed and since we want to detect new
+ * resource type, we need to have this one exist.
+ */
+ PUBLIC("public", "Public visibility modifier", Kind.SYNTHETIC),
+
+ /**
+ * This type is used for elements dynamically generated by the parsing of aapt:attr nodes. The
+ * "aapt:attr" allow to inline resources as part of a different resource, for example, a
+ * drawable as part of a layout. When the parser, encounters one of this nodes, it will generate
+ * a synthetic _aaptattr reference.
+ */
+ AAPT("_aapt", "Aapt Attribute", Kind.SYNTHETIC),
+
+ /**
+ * Represents item tags inside a style definition.
+ */
+ STYLE_ITEM("item", "Style Item", Kind.SYNTHETIC),
+
+ /**
+ * Not an actual resource type from AAPT. Used to provide sample data values in the tools
+ * namespace
+ */
+ SAMPLE_DATA("sample", "Sample data", Kind.SYNTHETIC),
+ ;
+
+ private enum Kind {
+ /**
+ * These types are used both in the R and as XML tag names.
+ */
+ REAL,
+
+ /**
+ * Styleables are handled by aapt but don't end up in the resource table. They have an R
+ * inner class (called {@code styleable}), are declared in XML (using {@code
+ * declare-styleable}) but cannot be referenced from XML.
+ */
+ STYLEABLE,
+
+ /**
+ * Other types that are not known to aapt, but are used by tools to represent some
+ * information in the resources system.
+ */
+ SYNTHETIC,
+ ;
+ }
+
+ private final String mName;
+ private final Kind mKind;
+ private final String mDisplayName;
+ private final String[] mAlternateXmlNames;
+
+ ResourceType(
+ String name,
+ String displayName,
+ String... alternateXmlNames) {
+ mName = name;
+ mKind = Kind.REAL;
+ mDisplayName = displayName;
+ mAlternateXmlNames = alternateXmlNames;
+ }
+
+ ResourceType(String name, String displayName, Kind kind) {
+ mName = name;
+ mKind = kind;
+ mDisplayName = displayName;
+ mAlternateXmlNames = new String[0];
+ }
+
+ /**
+ * The set of all types of resources that can be referenced by other resources.
+ */
+ public static final ImmutableSet<ResourceType> REFERENCEABLE_TYPES;
+
+ private static final ImmutableMap<String, ResourceType> TAG_NAMES;
+ private static final ImmutableMap<String, ResourceType> CLASS_NAMES;
+
+ static {
+ ImmutableMap.Builder<String, ResourceType> tagNames = ImmutableMap.builder();
+ tagNames.put(SdkConstants.TAG_DECLARE_STYLEABLE, STYLEABLE);
+ tagNames.put(SdkConstants.TAG_PUBLIC, PUBLIC);
+
+ ImmutableMap.Builder<String, ResourceType> classNames = ImmutableMap.builder();
+ classNames.put(STYLEABLE.mName, STYLEABLE);
+ classNames.put(AAPT.mName, AAPT);
+
+ for (ResourceType type : ResourceType.values()) {
+ if (type.mKind != Kind.REAL || type == STYLEABLE) {
+ continue;
+ }
+ classNames.put(type.getName(), type);
+ tagNames.put(type.getName(), type);
+ for (String alternateName : type.mAlternateXmlNames) {
+ tagNames.put(alternateName, type);
+ }
+ }
+
+ TAG_NAMES = tagNames.build();
+ CLASS_NAMES = classNames.build();
+ REFERENCEABLE_TYPES =
+ Arrays.stream(values())
+ .filter(ResourceType::getCanBeReferenced)
+ .collect(Sets.toImmutableEnumSet());
+ }
+
+ /**
+ * Returns the resource type name, as used by XML files.
+ */
+ public String getName() {
+ return mName;
+ }
+
+ /**
+ * Returns a translated display name for the resource type.
+ */
+ public String getDisplayName() {
+ return mDisplayName;
+ }
+
+ /**
+ * Returns the enum by its name as it appears in the R class.
+ *
+ * @param className name of the inner class of the R class, e.g. "string" or "styleable".
+ */
+
+ public static ResourceType fromClassName(String className) {
+ return CLASS_NAMES.get(className);
+ }
+
+ /**
+ * Returns the enum by its name as it appears as a folder name under {@code res/}.
+ *
+ * @param folderName name of the inner class of the R class, e.g. "drawable" or "color".
+ */
+
+ public static ResourceType fromFolderName(String folderName) {
+ return CLASS_NAMES.get(folderName);
+ }
+
+ /**
+ * Returns the enum by its name as it appears in XML as a tag name.
+ *
+ * @param tagName name of the XML tag, e.g. "string" or "declare-styleable".
+ */
+
+ public static ResourceType fromXmlTagName(String tagName) {
+ return TAG_NAMES.get(tagName);
+ }
+
+ /**
+ * Returns the enum by its name as it appears in a {@link ResourceUrl} string.
+ *
+ * @param xmlValue value of the type attribute or the prefix of a {@link ResourceUrl}, e.g.
+ * "string" or "array".
+ */
+
+ public static ResourceType fromXmlValue(String xmlValue) {
+ if (xmlValue.equals(SdkConstants.TAG_DECLARE_STYLEABLE)
+ || xmlValue.equals(STYLEABLE.mName)) {
+ return null;
+ }
+
+ if (xmlValue.equals(SAMPLE_DATA.mName)) {
+ return SAMPLE_DATA;
+ }
+
+ return CLASS_NAMES.get(xmlValue);
+ }
+
+ public static <T> ResourceType fromXmlTag(
+ T tag,
+ Function<T, String> nameFunction,
+ BiFunction<T, String, String> attributeFunction) {
+ String tagName = nameFunction.apply(tag);
+ switch (tagName) {
+ case SdkConstants.TAG_EAT_COMMENT:
+ return null;
+ case SdkConstants.TAG_ITEM:
+ String typeAttribute = attributeFunction.apply(tag, SdkConstants.ATTR_TYPE);
+ if (!Strings.isNullOrEmpty(typeAttribute)) {
+ return fromClassName(typeAttribute);
+ } else {
+ return null;
+ }
+ default:
+ return fromXmlTagName(tagName);
+ }
+ }
+
+ public static ResourceType fromXmlTag(Node domNode) {
+ if (!(domNode instanceof Element)) {
+ return null;
+ }
+
+ Element tag = (Element) domNode;
+ return fromXmlTag(
+ tag,
+ element -> firstNonNull(element.getLocalName(), element.getTagName()),
+ Element::getAttribute);
+ }
+
+ /**
+ * @deprecated Use other static methods in this class. Kept for layoutlib binary compatibility.
+ */
+ @Deprecated
+ public static ResourceType getEnum(String className) {
+ return fromClassName(className);
+ }
+
+ /**
+ * Returns true if the generated R class contains an inner class for this {@link ResourceType}.
+ */
+ public boolean getHasInnerClass() {
+ return mKind != Kind.SYNTHETIC;
+ }
+
+ /**
+ * Returns true if this {@link ResourceType} can be referenced using the {@link ResourceUrl}
+ * syntax: {@code @typeName/resourceName}.
+ */
+ public boolean getCanBeReferenced() {
+ return mKind == Kind.REAL && this != ATTR;
+ }
+
+ @Override
+ public String toString() {
+ // Unfortunately we still have code that relies on toString() returning the aapt name.
+ return getName();
+ }
+}
diff --git a/src/main/java/com/android/tools/metalava/ApiLint.kt b/src/main/java/com/android/tools/metalava/ApiLint.kt
new file mode 100644
index 0000000..2290f31
--- /dev/null
+++ b/src/main/java/com/android/tools/metalava/ApiLint.kt
@@ -0,0 +1,3292 @@
+/*
+ * Copyright (C) 2018 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 com.android.tools.metalava
+
+import com.android.resources.ResourceType
+import com.android.resources.ResourceType.AAPT
+import com.android.resources.ResourceType.ANIM
+import com.android.resources.ResourceType.ANIMATOR
+import com.android.resources.ResourceType.ARRAY
+import com.android.resources.ResourceType.ATTR
+import com.android.resources.ResourceType.BOOL
+import com.android.resources.ResourceType.COLOR
+import com.android.resources.ResourceType.DIMEN
+import com.android.resources.ResourceType.DRAWABLE
+import com.android.resources.ResourceType.FONT
+import com.android.resources.ResourceType.FRACTION
+import com.android.resources.ResourceType.ID
+import com.android.resources.ResourceType.INTEGER
+import com.android.resources.ResourceType.INTERPOLATOR
+import com.android.resources.ResourceType.LAYOUT
+import com.android.resources.ResourceType.MENU
+import com.android.resources.ResourceType.MIPMAP
+import com.android.resources.ResourceType.NAVIGATION
+import com.android.resources.ResourceType.PLURALS
+import com.android.resources.ResourceType.PUBLIC
+import com.android.resources.ResourceType.RAW
+import com.android.resources.ResourceType.SAMPLE_DATA
+import com.android.resources.ResourceType.STRING
+import com.android.resources.ResourceType.STYLE
+import com.android.resources.ResourceType.STYLEABLE
+import com.android.resources.ResourceType.STYLE_ITEM
+import com.android.resources.ResourceType.TRANSITION
+import com.android.resources.ResourceType.XML
+import com.android.sdklib.SdkVersionInfo
+import com.android.tools.metalava.doclava1.Errors.ABSTRACT_INNER
+import com.android.tools.metalava.doclava1.Errors.ACRONYM_NAME
+import com.android.tools.metalava.doclava1.Errors.ACTION_VALUE
+import com.android.tools.metalava.doclava1.Errors.ALL_UPPER
+import com.android.tools.metalava.doclava1.Errors.ARRAY_RETURN
+import com.android.tools.metalava.doclava1.Errors.AUTO_BOXING
+import com.android.tools.metalava.doclava1.Errors.BANNED_THROW
+import com.android.tools.metalava.doclava1.Errors.BUILDER_SET_STYLE
+import com.android.tools.metalava.doclava1.Errors.CALLBACK_INTERFACE
+import com.android.tools.metalava.doclava1.Errors.CALLBACK_METHOD_NAME
+import com.android.tools.metalava.doclava1.Errors.CALLBACK_NAME
+import com.android.tools.metalava.doclava1.Errors.COMMON_ARGS_FIRST
+import com.android.tools.metalava.doclava1.Errors.COMPILE_TIME_CONSTANT
+import com.android.tools.metalava.doclava1.Errors.CONCRETE_COLLECTION
+import com.android.tools.metalava.doclava1.Errors.CONFIG_FIELD_NAME
+import com.android.tools.metalava.doclava1.Errors.CONSISTENT_ARGUMENT_ORDER
+import com.android.tools.metalava.doclava1.Errors.CONTEXT_FIRST
+import com.android.tools.metalava.doclava1.Errors.CONTEXT_NAME_SUFFIX
+import com.android.tools.metalava.doclava1.Errors.ENDS_WITH_IMPL
+import com.android.tools.metalava.doclava1.Errors.ENUM
+import com.android.tools.metalava.doclava1.Errors.EQUALS_AND_HASH_CODE
+import com.android.tools.metalava.doclava1.Errors.EXCEPTION_NAME
+import com.android.tools.metalava.doclava1.Errors.EXECUTOR_REGISTRATION
+import com.android.tools.metalava.doclava1.Errors.EXTENDS_ERROR
+import com.android.tools.metalava.doclava1.Errors.Error
+import com.android.tools.metalava.doclava1.Errors.FRACTION_FLOAT
+import com.android.tools.metalava.doclava1.Errors.GENERIC_EXCEPTION
+import com.android.tools.metalava.doclava1.Errors.GETTER_SETTER_NAMES
+import com.android.tools.metalava.doclava1.Errors.HEAVY_BIT_SET
+import com.android.tools.metalava.doclava1.Errors.ILLEGAL_STATE_EXCEPTION
+import com.android.tools.metalava.doclava1.Errors.INTENT_BUILDER_NAME
+import com.android.tools.metalava.doclava1.Errors.INTENT_NAME
+import com.android.tools.metalava.doclava1.Errors.INTERFACE_CONSTANT
+import com.android.tools.metalava.doclava1.Errors.INTERNAL_CLASSES
+import com.android.tools.metalava.doclava1.Errors.KOTLIN_OPERATOR
+import com.android.tools.metalava.doclava1.Errors.LISTENER_INTERFACE
+import com.android.tools.metalava.doclava1.Errors.LISTENER_LAST
+import com.android.tools.metalava.doclava1.Errors.MANAGER_CONSTRUCTOR
+import com.android.tools.metalava.doclava1.Errors.MANAGER_LOOKUP
+import com.android.tools.metalava.doclava1.Errors.MENTIONS_GOOGLE
+import com.android.tools.metalava.doclava1.Errors.METHOD_NAME_TENSE
+import com.android.tools.metalava.doclava1.Errors.METHOD_NAME_UNITS
+import com.android.tools.metalava.doclava1.Errors.MIN_MAX_CONSTANT
+import com.android.tools.metalava.doclava1.Errors.MISSING_BUILD
+import com.android.tools.metalava.doclava1.Errors.NOT_CLOSEABLE
+import com.android.tools.metalava.doclava1.Errors.NO_BYTE_OR_SHORT
+import com.android.tools.metalava.doclava1.Errors.NO_CLONE
+import com.android.tools.metalava.doclava1.Errors.ON_NAME_EXPECTED
+import com.android.tools.metalava.doclava1.Errors.OVERLAPPING_CONSTANTS
+import com.android.tools.metalava.doclava1.Errors.PACKAGE_LAYERING
+import com.android.tools.metalava.doclava1.Errors.PAIRED_REGISTRATION
+import com.android.tools.metalava.doclava1.Errors.PARCELABLE_LIST
+import com.android.tools.metalava.doclava1.Errors.PARCEL_CONSTRUCTOR
+import com.android.tools.metalava.doclava1.Errors.PARCEL_CREATOR
+import com.android.tools.metalava.doclava1.Errors.PARCEL_NOT_FINAL
+import com.android.tools.metalava.doclava1.Errors.PERCENTAGE_INT
+import com.android.tools.metalava.doclava1.Errors.PROTECTED_MEMBER
+import com.android.tools.metalava.doclava1.Errors.RAW_AIDL
+import com.android.tools.metalava.doclava1.Errors.REGISTRATION_NAME
+import com.android.tools.metalava.doclava1.Errors.RESOURCE_FIELD_NAME
+import com.android.tools.metalava.doclava1.Errors.RESOURCE_STYLE_FIELD_NAME
+import com.android.tools.metalava.doclava1.Errors.RESOURCE_VALUE_FIELD_NAME
+import com.android.tools.metalava.doclava1.Errors.RETHROW_REMOTE_EXCEPTION
+import com.android.tools.metalava.doclava1.Errors.SERVICE_NAME
+import com.android.tools.metalava.doclava1.Errors.SETTER_RETURNS_THIS
+import com.android.tools.metalava.doclava1.Errors.SINGLETON_CONSTRUCTOR
+import com.android.tools.metalava.doclava1.Errors.SINGLE_METHOD_INTERFACE
+import com.android.tools.metalava.doclava1.Errors.SINGULAR_CALLBACK
+import com.android.tools.metalava.doclava1.Errors.START_WITH_LOWER
+import com.android.tools.metalava.doclava1.Errors.START_WITH_UPPER
+import com.android.tools.metalava.doclava1.Errors.STATIC_UTILS
+import com.android.tools.metalava.doclava1.Errors.STREAM_FILES
+import com.android.tools.metalava.doclava1.Errors.TOP_LEVEL_BUILDER
+import com.android.tools.metalava.doclava1.Errors.UNIQUE_KOTLIN_OPERATOR
+import com.android.tools.metalava.doclava1.Errors.USER_HANDLE
+import com.android.tools.metalava.doclava1.Errors.USER_HANDLE_NAME
+import com.android.tools.metalava.doclava1.Errors.USE_ICU
+import com.android.tools.metalava.doclava1.Errors.USE_PARCEL_FILE_DESCRIPTOR
+import com.android.tools.metalava.doclava1.Errors.VISIBLY_SYNCHRONIZED
+import com.android.tools.metalava.model.ClassItem
+import com.android.tools.metalava.model.Codebase
+import com.android.tools.metalava.model.ConstructorItem
+import com.android.tools.metalava.model.FieldItem
+import com.android.tools.metalava.model.Item
+import com.android.tools.metalava.model.MemberItem
+import com.android.tools.metalava.model.MethodItem
+import com.android.tools.metalava.model.PackageItem
+import com.android.tools.metalava.model.ParameterItem
+import com.android.tools.metalava.model.TypeItem
+import com.android.tools.metalava.model.visitors.ApiVisitor
+import java.util.Locale
+import java.util.function.Predicate
+
+/**
+ * The [ApiLint] analyzer checks the API against a known set of preferred API practices
+ * by the Android API council.
+ */
+class ApiLint {
+ fun check(codebase: Codebase) {
+ val prevCount = reporter.totalCount
+
+ // The previous Kotlin interop tests are also part of API lint now (though they can be
+ // run independently as well; therefore, only run them here if not running separately)
+ val kotlinInterop = if (!options.checkKotlinInterop) KotlinInteropChecks() else null
+
+ codebase.accept(object : ApiVisitor(
+ // Sort by source order such that warnings follow source line number order
+ methodComparator = MethodItem.sourceOrderComparator,
+ fieldComparator = FieldItem.comparator
+ ) {
+ override fun skip(item: Item): Boolean {
+ return super.skip(item) || item is ClassItem && !isInteresting(item)
+ }
+
+ private var isKotlin = false
+
+ override fun visitClass(cls: ClassItem) {
+ val methods = cls.filteredMethods(filterEmit).asSequence()
+ val fields = cls.filteredFields(filterEmit, showUnannotated).asSequence()
+ val constructors = cls.filteredConstructors(filterEmit)
+ val superClass = cls.filteredSuperclass(filterReference)
+ val interfaces = cls.filteredInterfaceTypes(filterReference).asSequence()
+ val allMethods = methods.asSequence() + constructors.asSequence()
+ checkClass(
+ cls, methods, constructors, allMethods, fields, superClass, interfaces,
+ filterReference
+ )
+
+ isKotlin = cls.isKotlin()
+ }
+
+ override fun visitMethod(method: MethodItem) {
+ checkMethod(method, filterReference)
+ val returnType = method.returnType()
+ if (returnType != null) {
+ checkType(returnType, method)
+ }
+ for (parameter in method.parameters()) {
+ checkType(parameter.type(), parameter)
+ }
+ kotlinInterop?.checkMethod(method, isKotlin)
+ }
+
+ override fun visitField(field: FieldItem) {
+ checkField(field)
+ checkType(field.type(), field)
+ kotlinInterop?.checkField(field, isKotlin)
+ }
+ })
+
+ val apiLintIssues = reporter.totalCount - prevCount
+ if (apiLintIssues > 0) {
+ // We've reported API lint violations; emit some verbiage to explain
+ // how to suppress the error rules.
+ options.stdout.println("$apiLintIssues new API lint issues were found. See tools/metalava/API-LINT.md for how to handle these.")
+ }
+ }
+
+ private fun report(id: Error, item: Item, message: String) {
+ // Don't flag api warnings on deprecated APIs; these are obviously already known to
+ // be problematic.
+ if (item.deprecated) {
+ return
+ }
+
+ reporter.report(id, item, message)
+ }
+
+ private fun checkType(type: TypeItem, item: Item) {
+ val typeString = type.toTypeString()
+ checkPfd(typeString, item)
+ checkNumbers(typeString, item)
+ checkCollections(type, item)
+ checkCollectionsOverArrays(type, typeString, item)
+ checkBoxed(type, item)
+ checkIcu(type, typeString, item)
+ checkBitSet(type, typeString, item)
+ }
+
+ private fun checkClass(
+ cls: ClassItem,
+ methods: Sequence<MethodItem>,
+ constructors: Sequence<ConstructorItem>,
+ methodsAndConstructors: Sequence<MethodItem>,
+ fields: Sequence<FieldItem>,
+ superClass: ClassItem?,
+ interfaces: Sequence<TypeItem>,
+ filterReference: Predicate<Item>
+ ) {
+ checkEquals(methods)
+ checkEnums(cls)
+ checkClassNames(cls)
+ checkCallbacks(cls, methods)
+ checkListeners(cls, methods)
+ checkParcelable(cls, methods, constructors, fields)
+ checkRegistrationMethods(cls, methods)
+ checkHelperClasses(cls, methods, fields)
+ checkBuilder(cls, methods, superClass)
+ checkAidl(cls, superClass, interfaces)
+ checkInternal(cls)
+ checkLayering(cls, methodsAndConstructors, fields)
+ checkBooleans(methods)
+ checkFlags(fields)
+ checkGoogle(cls, methods, fields)
+ checkManager(cls, methods, constructors)
+ checkStaticUtils(cls, methods, constructors, fields)
+ checkCallbackHandlers(cls, methodsAndConstructors, superClass)
+ checkResourceNames(cls, fields)
+ checkFiles(methodsAndConstructors)
+ checkManagerList(cls, methods)
+ checkAbstractInner(cls)
+ checkRuntimeExceptions(methodsAndConstructors, filterReference)
+ checkError(cls, superClass)
+ checkCloseable(cls, methods)
+ checkNotKotlinOperator(methods)
+ checkUserHandle(cls, methods)
+ checkParams(cls)
+ checkSingleton(cls, methods, constructors)
+
+ // TODO: Not yet working
+ // checkOverloadArgs(cls, methods)
+ }
+
+ private fun checkField(
+ field: FieldItem
+ ) {
+ val modifiers = field.modifiers
+ if (modifiers.isStatic() && modifiers.isFinal()) {
+ checkConstantNames(field)
+ checkActions(field)
+ checkIntentExtras(field)
+ }
+ checkProtected(field)
+ checkServices(field)
+ }
+
+ private fun checkMethod(
+ method: MethodItem,
+ filterReference: Predicate<Item>
+ ) {
+ if (!method.isConstructor()) {
+ checkMethodNames(method)
+ checkProtected(method)
+ checkSynchronized(method)
+ checkIntentBuilder(method)
+ checkUnits(method)
+ checkTense(method)
+ checkClone(method)
+ }
+ checkExceptions(method, filterReference)
+ checkContextFirst(method)
+ checkListenerLast(method)
+ }
+
+ private fun checkEnums(cls: ClassItem) {
+ /*
+ def verify_enums(clazz):
+ """Enums are bad, mmkay?"""
+ if "extends java.lang.Enum" in clazz.raw:
+ error(clazz, None, "F5", "Enums are not allowed")
+ */
+ if (cls.isEnum()) {
+ report(ENUM, cls, "Enums are discouraged in Android APIs")
+ }
+ }
+
+ private fun checkMethodNames(method: MethodItem) {
+ /*
+ def verify_method_names(clazz):
+ """Try catching malformed method names, like Foo() or getMTU()."""
+ if clazz.fullname.startswith("android.opengl"): return
+ if clazz.fullname.startswith("android.renderscript"): return
+ if clazz.fullname == "android.system.OsConstants": return
+
+ for m in clazz.methods:
+ if re.search("[A-Z]{2,}", m.name) is not None:
+ warn(clazz, m, "S1", "Method names with acronyms should be getMtu() instead of getMTU()")
+ if re.match("[^a-z]", m.name):
+ error(clazz, m, "S1", "Method name must start with lowercase char")
+ */
+
+ // Existing violations
+ val containing = method.containingClass().qualifiedName()
+ if (containing.startsWith("android.opengl") ||
+ containing.startsWith("android.renderscript") ||
+ containing.startsWith("android.database.sqlite.") ||
+ containing == "android.system.OsConstants"
+ ) {
+ return
+ }
+
+ val name = method.name()
+ val first = name[0]
+
+ when {
+ first !in 'a'..'z' -> report(START_WITH_LOWER, method, "Method name must start with lowercase char: $name")
+ hasAcronyms(name) -> {
+ report(
+ ACRONYM_NAME, method,
+ "Acronyms should not be capitalized in method names: was `$name`, should this be `${decapitalizeAcronyms(
+ name
+ )}`?"
+ )
+ }
+ }
+ }
+
+ private fun checkClassNames(cls: ClassItem) {
+ /*
+ def verify_class_names(clazz):
+ """Try catching malformed class names like myMtp or MTPUser."""
+ if clazz.fullname.startswith("android.opengl"): return
+ if clazz.fullname.startswith("android.renderscript"): return
+ if re.match("android\.R\.[a-z]+", clazz.fullname): return
+
+ if re.search("[A-Z]{2,}", clazz.name) is not None:
+ warn(clazz, None, "S1", "Class names with acronyms should be Mtp not MTP")
+ if re.match("[^A-Z]", clazz.name):
+ error(clazz, None, "S1", "Class must start with uppercase char")
+ if clazz.name.endswith("Impl"):
+ error(clazz, None, None, "Don't expose your implementation details")
+ */
+
+ // Existing violations
+ val qualifiedName = cls.qualifiedName()
+ if (qualifiedName.startsWith("android.opengl") ||
+ qualifiedName.startsWith("android.renderscript") ||
+ qualifiedName.startsWith("android.database.sqlite.") ||
+ qualifiedName.startsWith("android.R.")
+ ) {
+ return
+ }
+
+ val name = cls.simpleName()
+ val first = name[0]
+ when {
+ first !in 'A'..'Z' -> {
+ report(
+ START_WITH_UPPER, cls,
+ "Class must start with uppercase char: $name"
+ )
+ }
+ hasAcronyms(name) -> {
+ report(
+ ACRONYM_NAME, cls,
+ "Acronyms should not be capitalized in class names: was `$name`, should this be `${decapitalizeAcronyms(
+ name
+ )}`?"
+ )
+ }
+ name.endsWith("Impl") -> {
+ report(
+ ENDS_WITH_IMPL, cls,
+ "Don't expose your implementation details: `$name` ends with `Impl`"
+ )
+ }
+ }
+ }
+
+ private fun checkConstantNames(field: FieldItem) {
+ /*
+ def verify_constants(clazz):
+ """All static final constants must be FOO_NAME style."""
+ if re.match("android\.R\.[a-z]+", clazz.fullname): return
+ if clazz.fullname.startswith("android.os.Build"): return
+ if clazz.fullname == "android.system.OsConstants": return
+
+ req = ["java.lang.String","byte","short","int","long","float","double","boolean","char"]
+ for f in clazz.fields:
+ if "static" in f.split and "final" in f.split:
+ if re.match("[A-Z0-9_]+", f.name) is None:
+ error(clazz, f, "C2", "Constant field names must be FOO_NAME")
+ if f.typ != "java.lang.String":
+ if f.name.startswith("MIN_") or f.name.startswith("MAX_"):
+ warn(clazz, f, "C8", "If min/max could change in future, make them dynamic methods")
+ if f.typ in req and f.value is None:
+ error(clazz, f, None, "All constants must be defined at compile time")
+ */
+ // Existing violations
+ val qualified = field.containingClass().qualifiedName()
+ if (qualified.startsWith("android.os.Build") ||
+ qualified == "android.system.OsConstants" ||
+ qualified == "android.media.MediaCodecInfo" ||
+ qualified.startsWith("android.opengl.") ||
+ qualified.startsWith("android.R.")
+ ) {
+ return
+ }
+
+ val name = field.name()
+ if (!constantNamePattern.matches(name)) {
+ val suggested = SdkVersionInfo.camelCaseToUnderlines(name).toUpperCase(Locale.US)
+ report(
+ ALL_UPPER, field,
+ "Constant field names must be named with only upper case characters: `$qualified#$name`, should be `$suggested`?"
+ )
+ } else if ((name.startsWith("MIN_") || name.startsWith("MAX_")) && !field.type().isString()) {
+ report(
+ MIN_MAX_CONSTANT, field,
+ "If min/max could change in future, make them dynamic methods: $qualified#$name"
+ )
+ } else if ((field.type().primitive || field.type().isString()) && field.initialValue(true) == null) {
+ report(
+ COMPILE_TIME_CONSTANT, field,
+ "All constants must be defined at compile time: $qualified#$name"
+ )
+ }
+ }
+
+ private fun checkCallbacks(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_callbacks(clazz):
+ """Verify Callback classes.
+ All callback classes must be abstract.
+ All methods must follow onFoo() naming style."""
+ if clazz.fullname == "android.speech.tts.SynthesisCallback": return
+
+ if clazz.name.endswith("Callbacks"):
+ error(clazz, None, "L1", "Callback class names should be singular")
+ if clazz.name.endswith("Observer"):
+ warn(clazz, None, "L1", "Class should be named FooCallback")
+
+ if clazz.name.endswith("Callback"):
+ if "interface" in clazz.split:
+ error(clazz, None, "CL3", "Callbacks must be abstract class to enable extension in future API levels")
+
+ for m in clazz.methods:
+ if not re.match("on[A-Z][a-z]*", m.name):
+ error(clazz, m, "L1", "Callback method names must be onFoo() style")
+
+ )
+ */
+
+ // Existing violations
+ val qualified = cls.qualifiedName()
+ if (qualified == "android.speech.tts.SynthesisCallback") {
+ return
+ }
+
+ val name = cls.simpleName()
+ when {
+ name.endsWith("Callbacks") -> {
+ report(
+ SINGULAR_CALLBACK, cls,
+ "Callback class names should be singular: $name"
+ )
+ }
+ name.endsWith("Observer") -> {
+ val prefix = name.removeSuffix("Observer")
+ report(
+ CALLBACK_NAME, cls,
+ "Class should be named ${prefix}Callback"
+ )
+ }
+ name.endsWith("Callback") -> {
+ if (cls.isInterface()) {
+ report(
+ CALLBACK_INTERFACE, cls,
+ "Callbacks must be abstract class instead of interface to enable extension in future API levels: $name"
+ )
+ } else {
+ for (method in methods) {
+ val methodName = method.name()
+ if (!onCallbackNamePattern.matches(methodName)) {
+ report(
+ CALLBACK_METHOD_NAME, cls,
+ "Callback method names must follow the on<Something> style: $methodName"
+ )
+ }
+ }
+ }
+ }
+ }
+ }
+
+ private fun checkListeners(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_listeners(clazz):
+ """Verify Listener classes.
+ All Listener classes must be interface.
+ All methods must follow onFoo() naming style.
+ If only a single method, it must match class name:
+ interface OnFooListener { void onFoo() }"""
+
+ if clazz.name.endswith("Listener"):
+ if " abstract class " in clazz.raw:
+ error(clazz, None, "L1", "Listeners should be an interface, or otherwise renamed Callback")
+
+ for m in clazz.methods:
+ if not re.match("on[A-Z][a-z]*", m.name):
+ error(clazz, m, "L1", "Listener method names must be onFoo() style")
+
+ if len(clazz.methods) == 1 and clazz.name.startswith("On"):
+ m = clazz.methods[0]
+ if (m.name + "Listener").lower() != clazz.name.lower():
+ error(clazz, m, "L1", "Single listener method name must match class name")
+ */
+
+ val name = cls.simpleName()
+ if (name.endsWith("Listener")) {
+ if (cls.isClass()) {
+ report(
+ LISTENER_INTERFACE, cls,
+ "Listeners should be an interface, or otherwise renamed Callback: $name"
+ )
+ } else {
+ for (method in methods) {
+ val methodName = method.name()
+ if (!onCallbackNamePattern.matches(methodName)) {
+ report(
+ CALLBACK_METHOD_NAME, cls,
+ "Listener method names must follow the on<Something> style: $methodName"
+ )
+ }
+ }
+
+ if (methods.count() == 1) {
+ val method = methods.first()
+ val methodName = method.name()
+ if (methodName.startsWith("On") &&
+ !("${methodName}Listener").equals(cls.simpleName(), ignoreCase = true)
+ ) {
+ report(
+ SINGLE_METHOD_INTERFACE, cls,
+ "Single listener method name must match class name"
+ )
+ }
+ }
+ }
+ }
+ }
+
+ private fun checkActions(field: FieldItem) {
+ /*
+ def verify_actions(clazz):
+ """Verify intent actions.
+ All action names must be named ACTION_FOO.
+ All action values must be scoped by package and match name:
+ package android.foo {
+ String ACTION_BAR = "android.foo.action.BAR";
+ }"""
+ for f in clazz.fields:
+ if f.value is None: continue
+ if f.name.startswith("EXTRA_"): continue
+ if f.name == "SERVICE_INTERFACE" or f.name == "PROVIDER_INTERFACE": continue
+ if "INTERACTION" in f.name: continue
+
+ if "static" in f.split and "final" in f.split and f.typ == "java.lang.String":
+ if "_ACTION" in f.name or "ACTION_" in f.name or ".action." in f.value.lower():
+ if not f.name.startswith("ACTION_"):
+ error(clazz, f, "C3", "Intent action constant name must be ACTION_FOO")
+ else:
+ if clazz.fullname == "android.content.Intent":
+ prefix = "android.intent.action"
+ elif clazz.fullname == "android.provider.Settings":
+ prefix = "android.settings"
+ elif clazz.fullname == "android.app.admin.DevicePolicyManager" or clazz.fullname == "android.app.admin.DeviceAdminReceiver":
+ prefix = "android.app.action"
+ else:
+ prefix = clazz.pkg.name + ".action"
+ expected = prefix + "." + f.name[7:]
+ if f.value != expected:
+ error(clazz, f, "C4", "Inconsistent action value; expected '%s'" % (expected))
+ */
+
+ val name = field.name()
+ if (name.startsWith("EXTRA_") || name == "SERVICE_INTERFACE" || name == "PROVIDER_INTERFACE") {
+ return
+ }
+ if (!field.type().isString()) {
+ return
+ }
+ val value = field.initialValue(true) as? String ?: return
+ if (!(name.contains("_ACTION") || name.contains("ACTION_") || value.contains(".action."))) {
+ return
+ }
+ if (!name.startsWith("ACTION_")) {
+ report(
+ INTENT_NAME, field,
+ "Intent action constant name must be ACTION_FOO: $name"
+ )
+ return
+ }
+ val className = field.containingClass().qualifiedName()
+ val prefix = when (className) {
+ "android.content.Intent" -> "android.intent.action"
+ "android.provider.Settings" -> "android.settings"
+ "android.app.admin.DevicePolicyManager", "android.app.admin.DeviceAdminReceiver" -> "android.app.action"
+ else -> field.containingClass().containingPackage().qualifiedName() + ".action"
+ }
+ val expected = prefix + "." + name.substring(7)
+ if (value != expected) {
+ report(
+ ACTION_VALUE, field,
+ "Inconsistent action value; expected `$expected`, was `$value`"
+ )
+ }
+ }
+
+ private fun checkIntentExtras(field: FieldItem) {
+ /*
+ def verify_extras(clazz):
+ """Verify intent extras.
+ All extra names must be named EXTRA_FOO.
+ All extra values must be scoped by package and match name:
+ package android.foo {
+ String EXTRA_BAR = "android.foo.extra.BAR";
+ }"""
+ if clazz.fullname == "android.app.Notification": return
+ if clazz.fullname == "android.appwidget.AppWidgetManager": return
+
+ for f in clazz.fields:
+ if f.value is None: continue
+ if f.name.startswith("ACTION_"): continue
+
+ if "static" in f.split and "final" in f.split and f.typ == "java.lang.String":
+ if "_EXTRA" in f.name or "EXTRA_" in f.name or ".extra" in f.value.lower():
+ if not f.name.startswith("EXTRA_"):
+ error(clazz, f, "C3", "Intent extra must be EXTRA_FOO")
+ else:
+ if clazz.pkg.name == "android.content" and clazz.name == "Intent":
+ prefix = "android.intent.extra"
+ elif clazz.pkg.name == "android.app.admin":
+ prefix = "android.app.extra"
+ else:
+ prefix = clazz.pkg.name + ".extra"
+ expected = prefix + "." + f.name[6:]
+ if f.value != expected:
+ error(clazz, f, "C4", "Inconsistent extra value; expected '%s'" % (expected))
+
+
+ */
+ val className = field.containingClass().qualifiedName()
+ if (className == "android.app.Notification" || className == "android.appwidget.AppWidgetManager") {
+ return
+ }
+
+ val name = field.name()
+ if (name.startsWith("ACTION_") || !field.type().isString()) {
+ return
+ }
+ val value = field.initialValue(true) as? String ?: return
+ if (!(name.contains("_EXTRA") || name.contains("EXTRA_") || value.contains(".extra"))) {
+ return
+ }
+ if (!name.startsWith("EXTRA_")) {
+ report(
+ INTENT_NAME, field,
+ "Intent extra constant name must be EXTRA_FOO: $name"
+ )
+ return
+ }
+
+ val packageName = field.containingClass().containingPackage().qualifiedName()
+ val prefix = when {
+ className == "android.content.Intent" -> "android.intent.extra"
+ packageName == "android.app.admin" -> "android.app.extra"
+ else -> "$packageName.extra"
+ }
+ val expected = prefix + "." + name.substring(6)
+ if (value != expected) {
+ report(
+ ACTION_VALUE, field,
+ "Inconsistent extra value; expected `$expected`, was `$value`"
+ )
+ }
+ }
+
+ private fun checkEquals(methods: Sequence<MethodItem>) {
+ /*
+ def verify_equals(clazz):
+ """Verify that equals() and hashCode() must be overridden together."""
+ eq = False
+ hc = False
+ for m in clazz.methods:
+ if " static " in m.raw: continue
+ if "boolean equals(java.lang.Object)" in m.raw: eq = True
+ if "int hashCode()" in m.raw: hc = True
+ if eq != hc:
+ error(clazz, None, "M8", "Must override both equals and hashCode; missing one")
+ */
+ var equalsMethod: MethodItem? = null
+ var hashCodeMethod: MethodItem? = null
+
+ for (method in methods) {
+ if (isEqualsMethod(method)) {
+ equalsMethod = method
+ } else if (isHashCodeMethod(method)) {
+ hashCodeMethod = method
+ }
+ }
+ if ((equalsMethod == null) != (hashCodeMethod == null)) {
+ val method = equalsMethod ?: hashCodeMethod!!
+ report(
+ EQUALS_AND_HASH_CODE, method,
+ "Must override both equals and hashCode; missing one in ${method.containingClass().qualifiedName()}"
+ )
+ }
+ }
+
+ private fun isEqualsMethod(method: MethodItem): Boolean {
+ return method.name() == "equals" && method.parameters().size == 1 &&
+ method.parameters()[0].type().isJavaLangObject() &&
+ !method.modifiers.isStatic()
+ }
+
+ private fun isHashCodeMethod(method: MethodItem): Boolean {
+ return method.name() == "hashCode" && method.parameters().isEmpty() &&
+ !method.modifiers.isStatic()
+ }
+
+ private fun checkParcelable(
+ cls: ClassItem,
+ methods: Sequence<MethodItem>,
+ constructors: Sequence<MethodItem>,
+ fields: Sequence<FieldItem>
+ ) {
+ /*
+ def verify_parcelable(clazz):
+ """Verify that Parcelable objects aren't hiding required bits."""
+ if "implements android.os.Parcelable" in clazz.raw:
+ creator = [ i for i in clazz.fields if i.name == "CREATOR" ]
+ write = [ i for i in clazz.methods if i.name == "writeToParcel" ]
+ describe = [ i for i in clazz.methods if i.name == "describeContents" ]
+
+ if len(creator) == 0 or len(write) == 0 or len(describe) == 0:
+ error(clazz, None, "FW3", "Parcelable requires CREATOR, writeToParcel, and describeContents; missing one")
+
+ if ((" final class " not in clazz.raw) and
+ (" final deprecated class " not in clazz.raw)):
+ error(clazz, None, "FW8", "Parcelable classes must be final")
+
+ for c in clazz.ctors:
+ if c.args == ["android.os.Parcel"]:
+ error(clazz, c, "FW3", "Parcelable inflation is exposed through CREATOR, not raw constructors")
+ */
+
+ if (!cls.implements("android.os.Parcelable")) {
+ return
+ }
+
+ if (fields.none { it.name() == "CREATOR" }) {
+ report(
+ PARCEL_CREATOR, cls,
+ "Parcelable requires a `CREATOR` field; missing in ${cls.qualifiedName()}"
+ )
+ }
+ if (methods.none { it.name() == "writeToParcel" }) {
+ report(
+ PARCEL_CREATOR, cls,
+ "Parcelable requires `void writeToParcel(Parcel, int)`; missing in ${cls.qualifiedName()}"
+ )
+ }
+ if (methods.none { it.name() == "describeContents" }) {
+ report(
+ PARCEL_CREATOR, cls,
+ "Parcelable requires `public int describeContents()`; missing in ${cls.qualifiedName()}"
+ )
+ }
+
+ if (!cls.modifiers.isFinal()) {
+ report(
+ PARCEL_NOT_FINAL, cls,
+ "Parcelable classes must be final: ${cls.qualifiedName()} is not final"
+ )
+ }
+
+ val parcelConstructor = constructors.firstOrNull {
+ val parameters = it.parameters()
+ parameters.size == 1 && parameters[0].type().toTypeString() == "android.os.Parcel"
+ }
+
+ if (parcelConstructor != null) {
+ report(
+ PARCEL_CONSTRUCTOR, parcelConstructor,
+ "Parcelable inflation is exposed through CREATOR, not raw constructors, in ${cls.qualifiedName()}"
+ )
+ }
+ }
+
+ private fun checkProtected(member: MemberItem) {
+ /*
+ def verify_protected(clazz):
+ """Verify that no protected methods or fields are allowed."""
+ for m in clazz.methods:
+ if m.name == "finalize": continue
+ if "protected" in m.split:
+ error(clazz, m, "M7", "Protected methods not allowed; must be public")
+ for f in clazz.fields:
+ if "protected" in f.split:
+ error(clazz, f, "M7", "Protected fields not allowed; must be public")
+ */
+ val modifiers = member.modifiers
+ if (modifiers.isProtected()) {
+ if (member.name() == "finalize" && member is MethodItem && member.parameters().isEmpty()) {
+ return
+ }
+
+ report(
+ PROTECTED_MEMBER, member,
+ "Protected ${if (member is MethodItem) "methods" else "fields"} not allowed; must be public: ${member.describe()}}"
+ )
+ }
+ }
+
+ private fun checkRegistrationMethods(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_register(clazz):
+ """Verify parity of registration methods.
+ Callback objects use register/unregister methods.
+ Listener objects use add/remove methods."""
+ methods = [ m.name for m in clazz.methods ]
+ for m in clazz.methods:
+ if "Callback" in m.raw:
+ if m.name.startswith("register"):
+ other = "unregister" + m.name[8:]
+ if other not in methods:
+ error(clazz, m, "L2", "Missing unregister method")
+ if m.name.startswith("unregister"):
+ other = "register" + m.name[10:]
+ if other not in methods:
+ error(clazz, m, "L2", "Missing register method")
+
+ if m.name.startswith("add") or m.name.startswith("remove"):
+ error(clazz, m, "L3", "Callback methods should be named register/unregister")
+
+ if "Listener" in m.raw:
+ if m.name.startswith("add"):
+ other = "remove" + m.name[3:]
+ if other not in methods:
+ error(clazz, m, "L2", "Missing remove method")
+ if m.name.startswith("remove") and not m.name.startswith("removeAll"):
+ other = "add" + m.name[6:]
+ if other not in methods:
+ error(clazz, m, "L2", "Missing add method")
+
+ if m.name.startswith("register") or m.name.startswith("unregister"):
+ error(clazz, m, "L3", "Listener methods should be named add/remove")
+ */
+
+ /** Make sure that there is a corresponding method */
+ fun ensureMatched(cls: ClassItem, methods: Sequence<MethodItem>, method: MethodItem, name: String) {
+ for (candidate in methods) {
+ if (candidate.name() == name) {
+ return
+ }
+ }
+
+ report(
+ PAIRED_REGISTRATION, method,
+ "Found ${method.name()} but not $name in ${cls.qualifiedName()}"
+ )
+ }
+
+ for (method in methods) {
+ val name = method.name()
+ // the python version looks for any substring, but that includes a lot of other stuff, like plurals
+ if (name.endsWith("Callback")) {
+ if (name.startsWith("register")) {
+ val unregister = "unregister" + name.substring(8) // "register".length
+ ensureMatched(cls, methods, method, unregister)
+ } else if (name.startsWith("unregister")) {
+ val unregister = "register" + name.substring(10) // "unregister".length
+ ensureMatched(cls, methods, method, unregister)
+ }
+ if (name.startsWith("add") || name.startsWith("remove")) {
+ report(
+ REGISTRATION_NAME, method,
+ "Callback methods should be named register/unregister; was $name"
+ )
+ }
+ } else if (name.endsWith("Listener")) {
+ if (name.startsWith("add")) {
+ val unregister = "remove" + name.substring(3) // "add".length
+ ensureMatched(cls, methods, method, unregister)
+ } else if (name.startsWith("remove") && !name.startsWith("removeAll")) {
+ val unregister = "add" + name.substring(6) // "remove".length
+ ensureMatched(cls, methods, method, unregister)
+ }
+ if (name.startsWith("register") || name.startsWith("unregister")) {
+ report(
+ REGISTRATION_NAME, method,
+ "Listener methods should be named add/remove; was $name"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkSynchronized(method: MethodItem) {
+ /*
+ def verify_sync(clazz):
+ """Verify synchronized methods aren't exposed."""
+ for m in clazz.methods:
+ if "synchronized" in m.split:
+ error(clazz, m, "M5", "Internal locks must not be exposed")
+ */
+ if (method.modifiers.isSynchronized()) {
+ report(
+ VISIBLY_SYNCHRONIZED, method,
+ "Internal locks must not be exposed: ${method.describe()}"
+ )
+ }
+ }
+
+ private fun checkIntentBuilder(method: MethodItem) {
+ /*
+ def verify_intent_builder(clazz):
+ """Verify that Intent builders are createFooIntent() style."""
+ if clazz.name == "Intent": return
+
+ for m in clazz.methods:
+ if m.typ == "android.content.Intent":
+ if m.name.startswith("create") and m.name.endswith("Intent"):
+ pass
+ else:
+ warn(clazz, m, "FW1", "Methods creating an Intent should be named createFooIntent()")
+ */
+ if (method.returnType()?.toTypeString() == "android.content.Intent") {
+ val name = method.name()
+ if (name.startsWith("create") && name.endsWith("Intent")) {
+ return
+ }
+ if (method.containingClass().simpleName() == "Intent") {
+ return
+ }
+
+ report(
+ INTENT_BUILDER_NAME, method,
+ "Methods creating an Intent should be named `create<Foo>Intent()`, was `$name`"
+ )
+ }
+ }
+
+ private fun checkHelperClasses(cls: ClassItem, methods: Sequence<MethodItem>, fields: Sequence<FieldItem>) {
+ /*
+ def verify_helper_classes(clazz):
+ """Verify that helper classes are named consistently with what they extend.
+ All developer extendable methods should be named onFoo()."""
+ test_methods = False
+ if "extends android.app.Service" in clazz.raw:
+ test_methods = True
+ if not clazz.name.endswith("Service"):
+ error(clazz, None, "CL4", "Inconsistent class name; should be FooService")
+
+ found = False
+ for f in clazz.fields:
+ if f.name == "SERVICE_INTERFACE":
+ found = True
+ if f.value != clazz.fullname:
+ error(clazz, f, "C4", "Inconsistent interface constant; expected '%s'" % (clazz.fullname))
+
+ if "extends android.content.ContentProvider" in clazz.raw:
+ test_methods = True
+ if not clazz.name.endswith("Provider"):
+ error(clazz, None, "CL4", "Inconsistent class name; should be FooProvider")
+
+ found = False
+ for f in clazz.fields:
+ if f.name == "PROVIDER_INTERFACE":
+ found = True
+ if f.value != clazz.fullname:
+ error(clazz, f, "C4", "Inconsistent interface constant; expected '%s'" % (clazz.fullname))
+
+ if "extends android.content.BroadcastReceiver" in clazz.raw:
+ test_methods = True
+ if not clazz.name.endswith("Receiver"):
+ error(clazz, None, "CL4", "Inconsistent class name; should be FooReceiver")
+
+ if "extends android.app.Activity" in clazz.raw:
+ test_methods = True
+ if not clazz.name.endswith("Activity"):
+ error(clazz, None, "CL4", "Inconsistent class name; should be FooActivity")
+
+ if test_methods:
+ for m in clazz.methods:
+ if "final" in m.split: continue
+// Note: This regex seems wrong:
+ if not re.match("on[A-Z]", m.name):
+ if "abstract" in m.split:
+ warn(clazz, m, None, "Methods implemented by developers should be named onFoo()")
+ else:
+ warn(clazz, m, None, "If implemented by developer, should be named onFoo(); otherwise consider marking final")
+
+ */
+
+ fun ensureFieldValue(fields: Sequence<FieldItem>, fieldName: String, fieldValue: String) {
+ fields.firstOrNull { it.name() == fieldName }?.let { field ->
+ if (field.initialValue(true) != fieldValue) {
+ report(
+ INTERFACE_CONSTANT, field,
+ "Inconsistent interface constant; expected '$fieldValue'`"
+ )
+ }
+ }
+ }
+
+ fun ensureContextNameSuffix(cls: ClassItem, suffix: String) {
+ if (!cls.simpleName().endsWith(suffix)) {
+ report(
+ CONTEXT_NAME_SUFFIX, cls,
+ "Inconsistent class name; should be `<Foo>$suffix`, was `${cls.simpleName()}`"
+ )
+ }
+ }
+
+ var testMethods = false
+
+ when {
+ cls.extends("android.app.Service") -> {
+ testMethods = true
+ ensureContextNameSuffix(cls, "Service")
+ ensureFieldValue(fields, "SERVICE_INTERFACE", cls.fullName())
+ }
+ cls.extends("android.content.ContentProvider") -> {
+ testMethods = true
+ ensureContextNameSuffix(cls, "Provider")
+ ensureFieldValue(fields, "PROVIDER_INTERFACE", cls.fullName())
+ }
+ cls.extends("android.content.BroadcastReceiver") -> {
+ testMethods = true
+ ensureContextNameSuffix(cls, "Receiver")
+ }
+ cls.extends("android.app.Activity") -> {
+ testMethods = true
+ ensureContextNameSuffix(cls, "Activity")
+ }
+ }
+
+ if (testMethods) {
+ for (method in methods) {
+ val modifiers = method.modifiers
+ if (modifiers.isFinal()) {
+ continue
+ }
+ val name = method.name()
+ if (!onCallbackNamePattern.matches(name)) {
+ val message =
+ if (modifiers.isAbstract()) {
+ "Methods implemented by developers should follow the on<Something> style, was `$name`"
+ } else {
+ "If implemented by developer, should follow the on<Something> style; otherwise consider marking final"
+ }
+ report(ON_NAME_EXPECTED, method, message)
+ }
+ }
+ }
+ }
+
+ private fun checkBuilder(cls: ClassItem, methods: Sequence<MethodItem>, superClass: ClassItem?) {
+ /*
+ def verify_builder(clazz):
+ """Verify builder classes.
+ Methods should return the builder to enable chaining."""
+ if " extends " in clazz.raw: return
+ if not clazz.name.endswith("Builder"): return
+
+ if clazz.name != "Builder":
+ warn(clazz, None, None, "Builder should be defined as inner class")
+
+ has_build = False
+ for m in clazz.methods:
+ if m.name == "build":
+ has_build = True
+ continue
+
+ if m.name.startswith("get"): continue
+ if m.name.startswith("clear"): continue
+
+ if m.name.startswith("with"):
+ warn(clazz, m, None, "Builder methods names should use setFoo() style")
+
+ if m.name.startswith("set"):
+ if not m.typ.endswith(clazz.fullname):
+ warn(clazz, m, "M4", "Methods must return the builder object")
+
+ if not has_build:
+ warn(clazz, None, None, "Missing build() method")
+ */
+ if (!cls.simpleName().endsWith("Builder")) {
+ return
+ }
+ if (superClass != null && !superClass.isJavaLangObject()) {
+ return
+ }
+ if (cls.isTopLevelClass()) {
+ report(
+ TOP_LEVEL_BUILDER, cls,
+ "Builder should be defined as inner class: ${cls.qualifiedName()}"
+ )
+ }
+ var hasBuild = false
+ for (method in methods) {
+ val name = method.name()
+ if (name == "build") {
+ hasBuild = true
+ continue
+ } else if (name.startsWith("get") || name.startsWith("clear")) {
+ continue
+ } else if (name.startsWith("with")) {
+ report(
+ BUILDER_SET_STYLE, cls,
+ "Builder methods names should use setFoo() style: ${method.describe()}"
+ )
+ } else if (name.startsWith("set")) {
+ val returnType = method.returnType()?.toTypeString() ?: ""
+ if (returnType != cls.qualifiedName()) {
+ report(
+ SETTER_RETURNS_THIS, cls,
+ "Methods must return the builder object (return type ${cls.simpleName()} instead of $returnType): ${method.describe()}"
+ )
+ }
+ }
+ }
+ if (!hasBuild) {
+ report(
+ MISSING_BUILD, cls,
+ "Missing `build()` method in ${cls.qualifiedName()}"
+ )
+ }
+ }
+
+ private fun checkAidl(cls: ClassItem, superClass: ClassItem?, interfaces: Sequence<TypeItem>) {
+ /*
+ def verify_aidl(clazz):
+ """Catch people exposing raw AIDL."""
+ if "extends android.os.Binder" in clazz.raw or "implements android.os.IInterface" in clazz.raw:
+ error(clazz, None, None, "Raw AIDL interfaces must not be exposed")
+ */
+
+ // Instead of ClassItem.implements() and .extends() which performs hierarchy
+ // searches, here we only want to flag directly extending or implementing:
+ val extendsBinder = superClass?.qualifiedName() == "android.os.Binder"
+ val implementsIInterface = interfaces.any { it.toTypeString() == "android.os.IInterface" }
+ if (extendsBinder || implementsIInterface) {
+ val problem = if (extendsBinder) {
+ "extends Binder"
+ } else {
+ "implements IInterface"
+ }
+ report(
+ RAW_AIDL, cls,
+ "Raw AIDL interfaces must not be exposed: ${cls.simpleName()} $problem"
+ )
+ }
+ }
+
+ private fun checkInternal(cls: ClassItem) {
+ /*
+ def verify_internal(clazz):
+ """Catch people exposing internal classes."""
+ if clazz.pkg.name.startswith("com.android"):
+ error(clazz, None, None, "Internal classes must not be exposed")
+ */
+
+ if (cls.qualifiedName().startsWith("com.android.")) {
+ report(
+ INTERNAL_CLASSES, cls,
+ "Internal classes must not be exposed"
+ )
+ }
+ }
+
+ private fun checkLayering(
+ cls: ClassItem,
+ methodsAndConstructors: Sequence<MethodItem>,
+ fields: Sequence<FieldItem>
+ ) {
+ /*
+ def verify_layering(clazz):
+ """Catch package layering violations.
+ For example, something in android.os depending on android.app."""
+ ranking = [
+ ["android.service","android.accessibilityservice","android.inputmethodservice","android.printservice","android.appwidget","android.webkit","android.preference","android.gesture","android.print"],
+ "android.app",
+ "android.widget",
+ "android.view",
+ "android.animation",
+ "android.provider",
+ ["android.content","android.graphics.drawable"],
+ "android.database",
+ "android.text",
+ "android.graphics",
+ "android.os",
+ "android.util"
+ ]
+
+ def rank(p):
+ for i in range(len(ranking)):
+ if isinstance(ranking[i], list):
+ for j in ranking[i]:
+ if p.startswith(j): return i
+ else:
+ if p.startswith(ranking[i]): return i
+
+ cr = rank(clazz.pkg.name)
+ if cr is None: return
+
+ for f in clazz.fields:
+ ir = rank(f.typ)
+ if ir and ir < cr:
+ warn(clazz, f, "FW6", "Field type violates package layering")
+
+ for m in clazz.methods:
+ ir = rank(m.typ)
+ if ir and ir < cr:
+ warn(clazz, m, "FW6", "Method return type violates package layering")
+ for arg in m.args:
+ ir = rank(arg)
+ if ir and ir < cr:
+ warn(clazz, m, "FW6", "Method argument type violates package layering")
+
+ */
+
+ fun packageRank(pkg: PackageItem): Int {
+ return when (pkg.qualifiedName()) {
+ "android.service",
+ "android.accessibilityservice",
+ "android.inputmethodservice",
+ "android.printservice",
+ "android.appwidget",
+ "android.webkit",
+ "android.preference",
+ "android.gesture",
+ "android.print" -> 10
+
+ "android.app" -> 20
+ "android.widget" -> 30
+ "android.view" -> 40
+ "android.animation" -> 50
+ "android.provider" -> 60
+
+ "android.content",
+ "android.graphics.drawable" -> 70
+
+ "android.database" -> 80
+ "android.text" -> 90
+ "android.graphics" -> 100
+ "android.os" -> 110
+ "android.util" -> 120
+ else -> -1
+ }
+ }
+
+ fun getTypePackage(type: TypeItem?): PackageItem? {
+ return if (type == null || type.primitive) {
+ null
+ } else {
+ type.asClass()?.containingPackage()
+ }
+ }
+
+ fun getTypeRank(type: TypeItem?): Int {
+ type ?: return -1
+ val pkg = getTypePackage(type) ?: return -1
+ return packageRank(pkg)
+ }
+
+ val classPackage = cls.containingPackage()
+ val classRank = packageRank(classPackage)
+ if (classRank == -1) {
+ return
+ }
+ for (field in fields) {
+ val fieldTypeRank = getTypeRank(field.type())
+ if (fieldTypeRank != -1 && fieldTypeRank < classRank) {
+ report(
+ PACKAGE_LAYERING, cls,
+ "Field type `${field.type().toTypeString()}` violates package layering: nothing in `$classPackage` should depend on `${getTypePackage(
+ field.type()
+ )}`"
+ )
+ }
+ }
+
+ for (method in methodsAndConstructors) {
+ val returnType = method.returnType()
+ if (returnType != null) { // not a constructor
+ val returnTypeRank = getTypeRank(returnType)
+ if (returnTypeRank != -1 && returnTypeRank < classRank) {
+ report(
+ PACKAGE_LAYERING, cls,
+ "Method return type `${returnType.toTypeString()}` violates package layering: nothing in `$classPackage` should depend on `${getTypePackage(
+ returnType
+ )}`"
+ )
+ }
+ }
+
+ for (parameter in method.parameters()) {
+ val parameterTypeRank = getTypeRank(parameter.type())
+ if (parameterTypeRank != -1 && parameterTypeRank < classRank) {
+ report(
+ PACKAGE_LAYERING, cls,
+ "Method parameter type `${parameter.type().toTypeString()}` violates package layering: nothing in `$classPackage` should depend on `${getTypePackage(
+ parameter.type()
+ )}`"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkBooleans(methods: Sequence<MethodItem>) {
+ /*
+ def verify_boolean(clazz):
+ """Verifies that boolean accessors are named correctly.
+ For example, hasFoo() and setHasFoo()."""
+
+ def is_get(m): return len(m.args) == 0 and m.typ == "boolean"
+ def is_set(m): return len(m.args) == 1 and m.args[0] == "boolean"
+
+ gets = [ m for m in clazz.methods if is_get(m) ]
+ sets = [ m for m in clazz.methods if is_set(m) ]
+
+ def error_if_exists(methods, trigger, expected, actual):
+ for m in methods:
+ if m.name == actual:
+ error(clazz, m, "M6", "Symmetric method for %s must be named %s" % (trigger, expected))
+
+ for m in clazz.methods:
+ if is_get(m):
+ if re.match("is[A-Z]", m.name):
+ target = m.name[2:]
+ expected = "setIs" + target
+ error_if_exists(sets, m.name, expected, "setHas" + target)
+ elif re.match("has[A-Z]", m.name):
+ target = m.name[3:]
+ expected = "setHas" + target
+ error_if_exists(sets, m.name, expected, "setIs" + target)
+ error_if_exists(sets, m.name, expected, "set" + target)
+ elif re.match("get[A-Z]", m.name):
+ target = m.name[3:]
+ expected = "set" + target
+ error_if_exists(sets, m.name, expected, "setIs" + target)
+ error_if_exists(sets, m.name, expected, "setHas" + target)
+
+ if is_set(m):
+ if re.match("set[A-Z]", m.name):
+ target = m.name[3:]
+ expected = "get" + target
+ error_if_exists(sets, m.name, expected, "is" + target)
+ error_if_exists(sets, m.name, expected, "has" + target)
+ */
+
+ fun errorIfExists(methods: Sequence<MethodItem>, trigger: String, expected: String, actual: String) {
+ for (method in methods) {
+ if (method.name() == actual) {
+ report(
+ GETTER_SETTER_NAMES, method,
+ "Symmetric method for `$trigger` must be named `$expected`; was `$actual`"
+ )
+ }
+ }
+ }
+
+ fun isGetter(method: MethodItem): Boolean {
+ val returnType = method.returnType() ?: return false
+ return method.parameters().isEmpty() && returnType.primitive && returnType.toTypeString() == "boolean"
+ }
+
+ fun isSetter(method: MethodItem): Boolean {
+ return method.parameters().size == 1 && method.parameters()[0].type().toTypeString() == "boolean"
+ }
+
+ for (method in methods) {
+ val name = method.name()
+ if (isGetter(method)) {
+ if (name.startsWith("is") && name.length >= 3 && name[2].isUpperCase()) {
+ val target = name.substring(2)
+ val expected = "setIs$target"
+ errorIfExists(methods, name, expected, "setHas$target")
+ } else if (name.startsWith("has") && name.length >= 4 && name[3].isUpperCase()) {
+ val target = name.substring(3)
+ val expected = "setHas$target"
+ errorIfExists(methods, name, expected, "setIs$target")
+ errorIfExists(methods, name, expected, "set$target")
+ } else if (name.startsWith("get") && name.length >= 4 && name[3].isUpperCase()) {
+ val target = name.substring(3)
+ val expected = "set$target"
+ errorIfExists(methods, name, expected, "setIs$target")
+ errorIfExists(methods, name, expected, "setHas$target")
+ }
+ } else if (isSetter(method)) {
+ if (name.startsWith("set") && name.length >= 4 && name[3].isUpperCase()) {
+ val target = name.substring(3)
+ val expected = "get$target"
+ errorIfExists(methods, name, expected, "is$target")
+ errorIfExists(methods, name, expected, "has$target")
+ }
+ }
+ }
+ }
+
+ private fun checkCollections(
+ type: TypeItem,
+ item: Item
+ ) {
+ /*
+ def verify_collections(clazz):
+ """Verifies that collection types are interfaces."""
+ if clazz.fullname == "android.os.Bundle": return
+
+ bad = ["java.util.Vector", "java.util.LinkedList", "java.util.ArrayList", "java.util.Stack",
+ "java.util.HashMap", "java.util.HashSet", "android.util.ArraySet", "android.util.ArrayMap"]
+ for m in clazz.methods:
+ if m.typ in bad:
+ error(clazz, m, "CL2", "Return type is concrete collection; must be higher-level interface")
+ for arg in m.args:
+ if arg in bad:
+ error(clazz, m, "CL2", "Argument is concrete collection; must be higher-level interface")
+ */
+
+ if (type.primitive) {
+ return
+ }
+
+ val raw = type.asClass()?.qualifiedName()
+ when (raw) {
+ "java.util.Vector",
+ "java.util.LinkedList",
+ "java.util.ArrayList",
+ "java.util.Stack",
+ "java.util.HashMap",
+ "java.util.HashSet",
+ "android.util.ArraySet",
+ "android.util.ArrayMap" -> {
+ if (item.containingClass()?.qualifiedName() == "android.os.Bundle") {
+ return
+ }
+ val where = when (item) {
+ is MethodItem -> "Return type"
+ is FieldItem -> "Field type"
+ else -> "Parameter type"
+ }
+ val erased = type.toErasedTypeString()
+ report(
+ CONCRETE_COLLECTION, item,
+ "$where is concrete collection (`$erased`); must be higher-level interface"
+ )
+ }
+ }
+ }
+
+ fun Item.containingClass(): ClassItem? {
+ return when (this) {
+ is MemberItem -> this.containingClass()
+ is ParameterItem -> this.containingMethod().containingClass()
+ is ClassItem -> this
+ else -> null
+ }
+ }
+
+ private fun checkFlags(fields: Sequence<FieldItem>) {
+ /*
+ def verify_flags(clazz):
+ """Verifies that flags are non-overlapping."""
+ known = collections.defaultdict(int)
+ for f in clazz.fields:
+ if "FLAG_" in f.name:
+ try:
+ val = int(f.value)
+ except:
+ continue
+
+ scope = f.name[0:f.name.index("FLAG_")]
+ if val & known[scope]:
+ warn(clazz, f, "C1", "Found overlapping flag constant value")
+ known[scope] |= val
+
+ */
+ var known: MutableMap<String, Int>? = null
+ var valueToFlag: MutableMap<Int?, String>? = null
+ for (field in fields) {
+ val name = field.name()
+ val index = name.indexOf("FLAG_")
+ if (index != -1) {
+ val value = field.initialValue() as? Int ?: continue
+ val scope = name.substring(0, index)
+ val prev = known?.get(scope) ?: 0
+ if (known != null && (prev and value) != 0) {
+ val prevName = valueToFlag?.get(prev)
+ report(
+ OVERLAPPING_CONSTANTS, field,
+ "Found overlapping flag constant values: `$name` with value $value (0x${Integer.toHexString(
+ value
+ )}) and overlapping flag value $prev (0x${Integer.toHexString(prev)}) from `$prevName`"
+ )
+ }
+ if (known == null) {
+ known = mutableMapOf()
+ }
+ known[scope] = value
+ if (valueToFlag == null) {
+ valueToFlag = mutableMapOf()
+ }
+ valueToFlag[value] = name
+ }
+ }
+ }
+
+ private fun checkExceptions(method: MethodItem, filterReference: Predicate<Item>) {
+ /*
+ def verify_exception(clazz):
+ """Verifies that methods don't throw generic exceptions."""
+ for m in clazz.methods:
+ for t in m.throws:
+ if t in ["java.lang.Exception", "java.lang.Throwable", "java.lang.Error"]:
+ error(clazz, m, "S1", "Methods must not throw generic exceptions")
+
+ if t in ["android.os.RemoteException"]:
+ if clazz.name == "android.content.ContentProviderClient": continue
+ if clazz.name == "android.os.Binder": continue
+ if clazz.name == "android.os.IBinder": continue
+
+ error(clazz, m, "FW9", "Methods calling into system server should rethrow RemoteException as RuntimeException")
+
+ if len(m.args) == 0 and t in ["java.lang.IllegalArgumentException", "java.lang.NullPointerException"]:
+ warn(clazz, m, "S1", "Methods taking no arguments should throw IllegalStateException")
+ */
+ for (exception in method.filteredThrowsTypes(filterReference)) {
+ val qualifiedName = exception.qualifiedName()
+ when (qualifiedName) {
+ "java.lang.Exception",
+ "java.lang.Throwable",
+ "java.lang.Error" -> {
+ report(
+ GENERIC_EXCEPTION, method,
+ "Methods must not throw generic exceptions (`$qualifiedName`)"
+ )
+ }
+ "android.os.RemoteException" -> {
+ when (method.containingClass().qualifiedName()) {
+ "android.content.ContentProviderClient",
+ "android.os.Binder",
+ "android.os.IBinder" -> {
+ // exceptions
+ }
+ else -> {
+ report(
+ RETHROW_REMOTE_EXCEPTION, method,
+ "Methods calling into system server should rethrow `RemoteException` as `RuntimeException`"
+ )
+ }
+ }
+ }
+ "java.lang.IllegalArgumentException",
+ "java.lang.NullPointerException" -> {
+ if (method.parameters().isEmpty()) {
+ report(
+ ILLEGAL_STATE_EXCEPTION, method,
+ "Methods taking no arguments should throw `IllegalStateException` instead of `$qualifiedName`"
+ )
+ }
+ }
+ }
+ }
+ }
+
+ private fun checkGoogle(cls: ClassItem, methods: Sequence<MethodItem>, fields: Sequence<FieldItem>) {
+ /*
+ def verify_google(clazz):
+ """Verifies that APIs never reference Google."""
+
+ if re.search("google", clazz.raw, re.IGNORECASE):
+ error(clazz, None, None, "Must never reference Google")
+
+ test = []
+ test.extend(clazz.ctors)
+ test.extend(clazz.fields)
+ test.extend(clazz.methods)
+
+ for t in test:
+ if re.search("google", t.raw, re.IGNORECASE):
+ error(clazz, t, None, "Must never reference Google")
+ */
+
+ fun checkName(name: String, item: Item) {
+ if (name.contains("Google", ignoreCase = true)) {
+ report(
+ MENTIONS_GOOGLE, item,
+ "Must never reference Google (`$name`)"
+ )
+ }
+ }
+
+ checkName(cls.simpleName(), cls)
+ for (method in methods) {
+ checkName(method.name(), method)
+ }
+ for (field in fields) {
+ checkName(field.name(), field)
+ }
+ }
+
+ private fun checkBitSet(type: TypeItem, typeString: String, item: Item) {
+ if (typeString.startsWith("java.util.BitSet") &&
+ type.asClass()?.qualifiedName() == "java.util.BitSet"
+ ) {
+ report(
+ HEAVY_BIT_SET, item,
+ "Type must not be heavy BitSet (${item.describe()})"
+ )
+ }
+ }
+
+ private fun checkManager(cls: ClassItem, methods: Sequence<MethodItem>, constructors: Sequence<ConstructorItem>) {
+ /*
+ def verify_manager(clazz):
+ """Verifies that FooManager is only obtained from Context."""
+
+ if not clazz.name.endswith("Manager"): return
+
+ for c in clazz.ctors:
+ error(clazz, c, None, "Managers must always be obtained from Context; no direct constructors")
+
+ for m in clazz.methods:
+ if m.typ == clazz.fullname:
+ error(clazz, m, None, "Managers must always be obtained from Context")
+
+ */
+ if (!cls.simpleName().endsWith("Manager")) {
+ return
+ }
+ for (method in constructors) {
+ method.modifiers.isPublic()
+ method.modifiers.isPrivate()
+ report(
+ MANAGER_CONSTRUCTOR, method,
+ "Managers must always be obtained from Context; no direct constructors"
+ )
+ }
+ for (method in methods) {
+ if (method.returnType()?.asClass() == cls) {
+ report(
+ MANAGER_LOOKUP, method,
+ "Managers must always be obtained from Context (`${method.name()}`)"
+ )
+ }
+ }
+ }
+
+ private fun checkBoxed(type: TypeItem, item: Item) {
+ /*
+ def verify_boxed(clazz):
+ """Verifies that methods avoid boxed primitives."""
+
+ boxed = ["java.lang.Number","java.lang.Byte","java.lang.Double","java.lang.Float","java.lang.Integer","java.lang.Long","java.lang.Short"]
+
+ for c in clazz.ctors:
+ for arg in c.args:
+ if arg in boxed:
+ error(clazz, c, "M11", "Must avoid boxed primitives")
+
+ for f in clazz.fields:
+ if f.typ in boxed:
+ error(clazz, f, "M11", "Must avoid boxed primitives")
+
+ for m in clazz.methods:
+ if m.typ in boxed:
+ error(clazz, m, "M11", "Must avoid boxed primitives")
+ for arg in m.args:
+ if arg in boxed:
+ error(clazz, m, "M11", "Must avoid boxed primitives")
+ */
+
+ fun isBoxType(qualifiedName: String): Boolean {
+ return when (qualifiedName) {
+ "java.lang.Number",
+ "java.lang.Byte",
+ "java.lang.Double",
+ "java.lang.Float",
+ "java.lang.Integer",
+ "java.lang.Long",
+ "java.lang.Short" ->
+ true
+ else ->
+ false
+ }
+ }
+
+ val qualifiedName = type.asClass()?.qualifiedName() ?: return
+ if (isBoxType(qualifiedName)) {
+ report(
+ AUTO_BOXING, item,
+ "Must avoid boxed primitives (`$qualifiedName`)"
+ )
+ }
+ }
+
+ private fun checkStaticUtils(
+ cls: ClassItem,
+ methods: Sequence<MethodItem>,
+ constructors: Sequence<ConstructorItem>,
+ fields: Sequence<FieldItem>
+ ) {
+ /*
+ def verify_static_utils(clazz):
+ """Verifies that helper classes can't be constructed."""
+ if clazz.fullname.startswith("android.opengl"): return
+ if clazz.fullname.startswith("android.R"): return
+
+ # Only care about classes with default constructors
+ if len(clazz.ctors) == 1 and len(clazz.ctors[0].args) == 0:
+ test = []
+ test.extend(clazz.fields)
+ test.extend(clazz.methods)
+
+ if len(test) == 0: return
+ for t in test:
+ if "static" not in t.split:
+ return
+
+ error(clazz, None, None, "Fully-static utility classes must not have constructor")
+ */
+ if (!cls.isClass()) {
+ return
+ }
+
+ val hasDefaultConstructor = cls.hasImplicitDefaultConstructor() || run {
+ if (constructors.count() == 1) {
+ val constructor = constructors.first()
+ constructor.parameters().isEmpty() && constructor.modifiers.isPublic()
+ } else {
+ false
+ }
+ }
+
+ if (hasDefaultConstructor) {
+ val qualifiedName = cls.qualifiedName()
+ if (qualifiedName.startsWith("android.opengl.") ||
+ qualifiedName.startsWith("android.R.") ||
+ qualifiedName == "android.R"
+ ) {
+ return
+ }
+
+ if (methods.none() && fields.none()) {
+ return
+ }
+
+ if (methods.none { !it.modifiers.isStatic() } &&
+ fields.none { !it.modifiers.isStatic() }) {
+ report(
+ STATIC_UTILS, cls,
+ "Fully-static utility classes must not have constructor"
+ )
+ }
+ }
+ }
+
+ private fun checkOverloadArgs(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_overload_args(clazz):
+ """Verifies that method overloads add new arguments at the end."""
+ if clazz.fullname.startswith("android.opengl"): return
+
+ overloads = collections.defaultdict(list)
+ for m in clazz.methods:
+ if "deprecated" in m.split: continue
+ overloads[m.name].append(m)
+
+ for name, methods in overloads.items():
+ if len(methods) <= 1: continue
+
+ # Look for arguments common across all overloads
+ def cluster(args):
+ count = collections.defaultdict(int)
+ res = set()
+ for i in range(len(args)):
+ a = args[i]
+ res.add("%s#%d" % (a, count[a]))
+ count[a] += 1
+ return res
+
+ common_args = cluster(methods[0].args)
+ for m in methods:
+ common_args = common_args & cluster(m.args)
+
+ if len(common_args) == 0: continue
+
+ # Require that all common arguments are present at start of signature
+ locked_sig = None
+ for m in methods:
+ sig = m.args[0:len(common_args)]
+ if not common_args.issubset(cluster(sig)):
+ warn(clazz, m, "M2", "Expected common arguments [%s] at beginning of overloaded method" % (", ".join(common_args)))
+ elif not locked_sig:
+ locked_sig = sig
+ elif locked_sig != sig:
+ error(clazz, m, "M2", "Expected consistent argument ordering between overloads: %s..." % (", ".join(locked_sig)))
+ */
+
+ if (cls.qualifiedName().startsWith("android.opengl")) {
+ return
+ }
+
+ val overloads = mutableMapOf<String, MutableList<MethodItem>>()
+ for (method in methods) {
+ if (!method.deprecated) {
+ val name = method.name()
+ val list = overloads[name] ?: run {
+ val new = mutableListOf<MethodItem>()
+ overloads[name] = new
+ new
+ }
+ list.add(method)
+ }
+ }
+
+ // Look for arguments common across all overloads
+ fun cluster(args: List<ParameterItem>): MutableSet<String> {
+ val count = mutableMapOf<String, Int>()
+ val res = mutableSetOf<String>()
+ for (parameter in args) {
+ val a = parameter.type().toTypeString()
+ val currCount = count[a] ?: 1
+ res.add("$a#$currCount")
+ count[a] = currCount + 1
+ }
+ return res
+ }
+
+ for ((_, methodList) in overloads.entries) {
+ if (methodList.size <= 1) {
+ continue
+ }
+
+ val commonArgs = cluster(methodList[0].parameters())
+ for (m in methodList) {
+ val clustered = cluster(m.parameters())
+ commonArgs.removeAll(clustered)
+ }
+ if (commonArgs.isEmpty()) {
+ continue
+ }
+
+ // Require that all common arguments are present at the start of the signature
+ var lockedSig: List<ParameterItem>? = null
+ val commonArgCount = commonArgs.size
+ for (m in methodList) {
+ val sig = m.parameters().subList(0, commonArgCount)
+ val cluster = cluster(sig)
+ if (!cluster.containsAll(commonArgs)) {
+ report(
+ COMMON_ARGS_FIRST, m,
+ "Expected common arguments ${commonArgs.joinToString()}} at beginning of overloaded method ${m.describe()}"
+ )
+ } else if (lockedSig == null) {
+ lockedSig = sig
+ } else if (lockedSig != sig) {
+ report(
+ CONSISTENT_ARGUMENT_ORDER, m,
+ "Expected consistent argument ordering between overloads: ${lockedSig.joinToString()}}"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkCallbackHandlers(
+ cls: ClassItem,
+ methodsAndConstructors: Sequence<MethodItem>,
+ superClass: ClassItem?
+ ) {
+ /*
+ def verify_callback_handlers(clazz):
+ """Verifies that methods adding listener/callback have overload
+ for specifying delivery thread."""
+
+ # Ignore UI packages which assume main thread
+ skip = [
+ "animation",
+ "view",
+ "graphics",
+ "transition",
+ "widget",
+ "webkit",
+ ]
+ for s in skip:
+ if s in clazz.pkg.name_path: return
+ if s in clazz.extends_path: return
+
+ # Ignore UI classes which assume main thread
+ if "app" in clazz.pkg.name_path or "app" in clazz.extends_path:
+ for s in ["ActionBar","Dialog","Application","Activity","Fragment","Loader"]:
+ if s in clazz.fullname: return
+ if "content" in clazz.pkg.name_path or "content" in clazz.extends_path:
+ for s in ["Loader"]:
+ if s in clazz.fullname: return
+
+ found = {}
+ by_name = collections.defaultdict(list)
+ examine = clazz.ctors + clazz.methods
+ for m in examine:
+ if m.name.startswith("unregister"): continue
+ if m.name.startswith("remove"): continue
+ if re.match("on[A-Z]+", m.name): continue
+
+ by_name[m.name].append(m)
+
+ for a in m.args:
+ if a.endswith("Listener") or a.endswith("Callback") or a.endswith("Callbacks"):
+ found[m.name] = m
+
+ for f in found.values():
+ takes_handler = False
+ takes_exec = False
+ for m in by_name[f.name]:
+ if "android.os.Handler" in m.args:
+ takes_handler = True
+ if "java.util.concurrent.Executor" in m.args:
+ takes_exec = True
+ if not takes_exec:
+ warn(clazz, f, "L1", "Registration methods should have overload that accepts delivery Executor")
+
+ */
+
+ // Note: In the above we compute takes_handler but it's not used; is this an incomplete
+ // check?
+
+ fun packageContainsSegment(packageName: String?, segment: String): Boolean {
+ packageName ?: return false
+ return (packageName.contains(segment) &&
+ (packageName.contains(".$segment.") || packageName.endsWith(".$segment")))
+ }
+
+ fun skipPackage(packageName: String?): Boolean {
+ packageName ?: return false
+ for (segment in uiPackageParts) {
+ if (packageContainsSegment(packageName, segment)) {
+ return true
+ }
+ }
+
+ return false
+ }
+
+ // Ignore UI packages which assume main thread
+ val classPackage = cls.containingPackage().qualifiedName()
+ val extendsPackage = superClass?.containingPackage()?.qualifiedName()
+
+ if (skipPackage(classPackage) || skipPackage(extendsPackage)) {
+ return
+ }
+
+ // Ignore UI classes which assume main thread
+ if (packageContainsSegment(classPackage, "app") ||
+ packageContainsSegment(extendsPackage, "app")
+ ) {
+ val fullName = cls.fullName()
+ if (fullName.contains("ActionBar") ||
+ fullName.contains("Dialog") ||
+ fullName.contains("Application") ||
+ fullName.contains("Activity") ||
+ fullName.contains("Fragment") ||
+ fullName.contains("Loader")
+ ) {
+ return
+ }
+ }
+ if (packageContainsSegment(classPackage, "content") ||
+ packageContainsSegment(extendsPackage, "content")
+ ) {
+ val fullName = cls.fullName()
+ if (fullName.contains("Loader")) {
+ return
+ }
+ }
+
+ val found = mutableMapOf<String, MethodItem>()
+ val byName = mutableMapOf<String, MutableList<MethodItem>>()
+ for (method in methodsAndConstructors) {
+ val name = method.name()
+ if (name.startsWith("unregister")) {
+ continue
+ }
+ if (name.startsWith("remove")) {
+ continue
+ }
+ if (name.startsWith("on") && onCallbackNamePattern.matches(name)) {
+ continue
+ }
+
+ val list = byName[name] ?: run {
+ val new = mutableListOf<MethodItem>()
+ byName[name] = new
+ new
+ }
+ list.add(method)
+
+ for (parameter in method.parameters()) {
+ val type = parameter.type().toTypeString()
+ if (type.endsWith("Listener") ||
+ type.endsWith("Callback") ||
+ type.endsWith("Callbacks")
+ ) {
+ found[name] = method
+ }
+ }
+ }
+
+ for (f in found.values) {
+ var takesExec = false
+
+ // TODO: apilint computed takes_handler but did not use it; should we add more checks or conditions?
+ // var takesHandler = false
+
+ val name = f.name()
+ for (method in byName[name]!!) {
+ // if (method.parameters().any { it.type().toTypeString() == "android.os.Handler" }) {
+ // takesHandler = true
+ // }
+ if (method.parameters().any { it.type().toTypeString() == "java.util.concurrent.Executor" }) {
+ takesExec = true
+ }
+ }
+ if (!takesExec) {
+ report(
+ EXECUTOR_REGISTRATION, f,
+ "Registration methods should have overload that accepts delivery Executor: `$name`"
+ )
+ }
+ }
+ }
+
+ private fun checkContextFirst(method: MethodItem) {
+ /*
+ def verify_context_first(clazz):
+ """Verifies that methods accepting a Context keep it the first argument."""
+ examine = clazz.ctors + clazz.methods
+ for m in examine:
+ if len(m.args) > 1 and m.args[0] != "android.content.Context":
+ if "android.content.Context" in m.args[1:]:
+ error(clazz, m, "M3", "Context is distinct, so it must be the first argument")
+ if len(m.args) > 1 and m.args[0] != "android.content.ContentResolver":
+ if "android.content.ContentResolver" in m.args[1:]:
+ error(clazz, m, "M3", "ContentResolver is distinct, so it must be the first argument")
+ */
+ val parameters = method.parameters()
+ if (parameters.size > 1 && parameters[0].type().toTypeString() != "android.content.Context") {
+ for (i in 1 until parameters.size) {
+ val p = parameters[i]
+ if (p.type().toTypeString() == "android.content.Context") {
+ report(
+ CONTEXT_FIRST, p,
+ "Context is distinct, so it must be the first argument (method `${method.name()}`)"
+ )
+ }
+ }
+ }
+ if (parameters.size > 1 && parameters[0].type().toTypeString() != "android.content.ContentResolver") {
+ for (i in 1 until parameters.size) {
+ val p = parameters[i]
+ if (p.type().toTypeString() == "android.content.ContentResolver") {
+ report(
+ CONTEXT_FIRST, p,
+ "ContentResolver is distinct, so it must be the first argument (method `${method.name()}`)"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkListenerLast(method: MethodItem) {
+ /*
+ def verify_listener_last(clazz):
+ """Verifies that methods accepting a Listener or Callback keep them as last arguments."""
+ examine = clazz.ctors + clazz.methods
+ for m in examine:
+ if "Listener" in m.name or "Callback" in m.name: continue
+ found = False
+ for a in m.args:
+ if a.endswith("Callback") or a.endswith("Callbacks") or a.endswith("Listener"):
+ found = True
+ elif found:
+ warn(clazz, m, "M3", "Listeners should always be at end of argument list")
+ */
+
+ val name = method.name()
+ if (name.contains("Listener") || name.contains("Callback")) {
+ return
+ }
+
+ val parameters = method.parameters()
+ if (parameters.size > 1) {
+ var found = false
+ for (parameter in parameters) {
+ val type = parameter.type().toTypeString()
+ if (type.endsWith("Callback") || type.endsWith("Callbacks") || type.endsWith("Listener")) {
+ found = true
+ } else if (found) {
+ report(
+ LISTENER_LAST, parameter,
+ "Listeners should always be at end of argument list (method `${method.name()}`)"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkResourceNames(cls: ClassItem, fields: Sequence<FieldItem>) {
+ /*
+ def verify_resource_names(clazz):
+ """Verifies that resource names have consistent case."""
+ if not re.match("android\.R\.[a-z]+", clazz.fullname): return
+
+ # Resources defined by files are foo_bar_baz
+ if clazz.name in ["anim","animator","color","dimen","drawable","interpolator","layout","transition","menu","mipmap","string","plurals","raw","xml"]:
+ for f in clazz.fields:
+ if re.match("config_[a-z][a-zA-Z1-9]*$", f.name): continue
+ if f.name.startswith("config_"):
+ error(clazz, f, None, "Expected config name to be config_fooBarBaz style")
+
+ if re.match("[a-z1-9_]+$", f.name): continue
+ error(clazz, f, None, "Expected resource name in this class to be foo_bar_baz style")
+
+ # Resources defined inside files are fooBarBaz
+ if clazz.name in ["array","attr","id","bool","fraction","integer"]:
+ for f in clazz.fields:
+ if re.match("config_[a-z][a-zA-Z1-9]*$", f.name): continue
+ if re.match("layout_[a-z][a-zA-Z1-9]*$", f.name): continue
+ if re.match("state_[a-z_]*$", f.name): continue
+
+ if re.match("[a-z][a-zA-Z1-9]*$", f.name): continue
+ error(clazz, f, "C7", "Expected resource name in this class to be fooBarBaz style")
+
+ # Styles are FooBar_Baz
+ if clazz.name in ["style"]:
+ for f in clazz.fields:
+ if re.match("[A-Z][A-Za-z1-9]+(_[A-Z][A-Za-z1-9]+?)*$", f.name): continue
+ error(clazz, f, "C7", "Expected resource name in this class to be FooBar_Baz style")
+ */
+ if (!cls.qualifiedName().startsWith("android.R.")) {
+ return
+ }
+
+ val resourceType = ResourceType.fromClassName(cls.simpleName()) ?: return
+ when (resourceType) {
+ ANIM,
+ ANIMATOR,
+ COLOR,
+ DIMEN,
+ DRAWABLE,
+ FONT,
+ INTERPOLATOR,
+ LAYOUT,
+ MENU,
+ MIPMAP,
+ NAVIGATION,
+ PLURALS,
+ RAW,
+ STRING,
+ TRANSITION,
+ XML -> {
+ // Resources defined by files are foo_bar_baz
+ // Note: it's surprising that dimen, plurals and string are in this list since
+ // they are value resources, not file resources, but keeping api lint compatibility
+ // for now.
+
+ for (field in fields) {
+ val name = field.name()
+ if (name.startsWith("config_")) {
+ if (!configFieldPattern.matches(name)) {
+ report(
+ CONFIG_FIELD_NAME, field,
+ "Expected config name to be in the `config_fooBarBaz` style, was `$name`"
+ )
+ }
+ continue
+ }
+ if (!resourceFileFieldPattern.matches(name)) {
+ report(
+ RESOURCE_FIELD_NAME, field,
+ "Expected resource name in `${cls.qualifiedName()}` to be in the `foo_bar_baz` style, was `$name`"
+ )
+ }
+ }
+ }
+
+ ARRAY,
+ ATTR,
+ BOOL,
+ FRACTION,
+ ID,
+ INTEGER -> {
+ // Resources defined inside files are fooBarBaz
+ for (field in fields) {
+ val name = field.name()
+ if (name.startsWith("config_") && configFieldPattern.matches(name)) {
+ continue
+ }
+ if (name.startsWith("layout_") && layoutFieldPattern.matches(name)) {
+ continue
+ }
+ if (name.startsWith("state_") && stateFieldPattern.matches(name)) {
+ continue
+ }
+ if (resourceValueFieldPattern.matches(name)) {
+ continue
+ }
+ report(
+ RESOURCE_VALUE_FIELD_NAME, field,
+ "Expected resource name in `${cls.qualifiedName()}` to be in the `fooBarBaz` style, was `$name`"
+ )
+ }
+ }
+
+ STYLE -> {
+ for (field in fields) {
+ val name = field.name()
+ if (!styleFieldPattern.matches(name)) {
+ report(
+ RESOURCE_STYLE_FIELD_NAME, field,
+ "Expected resource name in `${cls.qualifiedName()}` to be in the `FooBar_Baz` style, was `$name`"
+ )
+ }
+ }
+ }
+
+ STYLEABLE, // appears as R class but name check is implicitly done as part of style class check
+ // DECLARE_STYLEABLE,
+ STYLE_ITEM,
+ PUBLIC,
+ SAMPLE_DATA,
+ AAPT -> {
+ // no-op; these are resource "types" in XML but not present as R classes
+ // Listed here explicitly to force compiler error as new resource types
+ // are added.
+ }
+ }
+ }
+
+ private fun checkFiles(methodsAndConstructors: Sequence<MethodItem>) {
+ /*
+ def verify_files(clazz):
+ """Verifies that methods accepting File also accept streams."""
+
+ has_file = set()
+ has_stream = set()
+
+ test = []
+ test.extend(clazz.ctors)
+ test.extend(clazz.methods)
+
+ for m in test:
+ if "java.io.File" in m.args:
+ has_file.add(m)
+ if "java.io.FileDescriptor" in m.args or "android.os.ParcelFileDescriptor" in m.args or "java.io.InputStream" in m.args or "java.io.OutputStream" in m.args:
+ has_stream.add(m.name)
+
+ for m in has_file:
+ if m.name not in has_stream:
+ warn(clazz, m, "M10", "Methods accepting File should also accept FileDescriptor or streams")
+ */
+
+ var hasFile: MutableSet<MethodItem>? = null
+ var hasStream: MutableSet<String>? = null
+ for (method in methodsAndConstructors) {
+ for (parameter in method.parameters()) {
+ val type = parameter.type().toTypeString()
+ when (type) {
+ "java.io.File" -> {
+ val set = hasFile ?: run {
+ val new = mutableSetOf<MethodItem>()
+ hasFile = new
+ new
+ }
+ set.add(method)
+ }
+ "java.io.FileDescriptor",
+ "android.os.ParcelFileDescriptor",
+ "java.io.InputStream",
+ "java.io.OutputStream" -> {
+ val set = hasStream ?: run {
+ val new = mutableSetOf<String>()
+ hasStream = new
+ new
+ }
+ set.add(method.name())
+ }
+ }
+ }
+ }
+ val files = hasFile
+ if (files != null) {
+ val streams = hasStream
+ for (method in files) {
+ if (streams == null || !streams.contains(method.name())) {
+ report(
+ STREAM_FILES, method,
+ "Methods accepting `File` should also accept `FileDescriptor` or streams: ${method.describe()}"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkManagerList(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_manager_list(clazz):
+ """Verifies that managers return List<? extends Parcelable> instead of arrays."""
+
+ if not clazz.name.endswith("Manager"): return
+
+ for m in clazz.methods:
+ if m.typ.startswith("android.") and m.typ.endswith("[]"):
+ warn(clazz, m, None, "Methods should return List<? extends Parcelable> instead of Parcelable[] to support ParceledListSlice under the hood")
+ */
+ if (!cls.simpleName().endsWith("Manager")) {
+ return
+ }
+ for (method in methods) {
+ val returnType = method.returnType() ?: continue
+ if (returnType.primitive) {
+ return
+ }
+ val type = returnType.toTypeString()
+ if (type.startsWith("android.") && returnType.isArray()) {
+ report(
+ PARCELABLE_LIST, method,
+ "Methods should return `List<? extends Parcelable>` instead of `Parcelable[]` to support `ParceledListSlice` under the hood: ${method.describe()}"
+ )
+ }
+ }
+ }
+
+ private fun checkAbstractInner(cls: ClassItem) {
+ /*
+ def verify_abstract_inner(clazz):
+ """Verifies that abstract inner classes are static."""
+
+ if re.match(".+?\.[A-Z][^\.]+\.[A-Z]", clazz.fullname):
+ if " abstract " in clazz.raw and " static " not in clazz.raw:
+ warn(clazz, None, None, "Abstract inner classes should be static to improve testability")
+ */
+ if (!cls.isTopLevelClass() && cls.isClass() && cls.modifiers.isAbstract() && !cls.modifiers.isStatic()) {
+ report(
+ ABSTRACT_INNER, cls,
+ "Abstract inner classes should be static to improve testability: ${cls.describe()}"
+ )
+ }
+ }
+
+ private fun checkRuntimeExceptions(
+ methodsAndConstructors: Sequence<MethodItem>,
+ filterReference: Predicate<Item>
+ ) {
+ /*
+ def verify_runtime_exceptions(clazz):
+ """Verifies that runtime exceptions aren't listed in throws."""
+
+ banned = [
+ "java.lang.NullPointerException",
+ "java.lang.ClassCastException",
+ "java.lang.IndexOutOfBoundsException",
+ "java.lang.reflect.UndeclaredThrowableException",
+ "java.lang.reflect.MalformedParametersException",
+ "java.lang.reflect.MalformedParameterizedTypeException",
+ "java.lang.invoke.WrongMethodTypeException",
+ "java.lang.EnumConstantNotPresentException",
+ "java.lang.IllegalMonitorStateException",
+ "java.lang.SecurityException",
+ "java.lang.UnsupportedOperationException",
+ "java.lang.annotation.AnnotationTypeMismatchException",
+ "java.lang.annotation.IncompleteAnnotationException",
+ "java.lang.TypeNotPresentException",
+ "java.lang.IllegalStateException",
+ "java.lang.ArithmeticException",
+ "java.lang.IllegalArgumentException",
+ "java.lang.ArrayStoreException",
+ "java.lang.NegativeArraySizeException",
+ "java.util.MissingResourceException",
+ "java.util.EmptyStackException",
+ "java.util.concurrent.CompletionException",
+ "java.util.concurrent.RejectedExecutionException",
+ "java.util.IllformedLocaleException",
+ "java.util.ConcurrentModificationException",
+ "java.util.NoSuchElementException",
+ "java.io.UncheckedIOException",
+ "java.time.DateTimeException",
+ "java.security.ProviderException",
+ "java.nio.BufferUnderflowException",
+ "java.nio.BufferOverflowException",
+ ]
+
+ examine = clazz.ctors + clazz.methods
+ for m in examine:
+ for t in m.throws:
+ if t in banned:
+ error(clazz, m, None, "Methods must not mention RuntimeException subclasses in throws clauses")
+
+ */
+ for (method in methodsAndConstructors) {
+ for (throws in method.filteredThrowsTypes(filterReference)) {
+ when (throws.qualifiedName()) {
+ "java.lang.NullPointerException",
+ "java.lang.ClassCastException",
+ "java.lang.IndexOutOfBoundsException",
+ "java.lang.reflect.UndeclaredThrowableException",
+ "java.lang.reflect.MalformedParametersException",
+ "java.lang.reflect.MalformedParameterizedTypeException",
+ "java.lang.invoke.WrongMethodTypeException",
+ "java.lang.EnumConstantNotPresentException",
+ "java.lang.IllegalMonitorStateException",
+ "java.lang.SecurityException",
+ "java.lang.UnsupportedOperationException",
+ "java.lang.annotation.AnnotationTypeMismatchException",
+ "java.lang.annotation.IncompleteAnnotationException",
+ "java.lang.TypeNotPresentException",
+ "java.lang.IllegalStateException",
+ "java.lang.ArithmeticException",
+ "java.lang.IllegalArgumentException",
+ "java.lang.ArrayStoreException",
+ "java.lang.NegativeArraySizeException",
+ "java.util.MissingResourceException",
+ "java.util.EmptyStackException",
+ "java.util.concurrent.CompletionException",
+ "java.util.concurrent.RejectedExecutionException",
+ "java.util.IllformedLocaleException",
+ "java.util.ConcurrentModificationException",
+ "java.util.NoSuchElementException",
+ "java.io.UncheckedIOException",
+ "java.time.DateTimeException",
+ "java.security.ProviderException",
+ "java.nio.BufferUnderflowException",
+ "java.nio.BufferOverflowException" -> {
+ report(
+ BANNED_THROW, method,
+ "Methods must not mention RuntimeException subclasses in throws clauses (was `${throws.qualifiedName()}`)"
+ )
+ }
+ }
+ }
+ }
+ }
+
+ private fun checkError(cls: ClassItem, superClass: ClassItem?) {
+ /*
+ def verify_error(clazz):
+ """Verifies that we always use Exception instead of Error."""
+ if not clazz.extends: return
+ if clazz.extends.endswith("Error"):
+ error(clazz, None, None, "Trouble must be reported through an Exception, not Error")
+ if clazz.extends.endswith("Exception") and not clazz.name.endswith("Exception"):
+ error(clazz, None, None, "Exceptions must be named FooException")
+ */
+ superClass ?: return
+ if (superClass.simpleName().endsWith("Error")) {
+ report(
+ EXTENDS_ERROR, cls,
+ "Trouble must be reported through an `Exception`, not an `Error` (`${cls.simpleName()}` extends `${superClass.simpleName()}`)"
+ )
+ }
+ if (superClass.simpleName().endsWith("Exception") && !cls.simpleName().endsWith("Exception")) {
+ report(
+ EXCEPTION_NAME, cls,
+ "Exceptions must be named `FooException`, was `${cls.simpleName()}`"
+ )
+ }
+ }
+
+ private fun checkUnits(method: MethodItem) {
+ /*
+ def verify_units(clazz):
+ """Verifies that we use consistent naming for units."""
+
+ # If we find K, recommend replacing with V
+ bad = {
+ "Ns": "Nanos",
+ "Ms": "Millis or Micros",
+ "Sec": "Seconds", "Secs": "Seconds",
+ "Hr": "Hours", "Hrs": "Hours",
+ "Mo": "Months", "Mos": "Months",
+ "Yr": "Years", "Yrs": "Years",
+ "Byte": "Bytes", "Space": "Bytes",
+ }
+
+ for m in clazz.methods:
+ if m.typ not in ["short","int","long"]: continue
+ for k, v in bad.iteritems():
+ if m.name.endswith(k):
+ error(clazz, m, None, "Expected method name units to be " + v)
+ if m.name.endswith("Nanos") or m.name.endswith("Micros"):
+ warn(clazz, m, None, "Returned time values are strongly encouraged to be in milliseconds unless you need the extra precision")
+ if m.name.endswith("Seconds"):
+ error(clazz, m, None, "Returned time values must be in milliseconds")
+
+ for m in clazz.methods:
+ typ = m.typ
+ if typ == "void":
+ if len(m.args) != 1: continue
+ typ = m.args[0]
+
+ if m.name.endswith("Fraction") and typ != "float":
+ error(clazz, m, None, "Fractions must use floats")
+ if m.name.endswith("Percentage") and typ != "int":
+ error(clazz, m, None, "Percentage must use ints")
+
+ */
+ val returnType = method.returnType() ?: return
+ var type = returnType.toTypeString()
+ val name = method.name()
+ if (type == "int" || type == "long" || type == "short") {
+ if (badUnits.any { name.endsWith(it.key) }) {
+ val badUnit = badUnits.keys.find { name.endsWith(it) }
+ val value = badUnits[badUnit]
+ report(
+ METHOD_NAME_UNITS, method,
+ "Expected method name units to be `$value`, was `$badUnit` in `$name`"
+ )
+ } else if (name.endsWith("Nanos") || name.endsWith("Micros")) {
+ report(
+ METHOD_NAME_UNITS, method,
+ "Returned time values are strongly encouraged to be in milliseconds unless you need the extra precision, was `$name`"
+ )
+ } else if (name.endsWith("Seconds")) {
+ report(
+ METHOD_NAME_UNITS, method,
+ "Returned time values must be in milliseconds, was `$name`"
+ )
+ }
+ } else if (type == "void") {
+ if (method.parameters().size != 1) {
+ return
+ }
+ type = method.parameters()[0].type().toTypeString()
+ }
+ if (name.endsWith("Fraction") && type != "float") {
+ report(
+ FRACTION_FLOAT, method,
+ "Fractions must use floats, was `$type` in `$name`"
+ )
+ } else if (name.endsWith("Percentage") && type != "int") {
+ report(
+ PERCENTAGE_INT, method,
+ "Percentage must use ints, was `$type` in `$name`"
+ )
+ }
+ }
+
+ private fun checkCloseable(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_closable(clazz):
+ """Verifies that classes are AutoClosable."""
+ if "implements java.lang.AutoCloseable" in clazz.raw: return
+ if "implements java.io.Closeable" in clazz.raw: return
+
+ for m in clazz.methods:
+ if len(m.args) > 0: continue
+ if m.name in ["close","release","destroy","finish","finalize","disconnect","shutdown","stop","free","quit"]:
+ warn(clazz, m, None, "Classes that release resources should implement AutoClosable and CloseGuard")
+ return
+ */
+ var requireCloseable = false
+ loop@ for (method in methods) {
+ val name = method.name()
+ when (name) {
+ "close", "release", "destroy", "finish", "finalize", "disconnect", "shutdown", "stop", "free", "quit" -> {
+ requireCloseable = true
+ break@loop
+ }
+ }
+ }
+ if (requireCloseable && !cls.implements("java.lang.AutoCloseable")) { // includes java.io.Closeable
+ report(
+ NOT_CLOSEABLE, cls,
+ "Classes that release resources should implement AutoClosable and CloseGuard: ${cls.describe()}"
+ )
+ }
+ }
+
+ private fun checkNotKotlinOperator(methods: Sequence<MethodItem>) {
+ /*
+ def verify_method_name_not_kotlin_operator(clazz):
+ """Warn about method names which become operators in Kotlin."""
+
+ binary = set()
+
+ def unique_binary_op(m, op):
+ if op in binary:
+ error(clazz, m, None, "Only one of '{0}' and '{0}Assign' methods should be present for Kotlin".format(op))
+ binary.add(op)
+
+ for m in clazz.methods:
+ if 'static' in m.split:
+ continue
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators
+ if m.name in ["unaryPlus", "unaryMinus", "not"] and len(m.args) == 0:
+ warn(clazz, m, None, "Method can be invoked as a unary operator from Kotlin")
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#increments-and-decrements
+ if m.name in ["inc", "dec"] and len(m.args) == 0 and m.typ != "void":
+ # This only applies if the return type is the same or a subtype of the enclosing class, but we have no
+ # practical way of checking that relationship here.
+ warn(clazz, m, None, "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin")
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#arithmetic
+ if m.name in ["plus", "minus", "times", "div", "rem", "mod", "rangeTo"] and len(m.args) == 1:
+ warn(clazz, m, None, "Method can be invoked as a binary operator from Kotlin")
+ unique_binary_op(m, m.name)
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#in
+ if m.name == "contains" and len(m.args) == 1 and m.typ == "boolean":
+ warn(clazz, m, None, "Method can be invoked as a "in" operator from Kotlin")
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#indexed
+ if (m.name == "get" and len(m.args) > 0) or (m.name == "set" and len(m.args) > 1):
+ warn(clazz, m, None, "Method can be invoked with an indexing operator from Kotlin")
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#invoke
+ if m.name == "invoke":
+ warn(clazz, m, None, "Method can be invoked with function call syntax from Kotlin")
+
+ # https://kotlinlang.org/docs/reference/operator-overloading.html#assignments
+ if m.name in ["plusAssign", "minusAssign", "timesAssign", "divAssign", "remAssign", "modAssign"] \
+ and len(m.args) == 1 \
+ and m.typ == "void":
+ warn(clazz, m, None, "Method can be invoked as a compound assignment operator from Kotlin")
+ unique_binary_op(m, m.name[:-6]) # Remove "Assign" suffix
+
+ */
+
+ for (method in methods) {
+ if (method.modifiers.isStatic()) {
+ continue
+ }
+ val name = method.name()
+ when (name) {
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#unary-prefix-operators
+ "unaryPlus", "unaryMinus", "not" -> {
+ if (method.parameters().isEmpty()) {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked as a unary operator from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#increments-and-decrements
+ "inc", "dec" -> {
+ if (method.parameters().isEmpty() && method.returnType()?.toTypeString() != "void") {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked as a pre/postfix inc/decrement operator from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#arithmetic
+ "plus", "minus", "times", "div", "rem", "mod", "rangeTo" -> {
+ if (method.parameters().size == 1) {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked as a binary operator from Kotlin: `$name`"
+ )
+ }
+ val assignName = name + "Assign"
+
+ if (methods.any {
+ it.name() == assignName &&
+ it.parameters().size == 1 &&
+ it.returnType()?.toTypeString() == "void"
+ }) {
+ report(
+ UNIQUE_KOTLIN_OPERATOR, method,
+ "Only one of `$name` and `${name}Assign` methods should be present for Kotlin"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#in
+ "contains" -> {
+ if (method.parameters().size == 1 && method.returnType()?.toTypeString() == "boolean") {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked as a \"in\" operator from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#indexed
+ "get" -> {
+ if (method.parameters().isNotEmpty()) {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked with an indexing operator from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#indexed
+ "set" -> {
+ if (method.parameters().size > 1) {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked with an indexing operator from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#invoke
+ "invoke" -> {
+ if (method.parameters().size > 1) {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked with function call syntax from Kotlin: `$name`"
+ )
+ }
+ }
+ // https://kotlinlang.org/docs/reference/operator-overloading.html#assignments
+ "plusAssign", "minusAssign", "timesAssign", "divAssign", "remAssign", "modAssign" -> {
+ if (method.parameters().size == 1 && method.returnType()?.toTypeString() == "void") {
+ report(
+ KOTLIN_OPERATOR, method,
+ "Method can be invoked as a compound assignment operator from Kotlin: `$name`"
+ )
+ }
+ }
+ }
+ }
+ }
+
+ private fun checkCollectionsOverArrays(type: TypeItem, typeString: String, item: Item) {
+ /*
+ def verify_collections_over_arrays(clazz):
+ """Warn that [] should be Collections."""
+
+ safe = ["java.lang.String[]","byte[]","short[]","int[]","long[]","float[]","double[]","boolean[]","char[]"]
+ for m in clazz.methods:
+ if m.typ.endswith("[]") and m.typ not in safe:
+ warn(clazz, m, None, "Method should return Collection<> (or subclass) instead of raw array")
+ for arg in m.args:
+ if arg.endswith("[]") and arg not in safe:
+ warn(clazz, m, None, "Method argument should be Collection<> (or subclass) instead of raw array")
+
+ */
+
+ if (!type.isArray() || typeString.endsWith("...")) {
+ return
+ }
+
+ when (typeString) {
+ "java.lang.String[]",
+ "byte[]",
+ "short[]",
+ "int[]",
+ "long[]",
+ "float[]",
+ "double[]",
+ "boolean[]",
+ "char[]" -> {
+ return
+ }
+ else -> {
+ val action = when (item) {
+ is MethodItem -> {
+ if (item.name() == "values" && item.containingClass().isEnum()) {
+ return
+ }
+ "Method should return"
+ }
+ is FieldItem -> "Field should be"
+ else -> "Method parameter should be"
+ }
+ val component = type.asClass()?.simpleName() ?: ""
+ report(
+ ARRAY_RETURN, item,
+ "$action Collection<$component> (or subclass) instead of raw array; was `$typeString`"
+ )
+ }
+ }
+ }
+
+ private fun checkUserHandle(cls: ClassItem, methods: Sequence<MethodItem>) {
+ /*
+ def verify_user_handle(clazz):
+ """Methods taking UserHandle should be ForUser or AsUser."""
+ if clazz.name.endswith("Listener") or clazz.name.endswith("Callback") or clazz.name.endswith("Callbacks"): return
+ if clazz.fullname == "android.app.admin.DeviceAdminReceiver": return
+ if clazz.fullname == "android.content.pm.LauncherApps": return
+ if clazz.fullname == "android.os.UserHandle": return
+ if clazz.fullname == "android.os.UserManager": return
+
+ for m in clazz.methods:
+ if re.match("on[A-Z]+", m.name): continue
+
+ has_arg = "android.os.UserHandle" in m.args
+ has_name = m.name.endswith("AsUser") or m.name.endswith("ForUser")
+
+ if clazz.fullname.endswith("Manager") and has_arg:
+ warn(clazz, m, None, "When a method overload is needed to target a specific "
+ "UserHandle, callers should be directed to use "
+ "Context.createPackageContextAsUser() and re-obtain the relevant "
+ "Manager, and no new API should be added")
+ elif has_arg and not has_name:
+ warn(clazz, m, None, "Method taking UserHandle should be named 'doFooAsUser' "
+ "or 'queryFooForUser'")
+
+ */
+ val qualifiedName = cls.qualifiedName()
+ if (qualifiedName == "android.app.admin.DeviceAdminReceiver" ||
+ qualifiedName == "android.content.pm.LauncherApps" ||
+ qualifiedName == "android.os.UserHandle" ||
+ qualifiedName == "android.os.UserManager"
+ ) {
+ return
+ }
+
+ for (method in methods) {
+ val parameters = method.parameters()
+ if (parameters.isEmpty()) {
+ continue
+ }
+ val name = method.name()
+ if (name.startsWith("on") && onCallbackNamePattern.matches(name)) {
+ continue
+ }
+ val hasArg = parameters.any { it.type().toTypeString() == "android.os.UserHandle" }
+ if (!hasArg) {
+ continue
+ }
+ if (qualifiedName.endsWith("Manager")) {
+ report(
+ USER_HANDLE, method,
+ "When a method overload is needed to target a specific " +
+ "UserHandle, callers should be directed to use " +
+ "Context.createPackageContextAsUser() and re-obtain the relevant " +
+ "Manager, and no new API should be added"
+ )
+ } else if (!(name.endsWith("AsUser") || name.endsWith("ForUser"))) {
+ report(
+ USER_HANDLE_NAME, method,
+ "Method taking UserHandle should be named `doFooAsUser` or `queryFooForUser`, was `$name`"
+ )
+ }
+ }
+ }
+
+ private fun checkParams(cls: ClassItem) {
+ /*
+ def verify_params(clazz):
+ """Parameter classes should be 'Params'."""
+ if clazz.name.endswith("Params"): return
+ if clazz.fullname == "android.app.ActivityOptions": return
+ if clazz.fullname == "android.app.BroadcastOptions": return
+ if clazz.fullname == "android.os.Bundle": return
+ if clazz.fullname == "android.os.BaseBundle": return
+ if clazz.fullname == "android.os.PersistableBundle": return
+
+ bad = ["Param","Parameter","Parameters","Args","Arg","Argument","Arguments","Options","Bundle"]
+ for b in bad:
+ if clazz.name.endswith(b):
+ error(clazz, None, None, "Classes holding a set of parameters should be called 'FooParams'")
+ */
+
+ val qualifiedName = cls.qualifiedName()
+ for (suffix in badParameterClassNames) {
+ if (qualifiedName.endsWith(suffix) && !((qualifiedName.endsWith("Params") ||
+ qualifiedName == "android.app.ActivityOptions" ||
+ qualifiedName == "android.app.BroadcastOptions" ||
+ qualifiedName == "android.os.Bundle" ||
+ qualifiedName == "android.os.BaseBundle" ||
+ qualifiedName == "android.os.PersistableBundle"))
+ ) {
+ report(
+ USER_HANDLE_NAME, cls,
+ "Classes holding a set of parameters should be called `FooParams`, was `${cls.simpleName()}`"
+ )
+ }
+ }
+ }
+
+ private fun checkServices(field: FieldItem) {
+ /*
+ def verify_services(clazz):
+ """Service name should be FOO_BAR_SERVICE = 'foo_bar'."""
+ if clazz.fullname != "android.content.Context": return
+
+ for f in clazz.fields:
+ if f.typ != "java.lang.String": continue
+ found = re.match(r"([A-Z_]+)_SERVICE", f.name)
+ if found:
+ expected = found.group(1).lower()
+ if f.value != expected:
+ error(clazz, f, "C4", "Inconsistent service value; expected '%s'" % (expected))
+ */
+ val type = field.type()
+ if (!type.isString()) {
+ return
+ }
+ val name = field.name()
+ if (name.endsWith("_SERVICE") && serviceFieldPattern.matches(name)) {
+ val value = field.initialValue() as? String
+ if (value != null) {
+ val service = name.substring(0, name.length - "_SERVICE".length)
+ if (!service.equals(value, ignoreCase = true)) {
+ if (field.containingClass().qualifiedName() == "android.content.Context") {
+ return
+ }
+ report(
+ SERVICE_NAME, field,
+ "Inconsistent service value; expected `$service`, was `$value`"
+ )
+ }
+ }
+ }
+ }
+
+ private fun checkTense(method: MethodItem) {
+ /*
+ def verify_tense(clazz):
+ """Verify tenses of method names."""
+ if clazz.fullname.startswith("android.opengl"): return
+
+ for m in clazz.methods:
+ if m.name.endswith("Enable"):
+ warn(clazz, m, None, "Unexpected tense; probably meant 'enabled'")
+ */
+ val name = method.name()
+ if (name.endsWith("Enable")) {
+ if (method.containingClass().qualifiedName().startsWith("android.opengl")) {
+ return
+ }
+ report(
+ METHOD_NAME_TENSE, method,
+ "Unexpected tense; probably meant `enabled`, was `$name`"
+ )
+ }
+ }
+
+ private fun checkIcu(type: TypeItem, typeString: String, item: Item) {
+ /*
+ def verify_icu(clazz):
+ """Verifies that richer ICU replacements are used."""
+ better = {
+ "java.util.TimeZone": "android.icu.util.TimeZone",
+ "java.util.Calendar": "android.icu.util.Calendar",
+ "java.util.Locale": "android.icu.util.ULocale",
+ "java.util.ResourceBundle": "android.icu.util.UResourceBundle",
+ "java.util.SimpleTimeZone": "android.icu.util.SimpleTimeZone",
+ "java.util.StringTokenizer": "android.icu.util.StringTokenizer",
+ "java.util.GregorianCalendar": "android.icu.util.GregorianCalendar",
+ "java.lang.Character": "android.icu.lang.UCharacter",
+ "java.text.BreakIterator": "android.icu.text.BreakIterator",
+ "java.text.Collator": "android.icu.text.Collator",
+ "java.text.DecimalFormatSymbols": "android.icu.text.DecimalFormatSymbols",
+ "java.text.NumberFormat": "android.icu.text.NumberFormat",
+ "java.text.DateFormatSymbols": "android.icu.text.DateFormatSymbols",
+ "java.text.DateFormat": "android.icu.text.DateFormat",
+ "java.text.SimpleDateFormat": "android.icu.text.SimpleDateFormat",
+ "java.text.MessageFormat": "android.icu.text.MessageFormat",
+ "java.text.DecimalFormat": "android.icu.text.DecimalFormat",
+ }
+
+ for m in clazz.ctors + clazz.methods:
+ types = []
+ types.extend(m.typ)
+ types.extend(m.args)
+ for arg in types:
+ if arg in better:
+ warn(clazz, m, None, "Type %s should be replaced with richer ICU type %s" % (arg, better[arg]))
+ */
+ if (type.primitive) {
+ return
+ }
+ val better = when (typeString) {
+ "java.util.TimeZone" -> "android.icu.util.TimeZone"
+ "java.util.Calendar" -> "android.icu.util.Calendar"
+ "java.util.Locale" -> "android.icu.util.ULocale"
+ "java.util.ResourceBundle" -> "android.icu.util.UResourceBundle"
+ "java.util.SimpleTimeZone" -> "android.icu.util.SimpleTimeZone"
+ "java.util.StringTokenizer" -> "android.icu.util.StringTokenizer"
+ "java.util.GregorianCalendar" -> "android.icu.util.GregorianCalendar"
+ "java.lang.Character" -> "android.icu.lang.UCharacter"
+ "java.text.BreakIterator" -> "android.icu.text.BreakIterator"
+ "java.text.Collator" -> "android.icu.text.Collator"
+ "java.text.DecimalFormatSymbols" -> "android.icu.text.DecimalFormatSymbols"
+ "java.text.NumberFormat" -> "android.icu.text.NumberFormat"
+ "java.text.DateFormatSymbols" -> "android.icu.text.DateFormatSymbols"
+ "java.text.DateFormat" -> "android.icu.text.DateFormat"
+ "java.text.SimpleDateFormat" -> "android.icu.text.SimpleDateFormat"
+ "java.text.MessageFormat" -> "android.icu.text.MessageFormat"
+ "java.text.DecimalFormat" -> "android.icu.text.DecimalFormat"
+ else -> return
+ }
+ report(
+ USE_ICU, item,
+ "Type `$typeString` should be replaced with richer ICU type `$better`"
+ )
+ }
+
+ private fun checkClone(method: MethodItem) {
+ /*
+ def verify_clone(clazz):
+ """Verify that clone() isn't implemented; see EJ page 61."""
+ for m in clazz.methods:
+ if m.name == "clone":
+ error(clazz, m, None, "Provide an explicit copy constructor instead of implementing clone()")
+ */
+ if (method.name() == "clone" && method.parameters().isEmpty()) {
+ report(
+ NO_CLONE, method,
+ "Provide an explicit copy constructor instead of implementing `clone()`"
+ )
+ }
+ }
+
+ private fun checkPfd(type: String, item: Item) {
+ /*
+ def verify_pfd(clazz):
+ """Verify that android APIs use PFD over FD."""
+ examine = clazz.ctors + clazz.methods
+ for m in examine:
+ if m.typ == "java.io.FileDescriptor":
+ error(clazz, m, "FW11", "Must use ParcelFileDescriptor")
+ if m.typ == "int":
+ if "Fd" in m.name or "FD" in m.name or "FileDescriptor" in m.name:
+ error(clazz, m, "FW11", "Must use ParcelFileDescriptor")
+ for arg in m.args:
+ if arg == "java.io.FileDescriptor":
+ error(clazz, m, "FW11", "Must use ParcelFileDescriptor")
+
+ for f in clazz.fields:
+ if f.typ == "java.io.FileDescriptor":
+ error(clazz, f, "FW11", "Must use ParcelFileDescriptor")
+
+ */
+ if (type == "java.io.FileDescriptor") {
+ report(
+ NO_CLONE, item,
+ "Must use ParcelFileDescriptor instead of FileDescriptor in ${item.describe()}"
+ )
+ } else if (type == "int" && item is MethodItem) {
+ val name = item.name()
+ if (name.contains("Fd") || name.contains("FD") || name.contains("FileDescriptor", ignoreCase = true)) {
+ report(
+ USE_PARCEL_FILE_DESCRIPTOR, item,
+ "Must use ParcelFileDescriptor instead of FileDescriptor in ${item.describe()}"
+ )
+ }
+ }
+ }
+
+ private fun checkNumbers(type: String, item: Item) {
+ /*
+ def verify_numbers(clazz):
+ """Discourage small numbers types like short and byte."""
+
+ discouraged = ["short","byte"]
+
+ for c in clazz.ctors:
+ for arg in c.args:
+ if arg in discouraged:
+ warn(clazz, c, "FW12", "Should avoid odd sized primitives; use int instead")
+
+ for f in clazz.fields:
+ if f.typ in discouraged:
+ warn(clazz, f, "FW12", "Should avoid odd sized primitives; use int instead")
+
+ for m in clazz.methods:
+ if m.typ in discouraged:
+ warn(clazz, m, "FW12", "Should avoid odd sized primitives; use int instead")
+ for arg in m.args:
+ if arg in discouraged:
+ warn(clazz, m, "FW12", "Should avoid odd sized primitives; use int instead")
+ */
+ if (type == "short" || type == "byte") {
+ report(
+ NO_BYTE_OR_SHORT, item,
+ "Should avoid odd sized primitives; use `int` instead of `$type` in ${item.describe()}"
+ )
+ }
+ }
+
+ private fun checkSingleton(
+ cls: ClassItem,
+ methods: Sequence<MethodItem>,
+ constructors: Sequence<ConstructorItem>
+ ) {
+ /*
+ def verify_singleton(clazz):
+ """Catch singleton objects with constructors."""
+
+ singleton = False
+ for m in clazz.methods:
+ if m.name.startswith("get") and m.name.endswith("Instance") and " static " in m.raw:
+ singleton = True
+
+ if singleton:
+ for c in clazz.ctors:
+ error(clazz, c, None, "Singleton classes should use getInstance() methods")
+ */
+ if (constructors.none()) {
+ return
+ }
+ if (methods.any { it.name().startsWith("get") && it.name().endsWith("Instance") && it.modifiers.isStatic() }) {
+ for (constructor in constructors) {
+ report(
+ SINGLETON_CONSTRUCTOR, constructor,
+ "Singleton classes should use `getInstance()` methods: `${cls.simpleName()}`"
+ )
+ }
+ }
+ }
+
+ /**
+ * Checks whether the given full package name is the same as the given root
+ * package or a sub package (if we just did full.startsWith("java"), then
+ * we'd return true for "javafx", and if we just use full.startsWith("java.")
+ * we'd miss the root package itself.
+ */
+ private fun inSubPackage(root: String, full: String): Boolean {
+ return root == full || full.startsWith(root) && full[root.length] == '.'
+ }
+
+ private fun isInteresting(cls: ClassItem): Boolean {
+ /*
+ def is_interesting(clazz):
+ """Test if given class is interesting from an Android PoV."""
+
+ if clazz.pkg.name.startswith("java"): return False
+ if clazz.pkg.name.startswith("junit"): return False
+ if clazz.pkg.name.startswith("org.apache"): return False
+ if clazz.pkg.name.startswith("org.xml"): return False
+ if clazz.pkg.name.startswith("org.json"): return False
+ if clazz.pkg.name.startswith("org.w3c"): return False
+ if clazz.pkg.name.startswith("android.icu."): return False
+ return True
+ */
+
+ val name = cls.qualifiedName()
+
+ // Fail fast for most common cases:
+ if (name.startsWith("android.")) {
+ if (!name.startsWith("android.icu")) {
+ return true
+ }
+ } else if (name.startsWith("java.")) {
+ return false
+ }
+
+ return when {
+ inSubPackage("java", name) ||
+ inSubPackage("javax", name) ||
+ inSubPackage("junit", name) ||
+ inSubPackage("org.apache", name) ||
+ inSubPackage("org.xml", name) ||
+ inSubPackage("org.json", name) ||
+ inSubPackage("org.w3c", name) ||
+ inSubPackage("org.xmlpull", name) ||
+ inSubPackage("android.icu", name) -> false
+ else -> true
+ }
+ }
+
+ companion object {
+ private val badParameterClassNames = listOf(
+ "Param", "Parameter", "Parameters", "Args", "Arg", "Argument", "Arguments", "Options", "Bundle"
+ )
+ private val badUnits = mapOf(
+ "Ns" to "Nanos",
+ "Ms" to "Millis or Micros",
+ "Sec" to "Seconds",
+ "Secs" to "Seconds",
+ "Hr" to "Hours",
+ "Hrs" to "Hours",
+ "Mo" to "Months",
+ "Mos" to "Months",
+ "Yr" to "Years",
+ "Yrs" to "Years",
+ "Byte" to "Bytes",
+ "Space" to "Bytes"
+ )
+ private val uiPackageParts = listOf(
+ "animation",
+ "view",
+ "graphics",
+ "transition",
+ "widget",
+ "webkit"
+ )
+
+ private val constantNamePattern = Regex("[A-Z0-9_]+")
+ private val onCallbackNamePattern = Regex("on[A-Z][a-z][a-zA-Z1-9]*")
+ private val configFieldPattern = Regex("config_[a-z][a-zA-Z1-9]*")
+ private val layoutFieldPattern = Regex("layout_[a-z][a-zA-Z1-9]*")
+ private val stateFieldPattern = Regex("state_[a-z_]+")
+ private val resourceFileFieldPattern = Regex("[a-z1-9_]+")
+ private val resourceValueFieldPattern = Regex("[a-z][a-zA-Z1-9]*")
+ private val styleFieldPattern = Regex("[A-Z][A-Za-z1-9]+(_[A-Z][A-Za-z1-9]+?)*")
+ private val serviceFieldPattern = Regex("([A-Z_]+)_SERVICE")
+
+ private val acronymPattern2 = Regex("([A-Z]){2,}")
+ private val acronymPattern3 = Regex("([A-Z]){3,}")
+
+ private fun hasAcronyms(name: String): Boolean {
+ // Require 3 capitals, or 2 if it's at the end of a word.
+ val result = acronymPattern2.find(name) ?: return false
+ return result.range.start == name.length - 2 || acronymPattern3.find(name) != null
+ }
+
+ private fun getFirstAcronym(name: String): String? {
+ // Require 3 capitals, or 2 if it's at the end of a word.
+ val result = acronymPattern2.find(name) ?: return null
+ if (result.range.start == name.length - 2) {
+ return name.substring(name.length - 2)
+ }
+ val result2 = acronymPattern3.find(name)
+ return if (result2 != null) {
+ name.substring(result2.range.start, result2.range.endInclusive + 1)
+ } else {
+ null
+ }
+ }
+
+ /** for something like "HTMLWriter", returns "HtmlWriter" */
+ private fun decapitalizeAcronyms(name: String): String {
+ var s = name
+
+ if (s.none { it.isLowerCase() }) {
+ // The entire thing is capitalized. If so, just perform
+ // normal capitalization, but try dropping _'s.
+ return SdkVersionInfo.underlinesToCamelCase(s.toLowerCase(Locale.US)).capitalize()
+ }
+
+ while (true) {
+ val acronym = getFirstAcronym(s) ?: return s
+ val index = s.indexOf(acronym)
+ if (index == -1) {
+ return s
+ }
+ // The last character, if not the end of the string, is probably the beginning of the
+ // next word so capitalize it
+ s = if (index == s.length - acronym.length) {
+ // acronym at the end of the word word
+ val decapitalized = acronym[0] + acronym.substring(1).toLowerCase(Locale.US)
+ s.replace(acronym, decapitalized)
+ } else {
+ val replacement = acronym[0] + acronym.substring(
+ 1,
+ acronym.length - 1
+ ).toLowerCase(Locale.US) + acronym[acronym.length - 1]
+ s.replace(acronym, replacement)
+ }
+ }
+ }
+ }
+}
diff --git a/src/main/java/com/android/tools/metalava/Driver.kt b/src/main/java/com/android/tools/metalava/Driver.kt
index 0de3436..e48ae23 100644
--- a/src/main/java/com/android/tools/metalava/Driver.kt
+++ b/src/main/java/com/android/tools/metalava/Driver.kt
@@ -732,6 +732,12 @@
// General API checks for Android APIs
AndroidApiChecks().check(codebase)
+ if (options.checkApi) {
+ val localTimer = Stopwatch.createStarted()
+ ApiLint().check(codebase)
+ progress("\n$PROGRAM_NAME ran api-lint in ${localTimer.elapsed(SECONDS)} seconds")
+ }
+
val filterEmit = ApiPredicate(ignoreShown = true, ignoreRemoved = false)
val apiEmit = ApiPredicate(ignoreShown = true)
val apiReference = ApiPredicate(ignoreShown = true)
diff --git a/src/main/java/com/android/tools/metalava/KotlinInteropChecks.kt b/src/main/java/com/android/tools/metalava/KotlinInteropChecks.kt
index 992fd0b..a904af5 100644
--- a/src/main/java/com/android/tools/metalava/KotlinInteropChecks.kt
+++ b/src/main/java/com/android/tools/metalava/KotlinInteropChecks.kt
@@ -17,6 +17,7 @@
package com.android.tools.metalava
import com.android.tools.metalava.doclava1.Errors
+import com.android.tools.metalava.model.ClassItem
import com.android.tools.metalava.model.Codebase
import com.android.tools.metalava.model.FieldItem
import com.android.tools.metalava.model.Item
@@ -35,34 +36,46 @@
// Also potentially makes other API suggestions.
class KotlinInteropChecks {
fun check(codebase: Codebase) {
+
codebase.accept(object : ApiVisitor(
// Sort by source order such that warnings follow source line number order
methodComparator = MethodItem.sourceOrderComparator,
fieldComparator = FieldItem.comparator
) {
+ private var isKotlin = false
+
+ override fun visitClass(cls: ClassItem) {
+ isKotlin = cls.isKotlin()
+ }
+
override fun visitMethod(method: MethodItem) {
- checkMethod(method)
+ checkMethod(method, isKotlin)
}
override fun visitField(field: FieldItem) {
- checkField(field)
+ checkField(field, isKotlin)
}
})
}
- fun checkField(field: FieldItem) {
- ensureCompanionFieldJvmField(field)
+ fun checkField(field: FieldItem, isKotlin: Boolean = field.isKotlin()) {
+ if (isKotlin) {
+ ensureCompanionFieldJvmField(field)
+ }
ensureFieldNameNotKeyword(field)
}
- fun checkMethod(method: MethodItem) {
+ fun checkMethod(method: MethodItem, isKotlin: Boolean = method.isKotlin()) {
if (!method.isConstructor()) {
- ensureMethodNameNotKeyword(method)
- ensureParameterNamesNotKeywords(method)
- ensureDefaultParamsHaveJvmOverloads(method)
- ensureCompanionJvmStatic(method)
+ if (isKotlin) {
+ ensureDefaultParamsHaveJvmOverloads(method)
+ ensureCompanionJvmStatic(method)
+ ensureExceptionsDocumented(method)
+ } else {
+ ensureMethodNameNotKeyword(method)
+ ensureParameterNamesNotKeywords(method)
+ }
ensureLambdaLastParameter(method)
- ensureExceptionsDocumented(method)
}
}
diff --git a/src/main/java/com/android/tools/metalava/Options.kt b/src/main/java/com/android/tools/metalava/Options.kt
index 61559ab..147be0b 100644
--- a/src/main/java/com/android/tools/metalava/Options.kt
+++ b/src/main/java/com/android/tools/metalava/Options.kt
@@ -113,6 +113,7 @@
const val ARG_CURRENT_CODENAME = "--current-codename"
const val ARG_CURRENT_JAR = "--current-jar"
const val ARG_CHECK_KOTLIN_INTEROP = "--check-kotlin-interop"
+const val ARG_API_LINT = "--api-lint"
const val ARG_PUBLIC = "--public"
const val ARG_PROTECTED = "--protected"
const val ARG_PACKAGE = "--package"
@@ -291,6 +292,9 @@
*/
var showUnannotated = false
+ /** Whether to validate the API for best practices */
+ var checkApi = false
+
/** Whether to validate the API for Kotlin interop */
var checkKotlinInterop = false
@@ -853,6 +857,7 @@
// lintsAreErrors = true
}
+ ARG_API_LINT -> checkApi = true
ARG_CHECK_KOTLIN_INTEROP -> checkKotlinInterop = true
ARG_COLOR -> color = true
@@ -1712,6 +1717,7 @@
"released API, respectively. Different compatibility checks apply in the two scenarios. " +
"For example, to check the code base against the current public API, use " +
"$ARG_CHECK_COMPATIBILITY:api:current.",
+ ARG_API_LINT, "Check API for Android API best practices",
ARG_CHECK_KOTLIN_INTEROP, "Check API intended to be used from both Kotlin and Java for interoperability " +
"issues",
"$ARG_MIGRATE_NULLNESS <api file>", "Compare nullness information with the previous stable API " +
diff --git a/src/main/java/com/android/tools/metalava/doclava1/Errors.java b/src/main/java/com/android/tools/metalava/doclava1/Errors.java
index ef1cabf..cc0e5db 100644
--- a/src/main/java/com/android/tools/metalava/doclava1/Errors.java
+++ b/src/main/java/com/android/tools/metalava/doclava1/Errors.java
@@ -147,6 +147,7 @@
public enum Category {
COMPATIBILITY("Compatibility", null),
DOCUMENTATION("Documentation", null),
+ API_LINT("API Lint", "go/android-api-guidelines"),
UNKNOWN("Default", null);
public final String description;
@@ -236,11 +237,6 @@
public static final Error INFIX_REMOVAL = new Error(138, ERROR, Category.COMPATIBILITY);
public static final Error VARARG_REMOVAL = new Error(139, ERROR, Category.COMPATIBILITY);
public static final Error ADD_SEALED = new Error(140, ERROR, Category.COMPATIBILITY);
- public static final Error KOTLIN_KEYWORD = new Error(141, WARNING);
- public static final Error SAM_SHOULD_BE_LAST = new Error(142, WARNING);
- public static final Error MISSING_JVMSTATIC = new Error(143, WARNING);
- public static final Error DEFAULT_VALUE_CHANGE = new Error(144, ERROR);
- public static final Error DOCUMENT_EXCEPTIONS = new Error(145, ERROR);
public static final Error ANNOTATION_EXTRACTION = new Error(146, ERROR);
public static final Error SUPERFLUOUS_PREFIX = new Error(147, WARNING);
public static final Error HIDDEN_TYPEDEF_CONSTANT = new Error(148, ERROR);
@@ -261,6 +257,91 @@
public static final Error IGNORING_SYMLINK = new Error(159, INFO);
public static final Error INVALID_NULLABILITY_ANNOTATION_WARNING = new Error(160, WARNING);
+ // API lint
+ public static final Error START_WITH_LOWER = new Error(300, ERROR, Category.API_LINT, "S1");
+ public static final Error START_WITH_UPPER = new Error(301, ERROR, Category.API_LINT, "S1");
+ public static final Error ALL_UPPER = new Error(302, ERROR, Category.API_LINT, "C2");
+ public static final Error ACRONYM_NAME = new Error(303, WARNING, Category.API_LINT, "S1");
+ public static final Error ENUM = new Error(304, ERROR, Category.API_LINT, "F5");
+ public static final Error ENDS_WITH_IMPL = new Error(305, ERROR, Category.API_LINT);
+ public static final Error MIN_MAX_CONSTANT = new Error(306, WARNING, Category.API_LINT, "C8");
+ public static final Error COMPILE_TIME_CONSTANT = new Error(307, ERROR, Category.API_LINT);
+ public static final Error SINGULAR_CALLBACK = new Error(308, ERROR, Category.API_LINT, "L1");
+ public static final Error CALLBACK_NAME = new Error(309, WARNING, Category.API_LINT, "L1");
+ public static final Error CALLBACK_INTERFACE = new Error(310, ERROR, Category.API_LINT, "CL3");
+ public static final Error CALLBACK_METHOD_NAME = new Error(311, ERROR, Category.API_LINT, "L1");
+ public static final Error LISTENER_INTERFACE = new Error(312, ERROR, Category.API_LINT, "L1");
+ public static final Error SINGLE_METHOD_INTERFACE = new Error(313, ERROR, Category.API_LINT, "L1");
+ public static final Error INTENT_NAME = new Error(314, ERROR, Category.API_LINT, "C3");
+ public static final Error ACTION_VALUE = new Error(315, ERROR, Category.API_LINT, "C4");
+ public static final Error EQUALS_AND_HASH_CODE = new Error(316, ERROR, Category.API_LINT, "M8");
+ public static final Error PARCEL_CREATOR = new Error(317, ERROR, Category.API_LINT, "FW3");
+ public static final Error PARCEL_NOT_FINAL = new Error(318, ERROR, Category.API_LINT, "FW8");
+ public static final Error PARCEL_CONSTRUCTOR = new Error(319, ERROR, Category.API_LINT, "FW3");
+ public static final Error PROTECTED_MEMBER = new Error(320, ERROR, Category.API_LINT, "M7");
+ public static final Error PAIRED_REGISTRATION = new Error(321, ERROR, Category.API_LINT, "L2");
+ public static final Error REGISTRATION_NAME = new Error(322, ERROR, Category.API_LINT, "L3");
+ public static final Error VISIBLY_SYNCHRONIZED = new Error(323, ERROR, Category.API_LINT, "M5");
+ public static final Error INTENT_BUILDER_NAME = new Error(324, WARNING, Category.API_LINT, "FW1");
+ public static final Error CONTEXT_NAME_SUFFIX = new Error(325, ERROR, Category.API_LINT, "C4");
+ public static final Error INTERFACE_CONSTANT = new Error(326, ERROR, Category.API_LINT, "C4");
+ public static final Error ON_NAME_EXPECTED = new Error(327, WARNING, Category.API_LINT);
+ public static final Error TOP_LEVEL_BUILDER = new Error(328, WARNING, Category.API_LINT);
+ public static final Error MISSING_BUILD = new Error(329, WARNING, Category.API_LINT);
+ public static final Error BUILDER_SET_STYLE = new Error(330, WARNING, Category.API_LINT);
+ public static final Error SETTER_RETURNS_THIS = new Error(331, WARNING, Category.API_LINT, "M4");
+ public static final Error RAW_AIDL = new Error(332, ERROR, Category.API_LINT);
+ public static final Error INTERNAL_CLASSES = new Error(333, ERROR, Category.API_LINT);
+ public static final Error PACKAGE_LAYERING = new Error(334, WARNING, Category.API_LINT, "FW6");
+ public static final Error GETTER_SETTER_NAMES = new Error(335, ERROR, Category.API_LINT, "M6");
+ public static final Error CONCRETE_COLLECTION = new Error(336, ERROR, Category.API_LINT, "CL2");
+ public static final Error OVERLAPPING_CONSTANTS = new Error(337, WARNING, Category.API_LINT, "C1");
+ public static final Error GENERIC_EXCEPTION = new Error(338, ERROR, Category.API_LINT, "S1");
+ public static final Error ILLEGAL_STATE_EXCEPTION = new Error(339, WARNING, Category.API_LINT, "S1");
+ public static final Error RETHROW_REMOTE_EXCEPTION = new Error(340, ERROR, Category.API_LINT, "FW9");
+ public static final Error MENTIONS_GOOGLE = new Error(341, ERROR, Category.API_LINT);
+ public static final Error HEAVY_BIT_SET = new Error(342, ERROR, Category.API_LINT);
+ public static final Error MANAGER_CONSTRUCTOR = new Error(343, ERROR, Category.API_LINT);
+ public static final Error MANAGER_LOOKUP = new Error(344, ERROR, Category.API_LINT);
+ public static final Error AUTO_BOXING = new Error(345, ERROR, Category.API_LINT, "M11");
+ public static final Error STATIC_UTILS = new Error(346, ERROR, Category.API_LINT);
+ public static final Error CONTEXT_FIRST = new Error(347, ERROR, Category.API_LINT, "M3");
+ public static final Error LISTENER_LAST = new Error(348, WARNING, Category.API_LINT, "M3");
+ public static final Error EXECUTOR_REGISTRATION = new Error(349, WARNING, Category.API_LINT, "L1");
+ public static final Error CONFIG_FIELD_NAME = new Error(350, ERROR, Category.API_LINT);
+ public static final Error RESOURCE_FIELD_NAME = new Error(351, ERROR, Category.API_LINT);
+ public static final Error RESOURCE_VALUE_FIELD_NAME = new Error(352, ERROR, Category.API_LINT, "C7");
+ public static final Error RESOURCE_STYLE_FIELD_NAME = new Error(353, ERROR, Category.API_LINT, "C7");
+ public static final Error STREAM_FILES = new Error(354, WARNING, Category.API_LINT, "M10");
+ public static final Error PARCELABLE_LIST = new Error(355, WARNING, Category.API_LINT);
+ public static final Error ABSTRACT_INNER = new Error(356, WARNING, Category.API_LINT);
+ public static final Error BANNED_THROW = new Error(358, ERROR, Category.API_LINT);
+ public static final Error EXTENDS_ERROR = new Error(359, ERROR, Category.API_LINT);
+ public static final Error EXCEPTION_NAME = new Error(360, ERROR, Category.API_LINT);
+ public static final Error METHOD_NAME_UNITS = new Error(361, ERROR, Category.API_LINT);
+ public static final Error FRACTION_FLOAT = new Error(362, ERROR, Category.API_LINT);
+ public static final Error PERCENTAGE_INT = new Error(363, ERROR, Category.API_LINT);
+ public static final Error NOT_CLOSEABLE = new Error(364, WARNING, Category.API_LINT);
+ public static final Error KOTLIN_OPERATOR = new Error(365, WARNING, Category.API_LINT);
+ public static final Error ARRAY_RETURN = new Error(366, WARNING, Category.API_LINT);
+ public static final Error USER_HANDLE = new Error(367, WARNING, Category.API_LINT);
+ public static final Error USER_HANDLE_NAME = new Error(368, WARNING, Category.API_LINT);
+ public static final Error SERVICE_NAME = new Error(369, ERROR, Category.API_LINT, "C4");
+ public static final Error METHOD_NAME_TENSE = new Error(370, WARNING, Category.API_LINT);
+ public static final Error NO_CLONE = new Error(371, ERROR, Category.API_LINT);
+ public static final Error USE_ICU = new Error(372, WARNING, Category.API_LINT);
+ public static final Error USE_PARCEL_FILE_DESCRIPTOR = new Error(373, ERROR, Category.API_LINT, "FW11");
+ public static final Error NO_BYTE_OR_SHORT = new Error(374, WARNING, Category.API_LINT, "FW12");
+ public static final Error SINGLETON_CONSTRUCTOR = new Error(375, ERROR, Category.API_LINT);
+ public static final Error COMMON_ARGS_FIRST = new Error(376, WARNING, Category.API_LINT, "M2");
+ public static final Error CONSISTENT_ARGUMENT_ORDER = new Error(377, ERROR, Category.API_LINT, "M2");
+ public static final Error KOTLIN_KEYWORD = new Error(378, ERROR, Category.API_LINT); // Formerly 141
+ public static final Error UNIQUE_KOTLIN_OPERATOR = new Error(379, ERROR, Category.API_LINT);
+ public static final Error SAM_SHOULD_BE_LAST = new Error(380, WARNING, Category.API_LINT); // Formerly 142
+ public static final Error MISSING_JVMSTATIC = new Error(381, WARNING, Category.API_LINT); // Formerly 143
+ public static final Error DEFAULT_VALUE_CHANGE = new Error(382, ERROR, Category.API_LINT); // Formerly 144
+ public static final Error DOCUMENT_EXCEPTIONS = new Error(383, ERROR, Category.API_LINT); // Formerly 145
+
static {
// Attempt to initialize error names based on the field names
try {
diff --git a/src/test/java/com/android/tools/metalava/ApiLintTest.kt b/src/test/java/com/android/tools/metalava/ApiLintTest.kt
new file mode 100644
index 0000000..e01bacb
--- /dev/null
+++ b/src/test/java/com/android/tools/metalava/ApiLintTest.kt
@@ -0,0 +1,1929 @@
+/*
+ * Copyright (C) 2018 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 com.android.tools.metalava
+
+import org.junit.Test
+
+class ApiLintTest : DriverTest() {
+
+ @Test
+ fun `Test names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/ALL_CAPS.java:3: warning: Acronyms should not be capitalized in class names: was `ALL_CAPS`, should this be `AllCaps`? [AcronymName] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/HTMLWriter.java:3: warning: Acronyms should not be capitalized in class names: was `HTMLWriter`, should this be `HtmlWriter`? [AcronymName] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyStringImpl.java:3: error: Don't expose your implementation details: `MyStringImpl` ends with `Impl` [EndsWithImpl]
+ src/android/pkg/badlyNamedClass.java:3: error: Class must start with uppercase char: badlyNamedClass [StartWithUpper] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/badlyNamedClass.java:5: error: Method name must start with lowercase char: BadlyNamedMethod1 [StartWithLower] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/badlyNamedClass.java:7: warning: Acronyms should not be capitalized in method names: was `fromHTMLToHTML`, should this be `fromHtmlToHtml`? [AcronymName] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/badlyNamedClass.java:8: warning: Acronyms should not be capitalized in method names: was `toXML`, should this be `toXml`? [AcronymName] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/badlyNamedClass.java:9: warning: Acronyms should not be capitalized in method names: was `getID`, should this be `getId`? [AcronymName] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/badlyNamedClass.java:4: error: Constant field names must be named with only upper case characters: `android.pkg.badlyNamedClass#BadlyNamedField`, should be `BADLY_NAMED_FIELD`? [AllUpper] [Rule C2 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class badlyNamedClass {
+ public static final int BadlyNamedField = 1;
+ public void BadlyNamedMethod1() { }
+
+ public void fromHTMLToHTML() { }
+ public void toXML() { }
+ public String getID() { return null; }
+ public void setZOrderOnTop() { } // OK
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class ALL_CAPS { // like android.os.Build.VERSION_CODES
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class HTMLWriter {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyStringImpl {
+ }
+ """
+ ),
+ java(
+ """
+ package android.icu;
+
+ // Same as above android.pkg.badlyNamedClass but in a package
+ // that API lint is supposed to ignore (see ApiLint#isInteresting)
+ public class badlyNamedClass {
+ public static final int BadlyNamedField = 1;
+ public void BadlyNamedMethod1() { }
+
+ public void toXML() { }
+ public String getID() { return null; }
+ public void setZOrderOnTop() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.icu.sub;
+
+ // Same as above android.pkg.badlyNamedClass but in a package
+ // that API lint is supposed to ignore (see ApiLint#isInteresting)
+ public class badlyNamedClass {
+ public static final int BadlyNamedField = 1;
+ public void BadlyNamedMethod1() { }
+
+ public void toXML() { }
+ public String getID() { return null; }
+ public void setZOrderOnTop() { }
+ }
+ """
+ )
+ ),
+ expectedOutput = "9 new API lint issues were found. See tools/metalava/API-LINT.md for how to handle these."
+ )
+ }
+
+ @Test
+ fun `Test constants`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/Constants.java:9: error: All constants must be defined at compile time: android.pkg.Constants#FOO [CompileTimeConstant]
+ src/android/pkg/Constants.java:8: warning: If min/max could change in future, make them dynamic methods: android.pkg.Constants#MAX_FOO [MinMaxConstant] [Rule C8 in go/android-api-guidelines]
+ src/android/pkg/Constants.java:7: warning: If min/max could change in future, make them dynamic methods: android.pkg.Constants#MIN_FOO [MinMaxConstant] [Rule C8 in go/android-api-guidelines]
+ src/android/pkg/Constants.java:6: error: Constant field names must be named with only upper case characters: `android.pkg.Constants#myStrings`, should be `MY_STRINGS`? [AllUpper] [Rule C2 in go/android-api-guidelines]
+ src/android/pkg/Constants.java:5: error: Constant field names must be named with only upper case characters: `android.pkg.Constants#strings`, should be `STRINGS`? [AllUpper] [Rule C2 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class Constants {
+ private Constants() { }
+ public static final String[] strings = { "NONE", "WPA_PSK" };
+ public static final String[] myStrings = { "NONE", "WPA_PSK" };
+ public static final int MIN_FOO = 1;
+ public static final int MAX_FOO = 10;
+ public static final String FOO = System.getProperty("foo");
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `No enums`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyEnum.java:3: error: Enums are discouraged in Android APIs [Enum] [Rule F5 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public enum MyEnum {
+ FOO, BAR
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test callbacks`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyCallback.java:3: error: Callback method names must follow the on<Something> style: bar [CallbackMethodName] [Rule L1 in go/android-api-guidelines]
+ src/android/pkg/MyCallbacks.java:3: error: Callback class names should be singular: MyCallbacks [SingularCallback] [Rule L1 in go/android-api-guidelines]
+ src/android/pkg/MyInterfaceCallback.java:3: error: Callbacks must be abstract class instead of interface to enable extension in future API levels: MyInterfaceCallback [CallbackInterface] [Rule CL3 in go/android-api-guidelines]
+ src/android/pkg/MyObserver.java:3: warning: Class should be named MyCallback [CallbackName] [Rule L1 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyCallbacks {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyObserver {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public interface MyInterfaceCallback {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyCallback {
+ public void onFoo() {
+ }
+ public void onAnimationStart() {
+ }
+ public void bar() {
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test listeners`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyCallback.java:3: error: Callback method names must follow the on<Something> style: bar [CallbackMethodName] [Rule L1 in go/android-api-guidelines]
+ src/android/pkg/MyClassListener.java:3: error: Listeners should be an interface, or otherwise renamed Callback: MyClassListener [ListenerInterface] [Rule L1 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyClassListener {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public interface OnFooBarListener {
+ void bar();
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public interface OnFooBarListener {
+ void onFooBar(); // OK
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public interface MyInterfaceListener {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyCallback {
+ public void onFoo() {
+ }
+ public void bar() {
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test actions`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/accounts/Actions.java:7: error: Intent action constant name must be ACTION_FOO: ACCOUNT_ADDED [IntentName] [Rule C3 in go/android-api-guidelines]
+ src/android/accounts/Actions.java:6: error: Inconsistent action value; expected `android.accounts.action.ACCOUNT_OPENED`, was `android.accounts.ACCOUNT_OPENED` [ActionValue] [Rule C4 in go/android-api-guidelines]
+ src/android/accounts/Actions.java:8: error: Intent action constant name must be ACTION_FOO: SOMETHING [IntentName] [Rule C3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.accounts;
+
+ public class Actions {
+ private Actions() { }
+ public static final String ACTION_ACCOUNT_REMOVED = "android.accounts.action.ACCOUNT_REMOVED";
+ public static final String ACTION_ACCOUNT_OPENED = "android.accounts.ACCOUNT_OPENED";
+ public static final String ACCOUNT_ADDED = "android.accounts.action.ACCOUNT_ADDED";
+ public static final String SOMETHING = "android.accounts.action.ACCOUNT_MOVED";
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test extras`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/accounts/Extras.java:5: error: Inconsistent extra value; expected `android.accounts.extra.AUTOMATIC_RULE_ID`, was `android.app.extra.AUTOMATIC_RULE_ID` [ActionValue] [Rule C4 in go/android-api-guidelines]
+ src/android/accounts/Extras.java:7: error: Intent extra constant name must be EXTRA_FOO: RULE_ID [IntentName] [Rule C3 in go/android-api-guidelines]
+ src/android/accounts/Extras.java:6: error: Intent extra constant name must be EXTRA_FOO: SOMETHING_EXTRA [IntentName] [Rule C3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.accounts;
+
+ public class Extras {
+ private Extras() { }
+ public static final String EXTRA_AUTOMATIC_RULE_ID = "android.app.extra.AUTOMATIC_RULE_ID";
+ public static final String SOMETHING_EXTRA = "something.here";
+ public static final String RULE_ID = "android.app.extra.AUTOMATIC_RULE_ID";
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test equals and hashCode`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MissingEquals.java:4: error: Must override both equals and hashCode; missing one in android.pkg.MissingEquals [EqualsAndHashCode] [Rule M8 in go/android-api-guidelines]
+ src/android/pkg/MissingHashCode.java:5: error: Must override both equals and hashCode; missing one in android.pkg.MissingHashCode [EqualsAndHashCode] [Rule M8 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ @SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
+ public class Ok {
+ public boolean equals(Object other) { return true; }
+ public int hashCode() { return 0; }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MissingEquals {
+ public int hashCode() { return 0; }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ @SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
+ public class MissingHashCode {
+ public boolean equals(Object other) { return true; }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class UnrelatedEquals {
+ @SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
+ public static boolean equals(Object other) { return true; } // static
+ public boolean equals(int other) { return false; } // wrong parameter type
+ public boolean equals(Object other, int bar) { return false; } // wrong signature
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Test Parcelable`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MissingCreator.java:3: error: Parcelable requires a `CREATOR` field; missing in android.pkg.MissingCreator [ParcelCreator] [Rule FW3 in go/android-api-guidelines]
+ src/android/pkg/MissingDescribeContents.java:3: error: Parcelable requires `public int describeContents()`; missing in android.pkg.MissingDescribeContents [ParcelCreator] [Rule FW3 in go/android-api-guidelines]
+ src/android/pkg/MissingWriteToParcel.java:3: error: Parcelable requires `void writeToParcel(Parcel, int)`; missing in android.pkg.MissingWriteToParcel [ParcelCreator] [Rule FW3 in go/android-api-guidelines]
+ src/android/pkg/NonFinalParcelable.java:3: error: Parcelable classes must be final: android.pkg.NonFinalParcelable is not final [ParcelNotFinal] [Rule FW8 in go/android-api-guidelines]
+ src/android/pkg/ParcelableConstructor.java:4: error: Parcelable inflation is exposed through CREATOR, not raw constructors, in android.pkg.ParcelableConstructor [ParcelConstructor] [Rule FW3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public final class ParcelableConstructor implements android.os.Parcelable {
+ public ParcelableConstructor(android.os.Parcel p) { }
+ public int describeContents() { return 0; }
+ public void writeToParcel(android.os.Parcel p, int f) { }
+ public static final android.os.Parcelable.Creator<android.app.WallpaperColors> CREATOR = null;
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class NonFinalParcelable implements android.os.Parcelable {
+ public NonFinalParcelable() { }
+ public int describeContents() { return 0; }
+ public void writeToParcel(android.os.Parcel p, int f) { }
+ public static final android.os.Parcelable.Creator<android.app.WallpaperColors> CREATOR = null;
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public final class MissingCreator implements android.os.Parcelable {
+ public MissingCreator() { }
+ public int describeContents() { return 0; }
+ public void writeToParcel(android.os.Parcel p, int f) { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public final class MissingDescribeContents implements android.os.Parcelable {
+ public MissingDescribeContents() { }
+ public void writeToParcel(android.os.Parcel p, int f) { }
+ public static final android.os.Parcelable.Creator<android.app.WallpaperColors> CREATOR = null;
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public final class MissingWriteToParcel implements android.os.Parcelable {
+ public MissingWriteToParcel() { }
+ public int describeContents() { return 0; }
+ public static final android.os.Parcelable.Creator<android.app.WallpaperColors> CREATOR = null;
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `No protected methods or fields are allowed`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:6: error: Protected methods not allowed; must be public: method android.pkg.MyClass.wrong()} [ProtectedMember] [Rule M7 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:8: error: Protected fields not allowed; must be public: field android.pkg.MyClass.wrong} [ProtectedMember] [Rule M7 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyClass implements AutoCloseable {
+ public void ok() { }
+ protected void finalize() { } // OK
+ protected void wrong() { }
+ public int ok = 42;
+ protected int wrong = 5;
+ private int ok2 = 2;
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Ensure registration methods are matched`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/RegistrationMethods.java:6: error: Found registerUnpairedCallback but not unregisterUnpairedCallback in android.pkg.RegistrationMethods [PairedRegistration] [Rule L2 in go/android-api-guidelines]
+ src/android/pkg/RegistrationMethods.java:7: error: Found unregisterMismatchedCallback but not registerMismatchedCallback in android.pkg.RegistrationMethods [PairedRegistration] [Rule L2 in go/android-api-guidelines]
+ src/android/pkg/RegistrationMethods.java:8: error: Callback methods should be named register/unregister; was addCallback [RegistrationName] [Rule L3 in go/android-api-guidelines]
+ src/android/pkg/RegistrationMethods.java:13: error: Found addUnpairedListener but not removeUnpairedListener in android.pkg.RegistrationMethods [PairedRegistration] [Rule L2 in go/android-api-guidelines]
+ src/android/pkg/RegistrationMethods.java:14: error: Found removeMismatchedListener but not addMismatchedListener in android.pkg.RegistrationMethods [PairedRegistration] [Rule L2 in go/android-api-guidelines]
+ src/android/pkg/RegistrationMethods.java:15: error: Listener methods should be named add/remove; was registerWrongListener [RegistrationName] [Rule L3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class RegistrationMethods {
+ public void registerOkCallback(Runnable r) { } // OK
+ public void unregisterOkCallback(Runnable r) { } // OK
+ public void registerUnpairedCallback(Runnable r) { }
+ public void unregisterMismatchedCallback(Runnable r) { }
+ public void addCallback(Runnable r) { }
+
+ public void addOkListener(Runnable r) { } // OK
+ public void removeOkListener(Runnable r) { } // OK
+
+ public void addUnpairedListener(Runnable r) { }
+ public void removeMismatchedListener(Runnable r) { }
+ public void registerWrongListener(Runnable r) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Api methods should not be synchronized in their signature`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/CheckSynchronization.java:5: error: Internal locks must not be exposed: method android.pkg.CheckSynchronization.method2(Runnable) [VisiblySynchronized] [Rule M5 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class CheckSynchronization {
+ public void method1(Runnable r) { } // OK
+ public synchronized void method2(Runnable r) { } // ERROR
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check intent builder names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/IntentBuilderNames.java:6: warning: Methods creating an Intent should be named `create<Foo>Intent()`, was `makeMyIntent` [IntentBuilderName] [Rule FW1 in go/android-api-guidelines]
+ src/android/pkg/IntentBuilderNames.java:7: warning: Methods creating an Intent should be named `create<Foo>Intent()`, was `createIntentNow` [IntentBuilderName] [Rule FW1 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.content.Intent;
+
+ public class IntentBuilderNames {
+ public Intent createEnrollIntent() { return null; } // OK
+ public Intent makeMyIntent() { return null; } // WARN
+ public Intent createIntentNow() { return null; } // WARN
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check helper classes`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass1.java:3: error: Inconsistent class name; should be `<Foo>Activity`, was `MyClass1` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines]
+ src/android/pkg/MyClass1.java:6: warning: Methods implemented by developers should follow the on<Something> style, was `badlyNamedAbstractMethod` [OnNameExpected]
+ src/android/pkg/MyClass1.java:7: warning: If implemented by developer, should follow the on<Something> style; otherwise consider marking final [OnNameExpected]
+ src/android/pkg/MyClass2.java:3: error: Inconsistent class name; should be `<Foo>Provider`, was `MyClass2` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines]
+ src/android/pkg/MyClass3.java:3: error: Inconsistent class name; should be `<Foo>Service`, was `MyClass3` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines]
+ src/android/pkg/MyClass4.java:3: error: Inconsistent class name; should be `<Foo>Receiver`, was `MyClass4` [ContextNameSuffix] [Rule C4 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass1 extends android.app.Activity {
+ public void onOk() { }
+ public final void ok() { }
+ public abstract void badlyNamedAbstractMethod();
+ public void badlyNamedMethod() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass2 extends android.content.ContentProvider {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass3 extends android.app.Service {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass4 extends android.content.BroadcastReceiver {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyOkActivity extends android.app.Activity {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check builders`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:4: warning: Methods must return the builder object (return type Builder instead of void): method android.pkg.MyClass.Builder.setSomething(int) [SetterReturnsThis] [Rule M4 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:4: warning: Builder methods names should use setFoo() style: method android.pkg.MyClass.Builder.withFoo(int) [BuilderSetStyle]
+ src/android/pkg/MyClass.java:4: warning: Missing `build()` method in android.pkg.MyClass.Builder [MissingBuild]
+ src/android/pkg/TopLevelBuilder.java:3: warning: Builder should be defined as inner class: android.pkg.TopLevelBuilder [TopLevelBuilder]
+ src/android/pkg/TopLevelBuilder.java:3: warning: Missing `build()` method in android.pkg.TopLevelBuilder [MissingBuild]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class TopLevelBuilder {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public class Builder {
+ public void clearAll() { }
+ public int getSomething() { return 0; }
+ public void setSomething(int s) { }
+ public Builder withFoo(int s) { return this; }
+ public Builder setOk(int s) { return this; }
+ }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class Ok {
+ public class OkBuilder {
+ public Ok build() { return null; }
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Raw AIDL`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass1.java:3: error: Raw AIDL interfaces must not be exposed: MyClass1 extends Binder [RawAidl]
+ src/android/pkg/MyClass2.java:3: error: Raw AIDL interfaces must not be exposed: MyClass2 implements IInterface [RawAidl]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyClass1 extends android.os.Binder {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyClass2 implements android.os.IInterface {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+ // Ensure that we don't flag transitively implementing IInterface
+ public class MyClass3 extends MyClass1 implements MyClass2 {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Internal packages`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/com/android/pkg/MyClass.java:3: error: Internal classes must not be exposed [InternalClasses]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package com.android.pkg;
+
+ public class MyClass {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check package layering`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/content/MyClass1.java:7: warning: Field type `android.view.View` violates package layering: nothing in `package android.content` should depend on `package android.view` [PackageLayering] [Rule FW6 in go/android-api-guidelines]
+ src/android/content/MyClass1.java:7: warning: Method return type `android.view.View` violates package layering: nothing in `package android.content` should depend on `package android.view` [PackageLayering] [Rule FW6 in go/android-api-guidelines]
+ src/android/content/MyClass1.java:7: warning: Method parameter type `android.view.View` violates package layering: nothing in `package android.content` should depend on `package android.view` [PackageLayering] [Rule FW6 in go/android-api-guidelines]
+ src/android/content/MyClass1.java:7: warning: Method parameter type `android.view.View` violates package layering: nothing in `package android.content` should depend on `package android.view` [PackageLayering] [Rule FW6 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.content;
+
+ import android.graphics.drawable.Drawable;
+ import android.graphics.Bitmap;
+ import android.view.View;
+
+ public class MyClass1 {
+ public View view = null;
+ public Drawable drawable = null;
+ public Bitmap bitmap = null;
+ public View ok(View view, Drawable drawable) { return null; }
+ public Bitmap wrong(View view, Bitmap bitmap) { return null; }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check boolean getter and setter naming patterns`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:8: error: Symmetric method for `setProp4` must be named `getProp4`; was `isProp4` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:12: error: Symmetric method for `hasError1` must be named `setHasError1`; was `setError1` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:11: error: Symmetric method for `setError1` must be named `getError1`; was `hasError1` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:14: error: Symmetric method for `isError2` must be named `setIsError2`; was `setHasError2` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:18: error: Symmetric method for `getError3` must be named `setError3`; was `setIsError3` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:16: error: Symmetric method for `getError3` must be named `setError3`; was `setHasError3` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:20: error: Symmetric method for `hasError5` must be named `setHasError5`; was `setError5` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:19: error: Symmetric method for `setError5` must be named `getError5`; was `hasError5` [GetterSetterNames] [Rule M6 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public int getProp1() { return 0; }
+ public boolean getProp2() { return false; }
+ public boolean getProp3() { return false; }
+ public void setProp3(boolean s) { }
+ public boolean isProp4() { return false; }
+ public void setProp4(boolean s) { }
+
+ public boolean hasError1() { return false; }
+ public void setError1(boolean s) { }
+ public boolean isError2() { return false; }
+ public void setHasError2(boolean s) { }
+ public boolean getError3() { return false; }
+ public void setHasError3(boolean s) { }
+ public boolean isError4() { return false; }
+ public void setIsError3(boolean s) { }
+ public boolean hasError5() { return false; }
+ public void setError5(boolean s) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check banned collections`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:4: error: Parameter type is concrete collection (`java.util.HashMap`); must be higher-level interface [ConcreteCollection] [Rule CL2 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:5: error: Return type is concrete collection (`java.util.Vector`); must be higher-level interface [ConcreteCollection] [Rule CL2 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:5: error: Parameter type is concrete collection (`java.util.LinkedList`); must be higher-level interface [ConcreteCollection] [Rule CL2 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public MyClass(java.util.HashMap<String,String> map1, java.util.Map<String,String> map2) { }
+ public java.util.Vector<String> getList(java.util.LinkedList<String> list) { return null; }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check non-overlapping flags`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/accounts/OverlappingFlags.java:19: warning: Found overlapping flag constant values: `TEST1_FLAG_SECOND` with value 3 (0x3) and overlapping flag value 1 (0x1) from `TEST1_FLAG_FIRST` [OverlappingConstants] [Rule C1 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.accounts;
+
+ public class OverlappingFlags {
+ private OverlappingFlags() { }
+ public static final int DRAG_FLAG_GLOBAL_PREFIX_URI_PERMISSION = 128; // 0x80
+ public static final int DRAG_FLAG_GLOBAL_URI_READ = 1; // 0x1
+ public static final int DRAG_FLAG_GLOBAL_URI_WRITE = 2; // 0x2
+ public static final int DRAG_FLAG_OPAQUE = 512; // 0x200
+ public static final int SYSTEM_UI_FLAG_FULLSCREEN = 4; // 0x4
+ public static final int SYSTEM_UI_FLAG_HIDE_NAVIGATION = 2; // 0x2
+ public static final int SYSTEM_UI_FLAG_IMMERSIVE = 2048; // 0x800
+ public static final int SYSTEM_UI_FLAG_IMMERSIVE_STICKY = 4096; // 0x1000
+ public static final int SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN = 1024; // 0x400
+ public static final int SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION = 512; // 0x200
+ public static final int SYSTEM_UI_FLAG_LAYOUT_STABLE = 256; // 0x100
+ public static final int SYSTEM_UI_FLAG_LIGHT_NAVIGATION_BAR = 16; // 0x10
+
+ public static final int TEST1_FLAG_FIRST = 1;
+ public static final int TEST1_FLAG_SECOND = 3;
+ public static final int TEST2_FLAG_FIRST = 5;
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check exception related issues`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT,
+ // Conflicting advice:
+ ARG_HIDE, "BannedThrow"),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:6: error: Methods must not throw generic exceptions (`java.lang.Exception`) [GenericException] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:7: error: Methods must not throw generic exceptions (`java.lang.Throwable`) [GenericException] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:8: error: Methods must not throw generic exceptions (`java.lang.Error`) [GenericException] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:9: warning: Methods taking no arguments should throw `IllegalStateException` instead of `java.lang.IllegalArgumentException` [IllegalStateException] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:10: warning: Methods taking no arguments should throw `IllegalStateException` instead of `java.lang.NullPointerException` [IllegalStateException] [Rule S1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:11: error: Methods calling into system server should rethrow `RemoteException` as `RuntimeException` [RethrowRemoteException] [Rule FW9 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.os.RemoteException;
+
+ @SuppressWarnings("RedundantThrows")
+ public class MyClass {
+ public void method1() throws Exception { }
+ public void method2() throws Throwable { }
+ public void method3() throws Error { }
+ public void method4() throws IllegalArgumentException { }
+ public void method4() throws NullPointerException { }
+ public void method5() throws RemoteException { }
+ public void ok(int p) throws NullPointerException { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check no mentions of Google in APIs`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:4: error: Must never reference Google (`MyGoogleService`) [MentionsGoogle]
+ src/android/pkg/MyClass.java:5: error: Must never reference Google (`callGoogle`) [MentionsGoogle]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public static class MyGoogleService {
+ public void callGoogle() { }
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check no usages of heavy BitSet`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:6: error: Type must not be heavy BitSet (method android.pkg.MyClass.reverse(java.util.BitSet)) [HeavyBitSet]
+ src/android/pkg/MyClass.java:6: error: Type must not be heavy BitSet (parameter bitset in android.pkg.MyClass.reverse(java.util.BitSet bitset)) [HeavyBitSet]
+ src/android/pkg/MyClass.java:5: error: Type must not be heavy BitSet (field android.pkg.MyClass.bitset) [HeavyBitSet]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import java.util.BitSet;
+
+ public class MyClass {
+ public BitSet bitset;
+ public BitSet reverse(BitSet bitset) { return null; }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check Manager related issues`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyFirstManager.java:4: error: Managers must always be obtained from Context; no direct constructors [ManagerConstructor]
+ src/android/pkg/MyFirstManager.java:6: error: Managers must always be obtained from Context (`get`) [ManagerLookup]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyFirstManager {
+ public MyFirstManager() {
+ }
+ public MyFirstManager get() { return null; }
+ public MySecondManager ok() { return null; }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MySecondManager {
+ private MySecondManager() {
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check boxed types`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:6: error: Must avoid boxed primitives (`java.lang.Long`) [AutoBoxing] [Rule M11 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:8: error: Must avoid boxed primitives (`java.lang.Short`) [AutoBoxing] [Rule M11 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:8: error: Must avoid boxed primitives (`java.lang.Double`) [AutoBoxing] [Rule M11 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:4: error: Must avoid boxed primitives (`java.lang.Integer`) [AutoBoxing] [Rule M11 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public Integer integer1;
+ public int integer2;
+ public MyClass(Long l) {
+ }
+ public Short getDouble(Double l) { return null; }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check static utilities`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyUtils1.java:3: error: Fully-static utility classes must not have constructor [StaticUtils]
+ src/android/pkg/MyUtils2.java:3: error: Fully-static utility classes must not have constructor [StaticUtils]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyUtils1 {
+ // implicit constructor
+ public static void foo() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyUtils2 {
+ public MyUtils2() { }
+ public static void foo() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyUtils3 {
+ private MyUtils3() { }
+ public static void foo() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyUtils4 {
+ // OK: instance method
+ public void foo() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyUtils5 {
+ // OK: instance field
+ public int foo = 42;
+ public static void foo() { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check context first`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:10: error: Context is distinct, so it must be the first argument (method `wrong`) [ContextFirst] [Rule M3 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:11: error: ContentResolver is distinct, so it must be the first argument (method `wrong`) [ContextFirst] [Rule M3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.content.Context;
+ import android.content.ContentResolver;
+
+ public class MyClass {
+ public MyClass(Context context1, Context context2) {
+ }
+ public void ok(ContentResolver resolver, int i) { }
+ public void ok(Context context, int i) { }
+ public void wrong(int i, Context context) { }
+ public void wrong(int i, ContentResolver resolver) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check listener last`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT, ARG_HIDE, "ExecutorRegistration"),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:6: warning: Listeners should always be at end of argument list (method `MyClass`) [ListenerLast] [Rule M3 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:9: warning: Listeners should always be at end of argument list (method `wrong`) [ListenerLast] [Rule M3 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.pkg.MyCallback;
+ import android.content.Context;
+
+ public class MyClass {
+ public MyClass(MyCallback listener, int i) {
+ }
+ public void ok(Context context, int i, MyCallback listener) { }
+ public void wrong(MyCallback listener, int i) { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ @SuppressWarnings("WeakerAccess")
+ public abstract class MyCallback {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check overloaded arguments`() {
+ // TODO: This check is not yet hooked up
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ private MyClass() {
+ }
+
+ public void name1() { }
+ public void name1(int i) { }
+ public void name1(int i, int j) { }
+ public void name1(int i, int j, int k) { }
+ public void name1(int i, int j, int k, float f) { }
+
+ public void name2(int i) { }
+ public void name2(int i, int j) { }
+ public void name2(int i, float j, float k) { }
+ public void name2(int i, int j, int k, float f) { }
+ public void name2(int i, float f, int j) { }
+
+ public void name3() { }
+ public void name3(int i) { }
+ public void name3(int i, int j) { }
+ public void name3(int i, float j, int k) { }
+
+ public void name4(int i, int j, float f, long z) { }
+ public void name4(double d, int i, int j, float f) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check Callback Handlers`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:12: warning: Registration methods should have overload that accepts delivery Executor: `registerWrongCallback` [ExecutorRegistration] [Rule L1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:4: warning: Registration methods should have overload that accepts delivery Executor: `MyClass` [ExecutorRegistration] [Rule L1 in go/android-api-guidelines]
+ src/android/pkg/MyClass.java:9: warning: SAM-compatible parameters (such as parameter 1, "executor", in android.pkg.MyClass.registerStreamEventCallback) should be last to improve Kotlin interoperability; see https://kotlinlang.org/docs/reference/java-interop.html#sam-conversions [SamShouldBeLast]
+ src/android/pkg/MyClass.java:10: warning: SAM-compatible parameters (such as parameter 1, "executor", in android.pkg.MyClass.unregisterStreamEventCallback) should be last to improve Kotlin interoperability; see https://kotlinlang.org/docs/reference/java-interop.html#sam-conversions [SamShouldBeLast]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ public MyClass(MyCallback callback) {
+ }
+
+ public void registerStreamEventCallback(MyCallback callback);
+ public void unregisterStreamEventCallback(MyCallback callback);
+ public void registerStreamEventCallback(java.util.concurrent.Executor executor, MyCallback callback);
+ public void unregisterStreamEventCallback(java.util.concurrent.Executor executor, MyCallback callback);
+
+ public void registerWrongCallback(MyCallback callback);
+ public void unregisterWrongCallback(MyCallback callback);
+ }
+ """
+ ),
+ java(
+ """
+ package android.graphics;
+ import android.pkg.MyCallback;
+
+ public class MyUiClass {
+ public void registerWrongCallback(MyCallback callback);
+ public void unregisterWrongCallback(MyCallback callback);
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ @SuppressWarnings("WeakerAccess")
+ public abstract class MyCallback {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check resource names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/R.java:11: error: Expected resource name in `android.R.id` to be in the `fooBarBaz` style, was `wrong_style` [ResourceValueFieldName] [Rule C7 in go/android-api-guidelines]
+ src/android/R.java:17: error: Expected config name to be in the `config_fooBarBaz` style, was `config_wrong_config_style` [ConfigFieldName]
+ src/android/R.java:20: error: Expected resource name in `android.R.layout` to be in the `foo_bar_baz` style, was `wrongNameStyle` [ResourceFieldName]
+ src/android/R.java:31: error: Expected resource name in `android.R.style` to be in the `FooBar_Baz` style, was `wrong_style_name` [ResourceStyleFieldName] [Rule C7 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android;
+
+ public final class R {
+ public static final class id {
+ public static final int text = 7000;
+ public static final int config_fooBar = 7001;
+ public static final int layout_fooBar = 7002;
+ public static final int state_foo = 7003;
+ public static final int foo = 7004;
+ public static final int fooBar = 7005;
+ public static final int wrong_style = 7006;
+ }
+ public static final class layout {
+ public static final int text = 7000;
+ public static final int config_fooBar = 7001;
+ public static final int config_foo = 7002;
+ public static final int config_wrong_config_style = 7003;
+
+ public static final int ok_name_style = 7004;
+ public static final int wrongNameStyle = 7005;
+ }
+ public static final class style {
+ public static final int TextAppearance_Compat_Notification = 0x7f0c00ec;
+ public static final int TextAppearance_Compat_Notification_Info = 0x7f0c00ed;
+ public static final int TextAppearance_Compat_Notification_Line2 = 0x7f0c00ef;
+ public static final int TextAppearance_Compat_Notification_Time = 0x7f0c00f2;
+ public static final int TextAppearance_Compat_Notification_Title = 0x7f0c00f4;
+ public static final int Widget_Compat_NotificationActionContainer = 0x7f0c015d;
+ public static final int Widget_Compat_NotificationActionText = 0x7f0c015e;
+
+ public static final int wrong_style_name = 7000;
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check files`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/CheckFiles.java:12: warning: Methods accepting `File` should also accept `FileDescriptor` or streams: method android.pkg.CheckFiles.error(int,java.io.File) [StreamFiles] [Rule M10 in go/android-api-guidelines]
+ src/android/pkg/CheckFiles.java:8: warning: Methods accepting `File` should also accept `FileDescriptor` or streams: constructor android.pkg.CheckFiles(android.content.Context,java.io.File) [StreamFiles] [Rule M10 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.content.Context;
+ import android.content.ContentResolver;
+ import java.io.File;
+ import java.io.InputStream;
+
+ public class CheckFiles {
+ public MyClass(Context context, File file) {
+ }
+ public void ok(int i, File file) { }
+ public void ok(int i, InputStream stream) { }
+ public void error(int i, File file) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check parcelable lists`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/CheckFiles.java:12: warning: Methods accepting `File` should also accept `FileDescriptor` or streams: method android.pkg.CheckFiles.error(int,java.io.File) [StreamFiles] [Rule M10 in go/android-api-guidelines]
+ src/android/pkg/CheckFiles.java:8: warning: Methods accepting `File` should also accept `FileDescriptor` or streams: constructor android.pkg.CheckFiles(android.content.Context,java.io.File) [StreamFiles] [Rule M10 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.content.Context;
+ import android.content.ContentResolver;
+ import java.io.File;
+ import java.io.InputStream;
+
+ public class CheckFiles {
+ public MyClass(Context context, File file) {
+ }
+ public void ok(int i, File file) { }
+ public void ok(int i, InputStream stream) { }
+ public void error(int i, File file) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check abstract inner`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyManager.java:9: warning: Abstract inner classes should be static to improve testability: class android.pkg.MyManager.MyInnerManager [AbstractInner]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.content.Context;
+ import android.content.ContentResolver;
+ import java.io.File;
+ import java.io.InputStream;
+
+ public abstract class MyManager {
+ private MyManager() {}
+ public abstract class MyInnerManager {
+ private MyInnerManager() {}
+ }
+ public abstract static class MyOkManager {
+ private MyOkManager() {}
+ }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check for banned runtime exceptions`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:7: error: Methods must not mention RuntimeException subclasses in throws clauses (was `java.lang.SecurityException`) [BannedThrow]
+ src/android/pkg/MyClass.java:6: error: Methods must not mention RuntimeException subclasses in throws clauses (was `java.lang.ClassCastException`) [BannedThrow]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass {
+ private MyClass() throws NullPointerException {} // OK, private
+ @SuppressWarnings("RedundantThrows") public MyClass(int i) throws java.io.IOException {} // OK, not runtime exception
+ public MyClass(double l) throws ClassCastException {} // error
+ public void foo() throws SecurityException {} // error
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check for extending errors`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyClass.java:3: error: Trouble must be reported through an `Exception`, not an `Error` (`MyClass` extends `Error`) [ExtendsError]
+ src/android/pkg/MySomething.java:3: error: Exceptions must be named `FooException`, was `MySomething` [ExceptionName]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MyClass extends Error {
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MySomething extends RuntimeException {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check units and method names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT, ARG_HIDE, "NoByteOrShort"),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/UnitNameTest.java:5: error: Expected method name units to be `Hours`, was `Hr` in `getErrorHr` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:6: error: Expected method name units to be `Nanos`, was `Ns` in `getErrorNs` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:7: error: Expected method name units to be `Bytes`, was `Byte` in `getErrorByte` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:8: error: Returned time values are strongly encouraged to be in milliseconds unless you need the extra precision, was `getErrorNanos` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:9: error: Returned time values are strongly encouraged to be in milliseconds unless you need the extra precision, was `getErrorMicros` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:10: error: Returned time values must be in milliseconds, was `getErrorSeconds` [MethodNameUnits]
+ src/android/pkg/UnitNameTest.java:16: error: Fractions must use floats, was `int` in `getErrorFraction` [FractionFloat]
+ src/android/pkg/UnitNameTest.java:17: error: Fractions must use floats, was `int` in `setErrorFraction` [FractionFloat]
+ src/android/pkg/UnitNameTest.java:21: error: Percentage must use ints, was `float` in `getErrorPercentage` [PercentageInt]
+ src/android/pkg/UnitNameTest.java:22: error: Percentage must use ints, was `float` in `setErrorPercentage` [PercentageInt]
+ src/android/pkg/UnitNameTest.java:24: error: Expected method name units to be `Bytes`, was `Byte` in `readSingleByte` [MethodNameUnits]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class UnitNameTest {
+ public int okay() { return 0; }
+ public int getErrorHr() { return 0; }
+ public int getErrorNs() { return 0; }
+ public short getErrorByte() { return (short)0; }
+ public int getErrorNanos() { return 0; }
+ public long getErrorMicros() { return 0L; }
+ public long getErrorSeconds() { return 0L; }
+ public float getErrorSeconds() { return 0; }
+
+ public float getOkFraction() { return 0f; }
+ public void setOkFraction(float f) { }
+ public void setOkFraction(int n, int d) { }
+ public int getErrorFraction() { return 0; }
+ public void setErrorFraction(int i) { }
+
+ public int getOkPercentage() { return 0f; }
+ public void setOkPercentage(int i) { }
+ public float getErrorPercentage() { return 0f; }
+ public void setErrorPercentage(float f) { }
+
+ public int readSingleByte() { return 0; }
+
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check closeable`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyErrorClass1.java:3: warning: Classes that release resources should implement AutoClosable and CloseGuard: class android.pkg.MyErrorClass1 [NotCloseable]
+ src/android/pkg/MyErrorClass2.java:3: warning: Classes that release resources should implement AutoClosable and CloseGuard: class android.pkg.MyErrorClass2 [NotCloseable]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyOkClass1 implements java.io.Closeable {
+ public void close() {}
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ // Ok: indirectly implementing AutoCloseable
+ public abstract class MyOkClass2 implements MyInterface {
+ public void close() {}
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public class MyInterface extends AutoCloseable {
+ public void close() {}
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyErrorClass1 {
+ public void close() {}
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MyErrorClass2 {
+ public void shutdown() {}
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check Kotlin keywords`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/KotlinKeywordTest.java:7: error: Avoid method names that are Kotlin hard keywords ("fun"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
+ src/android/pkg/KotlinKeywordTest.java:8: error: Avoid field names that are Kotlin hard keywords ("as"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class KotlinKeywordTest {
+ public void okay();
+ public int okay = 0;
+
+ public void fun() {} // error
+ public int as = 0; // error
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check Kotlin operators`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/KotlinOperatorTest.java:4: warning: Method can be invoked with an indexing operator from Kotlin: `get` [KotlinOperator]
+ src/android/pkg/KotlinOperatorTest.java:5: warning: Method can be invoked with an indexing operator from Kotlin: `set` [KotlinOperator]
+ src/android/pkg/KotlinOperatorTest.java:6: warning: Method can be invoked with function call syntax from Kotlin: `invoke` [KotlinOperator]
+ src/android/pkg/KotlinOperatorTest.java:7: warning: Method can be invoked as a binary operator from Kotlin: `plus` [KotlinOperator]
+ src/android/pkg/KotlinOperatorTest.java:7: error: Only one of `plus` and `plusAssign` methods should be present for Kotlin [UniqueKotlinOperator]
+ src/android/pkg/KotlinOperatorTest.java:8: warning: Method can be invoked as a compound assignment operator from Kotlin: `plusAssign` [KotlinOperator]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class KotlinOperatorTest {
+ public int get(int i) { return i + 2; }
+ public void set(int i, int j, int k) { }
+ public void invoke(int i, int j, int k) { }
+ public int plus(JavaClass other) { return 0; }
+ public void plusAssign(JavaClass other) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Return collections instead of arrays`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT, ARG_HIDE, "AutoBoxing"),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/ArrayTest.java:7: warning: Method should return Collection<Object> (or subclass) instead of raw array; was `java.lang.Object[]` [ArrayReturn]
+ src/android/pkg/ArrayTest.java:8: warning: Method parameter should be Collection<Number> (or subclass) instead of raw array; was `java.lang.Number[]` [ArrayReturn]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class ArrayTest {
+ public int[] ok1() { return null; }
+ public String[] ok2() { return null; }
+ public void ok3(int[] i) { }
+ public Object[] error1() { return null; }
+ public void error2(Number[] i) { }
+ public void ok(Number... args) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check user handle names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MyManager.java:6: warning: When a method overload is needed to target a specific UserHandle, callers should be directed to use Context.createPackageContextAsUser() and re-obtain the relevant Manager, and no new API should be added [UserHandle]
+ src/android/pkg/UserHandleTest.java:7: warning: Method taking UserHandle should be named `doFooAsUser` or `queryFooForUser`, was `error` [UserHandleName]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+ import android.os.UserHandle;
+
+ public class UserHandleTest {
+ public void doFooAsUser(int i, UserHandle handle) {} //OK
+ public void doFooForUser(int i, UserHandle handle) {} //OK
+ public void error(int i, UserHandle handle) {}
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+ import android.os.UserHandle;
+
+ public class MyManager {
+ private MyManager() { }
+ public void error(int i, UserHandle handle) {}
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check parameters`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/FooOptions.java:3: warning: Classes holding a set of parameters should be called `FooParams`, was `FooOptions` [UserHandleName]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class FooOptions {
+ }
+ """
+ ),
+ java(
+ """
+ package android.app;
+
+ public class ActivityOptions {
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check service names`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/ServiceNameTest.java:6: error: Inconsistent service value; expected `OTHER`, was `something` [ServiceName] [Rule C4 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class ServiceNameTest {
+ private ServiceNameTest() { }
+ public static final String FOO_BAR_SERVICE = "foo_bar";
+ public static final String OTHER_SERVICE = "something";
+ public static final int NON_STRING_SERVICE = 42;
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check method name tense`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MethodNameTest.java:6: warning: Unexpected tense; probably meant `enabled`, was `fooEnable` [MethodNameTense]
+ src/android/pkg/MethodNameTest.java:7: warning: Unexpected tense; probably meant `enabled`, was `mustEnable` [MethodNameTense]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class MethodNameTest {
+ private MethodNameTest() { }
+ public void enable() { } // ok, not Enable
+ public void fooEnable() { } // warn
+ public boolean mustEnable() { return true; } // warn
+ public boolean isEnabled() { return true; }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check no clone`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/CloneTest.java:5: error: Provide an explicit copy constructor instead of implementing `clone()` [NoClone]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public class CloneTest {
+ public void clone(int i) { } // ok
+ public CloneTest clone() { return super.clone(); } // error
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check ICU types`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/IcuTest.java:4: warning: Type `java.util.TimeZone` should be replaced with richer ICU type `android.icu.util.TimeZone` [UseIcu]
+ src/android/pkg/IcuTest.java:5: warning: Type `java.text.BreakIterator` should be replaced with richer ICU type `android.icu.text.BreakIterator` [UseIcu]
+ src/android/pkg/IcuTest.java:5: warning: Type `java.text.Collator` should be replaced with richer ICU type `android.icu.text.Collator` [UseIcu]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class IcuTest {
+ public IcuTest(java.util.TimeZone timeZone) { }
+ public abstract java.text.BreakIterator foo(java.text.Collator collator);
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check using parcel file descriptors`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/PdfTest.java:4: error: Must use ParcelFileDescriptor instead of FileDescriptor in parameter fd in android.pkg.PdfTest.error1(java.io.FileDescriptor fd) [NoClone]
+ src/android/pkg/PdfTest.java:5: error: Must use ParcelFileDescriptor instead of FileDescriptor in method android.pkg.PdfTest.getFileDescriptor() [UseParcelFileDescriptor] [Rule FW11 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class PdfTest {
+ public void error1(java.io.FileDescriptor fd) { }
+ public int getFileDescriptor() { return -1; }
+ public void ok(android.os.ParcelFileDescriptor fd) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check using bytes and shorts`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/ByteTest.java:4: warning: Should avoid odd sized primitives; use `int` instead of `byte` in parameter b in android.pkg.ByteTest.error1(byte b) [NoByteOrShort] [Rule FW12 in go/android-api-guidelines]
+ src/android/pkg/ByteTest.java:5: warning: Should avoid odd sized primitives; use `int` instead of `short` in parameter s in android.pkg.ByteTest.error2(short s) [NoByteOrShort] [Rule FW12 in go/android-api-guidelines]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class ByteTest {
+ public void error1(byte b) { }
+ public void error2(short s) { }
+ }
+ """
+ )
+ )
+ )
+ }
+
+ @Test
+ fun `Check singleton constructors`() {
+ check(
+ extraArguments = arrayOf(ARG_API_LINT),
+ compatibilityMode = false,
+ warnings = """
+ src/android/pkg/MySingleton.java:5: error: Singleton classes should use `getInstance()` methods: `MySingleton` [SingletonConstructor]
+ """,
+ sourceFiles = *arrayOf(
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MySingleton {
+ public static MySingleton getMyInstance() { return null; }
+ public MySingleton() { }
+ public void foo() { }
+ }
+ """
+ ),
+ java(
+ """
+ package android.pkg;
+
+ public abstract class MySingleton2 {
+ public static MySingleton2 getMyInstance() { return null; }
+ private MySingleton2() { } // OK, private
+ public void foo() { }
+ }
+ """
+ )
+ )
+ )
+ }
+}
diff --git a/src/test/java/com/android/tools/metalava/KotlinInteropChecksTest.kt b/src/test/java/com/android/tools/metalava/KotlinInteropChecksTest.kt
index 2ef7aaf..f3efebc 100644
--- a/src/test/java/com/android/tools/metalava/KotlinInteropChecksTest.kt
+++ b/src/test/java/com/android/tools/metalava/KotlinInteropChecksTest.kt
@@ -24,9 +24,9 @@
check(
extraArguments = arrayOf(ARG_CHECK_KOTLIN_INTEROP),
warnings = """
- src/test/pkg/Test.java:5: warning: Avoid method names that are Kotlin hard keywords ("fun"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
- src/test/pkg/Test.java:6: warning: Avoid parameter names that are Kotlin hard keywords ("typealias"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
- src/test/pkg/Test.java:7: warning: Avoid field names that are Kotlin hard keywords ("object"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
+ src/test/pkg/Test.java:5: error: Avoid method names that are Kotlin hard keywords ("fun"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
+ src/test/pkg/Test.java:6: error: Avoid parameter names that are Kotlin hard keywords ("typealias"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
+ src/test/pkg/Test.java:7: error: Avoid field names that are Kotlin hard keywords ("object"); see https://android.github.io/kotlin-guides/interop.html#no-hard-keywords [KotlinKeyword]
""",
sourceFiles = *arrayOf(
java(
diff --git a/src/test/java/com/android/tools/metalava/OptionsTest.kt b/src/test/java/com/android/tools/metalava/OptionsTest.kt
index db70cee..8dfcedc 100644
--- a/src/test/java/com/android/tools/metalava/OptionsTest.kt
+++ b/src/test/java/com/android/tools/metalava/OptionsTest.kt
@@ -201,6 +201,7 @@
to check the code base against the current
public API, use
--check-compatibility:api:current.
+--api-lint Check API for Android API best practices
--check-kotlin-interop Check API intended to be used from both Kotlin
and Java for interoperability issues
--migrate-nullness <api file> Compare nullness information with the previous
