Feat: add notNullable property for processor properties

[MartinWitt]

See #3339 for discussion why. I would add a simple boolean field to the property annotation because:

• Change the null to a nullObject like in SourcePosition would be backwards breaking
• Doesn't pollute the namespace with a new method.

Maybe a open issue issue is where to document it?

Edit: need to fix git sry
Edit2: git fixed

[monperrus]

Thanks a lot for the proposal.

The idea is interesting, but it does not make the log more useful.

I would propose to invert the annotation nullable -> notNullable.

If notNullable==true and properti is null we throw a SpoonException.

WDYT?

[monperrus]

ping @vmassol

[vmassol]

I think it's not the role of the spoon framework to handle that and that's the job of the Processor to tell what it needs and to report when it doesn't get what it needs to work (by throwing an exception if a property is mandatory and it cannot work without a value for it).

[MartinWitt]

@monperrus but wouldn't your change break the old behavior? If a user updates and gets your version, he could get exceptions for code working before.
Is this "issue" so important, that you would throw away backwards compatibility?
The obvious benefit of your change is, that it's way more starter friendly.

@vmassol Your message reads like you would remove the log message completely, would the proposed change still fit your needs?

[vmassol]

@vmassol Your message reads like you would remove the log message completely,

Yes I'd remove it completely and let the processor do the validation.

would the proposed change still fit your needs?

Sure.

[monperrus]

@monperrus but wouldn't your change break the old behavior?

No, it's fully backward compatible if properties are still nullable by default.

[MartinWitt]

Changed nullable -> notNullable and added the SpoonException. Should we still write into the log, even if there is a exception thrown?

[monperrus]

Great.

Should we still write into the log, even if there is a exception thrown?

no

could you add the //contract line in each test?

with that, we'll go towards merge

thanks!

[MartinWitt]

github seems to have some problems, but the CI looks green.