Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't use with AndroidAnnotations #14

Closed
hotchemi opened this issue Aug 27, 2015 · 23 comments
Closed

Can't use with AndroidAnnotations #14

hotchemi opened this issue Aug 27, 2015 · 23 comments
Assignees
Milestone

Comments

@hotchemi
Copy link
Member

Because they use _ ends with classes actually.

@hotchemi hotchemi added this to the 1.2.0 milestone Aug 27, 2015
@hotchemi hotchemi self-assigned this Aug 27, 2015
@mannodermaus
Copy link
Contributor

Rather than checking the class name for "Activity" or "Fragment", the processor should check if the class annotated with @RuntimePermissions is a subtype of android.app.Activity, android.app.Fragment or android.support.v4.app.Fragment.

In an annotation processor I recently wrote, I also only allow for some annotated elements to be of a certain type. Applying this to PermissionsDispatcher could look something like this:

// PermissionsProcessor.java:
public synchronized void init(ProcessingEnvironment env) {
    // ...
   this.types = env.getTypeUtils();
   this.elements = env.getElementUtils();
}

// RuntimePermissionsAnnotatedElement.java:
class RuntimePermissionsAnnotatedElement {
    //...

    private final TypeMirror type;

    RuntimePermissionsAnnotatedElement(TypeElement element) {
        // ...
        type = element.asType();
    }

    public TypeMirror asType() {
        return type;
    }
}

// Now, to check if an annotated element is an Activity:
// "types" are the TypeUtils retrieved from the ProcessingEnvironment at init time
// "elements" are the ElementUtils retrieved from the ProcessingEnvironment at init time
TypeMirror activityType = types.getDeclaredType(elements.getTypeElement("android.app.Activity"));
RuntimePermissionsAnnotatedElement element = // ...
if (types.isSubType(element.asType(), activityType)) {
    // This is an activity sub-class.
} else if (//Same for Fragment) {
} else if (// Same for Support Fragment) {
} else {
    // Invalid element
    throw new WrongClassException(...);
}

@hotchemi
Copy link
Member Author

Oh it's really nice idea!

@Fleker
Copy link

Fleker commented Sep 3, 2015

Any update on this? I also ran into the WrongClassException when trying to compile one of my activities that was an AppCompatActivity. I don't know why that was happening, but a fix would be appreciated.

@hotchemi
Copy link
Member Author

hotchemi commented Sep 3, 2015

@Fleker Do u use AndroidAnnotations?

@Fleker
Copy link

Fleker commented Sep 3, 2015

I spent some more time and figured out my problem. I think it had to do with trying to annotate a private method and it broke on compilation. Now my issue has been resolved.

@hotchemi
Copy link
Member Author

hotchemi commented Sep 4, 2015

OK but the error message is wired...

@hotchemi
Copy link
Member Author

hotchemi commented Sep 6, 2015

I'm gonna delete validation. If user don't annotate proper class, it causes compile error so it doesn't matter.

@WonderCsabo
Copy link
Contributor

@hotchemi sorry, i cannot say it is a good idea. Generating a non-compilable class is not user friendly, and should be always avoided. Moreover, it will not be clear to the user that the processor is buggy, or she/he used it in a wrong way.

Why don't you just apply the easy fix which was already proposed?

@hotchemi hotchemi modified the milestones: 1.2.1, 1.2.0 Sep 7, 2015
@hotchemi
Copy link
Member Author

hotchemi commented Sep 7, 2015

@WonderCsabo Thanks, I changed my mind!

@mannodermaus mannodermaus assigned mannodermaus and unassigned hotchemi Sep 7, 2015
mannodermaus pushed a commit to mannodermaus/PermissionsDispatcher that referenced this issue Sep 7, 2015
…rror comparison checks

* rather than checking if an annotated class ends in "Activity" or "Fragment", its type is compared to `android.app.Activity` and `android.support.v4.app.Fragment` in order to find their ClassType
* this fixes permissions-dispatcher#14 (Can't use with AndroidAnnotations)
mannodermaus pushed a commit to mannodermaus/PermissionsDispatcher that referenced this issue Sep 7, 2015
@hotchemi
Copy link
Member Author

I realized this implementation can't catch nested super class problem.
I mean if the target extends AppCompatActivity it doesn't go well in this case...

@WonderCsabo
Copy link
Contributor

@hotchemi I think you are wrong. types.isSubType(element.asType(), activityType)) will return true if the actual element is a subclass of AppCompatActivity.

@hotchemi
Copy link
Member Author

@WonderCsabo Oh really? I tried to add this test data but it doesn't go well...anything wrong?

@WonderCsabo
Copy link
Contributor

I think the test data is OK. However isSubTypeOf definitely works on multi-level inheritance. It should, as defined in the JLS, but i also double checked. Maybe you are calling it wrong somewhere.

@hotchemi
Copy link
Member Author

Oh thanks.
Actually we define checking in these lines and use them in these lines...
In this case, ClassType.getClassType returns null with the last data.

@hotchemi
Copy link
Member Author

@WonderCsabo @aurae
I realized wired thing.
Instead of original extended Activity, I tried to use actual FragmentActivity, and the test passed.
With debug, it seems that original activity doesn't have inheritance information, I'm not sure why...

@WonderCsabo
Copy link
Contributor

WonderCsabo commented Sep 13, 2015 via email

@hotchemi
Copy link
Member Author

I wrote like below.
static class SuperActivity extends Activity {}
I think it must extends android.app.Activity right? It's just static.

@WonderCsabo
Copy link
Contributor

You deleted the commit unfortunately, but i think your test case looked like this:

public class MyChildClass extends MyChildClass.SuperClass {

    static class SuperClass extends Object {}
}

This cannot compile, because of cyclic inheritance. So that is why isSuperType returned false.

@hotchemi
Copy link
Member Author

@WonderCsabo Oh sorry that was my mistake...

@BukT0p
Copy link

BukT0p commented Oct 27, 2015

Hi, its me, again. Tried to use this lib one more time and got this message:
`Error:Execution failed for task ':app:compileDebugJavaWithJavac'.

permissions.dispatcher.processor.exceptions.NotDefinedException: @NeedsPermission or @NeedsPermissions are not defined`

I have my fragment generated and dispatcher is generated too. But there is no code for calling the dispatcher inside the generated fragment. I can't find much details on that in log.

@hotchemi
Copy link
Member Author

Could u paste the code of fragment? I guess you don't define @NeedsPermission.

@BukT0p
Copy link

BukT0p commented Oct 28, 2015

As stated before I have my TestFragment_ generated as well as TestFragmentPermissionsDispatcher but in TestFragment_ there are no references to Dispatcher. I have compilation error https://gist.github.com/BukT0p/572e431288b698f25de0

@hotchemi
Copy link
Member Author

@BukT0p Umm it's make sense because u don't use original fragment. Could u send pull request?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants