groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pa...@apache.org
Subject [50/50] groovy git commit: GROOVY-8060: @Log annotation does not check logging enablement inside closures which are arguments to methods
Date Thu, 26 Jan 2017 05:31:24 GMT
GROOVY-8060: @Log annotation does not check logging enablement inside closures which are arguments
to methods


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8cff58c2
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8cff58c2
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8cff58c2

Branch: refs/heads/GROOVY_2_4_X
Commit: 8cff58c2d4c111a698010566df8815db75e86c20
Parents: 65daac5
Author: paulk <paulk@asert.com.au>
Authored: Mon Jan 23 21:42:52 2017 +1000
Committer: paulk <paulk@asert.com.au>
Committed: Wed Jan 25 21:59:30 2017 +1000

----------------------------------------------------------------------
 .../groovy/transform/LogASTTransformation.java  | 24 +++++++-----
 src/test/groovy/bugs/Groovy8060Bug.groovy       | 39 ++++++++++++++++++++
 2 files changed, 54 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/8cff58c2/src/main/org/codehaus/groovy/transform/LogASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/LogASTTransformation.java b/src/main/org/codehaus/groovy/transform/LogASTTransformation.java
index 29bcab7..2350aee 100644
--- a/src/main/org/codehaus/groovy/transform/LogASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/LogASTTransformation.java
@@ -129,24 +129,30 @@ public class LogASTTransformation extends AbstractASTTransformation
implements C
             }
 
             private Expression transformMethodCallExpression(Expression exp) {
-                MethodCallExpression mce = (MethodCallExpression) exp;
+                Expression modifiedCall = addGuard((MethodCallExpression) exp);
+                return modifiedCall == null ? super.transform(exp) : modifiedCall;
+            }
+
+            private Expression addGuard(MethodCallExpression mce) {
+                // only add guard to methods of the form: logVar.logMethod(params)
                 if (!(mce.getObjectExpression() instanceof VariableExpression)) {
-                    return exp;
+                    return null;
                 }
                 VariableExpression variableExpression = (VariableExpression) mce.getObjectExpression();
                 if (!variableExpression.getName().equals(logFieldName)
                         || !(variableExpression.getAccessedVariable() instanceof DynamicVariable))
{
-                    return exp;
+                    return null;
                 }
+
                 String methodName = mce.getMethodAsString();
-                if (methodName == null) return exp;
-                if (usesSimpleMethodArgumentsOnly(mce)) return exp;
+                if (methodName == null) return null;
+                if (!loggingStrategy.isLoggingMethod(methodName)) return null;
+                // also don't bother with guard if we have "simple" method args
+                // since there is no saving
+                if (usesSimpleMethodArgumentsOnly(mce)) return null;
 
                 variableExpression.setAccessedVariable(logNode);
-
-                if (!loggingStrategy.isLoggingMethod(methodName)) return exp;
-
-                return loggingStrategy.wrapLoggingMethodCall(variableExpression, methodName,
exp);
+                return loggingStrategy.wrapLoggingMethodCall(variableExpression, methodName,
mce);
             }
 
             private boolean usesSimpleMethodArgumentsOnly(MethodCallExpression mce) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/8cff58c2/src/test/groovy/bugs/Groovy8060Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8060Bug.groovy b/src/test/groovy/bugs/Groovy8060Bug.groovy
new file mode 100644
index 0000000..741929b
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8060Bug.groovy
@@ -0,0 +1,39 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package groovy.bugs
+
+class Groovy8060Bug extends GroovyTestCase {
+    void testLoggingWithinClosuresThatAreMethodArgsShouldHaveGuards() {
+        assertScript '''
+            @Grab('org.slf4j:slf4j-simple:1.7.21')
+            import groovy.util.logging.Slf4j
+
+            @Slf4j
+            class LogMain {
+                public static int count = 0
+
+                static void main(args) {
+                    assert !log.isTraceEnabled()
+                    1.times { log.trace("${count++}") }
+                    assert !count
+                }
+            }
+        '''
+    }
+}


Mime
View raw message