yahoo / squidb

SquiDB is a SQLite database library for Android and iOS
https://github.com/yahoo/squidb/wiki
Apache License 2.0
1.31k stars 132 forks source link

Support JSR305 Nonnull and Nullable annotations #241

Closed aminelaadhari closed 8 years ago

aminelaadhari commented 8 years ago

J2objc has a partial support of nullability annotation with swift. https://groups.google.com/forum/#!topic/j2objc-discuss/zNt2eMcLvlY

I think we can achieve this by writing a plugin, but I am not really sure.

I want to do is add the annotation to the spec fields: @Nonnull String exampleVariable;

And the expected getter/setter would be: @Nonnull public String getExampleVariable() { return get(EXAMPLE_VARIABLE); }

public String setExampleVariable(@Nonnull  exampleVariable) {
    set(EXAMPLE_VARIABLE, exampleVariable);
    return this;
}

Do you think that a plugin is the right way to do it? Do you have any hints on how can this be implemented ?

Thanks

sbosley commented 8 years ago

Yep! A plugin is definitely the right way to do this if you want this functionality right now. Note also though that issue #155 is still open, and I'm hoping to add the JSR305 annotations across the public API of squidb in a future release. This effort would probably include some kind of code generation enhancement for model objects like what you've suggested, so such a plugin would become unnecessary at that point. No timeline at this point, but the effort is ongoing, slowly and steadily.

Adding a nullability annotation to the getter/setter with a plugin are fairly easy. You could simply create a plugin that overrides beforeEmitGetter() and beforeEmitSetter(). In the former, you'd simply write out the proper annotation:

public void beforeEmitGetter(JavaFileWriter writer, PropertyGenerator propertyGenerator,
    MethodDeclarationParams params) throws IOException {
    if (propertyGenerator.getField() != null && propertyGenerator.getField().getAnnotation(Nonnull.class) != null) { 
        writer.writeAnnotation(new DeclaredTypeName("java.annotation", "Nonnull"));
    }
}

Doing it for the setter is a little hackier, but you could probably fake it by including the annotation in the setter parameter name:

public void beforeEmitSetter(JavaFileWriter writer, PropertyGenerator propertyGenerator,
    MethodDeclarationParams params) throws IOException {
    if (propertyGenerator.getField() != null && propertyGenerator.getField().getAnnotation(Nonnull.class) != null) { 
        String newArgName = "@Nonnull " + params.getArgumentNames().get(0);
        params.setArgumentNames(Arrays.asList(newArgName));
    }
}

Hope that helps! I'm going to close this issue as a duplicate of #155 and link back to it from there. You can watch that issue for updates on the effort to include nullability annotations.

aminelaadhari commented 8 years ago

Thanks @sbosley, very useful. I changed the beforeEmitSetter method because it writes the annotation after the type. It doesn't work with java 1.7. I created a plugin here: https://github.com/aminelaadhari/squidb-nullable

One remaining problem is that ModelMethodPlugin doesn't copy the annotations.

sbosley commented 8 years ago

Yeah, until we get full nullability support throughout the squidb codebase, the default code generator plugins won't support it either. We'll probably have to make some modifications to our apt-utils library too, since it was written for Java 1.6 and doesn't have good APIs for annotations on method params.

Still, if you like you can easily reimplement the ModelMethodPlugin yourself and disable the default one with a flag!

aminelaadhari commented 8 years ago

I will do that, thanks !