Merge pull request #549 from permissions-dispatcher/issues_436

compile time validation for detecting OnNeverAskAgain with special permissions
permissions-dispatcher · Dec 3, 2018 · 7ca6bd9 · 7ca6bd9
2 parents 3d063cc + 8a0fa29
commit 7ca6bd9
Show file tree

Hide file tree

Showing 13 changed files with 117 additions and 137 deletions.
diff --git a/processor/src/main/kotlin/permissions/dispatcher/processor/RuntimePermissionsElement.kt b/processor/src/main/kotlin/permissions/dispatcher/processor/RuntimePermissionsElement.kt
@@ -8,19 +8,7 @@ import permissions.dispatcher.NeedsPermission
 import permissions.dispatcher.OnNeverAskAgain
 import permissions.dispatcher.OnPermissionDenied
 import permissions.dispatcher.OnShowRationale
-import permissions.dispatcher.processor.util.GEN_CLASS_SUFFIX
-import permissions.dispatcher.processor.util.checkDuplicatedMethodName
-import permissions.dispatcher.processor.util.checkDuplicatedValue
-import permissions.dispatcher.processor.util.checkMethodParameters
-import permissions.dispatcher.processor.util.checkMethodSignature
-import permissions.dispatcher.processor.util.checkMixPermissionType
-import permissions.dispatcher.processor.util.checkNotEmpty
-import permissions.dispatcher.processor.util.checkPrivateMethods
-import permissions.dispatcher.processor.util.childElementsAnnotatedWith
-import permissions.dispatcher.processor.util.findMatchingMethodForNeeds
-import permissions.dispatcher.processor.util.packageName
-import permissions.dispatcher.processor.util.simpleString
-import permissions.dispatcher.processor.util.typeMirrorOf
+import permissions.dispatcher.processor.util.*
 import javax.lang.model.element.ExecutableElement
 import javax.lang.model.element.TypeElement
 
@@ -73,6 +61,7 @@ class RuntimePermissionsElement(e: TypeElement) {
         checkPrivateMethods(onNeverAskElements, OnNeverAskAgain::class.java)
         checkMethodSignature(onNeverAskElements)
         checkMethodParameters(onNeverAskElements, 0)
+        checkSpecialPermissionsWithNeverAskAgain(onNeverAskElements)
     }
 
     /* Begin public */

diff --git a/...ermissions/dispatcher/processor/exception/SpecialPermissionsWithNeverAskAgainException.kt b/...ermissions/dispatcher/processor/exception/SpecialPermissionsWithNeverAskAgainException.kt
@@ -0,0 +1,3 @@
+package permissions.dispatcher.processor.exception
+
+class SpecialPermissionsWithNeverAskAgainException : RuntimeException("'@NeverAskAgain' annotated method never being called with 'WRITE_SETTINGS' or 'SYSTEM_ALERT_WINDOW' permission.")
diff --git a/processor/src/main/kotlin/permissions/dispatcher/processor/util/Validators.kt b/processor/src/main/kotlin/permissions/dispatcher/processor/util/Validators.kt
@@ -1,18 +1,10 @@
 package permissions.dispatcher.processor.util
 
+import permissions.dispatcher.OnNeverAskAgain
 import permissions.dispatcher.processor.ProcessorUnit
 import permissions.dispatcher.processor.RuntimePermissionsElement
 import permissions.dispatcher.processor.TYPE_UTILS
-import permissions.dispatcher.processor.exception.DuplicatedMethodNameException
-import permissions.dispatcher.processor.exception.DuplicatedValueException
-import permissions.dispatcher.processor.exception.MixPermissionTypeException
-import permissions.dispatcher.processor.exception.NoAnnotatedMethodsException
-import permissions.dispatcher.processor.exception.NoParametersAllowedException
-import permissions.dispatcher.processor.exception.NoThrowsAllowedException
-import permissions.dispatcher.processor.exception.PrivateMethodException
-import permissions.dispatcher.processor.exception.WrongClassException
-import permissions.dispatcher.processor.exception.WrongParametersException
-import permissions.dispatcher.processor.exception.WrongReturnTypeException
+import permissions.dispatcher.processor.exception.*
 import java.util.*
 import javax.lang.model.element.Element
 import javax.lang.model.element.ExecutableElement
@@ -27,7 +19,7 @@ private const val SYSTEM_ALERT_WINDOW = "android.permission.SYSTEM_ALERT_WINDOW"
  * Obtains the [ProcessorUnit] implementation for the provided element.
  * Raises an exception if no suitable implementation exists
  */
-fun <K> findAndValidateProcessorUnit(units: List<ProcessorUnit<K>>, element: Element) : ProcessorUnit<K> {
+fun <K> findAndValidateProcessorUnit(units: List<ProcessorUnit<K>>, element: Element): ProcessorUnit<K> {
     val type = element.asType()
     try {
         return units.first { type.isSubtypeOf(it.getTargetType()) }
@@ -131,6 +123,15 @@ fun <A : Annotation> checkMixPermissionType(items: List<ExecutableElement>, anno
     }
 }
 
+fun checkSpecialPermissionsWithNeverAskAgain(items: List<ExecutableElement>, annotationClass: Class<OnNeverAskAgain> = OnNeverAskAgain::class.java) {
+    items.forEach {
+        val permissionValue = it.getAnnotation(annotationClass).permissionValue()
+        if (permissionValue.contains(WRITE_SETTINGS) || permissionValue.contains(SYSTEM_ALERT_WINDOW)) {
+            throw SpecialPermissionsWithNeverAskAgainException()
+        }
+    }
+}
+
 fun checkDuplicatedMethodName(items: List<ExecutableElement>) {
     items.forEach { item ->
         items.firstOrNull { it != item && it.simpleName == item.simpleName }?.let {

diff --git a/processor/src/test/java/permissions/dispatcher/processor/ProcessorTestSuite.java b/processor/src/test/java/permissions/dispatcher/processor/ProcessorTestSuite.java
@@ -131,6 +131,16 @@ public class ProcessorTestSuite extends TestSuite {
         assertJavaSource(Source.needsPermissionMethodOverloadFragment);
     }
 
+    @Test public void systemAlertWindowWithOnNeverAskAgain() {
+        expectRuntimeException("'@NeverAskAgain' annotated method never being called with 'WRITE_SETTINGS' or 'SYSTEM_ALERT_WINDOW' permission.");
+        assertJavaSource(Source.systemAlertWindowWithOnNeverAskAgain);
+    }
+
+    @Test public void writeSettingsWithOnNeverAskAgain() {
+        expectRuntimeException("'@NeverAskAgain' annotated method never being called with 'WRITE_SETTINGS' or 'SYSTEM_ALERT_WINDOW' permission.");
+        assertJavaSource(Source.writeSettingsWithOnNeverAskAgain);
+    }
+
     @Test public void methodOverloadWithoutNeedsPermission() {
         assertJavaSource(Source.methodOverloadWithoutNeedsPermission);
     }

diff --git a/processor/src/test/java/permissions/dispatcher/processor/data/Source.java b/processor/src/test/java/permissions/dispatcher/processor/data/Source.java
@@ -4963,9 +4963,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.WRITE_SETTINGS)",
                     "    void writeSettingOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.WRITE_SETTINGS)",
-                    "    void writeSettingOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5006,11 +5003,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target.requireActivity(), PERMISSION_WRITESETTINGS) || Settings.System.canWrite(target.requireActivity())) {",
                     "        target.writeSettings();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_WRITESETTINGS)) {",
-                    "          target.writeSettingOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.writeSettingOnPermissionDenied();",
-                    "        }",
+                    "        target.writeSettingOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -5070,9 +5063,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.SYSTEM_ALERT_WINDOW)",
                     "    void systemAlertWindowOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)",
-                    "    void systemAlertWindowOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5113,11 +5103,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target.requireActivity(), PERMISSION_SYSTEMALERTWINDOW) || Settings.canDrawOverlays(target.requireActivity())) {",
                     "        target.systemAlertWindow();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_SYSTEMALERTWINDOW)) {",
-                    "          target.systemAlertWindowOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.systemAlertWindowOnPermissionDenied();",
-                    "        }",
+                    "        target.systemAlertWindowOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -5177,9 +5163,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.WRITE_SETTINGS)",
                     "    void writeSettingsOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.WRITE_SETTINGS)",
-                    "    void writeSettingsOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5220,11 +5203,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target, PERMISSION_WRITESETTINGS) || Settings.System.canWrite(target)) {",
                     "        target.writeSettings();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_WRITESETTINGS)) {",
-                    "          target.writeSettingsOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.writeSettingsOnPermissionDenied();",
-                    "        }",
+                    "        target.writeSettingsOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -5284,9 +5263,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.SYSTEM_ALERT_WINDOW)",
                     "    void systemAlertWindowOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)",
-                    "    void systemAlertWindowOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5327,11 +5303,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target, PERMISSION_SYSTEMALERTWINDOW) || Settings.canDrawOverlays(target)) {",
                     "        target.systemAlertWindow();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_SYSTEMALERTWINDOW)) {",
-                    "          target.systemAlertWindowOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.systemAlertWindowOnPermissionDenied();",
-                    "        }",
+                    "        target.systemAlertWindowOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -5393,9 +5365,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.SYSTEM_ALERT_WINDOW)",
                     "    void systemAlertWindowOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)",
-                    "    void systemAlertWindowOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5439,11 +5408,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target.requireActivity(), PERMISSION_SYSTEMALERTWINDOW) || Settings.canDrawOverlays(target.requireActivity())) {",
                     "        target.systemAlertWindow();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_SYSTEMALERTWINDOW)) {",
-                    "          target.systemAlertWindowOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.systemAlertWindowOnPermissionDenied();",
-                    "        }",
+                    "        target.systemAlertWindowOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -5505,9 +5470,6 @@ protected String[] getActualSource() {
                     "    @OnPermissionDenied(Manifest.permission.SYSTEM_ALERT_WINDOW)",
                     "    void systemAlertWindowOnPermissionDenied() {",
                     "    }",
-                    "    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)",
-                    "    void systemAlertWindowOnNeverAskAgain() {",
-                    "    }",
                     "}"
             };
         }
@@ -5551,11 +5513,7 @@ protected String[] getExpectSource() {
                     "      if (PermissionUtils.hasSelfPermissions(target, PERMISSION_SYSTEMALERTWINDOW) || Settings.canDrawOverlays(target)) {",
                     "        target.systemAlertWindow();",
                     "      } else {",
-                    "        if (!PermissionUtils.shouldShowRequestPermissionRationale(target, PERMISSION_SYSTEMALERTWINDOW)) {",
-                    "          target.systemAlertWindowOnNeverAskAgain();",
-                    "        } else {",
-                    "          target.systemAlertWindowOnPermissionDenied();",
-                    "        }",
+                    "        target.systemAlertWindowOnPermissionDenied();",
                     "      }",
                     "      break;",
                     "      default:",
@@ -6043,6 +6001,90 @@ protected String[] getExpectSource() {
         }
     };
 
+    public static final BaseTest systemAlertWindowWithOnNeverAskAgain = new BaseTest() {
+        @Override
+        protected String getName() {
+            return "MyFragment";
+        }
+
+        @Override
+        protected String[] getActualSource() {
+            return new String[]{
+                    "package test;",
+                    "import android.Manifest;",
+                    "import androidx.fragment.app.Fragment;",
+                    "import permissions.dispatcher.NeedsPermission;",
+                    "import permissions.dispatcher.OnNeverAskAgain;",
+                    "import permissions.dispatcher.OnPermissionDenied;",
+                    "import permissions.dispatcher.OnShowRationale;",
+                    "import permissions.dispatcher.PermissionRequest;",
+                    "import permissions.dispatcher.RuntimePermissions;",
+                    "@RuntimePermissions",
+                    "public class MyFragment extends Fragment {",
+                    "    @NeedsPermission(Manifest.permission.SYSTEM_ALERT_WINDOW)",
+                    "    void systemAlertWindow() {",
+                    "    }",
+                    "    @OnShowRationale(Manifest.permission.SYSTEM_ALERT_WINDOW)",
+                    "    void systemAlertWindowOnShowRationale(PermissionRequest request) {",
+                    "    }",
+                    "    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)",
+                    "    void systemAlertWindowOnNeverAskAgain() {",
+                    "    }",
+                    "    @OnPermissionDenied(Manifest.permission.SYSTEM_ALERT_WINDOW)",
+                    "    void systemAlertWindowOnPermissionDenied() {",
+                    "    }",
+                    "}"
+            };
+        }
+
+        @Override
+        protected String[] getExpectSource() {
+            return EMPTY_SOURCE;
+        }
+    };
+
+    public static final BaseTest writeSettingsWithOnNeverAskAgain = new BaseTest() {
+        @Override
+        protected String getName() {
+            return "MyFragment";
+        }
+
+        @Override
+        protected String[] getActualSource() {
+            return new String[]{
+                    "package test;",
+                    "import android.Manifest;",
+                    "import androidx.fragment.app.Fragment;",
+                    "import permissions.dispatcher.NeedsPermission;",
+                    "import permissions.dispatcher.OnNeverAskAgain;",
+                    "import permissions.dispatcher.OnPermissionDenied;",
+                    "import permissions.dispatcher.OnShowRationale;",
+                    "import permissions.dispatcher.PermissionRequest;",
+                    "import permissions.dispatcher.RuntimePermissions;",
+                    "@RuntimePermissions",
+                    "public class MyFragment extends Fragment {",
+                    "    @NeedsPermission(Manifest.permission.WRITE_SETTINGS)",
+                    "    void writeSettings() {",
+                    "    }",
+                    "    @OnShowRationale(Manifest.permission.WRITE_SETTINGS)",
+                    "    void writeSettingOnShowRationale(PermissionRequest request) {",
+                    "    }",
+                    "    @OnNeverAskAgain(Manifest.permission.WRITE_SETTINGS)",
+                    "    void writeSettingOnNeverAskAgain() {",
+                    "    }",
+                    "    @OnPermissionDenied(Manifest.permission.WRITE_SETTINGS)",
+                    "    void writeSettingOnPermissionDenied() {",
+                    "    }",
+                    "}"
+            };
+        }
+
+        @Override
+        protected String[] getExpectSource() {
+            return EMPTY_SOURCE;
+        }
+    };
+
     public static final BaseTest methodOverloadWithoutNeedsPermission = new BaseTest() {
         @Override
         protected String getName() {

diff --git a/...rc/main/java/permissions/dispatcher/test/ActivityWithSystemAlertWindowAllAnnotations.java b/...rc/main/java/permissions/dispatcher/test/ActivityWithSystemAlertWindowAllAnnotations.java
@@ -5,7 +5,6 @@
 
 import androidx.appcompat.app.AppCompatActivity;
 import permissions.dispatcher.NeedsPermission;
-import permissions.dispatcher.OnNeverAskAgain;
 import permissions.dispatcher.OnPermissionDenied;
 import permissions.dispatcher.OnShowRationale;
 import permissions.dispatcher.PermissionRequest;
@@ -26,10 +25,6 @@ void showRationaleForSystemAlertWindow(final PermissionRequest request) {
     void showDeniedForSystemAlertWindow() {
     }
 
-    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)
-    void showNeverAskForSystemAlertWindow() {
-    }
-
     @Override
     protected void onActivityResult(int requestCode, int resultCode, Intent data) {
         super.onActivityResult(requestCode, resultCode, data);

diff --git a/...rc/main/java/permissions/dispatcher/test/ActivityWithSystemAlertWindowKtAllAnnotations.kt b/...rc/main/java/permissions/dispatcher/test/ActivityWithSystemAlertWindowKtAllAnnotations.kt
@@ -20,10 +20,6 @@ open class ActivityWithSystemAlertWindowKtAllAnnotations : AppCompatActivity() {
     internal fun showDeniedForSystemAlertWindow() {
     }
 
-    @OnNeverAskAgain(Manifest.permission.SYSTEM_ALERT_WINDOW)
-    internal fun showNeverAskForSystemAlertWindow() {
-    }
-
     override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
         super.onActivityResult(requestCode, resultCode, data)
         onActivityResult(requestCode)

diff --git a/test/src/main/java/permissions/dispatcher/test/ActivityWithWriteSettingAllAnnotations.java b/test/src/main/java/permissions/dispatcher/test/ActivityWithWriteSettingAllAnnotations.java
@@ -5,7 +5,6 @@
 
 import androidx.appcompat.app.AppCompatActivity;
 import permissions.dispatcher.NeedsPermission;
-import permissions.dispatcher.OnNeverAskAgain;
 import permissions.dispatcher.OnPermissionDenied;
 import permissions.dispatcher.OnShowRationale;
 import permissions.dispatcher.PermissionRequest;
@@ -26,10 +25,6 @@ void showRationaleForWriteSettings(final PermissionRequest request) {
     void showDeniedForWriteSettings() {
     }
 
-    @OnNeverAskAgain(Manifest.permission.WRITE_SETTINGS)
-    void showNeverAskForWriteSettings() {
-    }
-
     @Override
     protected void onActivityResult(int requestCode, int resultCode, Intent data) {
         super.onActivityResult(requestCode, resultCode, data);

diff --git a/test/src/main/java/permissions/dispatcher/test/ActivityWithWriteSettingKtAllAnnotations.kt b/test/src/main/java/permissions/dispatcher/test/ActivityWithWriteSettingKtAllAnnotations.kt
@@ -20,10 +20,6 @@ open class ActivityWithWriteSettingKtAllAnnotations : AppCompatActivity() {
     internal fun showDeniedForWriteSettings() {
     }
 
-    @OnNeverAskAgain(Manifest.permission.WRITE_SETTINGS)
-    internal fun showNeverAskForWriteSettings() {
-    }
-
     override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
         super.onActivityResult(requestCode, resultCode, data)
         onActivityResult(requestCode)
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		package permissions.dispatcher.processor.exception

		class SpecialPermissionsWithNeverAskAgainException : RuntimeException("'@NeverAskAgain' annotated method never being called with 'WRITE_SETTINGS' or 'SYSTEM_ALERT_WINDOW' permission.")