In delombok, we mark the AST as changed if we remove an annotation; this fixes the issue where delombok would leave lombok annotations in the file if that annotation had no actual effect (such as @Getter on a field if there is an explicit getX method for that field).

issue #443: delombok would screw up @SneakyThrows on methods or constructors with empty bodies. Now we generate warnings for this.

4 files changed, 43 insertions, 7 deletions

diff --git a/src/core/lombok/eclipse/handlers/HandleSneakyThrows.java b/src/core/lombok/eclipse/handlers/HandleSneakyThrows.java
index b7c8a5d8..aa78ca3b 100644
--- a/src/core/lombok/eclipse/handlers/HandleSneakyThrows.java
+++ b/src/core/lombok/eclipse/handlers/HandleSneakyThrows.java
@@ -40,6 +40,8 @@ import org.eclipse.jdt.internal.compiler.ast.Annotation;
 import org.eclipse.jdt.internal.compiler.ast.Argument;
 import org.eclipse.jdt.internal.compiler.ast.ArrayInitializer;
 import org.eclipse.jdt.internal.compiler.ast.Block;
+import org.eclipse.jdt.internal.compiler.ast.ConstructorDeclaration;
+import org.eclipse.jdt.internal.compiler.ast.ExplicitConstructorCall;
 import org.eclipse.jdt.internal.compiler.ast.Expression;
 import org.eclipse.jdt.internal.compiler.ast.MemberValuePair;
 import org.eclipse.jdt.internal.compiler.ast.MessageSend;
@@ -147,7 +149,21 @@ public class HandleSneakyThrows extends EclipseAnnotationHandler<SneakyThrows> {
 			return;
 		}
 		
-		if (method.statements == null) return;
+		if (method.statements == null || method.statements.length == 0) {
+			boolean hasConstructorCall = false;
+			if (method instanceof ConstructorDeclaration) {
+				ExplicitConstructorCall constructorCall = ((ConstructorDeclaration) method).constructorCall;
+				hasConstructorCall = constructorCall != null && !constructorCall.isImplicitSuper() && !constructorCall.isImplicitThis();
+			}
+			
+			if (hasConstructorCall) {
+				annotation.addWarning("Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.");
+			} else {
+				annotation.addWarning("This method or constructor is empty; @SneakyThrows has been ignored.");
+			}
+			
+			return;
+		}
 		
 		Statement[] contents = method.statements;
 		
@@ -160,9 +176,9 @@ public class HandleSneakyThrows extends EclipseAnnotationHandler<SneakyThrows> {
 	}
 	
 	private Statement buildTryCatchBlock(Statement[] contents, DeclaredException exception, ASTNode source, AbstractMethodDeclaration method) {
-		int methodStart =  method.bodyStart;
-		int methodEnd =  method.bodyEnd;
-		long methodPosEnd = methodEnd << 32 | (methodEnd & 0xFFFFFFFFL);
+		int methodStart = method.bodyStart;
+		int methodEnd = method.bodyEnd;
+		long methodPosEnd = ((long) methodEnd) << 32 | (methodEnd & 0xFFFFFFFFL);
 		
 		TryStatement tryStatement = new TryStatement();
 		setGeneratedBy(tryStatement, source);
diff --git a/src/core/lombok/javac/handlers/HandleSneakyThrows.java b/src/core/lombok/javac/handlers/HandleSneakyThrows.java
index a5bd74e7..c2394fc8 100644
--- a/src/core/lombok/javac/handlers/HandleSneakyThrows.java
+++ b/src/core/lombok/javac/handlers/HandleSneakyThrows.java
@@ -84,13 +84,20 @@ public class HandleSneakyThrows extends JavacAnnotationHandler<SneakyThrows> {
 			return;
 		}
 		
-		if (method.body == null) return;
-		if (method.body.stats.isEmpty()) return;
+		if (method.body == null || method.body.stats.isEmpty()) {
+			generateEmptyBlockWarning(methodNode, annotation, false);
+			return;
+		}
 		
 		final JCStatement constructorCall = method.body.stats.get(0);
 		final boolean isConstructorCall = isConstructorCall(constructorCall);
 		List<JCStatement> contents = isConstructorCall ? method.body.stats.tail : method.body.stats;
 		
+		if (contents == null || contents.isEmpty()) {
+			generateEmptyBlockWarning(methodNode, annotation, true);
+			return;
+		}
+		
 		for (String exception : exceptions) {
 			contents = List.of(buildTryCatchBlock(methodNode, contents, exception, annotation.get()));
 		}
@@ -99,6 +106,14 @@ public class HandleSneakyThrows extends JavacAnnotationHandler<SneakyThrows> {
 		methodNode.rebuild();
 	}
 	
+	private void generateEmptyBlockWarning(JavacNode methodNode, JavacNode annotation, boolean hasConstructorCall) {
+		if (hasConstructorCall) {
+			annotation.addWarning("Calls to sibling / super constructors are always excluded from @SneakyThrows; @SneakyThrows has been ignored because there is no other code in this constructor.");
+		} else {
+			annotation.addWarning("This method or constructor is empty; @SneakyThrows has been ignored.");
+		}
+	}
+
 	private boolean isConstructorCall(final JCStatement supect) {
 		if (!(supect instanceof JCExpressionStatement)) return false;
 		final JCExpression supectExpression = ((JCExpressionStatement) supect).expr;
diff --git a/src/core/lombok/javac/handlers/JavacHandlerUtil.java b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
index c2de5b05..e79dd5dc 100644
--- a/src/core/lombok/javac/handlers/JavacHandlerUtil.java
+++ b/src/core/lombok/javac/handlers/JavacHandlerUtil.java
@@ -305,6 +305,7 @@ public class JavacHandlerUtil {
 			return;
 		}
 		
+		parentNode.getAst().setChanged();
 		deleteImportFromCompilationUnit(annotation, annotationType.getName());
 	}
 	
diff --git a/website/features/SneakyThrows.html b/website/features/SneakyThrows.html
index 0f04b7d9..808a7879 100644
--- a/website/features/SneakyThrows.html
+++ b/website/features/SneakyThrows.html
@@ -64,7 +64,11 @@
 				statement in a try/catch block with just <code>e.printStackTrace()</code> in the catch block. This is so spectacularly non-productive
 				compared to just sneakily throwing the exception onwards, that Roel and Reinier feel more than justified in claiming that the
 				checked exception system is far from perfect, and thus an opt-out mechanism is warranted.
-				</p>
+				</p><p>
+				If you put <code>@SneakyThrows</code> on a constructor, any call to a sibling or super constructor is <em>excluded</em> from the <code>@SneakyThrows</code> treatment. This is a
+				java restriction we cannot work around: Calls to sibling/super constructors MUST be the first statement in the constructor; they cannot be placed inside try/catch blocks.
+				</p><p>
+				<code>@SneakyThrows</code> on an empty method, or a constructor that is empty or only has a call to a sibling / super constructor results in no try/catch block and a warning.
 			</div>
 		</div>
 		<div class="footer">