From 04e9397971d3689e0710df026f673f8b0a6cb685 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Fri, 18 Mar 2011 19:30:02 +0100 Subject: Rewritten @Cleanup's new null analysis prevention to not use Lombok.preventNullAnalysis but go with Collections.singletonList(expr).get(0) instead; while this does create a pointless object, it doesn't cause a clash when eclipse has lombok 0.10 installed but the project uses 0.9, which doesn't have preventNullAnalysis. Eventually, once 0.9 is long forgotten, this can be reverted. --- .../lombok/eclipse/handlers/HandleCleanup.java | 43 ++++++++++++++++------ src/core/lombok/javac/handlers/HandleCleanup.java | 8 +++- 2 files changed, 38 insertions(+), 13 deletions(-) (limited to 'src/core/lombok') diff --git a/src/core/lombok/eclipse/handlers/HandleCleanup.java b/src/core/lombok/eclipse/handlers/HandleCleanup.java index 964653bc..9a63ce47 100644 --- a/src/core/lombok/eclipse/handlers/HandleCleanup.java +++ b/src/core/lombok/eclipse/handlers/HandleCleanup.java @@ -42,6 +42,7 @@ import org.eclipse.jdt.internal.compiler.ast.CastExpression; import org.eclipse.jdt.internal.compiler.ast.EqualExpression; import org.eclipse.jdt.internal.compiler.ast.Expression; import org.eclipse.jdt.internal.compiler.ast.IfStatement; +import org.eclipse.jdt.internal.compiler.ast.IntLiteral; import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration; import org.eclipse.jdt.internal.compiler.ast.MemberValuePair; import org.eclipse.jdt.internal.compiler.ast.MessageSend; @@ -174,16 +175,7 @@ public class HandleCleanup implements EclipseAnnotationHandler { NullLiteral nullLiteral = new NullLiteral(pS, pE); Eclipse.setGeneratedBy(nullLiteral, ast); - MessageSend preventNullAnalysis = new MessageSend(); - Eclipse.setGeneratedBy(preventNullAnalysis, ast); - - preventNullAnalysis.receiver = createNameReference("lombok.Lombok", ast); - preventNullAnalysis.selector = "preventNullAnalysis".toCharArray(); - - preventNullAnalysis.arguments = new Expression[] { varName }; - preventNullAnalysis.nameSourcePosition = p; - preventNullAnalysis.sourceStart = pS; - preventNullAnalysis.sourceEnd = preventNullAnalysis.statementEnd = pE; + MessageSend preventNullAnalysis = preventNullAnalysis(ast, varName); EqualExpression equalExpression = new EqualExpression(preventNullAnalysis, nullLiteral, OperatorIds.NOT_EQUAL); equalExpression.sourceStart = pS; equalExpression.sourceEnd = pE; @@ -195,8 +187,6 @@ public class HandleCleanup implements EclipseAnnotationHandler { Eclipse.setGeneratedBy(closeBlock, ast); IfStatement ifStatement = new IfStatement(equalExpression, closeBlock, 0, 0); Eclipse.setGeneratedBy(ifStatement, ast); - - finallyBlock[0] = ifStatement; tryStatement.finallyBlock = new Block(0); @@ -219,6 +209,35 @@ public class HandleCleanup implements EclipseAnnotationHandler { return true; } + private MessageSend preventNullAnalysis(Annotation ast, Expression expr) { + MessageSend singletonList = new MessageSend(); + Eclipse.setGeneratedBy(singletonList, ast); + + int pS = ast.sourceStart, pE = ast.sourceEnd; + long p = (long)pS << 32 | pE; + + singletonList.receiver = createNameReference("java.util.Collections", ast); + singletonList.selector = "singletonList".toCharArray(); + + singletonList.arguments = new Expression[] { expr }; + singletonList.nameSourcePosition = p; + singletonList.sourceStart = pS; + singletonList.sourceEnd = singletonList.statementEnd = pE; + + MessageSend preventNullAnalysis = new MessageSend(); + Eclipse.setGeneratedBy(preventNullAnalysis, ast); + + preventNullAnalysis.receiver = singletonList; + preventNullAnalysis.selector = "get".toCharArray(); + + preventNullAnalysis.arguments = new Expression[] { new IntLiteral(new char[] { '0' }, pS, pE) }; + preventNullAnalysis.nameSourcePosition = p; + preventNullAnalysis.sourceStart = pS; + preventNullAnalysis.sourceEnd = singletonList.statementEnd = pE; + + return preventNullAnalysis; + } + private void doAssignmentCheck(EclipseNode node, Statement[] tryBlock, char[] varName) { for (Statement statement : tryBlock) doAssignmentCheck0(node, statement, varName); } diff --git a/src/core/lombok/javac/handlers/HandleCleanup.java b/src/core/lombok/javac/handlers/HandleCleanup.java index 9e2fddf6..b681ab28 100644 --- a/src/core/lombok/javac/handlers/HandleCleanup.java +++ b/src/core/lombok/javac/handlers/HandleCleanup.java @@ -117,7 +117,7 @@ public class HandleCleanup implements JavacAnnotationHandler { List cleanupCall = List.of(maker.Exec( maker.Apply(List.nil(), cleanupMethod, List.nil()))); - JCMethodInvocation preventNullAnalysis = maker.Apply(List.nil(), JavacHandlerUtil.chainDotsString(maker, annotationNode, "lombok.Lombok.preventNullAnalysis"), List.of(maker.Ident(decl.name))); + JCMethodInvocation preventNullAnalysis = preventNullAnalysis(maker, annotationNode, maker.Ident(decl.name)); JCBinary isNull = maker.Binary(Javac.getCTCint(JCTree.class, "NE"), preventNullAnalysis, maker.Literal(Javac.getCTCint(TypeTags.class, "BOT"), null)); JCIf ifNotNullCleanup = maker.If(isNull, maker.Block(0, cleanupCall), null); @@ -139,6 +139,12 @@ public class HandleCleanup implements JavacAnnotationHandler { return true; } + private JCMethodInvocation preventNullAnalysis(TreeMaker maker, JavacNode node, JCExpression expression) { + JCMethodInvocation singletonList = maker.Apply(List.nil(), JavacHandlerUtil.chainDotsString(maker, node, "java.util.Collections.singletonList"), List.of(expression)); + JCMethodInvocation cleanedExpr = maker.Apply(List.nil(), maker.Select(singletonList, node.toName("get")) , List.of(maker.Literal(TypeTags.INT, 0))); + return cleanedExpr; + } + private void doAssignmentCheck(JavacNode node, List statements, Name name) { for (JCStatement statement : statements) doAssignmentCheck0(node, statement, name); } -- cgit From 7ebf05aaec0c4ed0e70a3a2a8a0f56afd029bd46 Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Fri, 1 Apr 2011 18:07:17 +0200 Subject: Added ability to log warnings in eclipse error log. --- src/core/lombok/eclipse/Eclipse.java | 71 ++++++++++++++++++++++------- src/core/lombok/eclipse/HandlerLibrary.java | 2 +- 2 files changed, 56 insertions(+), 17 deletions(-) (limited to 'src/core/lombok') diff --git a/src/core/lombok/eclipse/Eclipse.java b/src/core/lombok/eclipse/Eclipse.java index ddba726a..8910bb3e 100644 --- a/src/core/lombok/eclipse/Eclipse.java +++ b/src/core/lombok/eclipse/Eclipse.java @@ -1,5 +1,5 @@ /* - * Copyright © 2009 Reinier Zwitserloot and Roel Spilker. + * Copyright © 2009-2011 Reinier Zwitserloot and Roel Spilker. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -89,46 +89,85 @@ public class Eclipse { /** * Generates an error in the Eclipse error log. Note that most people never look at it! + * + * @param cud The {@code CompilationUnitDeclaration} where the error occurred. + * An error will be generated on line 0 linking to the error log entry. Can be {@code null}. + * @param message Human readable description of the problem. + * @param error The associated exception. Can be {@code null}. */ - public static void error(CompilationUnitDeclaration cud, String message) { - error(cud, message, DEFAULT_BUNDLE, null); + public static void error(CompilationUnitDeclaration cud, String message, Throwable error) { + error(cud, message, null, error); } /** * Generates an error in the Eclipse error log. Note that most people never look at it! + * + * @param cud The {@code CompilationUnitDeclaration} where the error occurred. + * An error will be generated on line 0 linking to the error log entry. Can be {@code null}. + * @param message Human readable description of the problem. + * @param bundleName Can be {@code null} to default to {@code org.eclipse.jdt.core} which is usually right. + * @param error The associated exception. Can be {@code null}. */ - public static void error(CompilationUnitDeclaration cud, String message, Throwable error) { - error(cud, message, DEFAULT_BUNDLE, error); + public static void error(CompilationUnitDeclaration cud, String message, String bundleName, Throwable error) { + if (bundleName == null) bundleName = DEFAULT_BUNDLE; + try { + new EclipseWorkspaceLogger().error(message, bundleName, error); + } catch (NoClassDefFoundError e) { //standalone ecj does not jave Platform, ILog, IStatus, and friends. + new TerminalLogger().error(message, bundleName, error); + } + if (cud != null) EclipseAST.addProblemToCompilationResult(cud, false, message + " - See error log.", 0, 0); } /** - * Generates an error in the Eclipse error log. Note that most people never look at it! + * Generates a warning in the Eclipse error log. Note that most people never look at it! + * + * @param cud The {@code CompilationUnitDeclaration} where the error occurred. + * An error will be generated on line 0 linking to the error log entry. Can be {@code null}. + * @param message Human readable description of the problem. + * @param error The associated exception. Can be {@code null}. */ - public static void error(CompilationUnitDeclaration cud, String message, String bundleName) { - error(cud, message, bundleName, null); + public static void warning(String message, Throwable error) { + warning(message, null, error); } /** - * Generates an error in the Eclipse error log. Note that most people never look at it! + * Generates a warning in the Eclipse error log. Note that most people never look at it! + * + * @param message Human readable description of the problem. + * @param bundleName Can be {@code null} to default to {@code org.eclipse.jdt.core} which is usually right. + * @param error The associated exception. Can be {@code null}. */ - public static void error(CompilationUnitDeclaration cud, String message, String bundleName, Throwable error) { + public static void warning(String message, String bundleName, Throwable error) { + if (bundleName == null) bundleName = DEFAULT_BUNDLE; try { - new EclipseWorkspaceLogger().error(message, bundleName, error); - } catch (NoClassDefFoundError e) { //standalone ecj does not jave Platform, ILog, IStatus, and friends. - new TerminalLogger().error(message, bundleName, error); + new EclipseWorkspaceLogger().warning(message, bundleName, error); + } catch (NoClassDefFoundError e) { //standalone ecj does not jave Platform, ILog, IStatus, and friends. + new TerminalLogger().warning(message, bundleName, error); } - if (cud != null) EclipseAST.addProblemToCompilationResult(cud, false, message + " - See error log.", 0, 0); } private static class TerminalLogger { void error(String message, String bundleName, Throwable error) { System.err.println(message); - error.printStackTrace(); + if (error != null) error.printStackTrace(); + } + + void warning(String message, String bundleName, Throwable error) { + System.err.println(message); + if (error != null) error.printStackTrace(); } } private static class EclipseWorkspaceLogger { void error(String message, String bundleName, Throwable error) { + msg(IStatus.ERROR, message, bundleName, error); + } + + void warning(String message, String bundleName, Throwable error) { + msg(IStatus.WARNING, message, bundleName, error); + } + + private void msg(int msgType, String message, String bundleName, Throwable error) { Bundle bundle = Platform.getBundle(bundleName); if (bundle == null) { System.err.printf("Can't find bundle %s while trying to report error:\n%s\n", bundleName, message); @@ -137,7 +176,7 @@ public class Eclipse { ILog log = Platform.getLog(bundle); - log.log(new Status(IStatus.ERROR, bundleName, message, error)); + log.log(new Status(msgType, bundleName, message, error)); } } diff --git a/src/core/lombok/eclipse/HandlerLibrary.java b/src/core/lombok/eclipse/HandlerLibrary.java index df4681f0..ba7f891a 100644 --- a/src/core/lombok/eclipse/HandlerLibrary.java +++ b/src/core/lombok/eclipse/HandlerLibrary.java @@ -103,7 +103,7 @@ public class HandlerLibrary { SpiLoadUtil.findAnnotationClass(handler.getClass(), EclipseAnnotationHandler.class); AnnotationHandlerContainer container = new AnnotationHandlerContainer(handler, annotationClass); if (lib.annotationHandlers.put(container.annotationClass.getName(), container) != null) { - Eclipse.error(null, "Duplicate handlers for annotation type: " + container.annotationClass.getName()); + Eclipse.error(null, "Duplicate handlers for annotation type: " + container.annotationClass.getName(), null); } lib.typeLibrary.addType(container.annotationClass.getName()); } catch (Throwable t) { -- cgit From 1a589ca221a172845a108a6c759018c6e25d5cdd Mon Sep 17 00:00:00 2001 From: Reinier Zwitserloot Date: Fri, 1 Apr 2011 18:07:46 +0200 Subject: Now logging a warning with diagnostic info so that the next time a user runs into issue #164, we can get a more useful stracktrace from them. --- .../lombok/eclipse/handlers/EclipseHandlerUtil.java | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'src/core/lombok') diff --git a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java index 860cf44f..a83ba836 100644 --- a/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java +++ b/src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java @@ -500,6 +500,27 @@ public class EclipseHandlerUtil { method.annotations = createSuppressWarningsAll(method, method.annotations); TypeDeclaration parent = (TypeDeclaration) type.get(); + if (parent.scope != null && method.scope == null) { + // We think this means heisenbug #164 is about to happen later in some other worker thread. + // To improve our ability to figure out what the heck is going on, let's generate a log so we can ask those who stumble on this about it, + // and thus see a far more useful stack trace. + boolean report = true; + for (StackTraceElement elem : Thread.currentThread().getStackTrace()) { + // We intentionally hook into the middle of ClassScope filling in BlockScopes for PatchDelegate, + // meaning that will trigger a false positive. Detect it and do not report the occurence of #164 if so. + if ("lombok.eclipse.agent.PatchDelegate".equals(elem.getClassName())) { + report = false; + break; + } + } + + if (report) { + Eclipse.warning("We believe you may have just stumbled on lombok issue #164. Please " + + "report the stack trace associated with this message at:\n" + + "http://code.google.com/p/projectlombok/issues/detail?id=164", new Throwable()); + } + } + if (parent.methods == null) { parent.methods = new AbstractMethodDeclaration[1]; parent.methods[0] = method; -- cgit