mirror of
https://github.com/apple/pkl.git
synced 2026-05-31 11:00:44 +02:00
codegen-java: Change condition for generating equals/hashCode/toString/with/Serializable (#710)
Motivation:
- Currently, the condition for generating equals/hashCode/toString methods is "generated class declares properties".
As a result, classes that don't declare properties inherit these methods from their generated superclasses.
However, generated `equals` and `toString` methods aren't designed to be inherited
and will either fail or produce wrong results when called for a subclass.
- Currently, the condition for generating `with` methods is "class is not abstract".
However, it isn't useful to generate `with` methods for non-instantiable non-abstract classes.
- Currently, the condition for making classes serializable is "class is not a module class".
However, it isn't useful to make non-instantiable non-module classes serializable,
and it is useful to make instantiable module classes serializable.
Changes:
- Change condition for generating equals/hashCode/toString/with/Serializable to "class is instantiable".
This is a breaking change.
(A generated class is instantiable, i.e., declares a public constructor,
if it is neither abstract nor stateless. This behavior remains unchanged for now.)
- Overhaul JavaCodeGeneratorTest
- introduce classes JavaSourceCode and JavaSourceCodeAssert
- change assertions to use JavaSourceCodeAssert via `assertThat(javaCode)`
- use parameterized test instead of loop
- use explicit trimIndent() and trimMargin() for multiline string literals
- IntelliJ editor desperately wants to insert trimIndent()
- can potentially be exploited by kotlinc and ktfmt
Result:
- Fixes all motivating issues.
- Fixes #706.
This commit is contained in:
@@ -237,15 +237,14 @@ class JavaCodeGenerator(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun generateConstructor(): MethodSpec {
|
fun generateConstructor(isInstantiable: Boolean): MethodSpec {
|
||||||
val builder =
|
val builder =
|
||||||
MethodSpec.constructorBuilder()
|
MethodSpec.constructorBuilder()
|
||||||
// choose most restrictive access modifier possible
|
// choose most restrictive access modifier possible
|
||||||
.addModifiers(
|
.addModifiers(
|
||||||
when {
|
when {
|
||||||
pClass.isAbstract -> Modifier.PROTECTED
|
isInstantiable -> Modifier.PUBLIC
|
||||||
allProperties.isNotEmpty() -> Modifier.PUBLIC // if `false`, has no state
|
pClass.isAbstract || pClass.isOpen -> Modifier.PROTECTED
|
||||||
pClass.isOpen -> Modifier.PROTECTED
|
|
||||||
else -> Modifier.PRIVATE
|
else -> Modifier.PRIVATE
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
@@ -542,7 +541,10 @@ class JavaCodeGenerator(
|
|||||||
val builder =
|
val builder =
|
||||||
TypeSpec.classBuilder(javaPoetClassName.simpleName()).addModifiers(Modifier.PUBLIC)
|
TypeSpec.classBuilder(javaPoetClassName.simpleName()).addModifiers(Modifier.PUBLIC)
|
||||||
|
|
||||||
if (codegenOptions.implementSerializable && !isModuleClass) {
|
// stateless classes aren't instantiable by choice, i.e., no public ctor is generated
|
||||||
|
val isInstantiable = !pClass.isAbstract && allProperties.isNotEmpty()
|
||||||
|
|
||||||
|
if (codegenOptions.implementSerializable && isInstantiable) {
|
||||||
builder.addSuperinterface(java.io.Serializable::class.java)
|
builder.addSuperinterface(java.io.Serializable::class.java)
|
||||||
builder.addField(generateSerialVersionUIDField())
|
builder.addField(generateSerialVersionUIDField())
|
||||||
}
|
}
|
||||||
@@ -574,7 +576,7 @@ class JavaCodeGenerator(
|
|||||||
generateSpringBootAnnotations(builder)
|
generateSpringBootAnnotations(builder)
|
||||||
}
|
}
|
||||||
|
|
||||||
builder.addMethod(generateConstructor())
|
builder.addMethod(generateConstructor(isInstantiable))
|
||||||
|
|
||||||
superclass?.let { builder.superclass(it.toJavaPoetName()) }
|
superclass?.let { builder.superclass(it.toJavaPoetName()) }
|
||||||
|
|
||||||
@@ -590,12 +592,12 @@ class JavaCodeGenerator(
|
|||||||
builder.addMethod(generateGetter(name, property, isOverridden))
|
builder.addMethod(generateGetter(name, property, isOverridden))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (!pClass.isAbstract) {
|
if (isInstantiable) {
|
||||||
builder.addMethod(generateWithMethod(name, property))
|
builder.addMethod(generateWithMethod(name, property))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (properties.isNotEmpty()) {
|
if (isInstantiable) {
|
||||||
builder
|
builder
|
||||||
.addMethod(generateEqualsMethod())
|
.addMethod(generateEqualsMethod())
|
||||||
.addMethod(generateHashCodeMethod())
|
.addMethod(generateHashCodeMethod())
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
@@ -32,13 +32,23 @@ public final class Mod {
|
|||||||
public long getOne() {
|
public long getOne() {
|
||||||
return one;
|
return one;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
public static class None extends Foo {
|
||||||
|
public None(@Named("one") long one) {
|
||||||
|
super(one);
|
||||||
|
}
|
||||||
|
|
||||||
|
public None withOne(long one) {
|
||||||
|
return new None(one);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean equals(Object obj) {
|
public boolean equals(Object obj) {
|
||||||
if (this == obj) return true;
|
if (this == obj) return true;
|
||||||
if (obj == null) return false;
|
if (obj == null) return false;
|
||||||
if (this.getClass() != obj.getClass()) return false;
|
if (this.getClass() != obj.getClass()) return false;
|
||||||
Foo other = (Foo) obj;
|
None other = (None) obj;
|
||||||
if (!Objects.equals(this.one, other.one)) return false;
|
if (!Objects.equals(this.one, other.one)) return false;
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -53,23 +63,13 @@ public final class Mod {
|
|||||||
@Override
|
@Override
|
||||||
public String toString() {
|
public String toString() {
|
||||||
StringBuilder builder = new StringBuilder(100);
|
StringBuilder builder = new StringBuilder(100);
|
||||||
builder.append(Foo.class.getSimpleName()).append(" {");
|
builder.append(None.class.getSimpleName()).append(" {");
|
||||||
appendProperty(builder, "one", this.one);
|
appendProperty(builder, "one", this.one);
|
||||||
builder.append("\n}");
|
builder.append("\n}");
|
||||||
return builder.toString();
|
return builder.toString();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
public static class None extends Foo {
|
|
||||||
public None(@Named("one") long one) {
|
|
||||||
super(one);
|
|
||||||
}
|
|
||||||
|
|
||||||
public None withOne(long one) {
|
|
||||||
return new None(one);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public static class Bar extends None {
|
public static class Bar extends None {
|
||||||
protected final String two;
|
protected final String two;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user