nonNullCollections flag isn't being respected for alias types

[a-k-g]

What happened?

We have the following conjure type:

  AndSearchFilter:
    docs: |
      A SearchFilter used to combine multiple SearchFilters.
      A row satisfies this SearchFilter if and only if it satisfies all combined SearchFilters.
    alias: set<SearchFilter>

And we have enabled the nonNullCollections flag to enforce that the set cannot contain null entries. However, this is not being enforced.

What did you want to happen?

Constructing the AndSearchFilter with a set containing null should fail.

[mpritham]

When the nonNullCollections flag is set, we could update the constructor to create a copy of the list passed in (for lists, via a call to this method in ConjureCollections).

The generation of this code should be under a new flag (maybe --copyCollectionsOnConstruction or something): it's currently possible that the aliased collection is mutated in some code that uses Conjure. Specifically, something like this could exist:

ListAlias createListAlias() {
  List<String> arr = new ArrayList<>();
  ListAlias listAlias = ListAlias.of(arr);
  callThatPopulatesList(arr);
  return listAlias;
}

We don't want any new changes we introduce to conjure-java to automatically lead to failures for clients that have the above (discouraged) pattern.

Conjure objects are meant to be immutable. As part of this change, when the copyCollectionsOnConstruction flag is set and the nonNullCollections flag is not set, we could create copies of collections (that allow null values) in the constructors for aliased collection types.

[carterkozak]

bikeshed: The feature flag should be a bit more specific since we do already copy collections in most cases (conjure bean types, just not aliases!). Perhaps something along the lines of --defensiveCollectionAliases.

There are only two hard things in Computer Science: cache invalidation and naming things.