Default value provided by ObjectFactory.property is inconsistent with Property.getOrElse behavior

[grv87]

Expected Behavior

Consider Property instance of Java standard type (e.g. Integer) constructed by ObjectFactory.property. Suppose set was never called. Then getOrElse(someValue) should return someValue.

Current Behavior

getOrElse(someValue) returns default value for that type defined by Java specification.

Context

This also affects detection whether required property was set.

In the case of getOrElse, for optional properties, each time we have to reset property to null manually.

In the case of required property, if developer would like to provide default value he should better set it explicitly.

Steps to Reproduce (for bugs)

1. Call to getOrElse on non-set property:

class PrintNumberTask extends DefaultTask {
  @Optional
  @Input
  final Property<Integer> number = project.objects.property(Integer)
  /* WORKAROUND:
  PrintNumberTask() {
    number.set((Integer)null)
  }
  */
  
  @TaskAction
  void printNumber() {
    println number.getOrElse(42)
  }
}

task('printNumber', type: PrintNumberTask)

gradle printNumber prints 0 instead of 42.

2. Detection if required property was set:

class PrintNumber2Task extends DefaultTask {
  @Input
  final Property<Integer> number = project.objects.property(Integer)
  
  @TaskAction
  void printNumber() {
    println number.getOrElse(42)
  }
}

task('printNumber2', type: PrintNumber2Task)

class PrintString2Task extends DefaultTask {
  @Input
  final Property<Integer> string = project.objects.property(String)
  
  @TaskAction
  void printString() {
    println string.getOrElse('Gradle forever!')
  }
}

task('printString2', type: PrintString2Task

gradle printString2 fails without its property set, but gradle printNumber2 succeedes.

Your Environment

Gradle 4.9

[lacasseio]

Thanks @grv87 for the great issue. It seems this behavior was explicitly defined code in the factory. The Javadoc specify the following: Boxed primitive types have the default value of the primitive type as defined by the JLS. I agree it's surprising in the sense that a developer would expect Integer to default to null. The principle of least surprise would probably prefer setting those to null and having an API to specify the default value on creation if you need the default JLS.

@adammurdoch What do you think should we do on this? I feel we could remove the default value in the factory method and add either an API to define the default value objects.property(Integer, 0) or a builder-like method on the property with a signature like Property<T> withValue(T value) so we can write something like Property<Integer> number = project.objects.property(Integer).withValue(0). Or crazier idea would be to have an API that differentiate when you want your boxed primitive to behave like primitive (default to what JLS says), like primitiveProperty(Integer).

[adammurdoch]

I'd be tempted to remove the default value from the factory method. Let's see if we need a convenience for setting an initial value at creation time (vs. just calling set() immediately after creating the instance.

[grv87]

Maybe set could return Property instance, so that it could be chained?

[adammurdoch]

It could indeed. This would be a breaking change, however, because the signature baked into bytecode will be different. Which might be ok given that this API is incubating. I'm tempted perhaps to add a new method to replace set() that could allow chaining.

[adammurdoch]

Fix via #6695

[adammurdoch]

#6780 does the same for collection properties. It also adds a value() method that can be used to chain method calls to create the property instance and set an initial value, if required, in a field initializer or some other expression. This will probably end up replacing set(), but that will happen later.