From 2ab1e712f8e1e4080292aae4867a631f327d635b Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Thu, 11 Jan 2024 13:30:00 -0600 Subject: [PATCH] [Lombok] Constructor can have only non-final or not initialized fields When using the AllArgsConstructor annotation (directly or via meta-annotation), only fields that are not final or not initialized should be added as arguments. Previously, all fields were being included regardless of modality or initialization, which is not consistent with the behavior of Lombok. ^KT-54054 Fixed --- .../lombok/processor/AllArgsConstructorProcessor.kt | 11 ++++++++++- .../k2/generators/AllArgsConstructorGeneratorPart.kt | 12 +++++++++++- plugins/lombok/testData/box/allArgsConstructor.kt | 10 ++++++++-- .../lombok/testData/box/allArgsConstructorStatic.kt | 8 +++++++- 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/plugins/lombok/lombok.k1/src/org/jetbrains/kotlin/lombok/processor/AllArgsConstructorProcessor.kt b/plugins/lombok/lombok.k1/src/org/jetbrains/kotlin/lombok/processor/AllArgsConstructorProcessor.kt index 8b6c30c896d..63149ca3c12 100644 --- a/plugins/lombok/lombok.k1/src/org/jetbrains/kotlin/lombok/processor/AllArgsConstructorProcessor.kt +++ b/plugins/lombok/lombok.k1/src/org/jetbrains/kotlin/lombok/processor/AllArgsConstructorProcessor.kt @@ -5,11 +5,14 @@ package org.jetbrains.kotlin.lombok.processor +import com.intellij.psi.PsiField +import com.intellij.psi.PsiModifier import org.jetbrains.kotlin.descriptors.ClassDescriptor import org.jetbrains.kotlin.descriptors.PropertyDescriptor import org.jetbrains.kotlin.lombok.config.LombokAnnotations.AllArgsConstructor import org.jetbrains.kotlin.lombok.config.LombokAnnotations.Value import org.jetbrains.kotlin.lombok.utils.getJavaFields +import org.jetbrains.kotlin.resolve.source.getPsi class AllArgsConstructorProcessor : AbstractConstructorProcessor() { @@ -17,6 +20,12 @@ class AllArgsConstructorProcessor : AbstractConstructorProcessor = - classDescriptor.getJavaFields() + classDescriptor.getJavaFields().filter(this::isFieldAllowed) + private fun isFieldAllowed(field: PropertyDescriptor): Boolean { + val psi = field.source.getPsi() as? PsiField ?: return true + + val final = psi.modifierList?.hasModifierProperty(PsiModifier.FINAL) ?: false + return !final || !psi.hasInitializer() + } } diff --git a/plugins/lombok/lombok.k2/src/org/jetbrains/kotlin/lombok/k2/generators/AllArgsConstructorGeneratorPart.kt b/plugins/lombok/lombok.k2/src/org/jetbrains/kotlin/lombok/k2/generators/AllArgsConstructorGeneratorPart.kt index 857b2fb909e..0e73642345a 100644 --- a/plugins/lombok/lombok.k2/src/org/jetbrains/kotlin/lombok/k2/generators/AllArgsConstructorGeneratorPart.kt +++ b/plugins/lombok/lombok.k2/src/org/jetbrains/kotlin/lombok/k2/generators/AllArgsConstructorGeneratorPart.kt @@ -5,11 +5,13 @@ package org.jetbrains.kotlin.lombok.k2.generators +import com.intellij.psi.PsiField import org.jetbrains.kotlin.fir.FirSession import org.jetbrains.kotlin.fir.java.declarations.FirJavaField import org.jetbrains.kotlin.fir.symbols.SymbolInternals import org.jetbrains.kotlin.fir.symbols.impl.FirClassSymbol import org.jetbrains.kotlin.lombok.k2.config.ConeLombokAnnotations.AllArgsConstructor +import org.jetbrains.kotlin.psi class AllArgsConstructorGeneratorPart(session: FirSession) : AbstractConstructorGeneratorPart(session) { override fun getConstructorInfo(classSymbol: FirClassSymbol<*>): AllArgsConstructor? { @@ -19,6 +21,14 @@ class AllArgsConstructorGeneratorPart(session: FirSession) : AbstractConstructor @OptIn(SymbolInternals::class) override fun getFieldsForParameters(classSymbol: FirClassSymbol<*>): List { - return classSymbol.fir.declarations.filterIsInstance() + return classSymbol.fir.declarations + .filterIsInstance() + .filter { it.isFieldAllowed() } + } + + private fun FirJavaField.isFieldAllowed(): Boolean { + // TODO: consider adding `hasInitializer` property directly to java model + val hasInitializer = (source?.psi as? PsiField)?.hasInitializer() ?: false + return isVar || !hasInitializer } } diff --git a/plugins/lombok/testData/box/allArgsConstructor.kt b/plugins/lombok/testData/box/allArgsConstructor.kt index ac15b1bd3a7..95952ee2104 100644 --- a/plugins/lombok/testData/box/allArgsConstructor.kt +++ b/plugins/lombok/testData/box/allArgsConstructor.kt @@ -5,11 +5,17 @@ import lombok.*; @AllArgsConstructor public class ConstructorExample { + // Part of constructor because not final. @Getter @Setter private int age = 10; + // Part of constructor because not initialized. @Getter(AccessLevel.PROTECTED) private String name; - private boolean otherField; + // Part of constructor because not initialized. + private final boolean otherField; + + // Not part of constructor because final and initialized. + @Getter private final String result = "OK"; static void javaUsage() { val generated = new ConstructorExample(12, "sdf", true); @@ -21,5 +27,5 @@ public class ConstructorExample { fun box(): String { val generated = ConstructorExample(12, "sdf", true) - return "OK" + return generated.getResult() } diff --git a/plugins/lombok/testData/box/allArgsConstructorStatic.kt b/plugins/lombok/testData/box/allArgsConstructorStatic.kt index 3c61184a224..ec46409828b 100644 --- a/plugins/lombok/testData/box/allArgsConstructorStatic.kt +++ b/plugins/lombok/testData/box/allArgsConstructorStatic.kt @@ -5,12 +5,18 @@ import lombok.*; @AllArgsConstructor(staticName = "of") public class ConstructorExample { + // Part of constructor because not final. @Getter @Setter private int age = 10; + // Part of constructor because not initialized. @Getter(AccessLevel.PROTECTED) private String name; + // Part of constructor because not initialized. private boolean otherField; + // Not part of constructor because final and initialized. + @Getter private final String result = "OK"; + public ConstructorExample(String arg) { } @@ -27,5 +33,5 @@ public class ConstructorExample { fun box(): String { val existing: ConstructorExample = ConstructorExample("existing") val generated: ConstructorExample = ConstructorExample.of(45, "234", false) - return "OK" + return generated.getResult() }