fix(android): correct lint issues for various API mis-use

Highlights: - enforce CallSuper so TaskExecutors will always clean up correctly - enforce Locale translation in string manipulaton (Fixes #3917)
invertase · May 18, 2021 · eb8d893 · eb8d893
1 parent 70aba4b
commit eb8d893
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 17 deletions.
diff --git a/.../android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java b/.../android/src/reactnative/java/io/invertase/firebase/common/ReactNativeFirebaseModule.java
@@ -19,15 +19,18 @@
 
 import android.app.Activity;
 import android.content.Context;
-import com.facebook.react.bridge.*;
-import io.invertase.firebase.interfaces.ContextProvider;
-import io.invertase.firebase.common.TaskExecutorService;
+import androidx.annotation.CallSuper;
+import androidx.annotation.NonNull;
 
-import javax.annotation.Nonnull;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
 
+import com.facebook.react.bridge.*;
+
+import io.invertase.firebase.interfaces.ContextProvider;
+import io.invertase.firebase.common.TaskExecutorService;
+
 public class ReactNativeFirebaseModule extends ReactContextBaseJavaModule implements ContextProvider {
   private final TaskExecutorService executorService;
 
@@ -67,6 +70,7 @@ public static void rejectPromiseWithCodeAndMessage(
   }
 
   @Override
+  @CallSuper
   public void initialize() {
     super.initialize();
   }
@@ -75,24 +79,25 @@ public ReactContext getContext() {
     return getReactApplicationContext();
   }
 
-  public ExecutorService getExecutor() {
+  public final ExecutorService getExecutor() {
     return executorService.getExecutor();
   }
 
-  public ExecutorService getTransactionalExecutor() {
+  public final ExecutorService getTransactionalExecutor() {
     return executorService.getTransactionalExecutor();
   }
 
-  public ExecutorService getTransactionalExecutor(String identifier) {
+  public final ExecutorService getTransactionalExecutor(String identifier) {
     return executorService.getTransactionalExecutor(identifier);
   }
 
   @Override
+  @CallSuper
   public void onCatalystInstanceDestroy() {
     executorService.shutdown();
   }
 
-  public void removeEventListeningExecutor(String identifier) {
+  public final void removeEventListeningExecutor(String identifier) {
     String executorName = executorService.getExecutorName(true, identifier);
     executorService.removeExecutor(executorName);
   }
@@ -105,12 +110,13 @@ public Activity getActivity() {
     return getCurrentActivity();
   }
 
-  @Nonnull
+  @NonNull
   @Override
   public String getName() {
     return "RNFB" + moduleName + "Module";
   }
 
+  @NonNull
   @Override
   public Map<String, Object> getConstants() {
     return new HashMap<>();

diff --git a/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java b/packages/app/android/src/reactnative/java/io/invertase/firebase/common/SharedUtils.java
@@ -99,7 +99,7 @@ public static WritableMap getExceptionMap(Exception exception) {
 
   public static String timestampToUTC(long timestamp) {
     long millisTimestamp = timestamp * 1000;
-    SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'");
+    SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'", Locale.ROOT);
     format.setTimeZone(TimeZone.getTimeZone("UTC"));
     return format.format(millisTimestamp);
   }

diff --git a/.../auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java b/.../auth/android/src/main/java/io/invertase/firebase/auth/ReactNativeFirebaseAuthModule.java
@@ -61,6 +61,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
@@ -1885,7 +1886,7 @@ private WritableMap getJSError(Exception exception) {
     }
 
     code = code
-      .toLowerCase()
+      .toLowerCase(Locale.ROOT)
       .replace("error_", "")
       .replace('_', '-');
     error.putString("code", code);

diff --git a/...android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java b/...android/src/main/java/io/invertase/firebase/database/UniversalFirebaseDatabaseCommon.java
@@ -95,7 +95,7 @@ static void addEmulatorConfig(String appName, String dbURL, String host, int por
     String configKey = appName + dbURL;
     HashMap<String, Object> emulatorConfig = new HashMap<>();
     emulatorConfig.put("host", host);
-    emulatorConfig.put("port", new Integer(port));
+    emulatorConfig.put("port", Integer.valueOf(port));
     emulatorConfigs.put(configKey, emulatorConfig);
   }
 

diff --git a/...c/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java b/...c/main/java/io/invertase/firebase/dynamiclinks/ReactNativeFirebaseDynamicLinksModule.java
@@ -72,6 +72,7 @@ public class ReactNativeFirebaseDynamicLinksModule extends ReactNativeFirebaseMo
   public void onCatalystInstanceDestroy() {
     getReactApplicationContext().removeActivityEventListener(this);
     getReactApplicationContext().addLifecycleEventListener(this);
+    super.onCatalystInstanceDestroy();
   }
 
   @ReactMethod

diff --git a/...e/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java b/...e/java/io/invertase/firebase/firestore/ReactNativeFirebaseFirestoreTransactionModule.java
@@ -56,6 +56,7 @@ public void onCatalystInstanceDestroy() {
     }
 
     transactionHandlers.clear();
+    super.onCatalystInstanceDestroy();
   }
 
   @ReactMethod

diff --git a/...android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java b/...android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageCommon.java
@@ -35,6 +35,7 @@
 import com.google.firebase.storage.StorageReference;
 import com.google.firebase.storage.StorageTask;
 
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 
@@ -143,7 +144,7 @@ static StorageMetadata buildMetadataFromMap(ReadableMap metadataMap, @Nullable U
 
       if (mimeType == null) {
         String fileExt = MimeTypeMap.getFileExtensionFromUrl(file.toString());
-        mimeType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(fileExt.toLowerCase());
+        mimeType = MimeTypeMap.getSingleton().getMimeTypeFromExtension(fileExt.toLowerCase(Locale.ROOT));
       }
 
       if (mimeType != null) {

diff --git a/...android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java b/...android/src/main/java/io/invertase/firebase/storage/ReactNativeFirebaseStorageModule.java
@@ -54,6 +54,7 @@ public class ReactNativeFirebaseStorageModule extends ReactNativeFirebaseModule
   @Override
   public void onCatalystInstanceDestroy() {
     ReactNativeFirebaseStorageTask.destroyAllTasks();
+    super.onCatalystInstanceDestroy();
   }
 
   /**

diff --git a/tests/android/build.gradle b/tests/android/build.gradle
@@ -106,6 +106,7 @@ subprojects {
           abortOnError = true
           warningsAsErrors false
           lintConfig file('./lint.xml')
+          ignore 'UnknownNullness', 'SyntheticAccessor', 'LogConditional'
           checkReleaseBuilds = true
           checkAllWarnings true
           showAll true

diff --git a/tests/android/lint.xml b/tests/android/lint.xml
@@ -21,13 +21,15 @@
        <issue id="NewIssueId" severity="ignore"/>
     -->
 <lint>
+  <issue id="DefaultLocale" severity="fatal" />
   <issue id="NewApi" severity="fatal" />
   <issue id="HardcodedText" severity="fatal" />
   <issue id="InlinedApi" severity="fatal" />
+  <issue id="MissingSuperCall" severity="ignore" />
   <issue id="StringFormatCount" severity="fatal" />
   <issue id="StringFormatMatches" severity="fatal" />
   <issue id="StringFormatInvalid" severity="fatal" />
-  <issue id="UnusedResources" severity="ignore" />
+  <issue id="UseValueOf" severity="fatal" />
   <issue id="ViewConstructor" severity="fatal" />
   <issue id="ViewHolder" severity="fatal" />
   <issue id="ViewTag" severity="fatal" />
@@ -42,6 +44,7 @@
   <issue id="RtlSymmetry" severity="ignore" />
 
   <!-- <issue id="UseCompatTextViewDrawableXml" severity="ignore" /> -->
+  <issue id="UnusedResources" severity="ignore" />
   <issue id="UnknownNullness" severity="ignore" />
   <issue id="SyntheticAccessor" severity="ignore" />
   <!-- FIXME need to remove WRITE_EXTERNAL_STORAGE from storage -->
@@ -91,7 +94,6 @@
   <issue id="ButtonOrder" severity="ignore" />
   <issue id="ButtonStyle" severity="ignore" />
   <issue id="ByteOrderMark" severity="ignore" />
-  <issue id="MissingSuperCall" severity="ignore" />
   <issue id="AdapterViewChildren" severity="ignore" />
   <issue id="ScrollViewCount" severity="ignore" />
   <issue id="PermissionImpliesUnsupportedChromeOsHardware" severity="ignore" />
@@ -183,15 +185,13 @@
   <issue id="InvalidPackage" severity="ignore" />
   <issue id="DrawAllocation" severity="ignore" />
   <issue id="UseSparseArrays" severity="ignore" />
-  <issue id="UseValueOf" severity="ignore" />
   <issue id="JavascriptInterface" severity="ignore" />
   <issue id="JobSchedulerService" severity="ignore" />
   <issue id="KeyboardInaccessibleWidget" severity="ignore" />
   <issue id="LabelFor" severity="ignore" />
   <issue id="InconsistentLayout" severity="ignore" />
   <issue id="InflateParams" severity="ignore" />
   <issue id="StaticFieldLeak" severity="ignore" />
-  <issue id="DefaultLocale" severity="ignore" />
   <issue id="LocaleFolder" severity="ignore" />
   <issue id="GetLocales" severity="ignore" />
   <issue id="InvalidResourceFolder" severity="ignore" />
-Original file line number
+Diff line change
@@ Expand Up / @@ -56,6 +56,7 @@ public void onCatalystInstanceDestroy() { @@
         }
         transactionHandlers.clear();
+        super.onCatalystInstanceDestroy();
       }
       @ReactMethod
@@ Expand Down @@