immutables-vavr icon indicating copy to clipboard operation
immutables-vavr copied to clipboard

Option builder fields are not generated for Stage builders / values

Open Sorceror opened this issue 7 years ago • 6 comments

Thanks a lot for the vavr encoding, they're helping the the transition from javaslang to vavr a lot! 👍

Option<T> fields inside builders marked as stagedBuilder=true does not have appropriate methods generated to BuildFinal interface.

import io.vavr.control.Option;
import org.immutables.value.Value;
import org.immutables.vavr.encodings.VavrEncodingEnabled;

@Value.Immutable
@Value.Style(stagedBuilder = true)
@VavrEncodingEnabled
public interface StagingBuilderTest {

    @Value.Parameter
    Object testParameter();

    Option<Object> testOption();
}

is compiled into

public final class ImmutableStagingBuilderTest implements StagingBuilderTest {
  private final Object testParameter;
  private final Option<Object> testOption;

  // omitted ...

  public interface TestParameterBuildStage {
    BuildFinal testParameter(Object testParameter);
  }

  public interface BuildFinal {
    ImmutableStagingBuilderTest build();
  }
}

with javaslang 2.0.6 it's complied into

public final class ImmutableStagingBuilderTest implements StagingBuilderTest {
  private final Object testParameter;
  private final Option<Object> testOption;

  // omitted ...

  public interface TestParameterBuildStage {
    BuildFinal testParameter(Object testParameter);
  }

  public interface BuildFinal {
    BuildFinal testOption(Object testOption);
    BuildFinal testOption(Option<? extends Object> testOption);
    ImmutableStagingBuilderTest build();
  }
}

This might be issue also for other supported types (not tested though).

Sorceror avatar Aug 18 '17 20:08 Sorceror

Apologies for the delay, I'm just seeing this bug report today!

The definition of Option did change between vavr-encodings and javaslang-encodings. I'll look into it.

io7m avatar Oct 08 '17 09:10 io7m

@elucash Do you have any idea why this might be happening? I can't see anything in there that would suggest that staged builders would stop working...

Here's the old (working) definition:

https://raw.githubusercontent.com/immutables/immutables-vavr/7848130882b36c0b7c8709573fe2dba04d41aa78/javaslang-encodings/src/main/java/org/immutables/javaslang/encodings/JavaslangOptionEncoding.java

Here's the current definition:

https://raw.githubusercontent.com/immutables/immutables-vavr/develop/vavr-encodings/src/main/java/org/immutables/vavr/encodings/VavrOptionEncoding.java

We added some @Encoding.Copy methods, and added a default value for the field.

io7m avatar Oct 08 '17 15:10 io7m

Any update on this?

bowbahdoe avatar Apr 13 '18 23:04 bowbahdoe

I think I found the problem. Immutables project itself has a big list of all types that it considers to be Optional types, and it's outdated since the rename to vavr from javaslang: https://github.com/immutables/immutables/blob/dd249064688af65ee3ff00fa5819e9deeb1bcf67/value-processor/src/org/immutables/value/processor/meta/AttributeTypeKind.java#L90

I'll test and see.

MFAshby avatar Jan 06 '21 20:01 MFAshby

I think that list is unrelated, that list is for built-in support for certain types (including javaslang Optional, Atlassian's ones etc). The encodings (in theory) should replace any need for build-in support for them. The apparent problem is that while Optional encoding specifies types which are not mandatory to set, they are for some reason not picked up when BuildFinal is generated, so it's sorta falls out of both mandatory and optional attribute sets which should be exclusive either/or, but nothing in between. So there is some problem with the classification of attribute and encodings were never really tested with staged builders I guess so this went unnoticed.

elucash avatar Jan 06 '21 21:01 elucash

Ah, thanks for the clarification. As a workaround it's possible to call .build() first, then set the optional members using .withXyz method calls. This would only be a blocking problem if an immutable with a staged builder had additional @Check annotations which inspected the Option type members.

MFAshby avatar Jan 07 '21 09:01 MFAshby