From commits-return-8215-archive-asf-public=cust-asf.ponee.io@groovy.apache.org Thu Apr 4 04:57:27 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id C685D18072F for ; Thu, 4 Apr 2019 06:57:26 +0200 (CEST) Received: (qmail 165 invoked by uid 500); 4 Apr 2019 04:57:25 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 102 invoked by uid 99); 4 Apr 2019 04:57:25 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 04 Apr 2019 04:57:25 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 057D185CAF; Thu, 4 Apr 2019 04:57:25 +0000 (UTC) Date: Thu, 04 Apr 2019 04:57:26 +0000 To: "commits@groovy.apache.org" Subject: [groovy] 02/02: minor refactor: remove some codenarc violations (part 4) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: paulk@apache.org In-Reply-To: <155435384449.8226.15534228209684747667@gitbox.apache.org> References: <155435384449.8226.15534228209684747667@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: groovy X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Rev: e4cc76b8db9ddefff65e710de96eb2e9c364d44c X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20190404045725.057D185CAF@gitbox.apache.org> 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 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.. @@ -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.. 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.. 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 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 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 replaceWithThrowLoopException = { ReturnStatement statement -> new ReturnStatementToIterationConverter(recurStatement: AstHelper.recurByThrowStatement()).convert(statement, positionMapping)