Skip to content
New issue

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

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

Already on GitHub? # to your account

add Realm Object rule #245

Closed
wants to merge 1 commit into from
Closed

add Realm Object rule #245

wants to merge 1 commit into from

Conversation

jpsim
Copy link
Collaborator

@jpsim jpsim commented Dec 3, 2015

I wrote this quickly so it's pretty messy, but it shouldn't have any false positives and might correct some otherwise misconfigured Realm models. /cc @bdash

@bdash
Copy link

bdash commented Dec 3, 2015

This seems like it would generate false-positives in many codebases unless it skipped ignored properties.

@jpsim
Copy link
Collaborator Author

jpsim commented Dec 3, 2015

There are several limitations with the current implementation that make me think this rule should be disabled by default:

1) Doesn't validate Object or List properties

Object properties would be challenging to validate because we don't know what types are Realm models. If we had valid compiler arguments for the file being linted, we'd be able to resolve those types and determine if they subclass RealmSwift.Object, but as things are now, we can't tell.

List properties could feasibly be validated with moderately extra effort though.

2) Doesn't work with inferred types

Because SourceKit doesn't apply type inference (that's a significant chunk of the compilation process), the current implementation just validates explicitly defined types.

3) Doesn't consider ignored properties

If you have a let name: String as an ignored property, this rule will treat that as incorrect, yielding a false positive.

*4) Doesn't know if inheriting from RealmSwift.Object or some other .Object type

We could check for import RealmSwift within the same file which would be required for that code to compile, but I'm not sure how I feel about that.

@jpsim jpsim mentioned this pull request Jan 12, 2016
@jpsim
Copy link
Collaborator Author

jpsim commented Jan 14, 2016

This rule is much too limited in scope to be useful I think. Closing.

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

Successfully merging this pull request may close these issues.

2 participants