You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/04/04 04:57:26 UTC
[groovy] 02/02: minor refactor: remove some codenarc violations
(part 4)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e4cc76b8db9ddefff65e710de96eb2e9c364d44c
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu Apr 4 11:48:44 2019 +1000
minor refactor: remove some codenarc violations (part 4)
---
config/codenarc/codenarc.groovy | 6 +-
.../codehaus/groovy/classgen/genArrayAccess.groovy | 37 +++----
.../{genArrays.groovy => genArrayUtil.groovy} | 22 ++---
.../org/codehaus/groovy/classgen/genDgmMath.groovy | 43 +++++----
.../groovy/classgen/genMathModification.groovy | 106 ++++++++++-----------
.../groovy/transform/ASTTestTransformation.groovy | 17 ++--
.../tailrec/TailRecursiveASTTransformation.groovy | 26 ++---
7 files changed, 133 insertions(+), 124 deletions(-)
diff --git a/config/codenarc/codenarc.groovy b/config/codenarc/codenarc.groovy
index 1d58fac..e9c7179 100644
--- a/config/codenarc/codenarc.groovy
+++ b/config/codenarc/codenarc.groovy
@@ -168,7 +168,10 @@ ruleset {
ruleset('rulesets/logging.xml') {
exclude 'SystemOutPrint' // too many to worry about, review later
- exclude 'SystemErrPrint' // too many to worry about, review later
+ exclude 'SystemErrPrint' // too many to worry about, review later
+ 'Println' {
+ doNotApplyToFileNames='genArrayAccess.groovy,genArrayUtil.groovy,genDgmMath.groovy,genMathModification.groovy,'
+ }
}
ruleset('rulesets/braces.xml') {
exclude 'ForStatementBraces' // for statements without braces seems acceptable in our coding standards
@@ -221,7 +224,6 @@ ruleset {
}
ruleset('rulesets/dry.xml') {
exclude 'DuplicateNumberLiteral' // too many to worry about, review later
- exclude 'DuplicateStringLiteralRule' // too many to worry about, review later
exclude 'DuplicateStringLiteral' // too many to worry about, review later
}
ruleset('rulesets/design.xml') {
diff --git a/src/main/groovy/org/codehaus/groovy/classgen/genArrayAccess.groovy b/src/main/groovy/org/codehaus/groovy/classgen/genArrayAccess.groovy
index 08cb68a..46521c1 100644
--- a/src/main/groovy/org/codehaus/groovy/classgen/genArrayAccess.groovy
+++ b/src/main/groovy/org/codehaus/groovy/classgen/genArrayAccess.groovy
@@ -18,15 +18,18 @@
*/
package org.codehaus.groovy.classgen
+// TODO this generator template has drifted apart from the now modified generated classes
+// Is it worth keeping this template and/or getting it back up to date?
+
println """
package org.codehaus.groovy.runtime.dgmimpl;
import groovy.lang.MetaClassImpl;
import groovy.lang.MetaMethod;
-import org.codehaus.groovy.runtime.callsite.CallSite;
-import org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite;
import org.codehaus.groovy.reflection.CachedClass;
import org.codehaus.groovy.reflection.ReflectionCache;
+import org.codehaus.groovy.runtime.callsite.CallSite;
+import org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite;
public class ArrayOperations {
${genInners()}
@@ -34,17 +37,17 @@ public class ArrayOperations {
"""
def genInners () {
- def res = ""
+ def res = ''
final Map primitives = [
- "boolean": "Boolean",
- "byte": "Byte",
- "char": "Character",
- "short": "Short",
- "int": "Integer",
- "long": "Long",
- "float": "Float",
- "double": "Double"
+ 'boolean': 'Boolean',
+ 'byte': 'Byte',
+ 'char': 'Character',
+ 'short': 'Short',
+ 'int': 'Integer',
+ 'long': 'Long',
+ 'float': 'Float',
+ 'double': 'Double'
]
primitives.each {primName, clsName ->
@@ -62,7 +65,7 @@ def genInners () {
public Object invoke(Object object, Object[] args) {
final ${primName}[] objects = (${primName}[]) object;
- return objects[normaliseIndex(((Integer) args[0]).intValue(), objects.length)];
+ return objects[normaliseIndex((Integer) args[0], objects.length)];
}
public CallSite createPojoCallSite(CallSite site, MetaClassImpl metaClass, MetaMethod metaMethod, Class[] params, Object receiver, Object[] args) {
@@ -72,14 +75,14 @@ def genInners () {
return new PojoMetaMethodSite(site, metaClass, metaMethod, params) {
public Object invoke(Object receiver, Object[] args) {
final ${primName}[] objects = (${primName}[]) receiver;
- return objects[normaliseIndex(((Integer) args[0]).intValue(), objects.length)];
+ return objects[normaliseIndex((Integer) args[0], objects.length)];
}
public Object callBinop(Object receiver, Object arg) {
if ((receiver instanceof ${primName}[] && arg instanceof Integer)
&& checkMetaClass()) {
final ${primName}[] objects = (${primName}[]) receiver;
- return objects[normaliseIndex(((Integer) arg).intValue(), objects.length)];
+ return objects[normaliseIndex((Integer) arg, objects.length)];
}
else
return super.callBinop(receiver,arg);
@@ -87,7 +90,7 @@ def genInners () {
public Object invokeBinop(Object receiver, Object arg) {
final ${primName}[] objects = (${primName}[]) receiver;
- return objects[normaliseIndex(((Integer) arg).intValue(), objects.length)];
+ return objects[normaliseIndex((Integer) arg, objects.length)];
}
};
}
@@ -109,7 +112,7 @@ def genInners () {
public Object invoke(Object object, Object[] args) {
final ${primName}[] objects = (${primName}[]) object;
- final int index = normaliseIndex(((Integer) args[0]).intValue(), objects.length);
+ final int index = normaliseIndex((Integer) args[0], objects.length);
Object newValue = args[1];
if (!(newValue instanceof ${clsName})) {
Number n = (Number) newValue;
@@ -129,7 +132,7 @@ def genInners () {
if ((receiver instanceof ${primName}[] && args[0] instanceof Integer && args[1] instanceof ${clsName} )
&& checkMetaClass()) {
final ${primName}[] objects = (${primName}[]) receiver;
- objects[normaliseIndex(((Integer) args[0]).intValue(), objects.length)] = ((${clsName})args[1]).${primName}Value();
+ objects[normaliseIndex((Integer) args[0], objects.length)] = ((${clsName})args[1]).${primName}Value();
return null;
}
else
diff --git a/src/main/groovy/org/codehaus/groovy/classgen/genArrays.groovy b/src/main/groovy/org/codehaus/groovy/classgen/genArrayUtil.groovy
similarity index 78%
rename from src/main/groovy/org/codehaus/groovy/classgen/genArrays.groovy
rename to src/main/groovy/org/codehaus/groovy/classgen/genArrayUtil.groovy
index 9bbe3cf..61891a9 100644
--- a/src/main/groovy/org/codehaus/groovy/classgen/genArrays.groovy
+++ b/src/main/groovy/org/codehaus/groovy/classgen/genArrayUtil.groovy
@@ -27,27 +27,27 @@ public class ArrayUtil {
"""
def genMethods () {
- def res = ""
+ def res = ''
for (i in 1..250)
- res += "\n\n" + genMethod (i)
+ res += '\n\n' + genMethod (i)
res
}
def genMethod (int paramNum) {
- def res = "public static Object [] createArray ("
+ def res = 'public static Object [] createArray ('
for (k in 0..<paramNum) {
- res += "Object arg" + k
+ res += 'Object arg' + k
if (k != paramNum-1)
- res += ", "
+ res += ', '
}
- res += ") {\n"
- res += "return new Object [] {\n"
+ res += ') {\n'
+ res += 'return new Object [] {\n'
for (k in 0..<paramNum) {
- res += "arg" + k
+ res += 'arg' + k
if (k != paramNum-1)
- res += ", "
+ res += ', '
}
- res += "};\n"
- res += "}"
+ res += '};\n'
+ res += '}'
res
}
diff --git a/src/main/groovy/org/codehaus/groovy/classgen/genDgmMath.groovy b/src/main/groovy/org/codehaus/groovy/classgen/genDgmMath.groovy
index 71bdd5f..c1fbafa 100644
--- a/src/main/groovy/org/codehaus/groovy/classgen/genDgmMath.groovy
+++ b/src/main/groovy/org/codehaus/groovy/classgen/genDgmMath.groovy
@@ -18,22 +18,22 @@
*/
package org.codehaus.groovy.classgen
-def types = ["Integer", "Long", "Float", "Double"]
+def types = ['Integer', 'Long', 'Float', 'Double']
def getMath (a,b) {
- if (a == "Double" || b == "Double" || a == "Float" || b == "Float")
- return "FloatingPointMath"
+ if (a == 'Double' || b == 'Double' || a == 'Float' || b == 'Float')
+ return 'FloatingPointMath'
- if (a == "Long" || b == "Long")
- return "LongMath"
+ if (a == 'Long' || b == 'Long')
+ return 'LongMath'
- "IntegerMath"
+ 'IntegerMath'
}
-println """
+println '''
public CallSite createPojoCallSite(CallSite site, MetaClassImpl metaClass, MetaMethod metaMethod, Class[] params, Object receiver, Object[] args) {
NumberMath m = NumberMath.getMath((Number)receiver, (Number)args[0]);
-"""
+'''
types.each {
a ->
@@ -54,10 +54,10 @@ types.each {
};
"""
}
- println "}"
+ println '}'
}
-println """
+println '''
return new NumberNumberCallSite (site, metaClass, metaMethod, params, (Number)receiver, (Number)args[0]){
public final Object invoke(Object receiver, Object[] args) {
return math.addImpl((Number)receiver,(Number)args[0]);
@@ -67,21 +67,26 @@ println """
return math.addImpl((Number)receiver,(Number)arg);
}
}
-"""
+'''
for (i in 2..256) {
print "public Object invoke$i (Object receiver, "
- for (j in 1..(i-1)) {
- print "Object a$j, "
- }
+ printParams(i)
println "Object a$i) {"
+ print ' return invoke (receiver, new Object[] {'
+ printArgs(i)
+ println "a$i} );"
+ println '}'
+}
- print " return invoke (receiver, new Object[] {"
+private void printParams(int i) {
+ for (j in 1..(i - 1)) {
+ print "Object a$j, "
+ }
+}
- for (j in 1..(i-1)) {
+private void printArgs(int i) {
+ for (j in 1..(i - 1)) {
print "a$j, "
}
- println "a$i} );"
-
- println "}"
}
diff --git a/src/main/groovy/org/codehaus/groovy/classgen/genMathModification.groovy b/src/main/groovy/org/codehaus/groovy/classgen/genMathModification.groovy
index 10cc7eb..cf326dd 100644
--- a/src/main/groovy/org/codehaus/groovy/classgen/genMathModification.groovy
+++ b/src/main/groovy/org/codehaus/groovy/classgen/genMathModification.groovy
@@ -19,25 +19,25 @@
package org.codehaus.groovy.classgen
def ops = [
- "plus",
- "minus",
- "multiply",
- "div",
- "or",
- "and",
- "xor",
- "intdiv",
- "mod",
- "leftShift",
- "rightShift",
- "rightShiftUnsigned"
+ 'plus',
+ 'minus',
+ 'multiply',
+ 'div',
+ 'or',
+ 'and',
+ 'xor',
+ 'intdiv',
+ 'mod',
+ 'leftShift',
+ 'rightShift',
+ 'rightShiftUnsigned'
]
-def numbers = ["Byte":"byte", "Short":"short", "Integer":"int", "Long":"long", "Float":"float", "Double":"double"]
+def numbers = ['Byte':'byte', 'Short':'short', 'Integer':'int', 'Long':'long', 'Float':'float', 'Double':'double']
ops.each { op ->
numbers.each { wrappedType, type ->
- println "public boolean ${type}_${op};";
+ println "public boolean ${type}_${op};"
}
}
@@ -48,12 +48,12 @@ ops.each { op ->
${type}_${op} = true;
}"""
}
- println "if (klazz==Object.class) {"
+ println 'if (klazz==Object.class) {'
numbers.each { wrappedType, type ->
println "${type}_${op} = true;"
}
- println "}"
- println "}"
+ println '}'
+ println '}'
}
ops.each { op ->
@@ -66,7 +66,7 @@ ops.each { op ->
return ${op}Slow(op1, op2);
}
else {
- return ${math.resType != type1 ? "((" + math.resType+ ")op1)" : "op1"} ${math[op]} ${math.resType != type2 ? "((" + math.resType+ ")op2)" : "op2"};
+ return ${math.resType != type1 ? '((' + math.resType+ ')op1)' : 'op1'} ${math[op]} ${math.resType != type2 ? '((' + math.resType+ ')op2)' : 'op2'};
}
}"""
println """private static ${math.resType} ${op}Slow(${type1} op1,${type2} op2) {
@@ -78,56 +78,56 @@ ops.each { op ->
}
def isFloatingPoint(number) {
- return number == "Double" || number == "Float";
+ number == 'Double' || number == 'Float'
}
def isLong(number) {
- return number == "Long";
+ number == 'Long'
}
def getMath (left, right) {
if (isFloatingPoint(left) || isFloatingPoint(right)) {
return [
- resType : "double",
+ resType : 'double',
- plus : "+",
- minus : "-",
- multiply : "*",
- div : "/",
- ];
+ plus : '+',
+ minus : '-',
+ multiply : '*',
+ div : '/',
+ ]
}
if (isLong(left) || isLong(right)){
return [
- resType : "long",
+ resType : 'long',
- plus : "+",
- minus : "-",
- multiply : "*",
- div : "/",
- or : "|",
- and : "&",
- xor : "^",
- intdiv : "/",
- mod : "%",
- leftShift : "<<",
- rightShift : ">>",
- rightShiftUnsigned : ">>>"
+ plus : '+',
+ minus : '-',
+ multiply : '*',
+ div : '/',
+ or : '|',
+ and : '&',
+ xor : '^',
+ intdiv : '/',
+ mod : '%',
+ leftShift : '<<',
+ rightShift : '>>',
+ rightShiftUnsigned : '>>>'
]
}
- return [
- resType : "int",
+ [
+ resType : 'int',
- plus : "+",
- minus : "-",
- multiply : "*",
- div : "/",
- or : "|",
- and : "&",
- xor : "^",
- intdiv : "/",
- mod : "%",
- leftShift : "<<",
- rightShift : ">>",
- rightShiftUnsigned : ">>>"
+ plus : '+',
+ minus : '-',
+ multiply : '*',
+ div : '/',
+ or : '|',
+ and : '&',
+ xor : '^',
+ intdiv : '/',
+ mod : '%',
+ leftShift : '<<',
+ rightShift : '>>',
+ rightShiftUnsigned : '>>>'
]
}
diff --git a/src/main/groovy/org/codehaus/groovy/transform/ASTTestTransformation.groovy b/src/main/groovy/org/codehaus/groovy/transform/ASTTestTransformation.groovy
index 8201d50..6982b86 100644
--- a/src/main/groovy/org/codehaus/groovy/transform/ASTTestTransformation.groovy
+++ b/src/main/groovy/org/codehaus/groovy/transform/ASTTestTransformation.groovy
@@ -64,14 +64,14 @@ class ASTTestTransformation extends AbstractASTTransformation implements Compila
}
member = annotationNode.getMember('value')
if (member && !(member instanceof ClosureExpression)) {
- throw new SyntaxException('ASTTest value must be a closure', member.getLineNumber(), member.getColumnNumber())
+ throw new SyntaxException('ASTTest value must be a closure', member.lineNumber, member.columnNumber)
}
if (!member && !annotationNode.getNodeMetaData(ASTTestTransformation)) {
- throw new SyntaxException('Missing test expression', annotationNode.getLineNumber(), annotationNode.getColumnNumber())
+ throw new SyntaxException('Missing test expression', annotationNode.lineNumber, annotationNode.columnNumber)
}
// convert value into node metadata so that the expression doesn't mix up with other AST xforms like type checking
annotationNode.putNodeMetaData(ASTTestTransformation, member)
- annotationNode.getMembers().remove('value')
+ annotationNode.members.remove('value')
def pcallback = compilationUnit.progressCallback
def callback = new CompilationUnit.ProgressCallback() {
@@ -85,8 +85,8 @@ class ASTTestTransformation extends AbstractASTTransformation implements Compila
for (int i = testClosure.lineNumber; i <= testClosure.lastLineNumber; i++) {
sb.append(source.source.getLine(i, new Janitor())).append('\n')
}
- def testSource = sb.substring(testClosure.columnNumber, sb.length())
- testSource = testSource.substring(0, testSource.lastIndexOf('}'))
+ def testSource = sb[testClosure.columnNumber..<sb.length()]
+ testSource = testSource[0..<testSource.lastIndexOf('}')]
CompilerConfiguration config = new CompilerConfiguration()
def customizer = new ImportCustomizer()
config.addCompilationCustomizers(customizer)
@@ -124,8 +124,7 @@ class ASTTestTransformation extends AbstractASTTransformation implements Compila
callback = pcallback
}
- compilationUnit.setProgressCallback(callback)
-
+ compilationUnit.progressCallback = callback
}
void setCompilationUnit(final CompilationUnit unit) {
@@ -152,8 +151,8 @@ class ASTTestTransformation extends AbstractASTTransformation implements Compila
if (column > 40) {
int start = column - 30 - 1
int end = (column + 10 > text.length() ? text.length() : column + 10 - 1)
- sample = ' ' + text.substring(start, end) + Utilities.eol() + ' ' +
- marker.substring(start, marker.length())
+ sample = ' ' + text[start..<end] + Utilities.eol() + ' ' +
+ marker[start..<marker.length()]
} else {
sample = ' ' + text + Utilities.eol() + ' ' + marker
}
diff --git a/src/main/groovy/org/codehaus/groovy/transform/tailrec/TailRecursiveASTTransformation.groovy b/src/main/groovy/org/codehaus/groovy/transform/tailrec/TailRecursiveASTTransformation.groovy
index 05a15e6..0b71062 100644
--- a/src/main/groovy/org/codehaus/groovy/transform/tailrec/TailRecursiveASTTransformation.groovy
+++ b/src/main/groovy/org/codehaus/groovy/transform/tailrec/TailRecursiveASTTransformation.groovy
@@ -53,9 +53,9 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
private static final Class MY_CLASS = TailRecursive
private static final ClassNode MY_TYPE = new ClassNode(MY_CLASS)
- static final String MY_TYPE_NAME = "@" + MY_TYPE.getNameWithoutPackage()
- private HasRecursiveCalls hasRecursiveCalls = new HasRecursiveCalls()
- private TernaryToIfStatementConverter ternaryToIfStatement = new TernaryToIfStatementConverter()
+ static final String MY_TYPE_NAME = '@' + MY_TYPE.nameWithoutPackage
+ private final HasRecursiveCalls hasRecursiveCalls = new HasRecursiveCalls()
+ private final TernaryToIfStatementConverter ternaryToIfStatement = new TernaryToIfStatementConverter()
@Override
@@ -65,7 +65,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
MethodNode method = nodes[1] as MethodNode
if (method.isAbstract()) {
- addError("Annotation " + MY_TYPE_NAME + " cannot be used for abstract methods.", method)
+ addError("Annotation $MY_TYPE_NAME cannot be used for abstract methods.", method)
return
}
@@ -75,7 +75,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
if (annotationNode.classNode == MY_TYPE)
break
if (annotationNode.classNode == memoizedClassNode) {
- addError("Annotation " + MY_TYPE_NAME + " must be placed before annotation @Memoized.", annotationNode)
+ addError("Annotation $MY_TYPE_NAME must be placed before annotation @Memoized.", annotationNode)
return
}
}
@@ -83,7 +83,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
if (!hasRecursiveMethodCalls(method)) {
AnnotationNode annotationNode = method.getAnnotations(ClassHelper.make(TailRecursive))[0]
- addError("No recursive calls detected. You must remove annotation " + MY_TYPE_NAME + ".", annotationNode)
+ addError("No recursive calls detected. You must remove annotation ${MY_TYPE_NAME}.", annotationNode)
return
}
@@ -93,7 +93,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
private boolean hasAnnotation(MethodNode methodNode, ClassNode annotation) {
List annots = methodNode.getAnnotations(annotation)
- return (annots != null && annots.size() > 0)
+ annots != null && annots.size() > 0
}
@@ -106,7 +106,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
}
private void transformVoidMethodToIteration(MethodNode method) {
- addError("Void methods are not supported by @TailRecursive yet.", method)
+ addError('Void methods are not supported by @TailRecursive yet.', method)
}
private void transformNonVoidMethodToIteration(MethodNode method, SourceUnit source) {
@@ -133,7 +133,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
if (!(node instanceof ReturnStatement)) {
return false
}
- return (((ReturnStatement) node).expression instanceof TernaryExpression)
+ ((ReturnStatement) node).expression instanceof TernaryExpression
}
Closure<Statement> replaceWithIfStatement = { ReturnStatement statement ->
ternaryToIfStatement.convert(statement)
@@ -167,7 +167,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
String iterationVariableName = iterationVariableName(paramName)
nameAndTypeMapping[paramName] = [name: iterationVariableName, type: paramType]
}
- return nameAndTypeMapping
+ nameAndTypeMapping
}
// Public b/c there are tests for this method
@@ -179,7 +179,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
String iterationVariableName = this.iterationVariableName(paramName)
positionMapping[index] = [name: iterationVariableName, type: paramType]
}
- return positionMapping
+ positionMapping
}
private String iterationVariableName(String paramName) {
@@ -203,7 +203,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
if (!(inner instanceof MethodCallExpression) && !(inner instanceof StaticMethodCallExpression)) {
return false
}
- return isRecursiveIn(inner, method)
+ isRecursiveIn(inner, method)
}
Closure<Statement> replaceWithContinueBlock = { ReturnStatement statement ->
new ReturnStatementToIterationConverter().convert(statement, positionMapping)
@@ -224,7 +224,7 @@ class TailRecursiveASTTransformation extends AbstractASTTransformation {
if (!(inner instanceof MethodCallExpression) && !(inner instanceof StaticMethodCallExpression)) {
return false
}
- return isRecursiveIn(inner, method)
+ isRecursiveIn(inner, method)
}
Closure<Statement> replaceWithThrowLoopException = { ReturnStatement statement ->
new ReturnStatementToIterationConverter(recurStatement: AstHelper.recurByThrowStatement()).convert(statement, positionMapping)