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

Optional Builder methods should default to package private #6593

Merged
merged 2 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions builder/builder/src/main/java/io/helidon/builder/Builder.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@
*/
boolean DEFAULT_ALLOW_NULLS = false;

/**
* The default value for {@link #allowPublicOptionals()}.
*/
boolean DEFAULT_ALLOW_PUBLIC_OPTIONALS = false;

/**
* The default value for {@link #includeGeneratedAnnotation()}.
*/
Expand Down Expand Up @@ -191,6 +196,15 @@
*/
boolean allowNulls() default DEFAULT_ALLOW_NULLS;

/**
* Should any use of {@link java.util.Optional} builder methods be made public. By default this value is {@code false} resulting
* in these methods taking {@code Optional} to be made package private. Setting this value to {@code true} will result in these
* same methods to be made public instead.
*
* @return true to make {@code Optional} method public and false to make these methods package private
*/
boolean allowPublicOptionals() default DEFAULT_ALLOW_PUBLIC_OPTIONALS;

/**
* Should the code generated types included the {@code Generated} annotation. Including this annotation will require an
* additional module dependency on your modules to include {@code jakarta.annotation-api}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class BodyContext {
private final boolean requireLibraryDependencies;
private final boolean beanStyleRequired;
private final boolean allowNulls;
private final boolean allowPublicOptionals;
private final boolean includeGeneratedAnnotation;
private final String listType;
private final String mapType;
Expand Down Expand Up @@ -102,6 +103,7 @@ public class BodyContext {
this.requireLibraryDependencies = toRequireLibraryDependencies(builderTriggerAnnotation, typeInfo);
this.beanStyleRequired = toRequireBeanStyle(builderTriggerAnnotation, typeInfo);
this.allowNulls = toAllowNulls(builderTriggerAnnotation, typeInfo);
this.allowPublicOptionals = toAllowPublicOptionals(builderTriggerAnnotation, typeInfo);
this.includeGeneratedAnnotation = toIncludeGeneratedAnnotation(builderTriggerAnnotation, typeInfo);
this.listType = toListImplType(builderTriggerAnnotation, typeInfo);
this.mapType = toMapImplType(builderTriggerAnnotation, typeInfo);
Expand Down Expand Up @@ -275,6 +277,16 @@ protected boolean allowNulls() {
return allowNulls;
}

/**
* Returns true if public {@link Optional} setters are allowed.
* See {@link Builder#allowPublicOptionals()}.
*
* @return true if allow public optional methods
*/
protected boolean allowPublicOptionals() {
return allowPublicOptionals;
}

/**
* Returns true if {@code jakarta.annotations.Generated} annotation should be generated.
* See {@link io.helidon.builder.Builder#includeGeneratedAnnotation()}.
Expand Down Expand Up @@ -492,6 +504,15 @@ private static boolean toAllowNulls(AnnotationAndValue builderTriggerAnnotation,
return (val == null) ? Builder.DEFAULT_ALLOW_NULLS : Boolean.parseBoolean(val);
}

/**
* In support of {@link Builder#allowPublicOptionals()}.
*/
private static boolean toAllowPublicOptionals(AnnotationAndValue builderTriggerAnnotation,
TypeInfo typeInfo) {
String val = searchForBuilderAnnotation("allowPublicOptionals", builderTriggerAnnotation, typeInfo);
return (val == null) ? Builder.DEFAULT_ALLOW_PUBLIC_OPTIONALS : Boolean.parseBoolean(val);
}

/**
* In support of {@link io.helidon.builder.Builder#includeGeneratedAnnotation()}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -876,11 +876,12 @@ private void appendSetter(StringBuilder mainBuilder,
boolean isList = typeName.isList();
boolean isMap = !isList && typeName.isMap();
boolean isSet = !isMap && typeName.isSet();
String publicOrPkdPrivate = (!typeName.isOptional() || ctx.allowPublicOptionals()) ? "public " : "";
boolean upLevel = isSet || isList;

StringBuilder builder = new StringBuilder();
GenerateJavadoc.setter(builder, beanAttributeName);
builder.append("\t\tpublic ").append(ctx.genericBuilderAliasDecl()).append(" ")
builder.append("\t\t").append(publicOrPkdPrivate).append(ctx.genericBuilderAliasDecl()).append(" ")
.append(prefixName)
.append(methodName).append("(")
.append(toGenerics(method, upLevel)).append(" val) {\n");
Expand Down Expand Up @@ -1104,16 +1105,6 @@ private static String toGenerics(TypeName typeName,
return typeName.fqName();
}

private static String toGenericsDecl(TypedElementName method) {
List<TypeName> compTypeNames = method.typeName().typeArguments();
if (1 == compTypeNames.size()) {
return avoidWildcard(compTypeNames.get(0)) + " val";
} else if (2 == compTypeNames.size()) {
return avoidWildcard(compTypeNames.get(0)) + " key, " + avoidWildcard(compTypeNames.get(1)) + " val";
}
return "Object val";
}

private static String avoidWildcard(TypeName typeName) {
return typeName.wildcard() ? typeName.name() : typeName.fqName();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022 Oracle and/or its affiliates.
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,7 +26,7 @@
* child interface is a target for the builder. The net result is that the builder generated will handle both the parent and the
* child merged as one.
*/
@Builder(implPrefix = "", implSuffix = "Impl")
@Builder(implPrefix = "", implSuffix = "Impl", allowPublicOptionals = true)
public interface ChildInterfaceIsABuilder extends ParentInterfaceNotABuilder {

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022 Oracle and/or its affiliates.
* Copyright (c) 2022, 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -26,7 +26,7 @@
*
* @see ParentInterfaceNotABuilder
*/
@Builder(implPrefix = "", implSuffix = "Impl")
@Builder(implPrefix = "", implSuffix = "Impl", allowPublicOptionals = true)
public interface ParentOfParentInterfaceIsABuilder {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,22 @@

package io.helidon.builder.test;

import java.util.Optional;

import io.helidon.builder.test.testsubjects.DefaultPickle;
import io.helidon.builder.test.testsubjects.DefaultPickleBarrel;
import io.helidon.builder.test.testsubjects.Pickle;
import io.helidon.builder.test.testsubjects.PickleBarrel;

import org.junit.jupiter.api.Test;

import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.*;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

class PickleBarrelTest {

@Test
void testIt() {
DefaultPickle.Builder pickleBuilder = DefaultPickle.builder().size(Optional.of(Pickle.Size.MEDIUM));
DefaultPickle.Builder pickleBuilder = DefaultPickle.builder().size(Pickle.Size.MEDIUM);
Exception e = assertThrows(IllegalStateException.class, pickleBuilder::build);
assertThat(e.getMessage(),
equalTo("'type' is a required attribute and should not be null"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
/**
* For internal use only to Helidon. Applicable when {@link io.helidon.pico.api.PicoServicesConfig#TAG_DEBUG} is enabled.
*/
@Builder(interceptor = CallingContext.BuilderInterceptor.class)
@Builder(interceptor = CallingContext.BuilderInterceptor.class, allowPublicOptionals = true)
public abstract class CallingContext {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
/**
* Abstractly describes method or field elements of a managed service type (i.e., fields, constructors, injectable methods, etc.).
*/
@Builder
@Builder(allowPublicOptionals = true)
public interface ElementInfo {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
* @see Services
* @see ServiceInfoCriteria
*/
@Builder(interceptor = ServiceInfoBuildInterceptor.class)
@Builder(interceptor = ServiceInfoBuildInterceptor.class, allowPublicOptionals = true)
public interface ServiceInfo extends ServiceInfoBasics {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*
* @see ServiceInfo
*/
@Builder
@Builder(allowPublicOptionals = true)
public interface ServiceInfoBasics {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
/**
* Represents the injection plan targeting a given {@link io.helidon.pico.api.ServiceProvider}.
*/
@Builder
@Builder(allowPublicOptionals = true)
public interface InjectionPlan {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/**
* Applies only to the output paths that various {@code creators} will use (e.g., {@link io.helidon.pico.tools.spi.ActivatorCreator}).
*/
@Builder
@Builder(allowPublicOptionals = true)
public interface CodeGenPaths {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
/**
* General code gen information.
*/
@Builder
@Builder(allowPublicOptionals = true)
public interface GeneralCodeGenNames {

/**
Expand Down