Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option builder fields are not generated for Stage builders / values #9

Open
Sorceror opened this issue Aug 18, 2017 · 6 comments
Open

Comments

@Sorceror
Copy link

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).

@io7m
Copy link
Member

io7m commented Oct 8, 2017

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
Copy link
Member

io7m commented Oct 8, 2017

@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.

@bowbahdoe
Copy link

Any update on this?

@MFAshby
Copy link
Contributor

MFAshby commented Jan 6, 2021

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.

@elucash
Copy link
Member

elucash commented Jan 6, 2021

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.

@MFAshby
Copy link
Contributor

MFAshby commented Jan 7, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants