You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2018/12/29 14:13:01 UTC
[groovy] 08/28: GROOVY-7233: Pre-factor some non-consequential
changes
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch refine-groovydoc
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 38e279553fb76db38c90a627a6698e3243c9403d
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Dec 18 10:00:46 2018 +1000
GROOVY-7233: Pre-factor some non-consequential changes
whitespace, unneeded modifiers, cleanup @author tags, add @Override annotations
---
gradle/pomconfigurer.gradle | 44 +++++++++---
src/main/groovy/groovy/util/logging/Commons.java | 4 +-
src/main/groovy/groovy/util/logging/Log.java | 8 +--
src/main/groovy/groovy/util/logging/Log4j.java | 4 +-
src/main/groovy/groovy/util/logging/Log4j2.java | 2 +
src/main/groovy/groovy/util/logging/Slf4j.java | 4 +-
.../groovy/transform/LogASTTransformation.java | 14 ++--
src/test/groovy/util/logging/CommonsTest.groovy | 78 ++++++++++++++++++++--
src/test/groovy/util/logging/Log4jTest.groovy | 26 +++-----
src/test/groovy/util/logging/LogTest.groovy | 34 ++++------
src/test/groovy/util/logging/Slf4jTest.groovy | 5 +-
11 files changed, 148 insertions(+), 75 deletions(-)
diff --git a/gradle/pomconfigurer.gradle b/gradle/pomconfigurer.gradle
index 4d854a9..b4d5d5c 100644
--- a/gradle/pomconfigurer.gradle
+++ b/gradle/pomconfigurer.gradle
@@ -33,7 +33,7 @@ project.ext.pomConfigureClosureWithoutTweaks = {
inceptionYear '2003'
organization {
name 'Apache Software Foundation'
- url 'http://groovy-lang.org'
+ url 'http://apache.org'
}
developers {
developer {
@@ -41,7 +41,6 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Guillaume Laforge'
organization 'SpringSource'
roles {
- role 'Project Manager'
role 'Despot'
role 'Developer'
}
@@ -249,8 +248,9 @@ project.ext.pomConfigureClosureWithoutTweaks = {
id 'paulk'
name 'Paul King'
email 'paulk@asert.com.au'
- organization 'ASERT, Australia'
+ organization 'OCI, Australia'
roles {
+ role 'Project Manager'
role 'Developer'
role 'Despot'
}
@@ -365,6 +365,20 @@ project.ext.pomConfigureClosureWithoutTweaks = {
role 'Despot'
}
}
+ developer {
+ id 'rpopma'
+ name 'Remko Popma'
+ roles {
+ role 'Developer'
+ }
+ }
+ developer {
+ id 'grocher'
+ name 'Graeme Rocher'
+ roles {
+ role 'Developer'
+ }
+ }
}
contributors {
contributor {
@@ -568,6 +582,9 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Nikolay Chugunov'
}
contributor {
+ name 'Francesco Durbin'
+ }
+ contributor {
name 'Paolo Di Tommaso'
}
contributor {
@@ -577,6 +594,9 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Matias Bjarland'
}
contributor {
+ name 'Tomasz Bujok'
+ }
+ contributor {
name 'Richard Hightower'
}
contributor {
@@ -610,13 +630,22 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Martin Kempf'
}
contributor {
- name 'Stephane Landelle'
+ name 'Martin Ghados'
}
contributor {
- name 'Vladimir Vivien'
+ name 'Alberto Mijares'
}
contributor {
- name 'Graeme Rocher'
+ name 'Matthias Cullmann'
+ }
+ contributor {
+ name 'Tomek Bujok'
+ }
+ contributor {
+ name 'Stephane Landelle'
+ }
+ contributor {
+ name 'Vladimir Vivien'
}
contributor {
name 'Joe Wolf'
@@ -628,9 +657,6 @@ project.ext.pomConfigureClosureWithoutTweaks = {
name 'Tom Nichols'
}
contributor {
- name 'Remko Popma'
- }
- contributor {
name 'mgroovy'
}
contributor {
diff --git a/src/main/groovy/groovy/util/logging/Commons.java b/src/main/groovy/groovy/util/logging/Commons.java
index c833c00..d4e5ad6 100644
--- a/src/main/groovy/groovy/util/logging/Commons.java
+++ b/src/main/groovy/groovy/util/logging/Commons.java
@@ -56,8 +56,6 @@ import java.util.Locale;
* If the expression exp is a constant or only a variable access the method call will
* not be transformed. But this will still cause a call on the injected logger.
*
- * @author Hamlet D'Arcy
- * @author Matthias Cullmann
* @since 1.8.0
*/
@java.lang.annotation.Documented
@@ -88,10 +86,12 @@ public @interface Commons {
new ConstantExpression(getCategoryName(classNode, categoryName))));
}
+ @Override
public boolean isLoggingMethod(String methodName) {
return methodName.matches("fatal|error|warn|info|debug|trace");
}
+ @Override
public Expression wrapLoggingMethodCall(Expression logVariable, String methodName, Expression originalExpression) {
MethodCallExpression condition = new MethodCallExpression(
logVariable,
diff --git a/src/main/groovy/groovy/util/logging/Log.java b/src/main/groovy/groovy/util/logging/Log.java
index e52d39b..1a4e554 100644
--- a/src/main/groovy/groovy/util/logging/Log.java
+++ b/src/main/groovy/groovy/util/logging/Log.java
@@ -61,12 +61,6 @@ import java.util.Locale;
* the method call will not be transformed. But this will still cause a call on the injected
* logger.
*
- * @author Guillaume Laforge
- * @author Jochen Theodorou
- * @author Dinko Srkoc
- * @author Hamlet D'Arcy
- * @author Raffaele Cigni
- * @author Alberto Vilches Raton
* @since 1.8.0
*/
@java.lang.annotation.Documented
@@ -100,10 +94,12 @@ public @interface Log {
new ConstantExpression(getCategoryName(classNode, categoryName))));
}
+ @Override
public boolean isLoggingMethod(String methodName) {
return methodName.matches("severe|warning|info|fine|finer|finest");
}
+ @Override
public Expression wrapLoggingMethodCall(Expression logVariable, String methodName, Expression originalExpression) {
AttributeExpression logLevelExpression = new AttributeExpression(
new ClassExpression(LEVEL_CLASSNODE),
diff --git a/src/main/groovy/groovy/util/logging/Log4j.java b/src/main/groovy/groovy/util/logging/Log4j.java
index e976673..28633b5 100644
--- a/src/main/groovy/groovy/util/logging/Log4j.java
+++ b/src/main/groovy/groovy/util/logging/Log4j.java
@@ -57,8 +57,6 @@ import java.util.Locale;
* If the expression exp is a constant or only a variable access the method call will
* not be transformed. But this will still cause a call on the injected logger.
*
- * @author Hamlet D'Arcy
- * @author Tomek Bujok
* @since 1.8.0
*/
@java.lang.annotation.Documented
@@ -88,10 +86,12 @@ public @interface Log4j {
new ConstantExpression(getCategoryName(classNode, categoryName))));
}
+ @Override
public boolean isLoggingMethod(String methodName) {
return methodName.matches("fatal|error|warn|info|debug|trace");
}
+ @Override
public Expression wrapLoggingMethodCall(Expression logVariable, String methodName, Expression originalExpression) {
final MethodCallExpression condition;
if (!"trace".equals(methodName)) {
diff --git a/src/main/groovy/groovy/util/logging/Log4j2.java b/src/main/groovy/groovy/util/logging/Log4j2.java
index 7dd1d72..a53874c 100644
--- a/src/main/groovy/groovy/util/logging/Log4j2.java
+++ b/src/main/groovy/groovy/util/logging/Log4j2.java
@@ -85,10 +85,12 @@ public @interface Log4j2 {
new ConstantExpression(getCategoryName(classNode, categoryName))));
}
+ @Override
public boolean isLoggingMethod(String methodName) {
return methodName.matches("fatal|error|warn|info|debug|trace");
}
+ @Override
public Expression wrapLoggingMethodCall(Expression logVariable, String methodName, Expression originalExpression) {
MethodCallExpression condition = new MethodCallExpression(
logVariable,
diff --git a/src/main/groovy/groovy/util/logging/Slf4j.java b/src/main/groovy/groovy/util/logging/Slf4j.java
index c5fb392..7561cf7 100644
--- a/src/main/groovy/groovy/util/logging/Slf4j.java
+++ b/src/main/groovy/groovy/util/logging/Slf4j.java
@@ -56,8 +56,6 @@ import java.util.Locale;
* If the expression exp is a constant or only a variable access the method call will
* not be transformed. But this will still cause a call on the injected logger.
*
- * @author Hamlet D'Arcy
- * @author Alberto Mijares
* @since 1.8.0
*/
@java.lang.annotation.Documented
@@ -87,10 +85,12 @@ public @interface Slf4j {
new ConstantExpression(getCategoryName(classNode, categoryName))));
}
+ @Override
public boolean isLoggingMethod(String methodName) {
return methodName.matches("error|warn|info|debug|trace");
}
+ @Override
public Expression wrapLoggingMethodCall(Expression logVariable, String methodName, Expression originalExpression) {
MethodCallExpression condition = new MethodCallExpression(
logVariable,
diff --git a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
index ecf92bc..1895062 100644
--- a/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/LogASTTransformation.java
@@ -48,16 +48,6 @@ import java.lang.reflect.Modifier;
/**
* This class provides an AST Transformation to add a log field to a class.
- *
- * @author Guillaume Laforge
- * @author Jochen Theodorou
- * @author Dinko Srkoc
- * @author Hamlet D'Arcy
- * @author Raffaele Cigni
- * @author Alberto Vilches Raton
- * @author Tomasz Bujok
- * @author Martin Ghados
- * @author Matthias Cullmann
*/
@GroovyASTTransformation(phase = CompilePhase.SEMANTIC_ANALYSIS)
public class LogASTTransformation extends AbstractASTTransformation implements CompilationUnitAware {
@@ -70,6 +60,7 @@ public class LogASTTransformation extends AbstractASTTransformation implements C
private CompilationUnit compilationUnit;
+ @Override
public void visit(ASTNode[] nodes, final SourceUnit source) {
init(nodes, source);
AnnotatedNode targetClass = (AnnotatedNode) nodes[1];
@@ -96,6 +87,7 @@ public class LogASTTransformation extends AbstractASTTransformation implements C
return source;
}
+ @Override
public Expression transform(Expression exp) {
if (exp == null) return null;
if (exp instanceof MethodCallExpression) {
@@ -274,6 +266,7 @@ public class LogASTTransformation extends AbstractASTTransformation implements C
this(null);
}
+ @Override
public String getCategoryName(ClassNode classNode, String categoryName) {
if (categoryName.equals(DEFAULT_CATEGORY_NAME)) {
return classNode.getName();
@@ -291,6 +284,7 @@ public class LogASTTransformation extends AbstractASTTransformation implements C
}
}
+ @Override
public void setCompilationUnit(final CompilationUnit unit) {
this.compilationUnit = unit;
}
diff --git a/src/test/groovy/util/logging/CommonsTest.groovy b/src/test/groovy/util/logging/CommonsTest.groovy
index 48f928e..9e2a460 100644
--- a/src/test/groovy/util/logging/CommonsTest.groovy
+++ b/src/test/groovy/util/logging/CommonsTest.groovy
@@ -23,10 +23,6 @@ import java.lang.reflect.Modifier
/**
* Unit test for the commons logging @Log based annotation.
- *
- * @author Hamlet D'Arcy
- * @author Matthias Cullmann
- *
*/
class CommonsTest extends GroovyTestCase {
@@ -60,6 +56,80 @@ class CommonsTest extends GroovyTestCase {
}
}
+ void testExplicitPrivateFinalStaticLogFieldAppears() {
+ Class clazz = new GroovyClassLoader().parseClass('''
+ import static groovy.transform.options.Visibility.*
+ @groovy.transform.VisibilityOptions(value = PRIVATE)
+ @groovy.util.logging.Commons
+ class MyClass {
+ }
+ ''')
+
+ assert clazz.declaredFields.find { Field field ->
+ field.name == "log" &&
+ Modifier.isPrivate(field.getModifiers()) &&
+ Modifier.isStatic(field.getModifiers()) &&
+ Modifier.isTransient(field.getModifiers()) &&
+ Modifier.isFinal(field.getModifiers())
+ }
+ }
+
+ void testPackagePrivateFinalStaticLogFieldAppears() {
+ Class clazz = new GroovyClassLoader().parseClass('''
+ import static groovy.transform.options.Visibility.*
+ @groovy.transform.VisibilityOptions(value = PACKAGE_PRIVATE)
+ @groovy.util.logging.Commons
+ class MyClass {
+ }
+ ''')
+
+ assert clazz.declaredFields.find { Field field ->
+ field.name == "log" &&
+ !Modifier.isPrivate(field.getModifiers()) &&
+ !Modifier.isProtected(field.getModifiers()) &&
+ !Modifier.isPublic(field.getModifiers()) &&
+ Modifier.isStatic(field.getModifiers()) &&
+ Modifier.isTransient(field.getModifiers()) &&
+ Modifier.isFinal(field.getModifiers())
+ }
+ }
+
+ void testProtectedFinalStaticLogFieldAppears() {
+ Class clazz = new GroovyClassLoader().parseClass('''
+ import static groovy.transform.options.Visibility.*
+ @groovy.transform.VisibilityOptions(value = PROTECTED)
+ @groovy.util.logging.Commons
+ class MyClass {
+ }
+ ''')
+
+ assert clazz.declaredFields.find { Field field ->
+ field.name == "log" &&
+ Modifier.isProtected(field.getModifiers()) &&
+ Modifier.isStatic(field.getModifiers()) &&
+ Modifier.isTransient(field.getModifiers()) &&
+ Modifier.isFinal(field.getModifiers())
+ }
+ }
+
+ void testPublicFinalStaticLogFieldAppears() {
+ Class clazz = new GroovyClassLoader().parseClass('''
+ import static groovy.transform.options.Visibility.*
+ @groovy.transform.VisibilityOptions(value = PUBLIC)
+ @groovy.util.logging.Commons
+ class MyClass {
+ }
+ ''')
+
+ assert clazz.declaredFields.find { Field field ->
+ field.name == "log" &&
+ Modifier.isPublic(field.getModifiers()) &&
+ Modifier.isStatic(field.getModifiers()) &&
+ Modifier.isTransient(field.getModifiers()) &&
+ Modifier.isFinal(field.getModifiers())
+ }
+ }
+
void testPrivateFinalStaticNamedLogFieldAppears() {
Class clazz = new GroovyClassLoader().parseClass('''
@groovy.util.logging.Commons('logger')
diff --git a/src/test/groovy/util/logging/Log4jTest.groovy b/src/test/groovy/util/logging/Log4jTest.groovy
index 5472a67..febc05f 100644
--- a/src/test/groovy/util/logging/Log4jTest.groovy
+++ b/src/test/groovy/util/logging/Log4jTest.groovy
@@ -19,17 +19,13 @@
package groovy.util.logging
import java.lang.reflect.*
-import org.codehaus.groovy.ast.*
-import org.codehaus.groovy.control.*
-import org.codehaus.groovy.tools.ast.*
-import org.codehaus.groovy.transform.*
import org.apache.log4j.AppenderSkeleton
import org.apache.log4j.spi.LoggingEvent
import org.apache.log4j.Level
import org.apache.log4j.Logger
/**
- * @author Tomasz Bujok
+ * Tests for Log4j AST transformation
*/
class Log4jTest extends GroovyTestCase {
@@ -50,7 +46,7 @@ class Log4jTest extends GroovyTestCase {
logger.removeAllAppenders()
}
- public void testPrivateFinalStaticLogFieldAppears() {
+ void testPrivateFinalStaticLogFieldAppears() {
Class clazz = new GroovyClassLoader().parseClass('''
@groovy.util.logging.Log4j
@@ -66,7 +62,7 @@ class Log4jTest extends GroovyTestCase {
}
}
- public void testClassAlreadyHasLogField() {
+ void testClassAlreadyHasLogField() {
shouldFail {
@@ -80,7 +76,7 @@ class Log4jTest extends GroovyTestCase {
}
}
- public void testClassAlreadyHasNamedLogField() {
+ void testClassAlreadyHasNamedLogField() {
shouldFail {
@@ -94,7 +90,7 @@ class Log4jTest extends GroovyTestCase {
}
}
- public void testLogInfo() {
+ void testLogInfo() {
Class clazz = new GroovyClassLoader().parseClass('''
@groovy.util.logging.Log4j
@@ -150,7 +146,7 @@ class Log4jTest extends GroovyTestCase {
assert events[0].message == "(static) info called"
}
- public void testLogInfoForNamedLogger() {
+ void testLogInfoForNamedLogger() {
Class clazz = new GroovyClassLoader().parseClass('''
@groovy.util.logging.Log4j('logger')
@@ -187,7 +183,7 @@ class Log4jTest extends GroovyTestCase {
assert events[ind].message == "trace called"
}
- public void testLogGuard() {
+ void testLogGuard() {
Class clazz = new GroovyClassLoader().parseClass('''
@groovy.util.logging.Log4j
class MyClass {
@@ -229,7 +225,7 @@ class Log4jTest extends GroovyTestCase {
assert appender.getEvents().size() == 1
}
- public void testCustomCategory() {
+ void testCustomCategory() {
Log4jInterceptingAppender appenderForCustomCategory = new Log4jInterceptingAppender()
Logger loggerForCustomCategory = Logger.getLogger('customCategory')
@@ -251,15 +247,15 @@ class Log4jTest extends GroovyTestCase {
}
}
-public class Log4jInterceptingAppender extends AppenderSkeleton {
+class Log4jInterceptingAppender extends AppenderSkeleton {
List<LoggingEvent> events
boolean isLogGuarded = true
- public Log4jInterceptingAppender() {
+ Log4jInterceptingAppender() {
this.events = new ArrayList<LoggingEvent>()
}
- public List<LoggingEvent> getEvents() {
+ List<LoggingEvent> getEvents() {
return events
}
diff --git a/src/test/groovy/util/logging/LogTest.groovy b/src/test/groovy/util/logging/LogTest.groovy
index 4db46bd..a52788a 100644
--- a/src/test/groovy/util/logging/LogTest.groovy
+++ b/src/test/groovy/util/logging/LogTest.groovy
@@ -26,18 +26,10 @@ import org.codehaus.groovy.control.MultipleCompilationErrorsException
/**
* Test to make sure the @Log annotation is working correctly.
- *
- * @author Guillaume Laforge
- * @author Jochen Theodorou
- * @author Dinko Srkoc
- * @author Hamlet D'Arcy
- * @author Raffaele Cigni
- * @author Alberto Vilches Raton
- * @author Tomasz Bujok
*/
class LogTest extends GroovyTestCase {
- public void testPrivateFinalStaticLogFieldAppears() {
+ void testPrivateFinalStaticLogFieldAppears() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log
@@ -53,7 +45,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testPrivateFinalStaticNamedLogFieldAppears() {
+ void testPrivateFinalStaticNamedLogFieldAppears() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log('logger')
@@ -69,7 +61,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testClassAlreadyHasLogField() {
+ void testClassAlreadyHasLogField() {
shouldFail {
@@ -83,7 +75,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testClassAlreadyHasNamedLogField() {
+ void testClassAlreadyHasNamedLogField() {
shouldFail {
@@ -116,7 +108,7 @@ class LogTest extends GroovyTestCase {
assert logSpy.infoParameter == 'info called'
}
- public void testLogInfo() {
+ void testLogInfo() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log
@@ -148,7 +140,7 @@ class LogTest extends GroovyTestCase {
assert logSpy.finestParameter == 'finest called'
}
- public void testLogInfoWithName() {
+ void testLogInfoWithName() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log('logger')
@@ -180,7 +172,7 @@ class LogTest extends GroovyTestCase {
assert logSpy.finestParameter == 'finest called'
}
- public void testLogGuard() {
+ void testLogGuard() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log
class MyClass {
@@ -216,7 +208,7 @@ class LogTest extends GroovyTestCase {
assert !logSpy.finestParameter
}
- public void testInheritance() {
+ void testInheritance() {
def clazz = new GroovyShell().evaluate("""
class MyParent {
@@ -251,7 +243,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testInheritance_ProtectedShadowing() {
+ void testInheritance_ProtectedShadowing() {
shouldFail(MultipleCompilationErrorsException) {
new GroovyClassLoader().parseClass("""
@@ -265,7 +257,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testInheritance_PublicShadowing() {
+ void testInheritance_PublicShadowing() {
shouldFail(MultipleCompilationErrorsException) {
new GroovyClassLoader().parseClass("""
@@ -279,7 +271,7 @@ class LogTest extends GroovyTestCase {
}
}
- public void testDefaultCategory() {
+ void testDefaultCategory() {
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log
class MyClass {
@@ -294,7 +286,7 @@ class LogTest extends GroovyTestCase {
assert logFormatterSpy.messageReceived
}
- public void testCustomCategory() {
+ void testCustomCategory() {
String categoryName = 'customCategory'
Class clazz = new GroovyClassLoader().parseClass("""
@groovy.util.logging.Log(category='$categoryName')
@@ -375,7 +367,7 @@ class LogFormatterSpy extends Formatter {
boolean messageReceived = false
@Override
- public String format(LogRecord record) {
+ String format(LogRecord record) {
messageReceived = true
return record.message
}
diff --git a/src/test/groovy/util/logging/Slf4jTest.groovy b/src/test/groovy/util/logging/Slf4jTest.groovy
index c63e04d..6a6324b 100644
--- a/src/test/groovy/util/logging/Slf4jTest.groovy
+++ b/src/test/groovy/util/logging/Slf4jTest.groovy
@@ -30,10 +30,7 @@ import java.lang.reflect.Field
import java.lang.reflect.Modifier
/**
- * @author Hamlet D'Arcy
- * @author Francesco Durbin
- * @author Tomasz Bujok
- * @author Paul King
+ * Tests for Slf4j AST transformation
*/
class Slf4jTest extends GroovyTestCase {