[Fixes #945] wither methods for abstract classes now slightly less useless.

7 files changed, 129 insertions, 89 deletions

diff --git a/doc/changelog.markdown b/doc/changelog.markdown
index 7b858ce7..1ca7f659 100644
--- a/doc/changelog.markdown
+++ b/doc/changelog.markdown
@@ -7,6 +7,7 @@ Lombok Changelog
 * FEATURE: `@Builder` updates: `@Singular` now supports guava's ImmutableTable [Issue #937](https://github.com/rzwitserloot/lombok/issues/937).
 * FEATURE: A `lombok.config` key can now be used to make your fields `final` and/or `private`... __everywhere__. We'll be monitoring the performance impact of this for a while. We'll touch every source file if you turn these on, and even if you don't, we have to call into the lombok config system for every file.
 * FEATURE: A `lombok.config` key can now be used to set the default behaviour of `@EqualsAndHashCode` when generating methods for a class that extends something in regards to calling the superclass implementations of `equals` and `hashCode` or not. [Issue #965](https://github.com/rzwitserloot/lombok/issues/965).
+* FEATURE: Putting `@Wither` on abstract classes now generates something slightly more useful: An abstract wither method. [Issue #945](https://github.com/rzwitserloot/lombok/issues/945).
 * BUGFIX: lambdas with 1 argument that has an explicit type did not pretty print correctly. [Issue #972](https://github.com/rzwitserloot/lombok/issues/972).
 * BUGFIX: `@Value` and `@FieldDefaults` no longer make uninitialized static fields final. [Issue #928](https://github.com/rzwitserloot/lombok/issues/928).
 * BUGFIX: When using delombok, a source file with only `@NonNull` annotations on parameters as lombok feature would not get properly delomboked.  [Issue #950](https://github.com/rzwitserloot/lombok/issues/950).
diff --git a/src/core/lombok/eclipse/handlers/HandleWither.java b/src/core/lombok/eclipse/handlers/HandleWither.java
index 29a6edb7..d8ac8df6 100644
--- a/src/core/lombok/eclipse/handlers/HandleWither.java
+++ b/src/core/lombok/eclipse/handlers/HandleWither.java
@@ -25,7 +25,6 @@ import static lombok.core.handlers.HandlerUtil.*;
 import static lombok.eclipse.Eclipse.*;
 import static lombok.eclipse.handlers.EclipseHandlerUtil.*;
 
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -57,6 +56,7 @@ import org.eclipse.jdt.internal.compiler.ast.ThisReference;
 import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
 import org.eclipse.jdt.internal.compiler.ast.TypeReference;
 import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
+import org.eclipse.jdt.internal.compiler.lookup.ExtraCompilerModifiers;
 import org.mangosdk.spi.ProviderFor;
 
 @ProviderFor(EclipseAnnotationHandler.class)
@@ -163,6 +163,9 @@ public class HandleWither extends EclipseAnnotationHandler<Wither> {
 			return;
 		}
 		
+		EclipseNode typeNode = fieldNode.up();
+		boolean makeAbstract = typeNode != null && typeNode.getKind() == Kind.TYPE && (((TypeDeclaration) typeNode.get()).modifiers & ClassFileConstants.AccAbstract) != 0;
+		
 		FieldDeclaration field = (FieldDeclaration) fieldNode.get();
 		TypeReference fieldType = copyType(field.type, source);
 		boolean isBoolean = isBoolean(fieldType);
@@ -208,17 +211,18 @@ public class HandleWither extends EclipseAnnotationHandler<Wither> {
 		
 		int modifier = toEclipseModifier(level);
 		
-		MethodDeclaration method = createWither((TypeDeclaration) fieldNode.up().get(), fieldNode, witherName, modifier, sourceNode, onMethod, onParam);
+		MethodDeclaration method = createWither((TypeDeclaration) fieldNode.up().get(), fieldNode, witherName, modifier, sourceNode, onMethod, onParam, makeAbstract);
 		injectMethod(fieldNode.up(), method);
 	}
 	
-	public MethodDeclaration createWither(TypeDeclaration parent, EclipseNode fieldNode, String name, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam) {
+	public MethodDeclaration createWither(TypeDeclaration parent, EclipseNode fieldNode, String name, int modifier, EclipseNode sourceNode, List<Annotation> onMethod, List<Annotation> onParam, boolean makeAbstract ) {
 		ASTNode source = sourceNode.get();
 		if (name == null) return null;
 		FieldDeclaration field = (FieldDeclaration) fieldNode.get();
 		int pS = source.sourceStart, pE = source.sourceEnd;
-		long p = (long)pS << 32 | pE;
+		long p = (long) pS << 32 | pE;
 		MethodDeclaration method = new MethodDeclaration(parent.compilationResult);
+		if (makeAbstract) modifier = modifier | ClassFileConstants.AccAbstract | ExtraCompilerModifiers.AccSemicolonBody;
 		method.modifiers = modifier;
 		method.returnType = cloneSelfType(fieldNode, source);
 		if (method.returnType == null) return null;
@@ -228,7 +232,7 @@ public class HandleWither extends EclipseAnnotationHandler<Wither> {
 			deprecated = new Annotation[] { generateDeprecatedAnnotation(source) };
 		}
 		method.annotations = copyAnnotations(source, onMethod.toArray(new Annotation[0]), deprecated);
-		Argument param = new Argument(field.name, p, copyType(field.type, source), Modifier.FINAL);
+		Argument param = new Argument(field.name, p, copyType(field.type, source), ClassFileConstants.AccFinal);
 		param.sourceStart = pS; param.sourceEnd = pE;
 		method.arguments = new Argument[] { param };
 		method.selector = name.toCharArray();
@@ -237,49 +241,51 @@ public class HandleWither extends EclipseAnnotationHandler<Wither> {
 		method.typeParameters = null;
 		method.bits |= ECLIPSE_DO_NOT_TOUCH_FLAG;
 		
-		List<Expression> args = new ArrayList<Expression>();
-		for (EclipseNode child : fieldNode.up().down()) {
-			if (child.getKind() != Kind.FIELD) continue;
-			FieldDeclaration childDecl = (FieldDeclaration) child.get();
-			// Skip fields that start with $
-			if (childDecl.name != null && childDecl.name.length > 0 && childDecl.name[0] == '$') continue;
-			long fieldFlags = childDecl.modifiers;
-			// Skip static fields.
-			if ((fieldFlags & ClassFileConstants.AccStatic) != 0) continue;
-			// Skip initialized final fields.
-			if (((fieldFlags & ClassFileConstants.AccFinal) != 0) && childDecl.initialization != null) continue;
-			if (child.get() == fieldNode.get()) {
-				args.add(new SingleNameReference(field.name, p));
-			} else {
-				args.add(createFieldAccessor(child, FieldAccess.ALWAYS_FIELD, source));
-			}
-		}
-		
-		AllocationExpression constructorCall = new AllocationExpression();
-		constructorCall.arguments = args.toArray(new Expression[0]);
-		constructorCall.type = cloneSelfType(fieldNode, source);
-		
-		Expression identityCheck = new EqualExpression(
-				createFieldAccessor(fieldNode, FieldAccess.ALWAYS_FIELD, source),
-				new SingleNameReference(field.name, p),
-				OperatorIds.EQUAL_EQUAL);
-		ThisReference thisRef = new ThisReference(pS, pE);
-		Expression conditional = new ConditionalExpression(identityCheck, thisRef, constructorCall);
-		Statement returnStatement = new ReturnStatement(conditional, pS, pE);
-		method.bodyStart = method.declarationSourceStart = method.sourceStart = source.sourceStart;
-		method.bodyEnd = method.declarationSourceEnd = method.sourceEnd = source.sourceEnd;
-		
 		Annotation[] nonNulls = findAnnotations(field, NON_NULL_PATTERN);
 		Annotation[] nullables = findAnnotations(field, NULLABLE_PATTERN);
-		List<Statement> statements = new ArrayList<Statement>(5);
-		if (nonNulls.length > 0) {
-			Statement nullCheck = generateNullCheck(field, sourceNode);
-			if (nullCheck != null) statements.add(nullCheck);
-		}
-		statements.add(returnStatement);
-		
-		method.statements = statements.toArray(new Statement[0]);
 		
+		if (!makeAbstract) {
+			List<Expression> args = new ArrayList<Expression>();
+			for (EclipseNode child : fieldNode.up().down()) {
+				if (child.getKind() != Kind.FIELD) continue;
+				FieldDeclaration childDecl = (FieldDeclaration) child.get();
+				// Skip fields that start with $
+				if (childDecl.name != null && childDecl.name.length > 0 && childDecl.name[0] == '$') continue;
+				long fieldFlags = childDecl.modifiers;
+				// Skip static fields.
+				if ((fieldFlags & ClassFileConstants.AccStatic) != 0) continue;
+				// Skip initialized final fields.
+				if (((fieldFlags & ClassFileConstants.AccFinal) != 0) && childDecl.initialization != null) continue;
+				if (child.get() == fieldNode.get()) {
+					args.add(new SingleNameReference(field.name, p));
+				} else {
+					args.add(createFieldAccessor(child, FieldAccess.ALWAYS_FIELD, source));
+				}
+			}
+			
+			AllocationExpression constructorCall = new AllocationExpression();
+			constructorCall.arguments = args.toArray(new Expression[0]);
+			constructorCall.type = cloneSelfType(fieldNode, source);
+			
+			Expression identityCheck = new EqualExpression(
+					createFieldAccessor(fieldNode, FieldAccess.ALWAYS_FIELD, source),
+					new SingleNameReference(field.name, p),
+					OperatorIds.EQUAL_EQUAL);
+			ThisReference thisRef = new ThisReference(pS, pE);
+			Expression conditional = new ConditionalExpression(identityCheck, thisRef, constructorCall);
+			Statement returnStatement = new ReturnStatement(conditional, pS, pE);
+			method.bodyStart = method.declarationSourceStart = method.sourceStart = source.sourceStart;
+			method.bodyEnd = method.declarationSourceEnd = method.sourceEnd = source.sourceEnd;
+			
+			List<Statement> statements = new ArrayList<Statement>(5);
+			if (nonNulls.length > 0) {
+				Statement nullCheck = generateNullCheck(field, sourceNode);
+				if (nullCheck != null) statements.add(nullCheck);
+			}
+			statements.add(returnStatement);
+			
+			method.statements = statements.toArray(new Statement[0]);
+		}
 		param.annotations = copyAnnotations(source, nonNulls, nullables, onParam.toArray(new Annotation[0]));
 		
 		method.traverse(new SetGeneratedByVisitor(source), parent.scope);
diff --git a/src/core/lombok/javac/handlers/HandleWither.java b/src/core/lombok/javac/handlers/HandleWither.java
index 5de18686..f8b6152a 100644
--- a/src/core/lombok/javac/handlers/HandleWither.java
+++ b/src/core/lombok/javac/handlers/HandleWither.java
@@ -150,7 +150,10 @@ public class HandleWither extends JavacAnnotationHandler<Wither> {
 		}
 	}
 	
-	public void createWitherForField(AccessLevel level, JavacNode fieldNode, JavacNode source, boolean whineIfExists, List<JCAnnotation> onMethod, List<JCAnnotation> onParam) {
+	public void createWitherForField(AccessLevel level, JavacNode fieldNode, JavacNode source, boolean strictMode, List<JCAnnotation> onMethod, List<JCAnnotation> onParam) {
+		JavacNode typeNode = fieldNode.up();
+		boolean makeAbstract = typeNode != null && typeNode.getKind() == Kind.TYPE && (((JCClassDecl) typeNode.get()).mods.flags & Flags.ABSTRACT) != 0;
+		
 		if (fieldNode.getKind() != Kind.FIELD) {
 			fieldNode.addError("@Wither is only supported on a class or a field.");
 			return;
@@ -165,17 +168,23 @@ public class HandleWither extends JavacAnnotationHandler<Wither> {
 		}
 		
 		if ((fieldDecl.mods.flags & Flags.STATIC) != 0) {
-			fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for static fields.");
+			if (strictMode) {
+				fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for static fields.");
+			}
 			return;
 		}
 		
 		if ((fieldDecl.mods.flags & Flags.FINAL) != 0 && fieldDecl.init != null) {
-			fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for final, initialized fields.");
+			if (strictMode) {
+				fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for final, initialized fields.");
+			}
 			return;
 		}
 		
 		if (fieldDecl.name.toString().startsWith("$")) {
-			fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for fields starting with $.");
+			if (strictMode) {
+				fieldNode.addWarning("Not generating wither for this field: Withers cannot be generated for fields starting with $.");
+			}
 			return;
 		}
 		
@@ -184,7 +193,7 @@ public class HandleWither extends JavacAnnotationHandler<Wither> {
 			case EXISTS_BY_LOMBOK:
 				return;
 			case EXISTS_BY_USER:
-				if (whineIfExists) {
+				if (strictMode) {
 					String altNameExpl = "";
 					if (!altName.equals(methodName)) altNameExpl = String.format(" (%s)", altName);
 					fieldNode.addWarning(
@@ -199,63 +208,68 @@ public class HandleWither extends JavacAnnotationHandler<Wither> {
 		
 		long access = toJavacModifier(level);
 		
-		JCMethodDecl createdWither = createWither(access, fieldNode, fieldNode.getTreeMaker(), source, onMethod, onParam);
-		injectMethod(fieldNode.up(), createdWither);
+		JCMethodDecl createdWither = createWither(access, fieldNode, fieldNode.getTreeMaker(), source, onMethod, onParam, makeAbstract);
+		injectMethod(typeNode, createdWither);
 	}
 	
-	public JCMethodDecl createWither(long access, JavacNode field, JavacTreeMaker maker, JavacNode source, List<JCAnnotation> onMethod, List<JCAnnotation> onParam) {
+	public JCMethodDecl createWither(long access, JavacNode field, JavacTreeMaker maker, JavacNode source, List<JCAnnotation> onMethod, List<JCAnnotation> onParam, boolean makeAbstract) {
 		String witherName = toWitherName(field);
 		if (witherName == null) return null;
 		
 		JCVariableDecl fieldDecl = (JCVariableDecl) field.get();
 		
-		ListBuffer<JCStatement> statements = new ListBuffer<JCStatement>();
 		List<JCAnnotation> nonNulls = findAnnotations(field, NON_NULL_PATTERN);
 		List<JCAnnotation> nullables = findAnnotations(field, NULLABLE_PATTERN);
 		
 		Name methodName = field.toName(witherName);
-		List<JCAnnotation> annsOnParam = copyAnnotations(onParam).appendList(nonNulls).appendList(nullables);
 		
+		JCExpression returnType = cloneSelfType(field);
+		
+		JCBlock methodBody = null;
 		long flags = JavacHandlerUtil.addFinalIfNeeded(Flags.PARAMETER, field.getContext());
-		JCVariableDecl param = maker.VarDef(maker.Modifiers(flags, annsOnParam), fieldDecl.name, fieldDecl.vartype, null);
+		List<JCAnnotation> annsOnParam = copyAnnotations(onParam).appendList(nonNulls).appendList(nullables);
 		
-		JCExpression selfType = cloneSelfType(field);
-		if (selfType == null) return null;
+		JCVariableDecl param = maker.VarDef(maker.Modifiers(flags, annsOnParam), fieldDecl.name, fieldDecl.vartype, null);
 		
-		ListBuffer<JCExpression> args = new ListBuffer<JCExpression>();
-		for (JavacNode child : field.up().down()) {
-			if (child.getKind() != Kind.FIELD) continue;
-			JCVariableDecl childDecl = (JCVariableDecl) child.get();
-			// Skip fields that start with $
-			if (childDecl.name.toString().startsWith("$")) continue;
-			long fieldFlags = childDecl.mods.flags;
-			// Skip static fields.
-			if ((fieldFlags & Flags.STATIC) != 0) continue;
-			// Skip initialized final fields.
-			if (((fieldFlags & Flags.FINAL) != 0) && childDecl.init != null) continue;
-			if (child.get() == field.get()) {
-				args.append(maker.Ident(fieldDecl.name));
+		if (!makeAbstract) {
+			ListBuffer<JCStatement> statements = new ListBuffer<JCStatement>();
+			
+			JCExpression selfType = cloneSelfType(field);
+			if (selfType == null) return null;
+			
+			ListBuffer<JCExpression> args = new ListBuffer<JCExpression>();
+			for (JavacNode child : field.up().down()) {
+				if (child.getKind() != Kind.FIELD) continue;
+				JCVariableDecl childDecl = (JCVariableDecl) child.get();
+				// Skip fields that start with $
+				if (childDecl.name.toString().startsWith("$")) continue;
+				long fieldFlags = childDecl.mods.flags;
+				// Skip static fields.
+				if ((fieldFlags & Flags.STATIC) != 0) continue;
+				// Skip initialized final fields.
+				if (((fieldFlags & Flags.FINAL) != 0) && childDecl.init != null) continue;
+				if (child.get() == field.get()) {
+					args.append(maker.Ident(fieldDecl.name));
+				} else {
+					args.append(createFieldAccessor(maker, child, FieldAccess.ALWAYS_FIELD));
+				}
+			}
+			
+			JCNewClass newClass = maker.NewClass(null, List.<JCExpression>nil(), selfType, args.toList(), null);
+			JCExpression identityCheck = maker.Binary(CTC_EQUAL, createFieldAccessor(maker, field, FieldAccess.ALWAYS_FIELD), maker.Ident(fieldDecl.name));
+			JCConditional conditional = maker.Conditional(identityCheck, maker.Ident(field.toName("this")), newClass);
+			JCReturn returnStatement = maker.Return(conditional);
+			
+			if (nonNulls.isEmpty()) {
+				statements.append(returnStatement);
 			} else {
-				args.append(createFieldAccessor(maker, child, FieldAccess.ALWAYS_FIELD));
+				JCStatement nullCheck = generateNullCheck(maker, field, source);
+				if (nullCheck != null) statements.append(nullCheck);
+				statements.append(returnStatement);
 			}
+			
+			methodBody = maker.Block(0, statements.toList());
 		}
-		
-		JCNewClass newClass = maker.NewClass(null, List.<JCExpression>nil(), selfType, args.toList(), null);
-		JCExpression identityCheck = maker.Binary(CTC_EQUAL, createFieldAccessor(maker, field, FieldAccess.ALWAYS_FIELD), maker.Ident(fieldDecl.name));
-		JCConditional conditional = maker.Conditional(identityCheck, maker.Ident(field.toName("this")), newClass);
-		JCReturn returnStatement = maker.Return(conditional);
-		
-		if (nonNulls.isEmpty()) {
-			statements.append(returnStatement);
-		} else {
-			JCStatement nullCheck = generateNullCheck(maker, field, source);
-			if (nullCheck != null) statements.append(nullCheck);
-			statements.append(returnStatement);
-		}
-		
-		JCExpression returnType = cloneSelfType(field);
-		
-		JCBlock methodBody = maker.Block(0, statements.toList());
 		List<JCTypeParameter> methodGenericParams = List.nil();
 		List<JCVariableDecl> parameters = List.of(param);
 		List<JCExpression> throwsClauses = List.nil();
@@ -266,6 +280,7 @@ public class HandleWither extends JavacAnnotationHandler<Wither> {
 		if (isFieldDeprecated(field)) {
 			annsOnMethod = annsOnMethod.prepend(maker.Annotation(genJavaLangTypeRef(field, "Deprecated"), List.<JCExpression>nil()));
 		}
+		if (makeAbstract) access = access | Flags.ABSTRACT;
 		JCMethodDecl decl = recursiveSetGeneratedBy(maker.MethodDef(maker.Modifiers(access, annsOnMethod), methodName, returnType,
 				methodGenericParams, parameters, throwsClauses, methodBody, annotationMethodDefaultValue), source.get(), field.getContext());
 		copyJavadoc(field, decl, CopyJavadoc.WITHER);
diff --git a/test/transform/resource/after-delombok/WitherWithAbstract.java b/test/transform/resource/after-delombok/WitherWithAbstract.java
new file mode 100644
index 00000000..d5bfeef8
--- /dev/null
+++ b/test/transform/resource/after-delombok/WitherWithAbstract.java
@@ -0,0 +1,6 @@
+abstract class WitherWithAbstract {
+	String foo;
+	@java.lang.SuppressWarnings("all")
+	@javax.annotation.Generated("lombok")
+	public abstract WitherWithAbstract withFoo(final String foo);
+}
\ No newline at end of file
diff --git a/test/transform/resource/after-ecj/WitherWithAbstract.java b/test/transform/resource/after-ecj/WitherWithAbstract.java
new file mode 100644
index 00000000..90fd6448
--- /dev/null
+++ b/test/transform/resource/after-ecj/WitherWithAbstract.java
@@ -0,0 +1,7 @@
+abstract class WitherWithAbstract {
+  @lombok.experimental.Wither String foo;
+  WitherWithAbstract() {
+    super();
+  }
+  public abstract @java.lang.SuppressWarnings("all") @javax.annotation.Generated("lombok") WitherWithAbstract withFoo(final String foo);
+}
diff --git a/test/transform/resource/before/WitherWithAbstract.java b/test/transform/resource/before/WitherWithAbstract.java
new file mode 100644
index 00000000..acc9094b
--- /dev/null
+++ b/test/transform/resource/before/WitherWithAbstract.java
@@ -0,0 +1,3 @@
+abstract class WitherWithAbstract {
+	@lombok.experimental.Wither String foo;
+}
\ No newline at end of file
diff --git a/website/features/experimental/Wither.html b/website/features/experimental/Wither.html
index 81ef475e..60dd0464 100644
--- a/website/features/experimental/Wither.html
+++ b/website/features/experimental/Wither.html
@@ -74,6 +74,8 @@
 				<p>
 					Withers cannot be generated for static fields because that makes no sense.
 				</p><p>
+					Withers can be generated for abstract classes, but this generates an abstract method with the appropriate signature.
+				</p><p>
 					When applying <code>@Wither</code> to a type, static fields and fields whose name start with a $ are skipped.
 				</p><p>
 					For generating the method names, the first character of the field, if it is a lowercase character, is title-cased, otherwise, it is left unmodified.