mozilla-mobile / gradle-apilint Goto Github PK

Integrating API lint into Mozilla's Phabricator CI (see Bug 1512487) comes with some requirements. Fundamentally, Phab can only display lint issues as comments anchored to a file and a line, and then only when the file and line are actually part of the commit in question.

Can we teach API lint to report the file and line of changed signatures? I feel like that information is probably already present but not captured in output by default, but I know there are layers of APIs in between the sources and the outputs, and maybe the source mapping is lost too early to achieve this.

As of January 1 2019, Mozilla requires that all GitHub projects include this CODE_OF_CONDUCT.md file in the project root. The file has two parts:

1. Required Text - All text under the headings Community Participation Guidelines and How to Report, are required, and should not be altered.
2. Optional Text - The Project Specific Etiquette heading provides a space to speak more specifically about ways people can work effectively and inclusively together. Some examples of those can be found on the Firefox Debugger project, and Common Voice. (The optional part is commented out in the raw template file, and will not be visible until you modify and uncomment that part.)

If you have any questions about this file, or Code of Conduct policies and procedures, please see Mozilla-GitHub-Standards or email [email protected].

(Message COC001)

This is the tracking issue to support kotlin in apilint.

We need a new mode in api-lint to enforce that only APIs that have been deprecated for a certain amount of time can be removed.

This way we can add BuildConfig to the classpath and avoid adding it to the api at the same time. See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1521404

Right now default methods are completely ignored, it would be nice if they were exposed to apilint.

MethodDoc#isDefault exposes whether a method has a default impl or not to the javadoc.

That way the check command would automatically run apiLint.

Annotations are part of methods identity in apilint, which causes apilint to not recognize when annotations are added or removed and instead causing apilint to think that the method has been removed giving a confusing error.

E.g. apilint gives this error:

Error: Method removed or incompatible change
    in method public org.mozilla.geckoview.GeckoResult<org.mozilla.gecko.util.GeckoBundle> getSnapshots(boolean)
    in public final class RuntimeTelemetry
    in package org.mozilla.geckoview
    at line 846

for this change

   public final class RuntimeTelemetry {
-    method public org.mozilla.geckoview.GeckoResult<org.mozilla.gecko.util.GeckoBundle> getSnapshots(boolean);
+    method @android.support.annotation.AnyThread public org.mozilla.geckoview.GeckoResult<org.mozilla.gecko.util.GeckoBundle> getSnapshots(boolean);
   }

While really what we would want is "Annotation added" which should probably not be a compat issue.

E.g. from GeckoView:

> Configure project :geckoview
WARNING: API 'variant.getGenerateBuildConfig()' is obsolete and has been replaced with 'variant.getGenerateBuildConfigProvider()'.
It will be removed at the end of 2019.
For more information, see https://d.android.com/r/tools/task-configuration-avoidance.
To determine what is calling variant.getGenerateBuildConfig(), use -Pandroid.debug.obsoleteApi=true on the command line to display more information.
WARNING: API 'variant.getAidlCompile()' is obsolete and has been replaced with 'variant.getAidlCompileProvider()'.
It will be removed at the end of 2019.
For more information, see https://d.android.com/r/tools/task-configuration-avoidance.
To determine what is calling variant.getAidlCompile(), use -Pandroid.debug.obsoleteApi=true on the command line to display more information.
WARNING: API 'variant.getPackageLibrary()' is obsolete and has been replaced with 'variant.getPackageLibraryProvider()'.
It will be removed at the end of 2019.
For more information, see https://d.android.com/r/tools/task-configuration-avoidance.
To determine what is calling variant.getPackageLibrary(), use -Pandroid.debug.obsoleteApi=true on the command line to display more information.

For automation, it really helps having a json result output that we can parse.

This is annoying because it's not clear to the user what is changing in the api that needs updating.

Maybe all annotations lint don't? We should fix this.

This helps every library that wants to be kotlin-friendly.

e.g. something like

public class TestClass<T> {
    public static class TestNestedClass {
        public T getThing();
    }
}

results in:

Error GV7: Class T is not allowed. Allowed packages: test.*.
    in method public T getThing()
    in public class TestClass.TestNestedClass
    in package test
    at line api.txt:16:0

We check that a field with @deprecated has @DeprecationSchedule but not the other way around.

We need a lint that makes sure that every API has a threading annotation like @UiThread, @AnyThread etc.

To properly inspect annotations in lints we need to know what annotations are on super classes and super methods, but there is no way to do that right now. We should inspect the super members and add their annotations to the overridden methods and classes.

Right now the plugin only supports Android libraries, we should add support for android applications too.

mozilla-central is migrating to python3 so we should just use it by default.

e.g. these two will pass even though not_allowed is not an allowed package:

public class TestClass extends not_allowed.TestA {

public class TestClass implements not_allowed.InterfaceA {

Java 9 introduced a non-backward compatible doclet API. We should support it in apidoc.

More Info: https://docs.oracle.com/javase/9/docs/api/jdk/javadoc/doclet/package-summary.html

This is because we add a dependency to apidoc-plugin without checking if we already have one.

> Cannot add a configuration with name 'apidoc-plugin' as a configuration with that name already exists.

We should build a lint to makefield-only classes like the following:

  public static class GeckoSession.NavigationDelegate.LoadRequest {
    field public final boolean isUserTriggered;
    field public final int target; 
    field public final java.lang.String triggerUri;
    field public final java.lang.String uri;
  }

have a protected or public constructor in order to let embedders mock (or construct) these classes in tests.

We need support for banning certain packages in the API, e.g. in GeckoView we don't want any object from the package org.mozilla.gecko in the API.

We need to integrate with a maven publishing service so we can publish the library to plugins.gradle.org.

apidoc generates an incorrect outline for Annotation, e.g.:

public static interface class Annotation implements java.lang.annotation.Annotation {

Notice the interface class part. The correct one would be something like:

public static interface Annotation implements java.lang.annotation.Annotation {

We should add support for thread annotations to make sure we track that in the api.

We could simply add @UiThread, @anythread, and any other relevant annotation to the API in api.txt, e.g.

    @UiThread public void onlyCallOnUiThread();

and then make sure that the apilint.py script picks these annotations and errors out if they change between revisions, similar to what it already does for arguments.

This should pass the lint but it doesn't:

        final class Type {
            private Type() {};
            final public static long CACHE = 1 << 0;
            final public static long COOKIES = 1 << 1;
            final public static long DOWNLOADS = 1 << 2;
            final public static long FORM_DATA = 1 << 3;
            final public static long HISTORY = 1 << 4;
            final public static long LOCAL_STORAGE = 1 << 5;
            final public static long PASSWORDS = 1 << 6;
        }   

Generics are ignored in apidoc, we should change that.

It would be nice to have a built-in way of enforcing a hand-written changelog for the API every time the api.txt file changes.

And probably a lint to make sure that every API has either of the annotation on it. This is especially useful for libraries that work with Kotlin consumers

@ncalexan brought this up. To make the api file more stable we should sort all members by name. We already sort classes but not packages and class members.

see https://bugzilla.mozilla.org/show_bug.cgi?id=1631603

looks like we're accessing an invalid index:

Traceback (most recent call last):
  File "/var/folders/n8/xgc43vqs3zbdgn8ccxjxr6v00000gn/T/script-7402692432473735778.py", line 1957, in <module>
    prev_fail, prev_noticed, prev = examine_stream(f, api_map)
  File "/var/folders/n8/xgc43vqs3zbdgn8ccxjxr6v00000gn/T/script-7402692432473735778.py", line 1702, in examine_stream
    api = _parse_stream(stream, api_map, examine_clazz)
  File "/var/folders/n8/xgc43vqs3zbdgn8ccxjxr6v00000gn/T/script-7402692432473735778.py", line 380, in _parse_stream
    location = read_map(api_map, line)
  File "/var/folders/n8/xgc43vqs3zbdgn8ccxjxr6v00000gn/T/script-7402692432473735778.py", line 459, in read_map
    mapString = api_map[lineNumber-1].strip()
IndexError: list index out of range

Just noticed: apilint should make sure that all types in the API are accessible (e.g. using a private type in a method argument should not be allowed).

It would be nice if we could have a way to automatically fix a git conflict in the CHANGELOG for the [api-version] parameter.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1685390#c0

Enum classes show up like normal classes:

public final class AllowOrDeny extends java.lang.Enum<E extends java.lang.Enum<E>> {

we probably want something more like

public enum AllowOrDeny {

Just noticed there's a regression in 0.5 where an overridden method that upgrades visibility is missed in the api.txt:

Test case:

class TestClass {
    public  class TestPackageProtected {
        private TestPackageProtected() {}
        // This shouldn't appear in the API
        /* package */ void testPackageProtected();
    }

    public static class TestOverrideNonVisibleApi extends TestPackageProtected {
        private TestOverrideNonVisibleApi() {}
        // This should appear in the API
        @Override
        public void testPackageProtected();
    }
 }

Which results in:

   public static class TestClass.TestOverrideNonVisibleApi extends TestClass.TestPackageProtected {
   }
 
   public static class TestClass.TestPackageProtected {
   }

instead of the expected

   public static class TestClass.TestOverrideNonVisibleApi extends TestClass.TestPackageProtected {
     method public void testPackageProtected();
   }
 
   public static class TestClass.TestPackageProtected {
   }

Motivated by: https://bugzilla.mozilla.org/show_bug.cgi?id=1571326

CHANGELOG-related errors do not appear in the json report.

Currently we require classes to not be final and to have a public or protected constructor so that they can be mocked; for field-only classes this is an unnecessary requirement, as there's no situation where mocking should be necessary.

We should have a simple project that uses apilint as example. I think this could work as an end-to-end integration test too.

These @interface enums should never appear in the API as they can't be used anywhere. We should have a lint for that.

In the light of jcenter being sunsetted (see here https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/) it has to be published in Maven.

My understanding is, after March 31, 2021, it will be impossible to publish updates for apilint in jcenter, and after May 1, 2021, the package will be potentially unusable.

Reference: https://phabricator.services.mozilla.com/D146245#inline-808465

When the return type is a non-null GeckoResult<> we should catch that with the linter.

apidoc right now includes overridden methods as part of the API. This is not necessary as overridden methods are implicitly part of the API as part of the "extends" clause.

We need a way to select the lints that run as some of them are very specific to some applications.

Some wanted features:

• Select list of lints that run (right now everything runs) or maybe a blacklist
• Select list of lints that run and fail the build (right now none of the lints do this)
• Have a whitelist for classes that fail the lint

We can probably have a apilint.json configuration file somewhere to store all of those.