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

Introduce linting rules covering annotations configuration validation #84

Open
Norbert515 opened this issue Sep 27, 2020 · 5 comments
Open
Assignees
Labels
enhancement New feature or request question Further information is requested sponsorware https://calebporzio.com/sponsorware

Comments

@Norbert515
Copy link
Contributor

Norbert515 commented Sep 27, 2020

Currently, when trying to call JsonMapper.fromJson<SomeObject>(object) it may throw a runtime exception like:

Unhandled Exception: It seems your class 'SomeObject' has not been annotated with @jsonSerializable

This can happen for multiple reasons (forgot @jsonSerializable, some missing type adapter, or by forgetting to run the code generation). From my experience with using this package, it can be pretty difficult to debug this issue as there is no help from the compiler.

My proposition is to add a code generator that generates type-safe code for (de)serialization. This could even be in the form of an extension function on JsonMapper like:

extension on JsonMapper {
  SomeObject deserializeSomeObject(String json) {...}
}

As this package is using reflectable, we'd need to write a new code generator that writes this class. This class could be purely optional and just a compile-safe addition to the package.

If this is something that people find interesting and you'd like to see implemented. I'd be happy to help out/ write a PR.

@k-paxian k-paxian added the question Further information is requested label Sep 27, 2020
@k-paxian
Copy link
Owner

Hey Norbert,

Thank you for an idea and intention to improve error handling approach.

Let's try to separate those concerns.

  • forgot @jsonSerializable (✔️) covered by error handling via runtime exception with clear informative message
  • some missing type adapter (🔴)
  • forgetting to run the code generation (🔴)

Your proposition is to have a pre-built runtime exceptions free methods like

extension on JsonMapper {
  SomeObject deserializeSomeObject(String json) {...}
}

What are the expectations? those methods deserializeSomeObject should return Object OR null ? w/o producing runtime exceptions? In other words you are not happy with runtime exceptions, and you would like to ease the pain?

Could you define a distinct use case and illustrate your solution for it please 🥇

@Norbert515
Copy link
Contributor Author

Thanks for the quick response!

Catching errors as soon as possible greatly reduces iteration lengths (if I want to be sure that I set up everything correctly I first have to run code-gen and then execute the code versus just the code-gen).

The idea behind the type-safe accessor is that, if it is present, you can be sure that the code to handle that specific object is correctly set up. Of course, this could still throw a runtime exception if the provided JSON doesn't fit the object, this is purely for the sake that you know that the actual setup is correct.

A couple of examples:

I got an enum in one of my model classes that I forgot to annotate correctly. To catch this error, I have to first run code-gen, run the app, and trigger an action that serializes/ deserializes the object containing the enum.
With a compile safe accessor, this bug could be caught before even running the app.

Another use-case (that I'm currently running into). I recently moved around quite a few model classes.


// @jsonSerialiable <---- Removed this here
class OldModel {
}

@jsonSerializable // <---- Added it here
class NewModel {
}

The compiler doesn't complain if I still have JsonMapper.serialize(oldModel) calls in my code. To refactor this, I now have to
a) Manually search for occurrences of the old model class
b) Run the app and fix these bugs on the fly

With the type-safe accessors, the generated code would simply lack the

  OldModel deserializeSomeObject(String json) {...}

method, therefore making fixing this way easier

@k-paxian
Copy link
Owner

k-paxian commented Sep 27, 2020

I see, thank you for clarifying use cases.

My original thouts in this field were to reuse
Static code analysis for dart code
https://pub.dev/packages/analyzer

I'm not sure yet how feasible it is to throw in few custom linter rules which will address json mapper usage validation. But theoretically, this might be more efficient way to solve this type of issues.

This way you'll get first class support for this kind of issues from IDE of your choice, wether it is Vscode or Android studio.

Sadly this issue is still not resolved for 3 years 😞

@k-paxian k-paxian added the enhancement New feature or request label Sep 27, 2020
@k-paxian k-paxian changed the title Add compile time safety Introduce linting rules covering annotations configuration validation Oct 30, 2020
@k-paxian k-paxian added the sponsorware https://calebporzio.com/sponsorware label Dec 19, 2020
@k-paxian
Copy link
Owner

k-paxian commented Feb 9, 2021

@k-paxian k-paxian self-assigned this Apr 20, 2021
@k-paxian
Copy link
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested sponsorware https://calebporzio.com/sponsorware
Projects
None yet
Development

No branches or pull requests

2 participants