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

@NotNull as alias for @Required #5161

Merged
merged 3 commits into from
Aug 25, 2017
Merged

@NotNull as alias for @Required #5161

merged 3 commits into from
Aug 25, 2017

Conversation

cmelchior
Copy link
Contributor

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @Required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @Required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @Required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

@cmelchior cmelchior self-assigned this Aug 25, 2017
Copy link
Contributor

@zaki50 zaki50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -441,13 +459,13 @@ private boolean categorizeIndexField(Element element, VariableElement variableEl
private void categorizeRequiredField(Element element, VariableElement variableElement) {
if (Utils.isPrimitiveType(variableElement)) {
Utils.error(String.format(Locale.US,
"@Required annotation is unnecessary for primitive field \"%s\".", element));
"@Required and @NotNull annotation is unnecessary for primitive field \"%s\".", element));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// In order to fully support the Kotlin type system we interpret `@NotNull` as an alias
// for `@Required`
for (AnnotationMirror annotation : field.getAnnotationMirrors()) {
if (annotation.getAnnotationType().toString().equals("org.jetbrains.annotations.NotNull")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use field.getAnnotation(Required.class) ? you don't want to pull the dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean @NotNull, but yes.

return;
}

if (Utils.isRealmList(variableElement) || Utils.isRealmModel(variableElement)) {
if (Utils.isRealmModel(variableElement)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why removing the isRealmList check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it conflicted with Kotlin mostly. RealmList is always non-null for managed object. I don't know why we didn't allow this to be set. At worst it would be redundant information.

@cmelchior cmelchior merged commit 39bb67c into master Aug 25, 2017
@cmelchior cmelchior deleted the cm/nonnull-alias branch August 25, 2017 12:27
GulajavaMinistudio added a commit to GulajavaMinistudio/realm-java that referenced this pull request Aug 26, 2017
zaki50 pushed a commit that referenced this pull request Aug 29, 2017
This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.
zaki50 added a commit that referenced this pull request Sep 5, 2017
…sor) (#5168)

* @NotNull as alias for @required (#5161)

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

* implement elemenrt type check of RealmList

* fix error message

* remove extra line

* fix typo

* address review comments
zaki50 added a commit that referenced this pull request Sep 5, 2017
* Add Primitive List fields to test classes

* removed tests for Primitive List for now

* removed tests for Primitive List for now

* Primitive List feature(Part5: Element type check in annotation processor) (#5168)

* @NotNull as alias for @required (#5161)

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

* implement elemenrt type check of RealmList

* fix error message

* remove extra line

* fix typo

* address review comments
zaki50 added a commit that referenced this pull request Sep 5, 2017
…sts) (#5151)

* solve compilation errors related to Primitive List API changes

* revert test

* fix FB warnings

* fix failing tests

* Primitive List feature(Part4: Tests) (#5152)

* Add Primitive List fields to test classes

* removed tests for Primitive List for now

* removed tests for Primitive List for now

* Primitive List feature(Part5: Element type check in annotation processor) (#5168)

* @NotNull as alias for @required (#5161)

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

* implement elemenrt type check of RealmList

* fix error message

* remove extra line

* fix typo

* address review comments
zaki50 added a commit that referenced this pull request Sep 5, 2017
* Nullability changes for Primitive List support

* more @nullable annotations

* fix annotation order

* Primitive List feature(Part3: Temporary fix of implementations and tests) (#5151)

* solve compilation errors related to Primitive List API changes

* revert test

* fix FB warnings

* fix failing tests

* Primitive List feature(Part4: Tests) (#5152)

* Add Primitive List fields to test classes

* removed tests for Primitive List for now

* removed tests for Primitive List for now

* Primitive List feature(Part5: Element type check in annotation processor) (#5168)

* @NotNull as alias for @required (#5161)

This PR adds support for org.jetbrains.annotations.NotNull as an alias for @required. This means that Realm now understands the Kotlin type-system in the Realm schema.

Previously we reported an error if you did @required RealmList<MyType> list = new RealmList<>();. It is a bit unclear why, since a RealmList is always non-null on managed objects. Since it made the interop with Kotlin kinda strange, I made a change so you can now do @required RealmList<MyType> list = new RealmLis<>() as well as val list : RealmList<MyType> = RealmList();

After this change it is no longer possible to do val person : Person either, you have to do val person : Person?. This reflects the constraints of Realm, but if people are able to maintain the variant themselves (hint: not possible when using sync), then they can use a custom non-null getter.

* implement elemenrt type check of RealmList

* fix error message

* remove extra line

* fix typo

* address review comments
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants