eclipse / xtext-lib

Eclipse Xtext™ Libraries
https://www.eclipse.org/Xtext/
Eclipse Public License 2.0
19 stars 33 forks source link

Accessors annotation with null check #246

Closed Hanksha closed 1 year ago

Hanksha commented 5 years ago

I love using the Accessors active annotation but in some cases I want to prevent null values to be assigned via setter or constructor and I can't use that annotation anymore.

I was thinking we could improve the Accessors active annotation to handle it. We can either add a boolean field allowNull=false or have another AccessorType called NULL_SAFE. The generated code will then check for null before to assign the value with this.myField = Objects.requireNonNull(myField, "null myField")

kthoms commented 5 years ago

Hmm, why not? @szarnekow WDYT?

szarnekow commented 5 years ago

Active annotations are rather easy to implement for project specific needs. I’m not excited to add more flags and options to the default annotations. Other may want to use Preconditions, the next user may want to have another error message. The Data annotation would need to be changed, too. For not-null fields it would make sense to generate not-null annotations, too - again the question: which ones... @Hanksha did you try to use a custom annotation for your use case / code style?

Hanksha commented 5 years ago

@szarnekow Yes you're right it is easy to create active annotations for project specific needs and I've done it for a good number of cases. But I consider this general purpose programming and not project specific need. If the Data annotation needs to be changed as well we could opt for a NotNull annotation (or reuse the existing @Notnull). Each field annotated with that annotation would indicate to the Data and Accessors annotations that a null value cannot be assigned.

Regarding the use of Objects.requireNonNull or Preconditions.checkNotNull they both throw a NullPointerException so there is no difference unless you meant to use Preconditions.checkArgument which throws a IllegalArgumentException.

szarnekow commented 5 years ago

@Hanksha I'm aware of the fact that we can make any kind of decision here and implement it in that way - being it based on annotations, about using Objects or Preconditions etc. What I'm trying to say: About the strict semantics here, there is no such kind of one-size-fits all. That's why I'm not super excited about more semantics baked into the basic active annotations. I wouldn't mind promoting a third party library with a couple of opinionated annotation implementations, though.

Hanksha commented 5 years ago

@szarnekow I find it ridiculous to have to do my own library to add this very general purpose feature (fail fast null pointer checks is as much a pattern/code style as using getters and setters). That would mean copy/paste the current implementation (which means I'll have to maintain it as well) and add that behavior (then call it what, AccessorsWithNotNull?) as the annotation cannot be easily extended.

cdietrich commented 5 years ago

the problem is: who will maintain the pile of AA

cdietrich commented 1 year ago

will close this for now. feel free to recreated in eclipse/xtext