ScaCap / spring-auto-restdocs

Spring Auto REST Docs is an extension to Spring REST Docs
https://scacap.github.io/spring-auto-restdocs/
Apache License 2.0
310 stars 86 forks source link

It would be nice to doc only @Nullable object types as nullable #115

Closed m-titov closed 5 years ago

m-titov commented 7 years ago

Subj.

jmisur commented 7 years ago

Could you please elaborate more on the subject? Do you mean that if a field is annotated as @Nullable it is optional, but without that annotation it's mandatory?

m-titov commented 7 years ago

Do you mean that if a field is annotated as @Nullable it is optional, but without that annotation it's mandatory?

Yes

jmisur commented 7 years ago

If I understand it correctly, all plain non-annotated fields should be then mandatory. Honestly I would like that, but it goes against java language where all fields are nullable by default. This would completely invert the meaning of all current implementations where this aspect of java is taken into account and remedied by @NotNull annotation.

m-titov commented 7 years ago

Disagree. In best practice developers shouldn't use nullable types usually. Nullable types should be used in rare cases and they should be annotated with @Nullable. Besides it's hard to find and annotate each variable with @NotNull in whole code base (not only for spring-auto-restdocs).

jmisur commented 7 years ago

I understand you point, but we have to conform to java language not best practice. Spring also supports JSR303 as part of it's validation facilities, recognising that nullable is the unfortunate default https://docs.spring.io/spring/docs/current/spring-framework-reference/html/validation.html#validation-beanvalidation-overview.

sebastientromp commented 5 years ago

I would love something like this as well.

On our project we have an annotation at package level that tells the code inspectors that all fields are @Nonnull by default:

@Documented
@Nonnull
@TypeQualifierDefault(ElementType.FIELD)
@Retention(RetentionPolicy.RUNTIME)
public @interface FieldsAreNonnullByDefault {
}

The fields that can be null are explicitely marked as @Nullable.

Is there a way to consider @Nonnull like @NotBlank when introspecting the code, to mark the relevant fields as mandatory?

jmisur commented 5 years ago

It would be possible, but not as a default behavior, but some kind of switch when configuring SARD. We would need to invert the optionality logic and add support for Nullable annotations.

sebastientromp commented 5 years ago

Supporting @Nonnull in addition to @NotNull should work as well I think, at least for our use case.

I could try to add the annotation at https://github.com/ScaCap/spring-auto-restdocs/blob/0fa013fea8ea3c1434eff312d2918b8aa07dd7da/spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/constraints/SkippableConstraintResolver.java#L34 and see how it goes

Edit: I spoke too fast, it probably wouldn't work, as it would still have to understand the @Nullable annotation, which ties back to what you were saying - invert the optionality logic

jmisur commented 5 years ago

If you made any progress into this, let us know or submit a PR. Of course it's not that easy, on many fronts. From our side, it's very unlikely that we will implement such solution. We started to embrace Kotlin, which has excellent nullability rules and it also works with SARD, without using a single annotation.