[ https://issues.apache.org/jira/browse/OFBIZ-11167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17561935#comment-17561935 ]
Paul Foxworthy edited comment on OFBIZ-11167 at 7/19/23 10:32 PM: ------------------------------------------------------------------ I wrote a little script to extract the rules in order of occurrence. I will launch an email to the dev list to discuss about the rules to be respected, and the implied changes. There are the results : ||Number of occurence||Rule name and details|| |9883|UnnecessaryGString - String objects should be created with single quotes, and GString objects created with double quotes. Creating normal String objects with double quotes is confusing to readers.| |4569|DuplicateStringLiteral - Code containing duplicate String literals can usually be improved by declaring the String as a constant field. The ignoreStrings property () can optionally specify a comma-separated list of Strings to ignore.| |4209|SpaceAroundMapEntryColon - Check for configured formatting of whitespace around colons for literal Map entries. The characterBeforeColonRegex and characterAfterColonRegex properties specify a regular expression that must match the character before/after the colon.| |1448|LineLength - Checks the maximum length for each line of source code. It checks for number of characters, so lines that include tabs may appear longer than the allowed number when viewing the file. The maximum line length can be configured by setting the length property, which defaults to 120.| |885|UnnecessaryGetter - Checks for explicit calls to getter/accessor methods which can, for the most part, be replaced by property access. A getter is defined as a method call that matches get[A-Z] but not getClass() or get[A-Z][A-Z] such as getURL(). Getters do not take method arguments. The ignoreMethodNames property (null) can specify method names that should be ignored, optionally containing wildcard characters ('*' or '?').| |601|NoDef - def should not be used. You should replace it with concrete type.| |485|MethodReturnTypeRequired - Checks that method return types are not dynamic, that is they are explicitly stated and different than def.| |484|Indentation - Check indentation for class and method declarations, and initial statements.| |482|UnnecessaryReturnKeyword - In Groovy, the return keyword is often optional. If a statement is the last line in a method or closure then you do not need to have the return keyword.| |407|UnnecessaryObjectReferences - Violations are triggered when an excessive set of consecutive statements all reference the same variable. This can be made more readable by using a with or identity block.| |345|CompileStatic - Check that classes are explicitely annotated with either @GrailsCompileStatic, @CompileStatic or @CompileDynamic| |320|ExplicitCallToEqualsMethod - This rule detects when the equals(Object) method is called directly in code instead of using the == or != operator. A groovier way to express this: a.equals(b) is this: a == b and a groovier way to express : !a.equals(b) is : a != b| |235|IfStatementBraces - Use braces for if statements, even for a single statement.| |235|SpaceAroundOperator - Check that there is at least one space (blank) or whitespace around each binary operator.| |211|NestedBlockDepth - Checks for blocks or closures nested more than maxNestedBlockDepth (5) levels deep.| |201|TrailingWhitespace - Checks that no lines of source code end with whitespace characters.| |190|DuplicateNumberLiteral - Code containing number String literals can usually be improved by declaring the number as a constant field. The ignoreNumbers property (0,1) can optionally specify a comma-separated list of numbers to ignore.| |190|NoWildcardImports - Wildcard imports, static or otherwise, should not be used.| |173|TrailingComma - Check whether list and map literals contain optional trailing comma.| |167|InvertedCondition - An inverted condition is one where a constant expression is used on the left hand side of the equals comparision. Such conditions can be confusing especially when used in assertions where the expected value is by convention placed on the right hand side of the comparision.| |167|JavadocEmptyReturnTag - Checks for empty @return tags within javadoc.| |164|UnnecessarySetter - Checks for explicit calls to setter methods which can, for the most part, be replaced by assignment to property. A setter is defined as a method call that matches set[A-Z] but not set[A-Z][A-Z] such as setURL(). Setters take one method argument.| |146|CouldBeElvis - Catch an if block that could be written as an elvis expression.| |142|UnusedImport - Imports for a class that is never referenced within the source file is unnecessary.| |141|AbcMetric - Checks the ABC size metric for methods/classes. A method (or "closure field") with an ABC score greater than the maxMethodAbcScore property (60) causes a violation. Likewise, a class that has an (average method) ABC score greater than the maxClassAverageMethodAbcScore property (60) causes a violation.| |117|ConsecutiveBlankLines - Makes sure there are no consecutive lines that are either blank or whitespace only.| |104|SpaceAfterComma - Checks that there is at least one space or whitespace following each comma. That includes checks for method and closure declaration parameter lists, method call parameter lists, Map literals and List literals.| |103|FactoryMethodName - A factory method is a method that creates objects, and they are typically named either buildFoo(), makeFoo(), or createFoo(). This rule enforces that only one naming convention is used. It defaults to makeFoo(), but that can be changed using the property 'regex'.| |102|ImplicitClosureParameter - Checks that the implicit it closure parameter is not used and that parameters are declared explicitly instead.| |91|VariableTypeRequired - Checks that variable types are explicitly specified in declarations (and not using def)| |85|InvertedIfElse - An inverted if-else statement is one in which there is a single if statement with a single else branch and the boolean test of the if is negated. For instance if (!x) false else true. It is usually clearer to write this as if (x) true else false.| |74|UnnecessarySemicolon - Semicolons as line terminators are not required in Groovy: remove them. Do not use a semicolon as a replacement for empty braces on for and while loops; this is a confusing practice.| |69|SpaceBeforeOpeningBrace - Check that there is at least one space (blank) or whitespace before each opening brace ("{") for method/class/interface declarations, closure expressions and block statements.| |68|SpaceAfterIf - Check that there is exactly one space (blank) after the if keyword and before the opening parenthesis.| |51|UnnecessaryGroovyImport - A Groovy file does not need to include an import for classes from java.lang, java.util, java.io, java.net, groovy.lang and groovy.util, as well as the classes java.math.BigDecimal and java.math.BigInteger.| |50|CyclomaticComplexity - Checks the cyclomatic complexity for methods/classes.A method (or "closure field") with a cyclomatic complexity value greater than the maxMethodComplexity property (20) causes a violation. Likewise, a class that has an (average method) cyclomatic complexityvalue greater than the maxClassAverageMethodComplexity property (20) causes a violation.| |46|BlockStartsWithBlankLine - Checks that code blocks such as method bodies, closures and control structure bodies do not start with an empty line.| |45|CatchException - Catching Exception is often too broad or general. It should usually be restricted to framework or infrastructure code, rather than application code.| |44|FileEndsWithoutNewline - Makes sure the source code file ends with a newline character.| |41|ExplicitCallToCompareToMethod - This rule detects when the compareTo(Object) method is called directly in code instead of using the <=>, >, >=, <, and <= operators. A groovier way to express this: a.compareTo(b) is this: a <=> b, or using the other operators.| |41|ParameterReassignment - Checks for a method or closure parameter being reassigned to a new value within the body of the method/closure, which is a confusing and questionable practice. Use a temporary variable instead.| |34|ExplicitCallToMultiplyMethod - This rule detects when the minus(Object) method is called directly in code instead of using the * operator. A groovier way to express this: a.multiply(b) is this: a * b| |34|UnusedVariable - Checks for variables that are never referenced. The ignoreVariableNames property (null) specifies one or more variable names that should be ignored, optionally containing wildcard characters ('*' or '?').| |33|DuplicateMapLiteral - Code containing duplicate Map literals can usually be improved by declaring the Map as a constant field.| |30|UnnecessaryPackageReference - Checks for explicit package reference for classes that Groovy imports by default, such as java.lang.String, java.util.Map and groovy.lang.Closure.| |28|JavaIoPackageAccess - This rule reports violations of the Enterprise JavaBeans specification by using the java.io package to access files or the file system.| |27|BlockEndsWithBlankLine - Checks that code blocks such as method bodies, closures and control structure bodies do not end with an empty line.| |27|NestedForLoop - Reports classes with nested for loops.| |25|MethodParameterTypeRequired - Checks that method parameters are not dynamically typed, that is they are explicitly stated and different than def.| |24|UnnecessaryElseStatement - When an if statement block ends with a return statement the else is unnecessary. The logic in the else branch can be run without being in a new scope.| |23|UnnecessaryParenthesesForMethodCallWithClosure - If a method is called and the only parameter to that method is an inline closure then the parentheses of the method call can be omitted.| |21|Instanceof - Checks for use of the instanceof operator. Use the ignoreTypeNames property to configure ignored type names.| |18|MethodSize - Checks if the size of a method exceeds the number of lines specified by the maxLines property (100).| |16|NoJavaUtilDate - Do not use java.util.Date. Prefer the classes in the java.time.* packages.| |14|ConfusingTernary - In a ternary expression avoid negation in the test. For example, rephrase: "(x != y) ? diff : same" as: "(x == y) ? same : diff". Consistent use of this rule makes the code easier to read. Also, this resolves trivial ordering problems, such as "does the error case go first?" or "does the common case go first?".| |13|JavadocMissingParamDescription - Checks for missing description within @param javadoc tags.| |11|UnnecessaryCast - Checks for unnecessary cast operations| |10|LongLiteralWithLowerCaseL - In Java and Groovy, you can specify long literals with the L or l character, for instance 55L or 24l. It is best practice to always use an uppercase L and never a lowercase l. This is because 11l rendered in some fonts may look like 111 instead of 11L.| |9|DuplicateListLiteral - Code containing duplicate List literals can usually be improved by declaring the List as a constant field.| |9|ExplicitArrayListInstantiation - This rule checks for the explicit instantiation of an ArrayList using the no-arg constructor. In Groovy, it is best to write new ArrayList() as [], which creates the same object.| |9|NoDouble - Checks for use of the double or Double types, in fields, variables, method parameters and method return types.| |8|AddEmptyString - Finds empty string literals which are being added. This is an inefficient way to convert any type to a String.| |8|SpaceAfterOpeningBrace - Check that there is at least one space (blank) or whitespace after each opening brace (" \{") for method/class/interface declarations, closure expressions and block statements. |7|CouldBeSwitchStatement - Checks for multiple if statements that could be converted to a switch| |7|UnnecessaryBooleanInstantiation - Use Boolean.valueOf() for variable values or Boolean.TRUE and Boolean.FALSE for constant values instead of calling the Boolean() constructor directly or calling Boolean.valueOf(true) or Boolean.valueOf(false).| | 6|MissingBlankLineAfterImports - Makes sure there is a blank line after the imports of a source code file.| |6|ParameterCount - Checks if the number of parameters in method/constructor exceeds the number of parameters specified by the maxParameters property.| |6|ReturnNullFromCatchBlock - Returning null from a catch block often masks errors and requires the client to handle error codes. In some coding styles this is discouraged.| |6| SpaceAfterCatch - Check that there is exactly one space (blank) after the catch keyword and before the opening parenthesis.| |6| SpaceAfterClosingBrace - Check that there is at least one space (blank) or whitespace after each closing brace ("}") for method/class/interface declarations, closure expressions and block statements.| |5|ExplicitHashMapInstantiation - This rule checks for the explicit instantiation of a HashMap using the no-arg constructor. In Groovy, it is best to write new HashMap() as [:], which creates the same object.| |5|PublicMethodsBeforeNonPublicMethods - Enforce that all public methods are above protected and private methods.| |5|SpaceAfterFor - Check that there is exactly one space (blank) after the for keyword and before the opening parenthesis.| |4|NoTabCharacter - Checks that all source files do not contain the tab character| |4|SimpleDateFormatMissingLocale - Be sure to specify a Locale when creating a new instance of SimpleDateFormat; the class is locale-sensitive. If you instantiate SimpleDateFormat without a Locale parameter, it will format the date and time according to the default Locale. Both the pattern and the Locale determine the format. For the same pattern, SimpleDateFormat may format a date and time differently if the Locale varies.| |4|SpaceBeforeClosingBrace - Check that there is at least one space (blank) or whitespace before each closing brace ("}") for method/class/interface declarations, closure expressions and block statements.| |3|DuplicateImport - Duplicate import statements are unnecessary.| |3|GStringAsMapKey - A GString should not be used as a map key since its hashcode is not guaranteed to be stable. Consider calling key.toString().| |3|SpaceAroundClosureArrow - Checks that there is whitespace around the closure arrow (->) symbol| |3|TernaryCouldBeElvis - Checks for ternary expressions where the boolean and true expressions are the same. These can be simplified to an Elvis expression.| |3|UnnecessaryPublicModifier - The 'public' modifier is not required on methods or classes.| |2|ExplicitHashSetInstantiation - This rule checks for the explicit instantiation of a HashSet using the no-arg constructor. In Groovy, it is best to write new HashSet() as [] as Set, which creates the same object.| |2|ExplicitLinkedListInstantiation - This rule checks for the explicit instantiation of a LinkedList using the no-arg constructor. In Groovy, it is best to write new LinkedList() as [] as Queue, which creates the same object.| |2|ExplicitTreeSetInstantiation - This rule checks for the explicit instantiation of a TreeSet using the no-arg constructor. In Groovy, it is best to write new TreeSet() as [] as SortedSet, which creates the same object.| |2|IfStatementCouldBeTernary - Checks for if statements where both the if and else blocks contain only a single return statement with a constant or literal value| |2|JavadocEmptyFirstLine - Check for javadoc comments with an empty top line.| |2|SpaceAfterWhile - Check that there is exactly one space (blank) after the while keyword and before the opening parenthesis.| |2|UnnecessaryCallForLastElement - This rule checks for excessively verbose methods of accessing the last element of an array or list. For instance, it is possible to access the last element of an array by performing array[array.length - 1], in Groovy it is simpler to either call array.last() or array[-1]. The same is true for lists. This violation is triggered whenever a get, getAt, or array-style access is used with an object size check.| |2|UnnecessaryDotClass - To make a reference to a class, it is unnecessary to specify the '.class' identifier. For instance String.class can be shortened to String.| |2|VariableName - Verifies that the name of each method matches a regular expression. By default it checks that non-'final' variable names start with a lowercase letter and contains only letters or numbers, and 'final' variable names start with an uppercase letter and contain only uppercase letters, numbers and underscores. The regex property specifies the default regular expression used to validate a non-'final' variable name. The finalRegex property specifies the regular expression used to validate 'final' variable names. The ignoreVariableNames property (null) can specify variable names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|BracesForIfElse - Checks the location of the opening brace ({) for if statements. By default, requires them on the same line, but the sameLine property can be set to false to override this.| |1|CatchNullPointerException - Catching NullPointerException is never appropriate. It should be avoided in the first place with proper null checking, and it can mask underlying errors.| |1|ClassEndsWithBlankLine - Check whether the class ends with a blank line.By default, it enforces that there must be a blank line before the closing class brace, except if the class is empty and is written in a single line. A blank line is defined as any line that does not contain any visible characters.| |1|ClassName - Verifies that the name of a class matches a regular expression. By default it checks that the class name starts with an uppercase letter and is followed by zero or more word characters (letters, numbers or underscores). The regex property specifies the regular expression used to validate the class name.| |1|ClassNameSameAsFilename - Reports files containing only one top level class / enum / interface which is named differently than the file.| |1|ClassStartsWithBlankLine - Check whether the class starts with a blank line By default, it enforces that there must be a blank line after the opening class brace, except if the class is empty and is written in a single line. A blank line is defined as any line that does not contain any visible characters.| |1|ExplicitCallToPlusMethod - This rule detects when the plus(Object) method is called directly in code instead of using the + operator. A groovier way to express this: a.plus(b) is this: a + b| |1|GStringExpressionWithinString - Check for regular (single quote) strings containing a GString-type expression (${...}).| |1|ImplementationAsType - Checks for use of a predefined set of concrete classes (e.g. ArrayList, Hashtable, ConcurrentHashMap) when specifying the type of a method parameter, closure parameter, constructor parameter, method return type or field type. The associated interfaces should be used to specify the type instead.| |1|InsecureRandom - Reports usages of java.util.Random, which can produce very predictable results. If two instances of Random are created with the same seed and sequence of method calls, they will generate the exact same results. Use java.security.SecureRandom instead, which provides a cryptographically strong random number generator. SecureRandom uses PRNG, which means they are using a deterministic algorithm to produce a pseudo-random number from a true random seed. SecureRandom produces non-deterministic output.| |1|JavadocEmptyLastLine - Check for javadoc comments with an empty line at the bottom.| |1|JdbcResultSetReference - Check for direct use of java.sql.ResultSet, which is not necessary if using the Groovy Sql facility or an ORM framework such as Hibernate.| |1|MethodName - Verifies that the name of each method matches a regular expression. By default it checks that the method name starts with a lowercase letter. The regex property specifies the regular expression to check the method name against. The ignoreMethodNames property (null) can specify method names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|MissingBlankLineAfterPackage - Makes sure there is a blank line after the package statement of a source code file.| |1|NoFloat - Checks for use of the float or Float types, in fields, variables, method parameters and method return types.| |1|ParameterName - Verifies that the name of each parameter matches a regular expression. This rule applies to method parameters, constructor parameters and closure parameters. By default it checks that parameter names start with a lowercase letter and contains only letters or numbers. The regex property specifies the default regular expression used to validate the parameter name. The ignoreParameterNames property (null) can specify parameter names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|ReturnsNullInsteadOfEmptyCollection - Consider returning a zero length collection rather than null. It is often a better design to return a length zero collection rather than a null reference to indicate that there are no results (i.e., an empty list of results). This way, no explicit check for null is needed by clients of the method.| |1|UnnecessaryBigDecimalInstantiation - It is unnecessary to instantiate BigDecimal objects. Instead just use the decimal literal or the 'G' identifier to force the type, such as 123.45 or 123.45G.| |1|UnnecessaryBooleanExpression - Checks for unnecessary boolean expressions, including ANDing (&&) or ORing (\|\|) with true, false, null, or a Map/List/String/Number literal. Also checks for negation (!) of true, false, null, or a Map/List/String/Number literal.| |1|UnnecessaryCollectCall - Some method calls to Object.collect(Closure) can be replaced with the spread operator. For instance, list.collect \{ it.multiply(2) }can be replaced by list*.multiply(2). Warning: if a collection is null, collect will return an empty list, while *. will return null.| |1|UnnecessaryDefInMethodDeclaration - If a method has a visibility modifier or a type declaration, then the def keyword is unneeded. For instance 'def private method() {}' is redundant and can be simplified to 'private method() {}'.| |1|UnnecessaryLongInstantiation - It is unnecessary to instantiate Long objects. Instead just use the literal with the 'L' identifier to force the type, such as 8L or 42L.| |1|UnnecessaryToString - Checks for unnecessary calls to toString().| was (Author: gil portenseigne): I wrote a little script to extract the rules in order of occurrence. I will launch an email to the dev list to discuss about the rules to be respected, and the implied changes. There are the results : ||Number of occurence||Rule name and details|| |9883|UnnecessaryGString - String objects should be created with single quotes, and GString objects created with double quotes. Creating normal String objects with double quotes is confusing to readers.| |4569|DuplicateStringLiteral - Code containing duplicate String literals can usually be improved by declaring the String as a constant field. The ignoreStrings property () can optionally specify a comma-separated list of Strings to ignore.| |4209|SpaceAroundMapEntryColon - Check for configured formatting of whitespace around colons for literal Map entries. The characterBeforeColonRegex and characterAfterColonRegex properties specify a regular expression that must match the character before/after the colon.| |1448|LineLength - Checks the maximum length for each line of source code. It checks for number of characters, so lines that include tabs may appear longer than the allowed number when viewing the file. The maximum line length can be configured by setting the length property, which defaults to 120.| |885|UnnecessaryGetter - Checks for explicit calls to getter/accessor methods which can, for the most part, be replaced by property access. A getter is defined as a method call that matches get[A-Z] but not getClass() or get[A-Z][A-Z] such as getURL(). Getters do not take method arguments. The ignoreMethodNames property (null) can specify method names that should be ignored, optionally containing wildcard characters ('*' or '?').| |601|NoDef - def should not be used. You should replace it with concrete type.| |485|MethodReturnTypeRequired - Checks that method return types are not dynamic, that is they are explicitly stated and different than def.| |484|Indentation - Check indentation for class and method declarations, and initial statements.| |482|UnnecessaryReturnKeyword - In Groovy, the return keyword is often optional. If a statement is the last line in a method or closure then you do not need to have the return keyword.| |407|UnnecessaryObjectReferences - Violations are triggered when an excessive set of consecutive statements all reference the same variable. This can be made more readable by using a with or identity block.| |345|CompileStatic - Check that classes are explicitely annotated with either @GrailsCompileStatic, @CompileStatic or @CompileDynamic| |320|ExplicitCallToEqualsMethod - This rule detects when the equals(Object) method is called directly in code instead of using the == or != operator. A groovier way to express this: a.equals(b) is this: a == b and a groovier way to express : !a.equals(b) is : a != b| |235|IfStatementBraces - Use braces for if statements, even for a single statement.| |235|SpaceAroundOperator - Check that there is at least one space (blank) or whitespace around each binary operator.| |211|NestedBlockDepth - Checks for blocks or closures nested more than maxNestedBlockDepth (5) levels deep.| |201|TrailingWhitespace - Checks that no lines of source code end with whitespace characters.| |190|DuplicateNumberLiteral - Code containing number String literals can usually be improved by declaring the number as a constant field. The ignoreNumbers property (0,1) can optionally specify a comma-separated list of numbers to ignore.| |190|NoWildcardImports - Wildcard imports, static or otherwise, should not be used.| |173|TrailingComma - Check whether list and map literals contain optional trailing comma.| |167|InvertedCondition - An inverted condition is one where a constant expression is used on the left hand side of the equals comparision. Such conditions can be confusing especially when used in assertions where the expected value is by convention placed on the right hand side of the comparision.| |167|JavadocEmptyReturnTag - Checks for empty @return tags within javadoc.| |164|UnnecessarySetter - Checks for explicit calls to setter methods which can, for the most part, be replaced by assignment to property. A setter is defined as a method call that matches set[A-Z] but not set[A-Z][A-Z] such as setURL(). Setters take one method argument.| |146|CouldBeElvis - Catch an if block that could be written as an elvis expression.| |142|UnusedImport - Imports for a class that is never referenced within the source file is unnecessary.| |141|AbcMetric - Checks the ABC size metric for methods/classes. A method (or "closure field") with an ABC score greater than the maxMethodAbcScore property (60) causes a violation. Likewise, a class that has an (average method) ABC score greater than the maxClassAverageMethodAbcScore property (60) causes a violation.| |117|ConsecutiveBlankLines - Makes sure there are no consecutive lines that are either blank or whitespace only.| |104|SpaceAfterComma - Checks that there is at least one space or whitespace following each comma. That includes checks for method and closure declaration parameter lists, method call parameter lists, Map literals and List literals.| |103|FactoryMethodName - A factory method is a method that creates objects, and they are typically named either buildFoo(), makeFoo(), or createFoo(). This rule enforces that only one naming convention is used. It defaults to makeFoo(), but that can be changed using the property 'regex'.| |102|ImplicitClosureParameter - Checks that the implicit it closure parameter is not used and that parameters are declared explicitly instead.| |91|VariableTypeRequired - Checks that variable types are explicitly specified in declarations (and not using def)| |85|InvertedIfElse - An inverted if-else statement is one in which there is a single if statement with a single else branch and the boolean test of the if is negated. For instance if (!x) false else true. It is usually clearer to write this as if (x) true else false.| |74|UnnecessarySemicolon - Semicolons as line terminators are not required in Groovy: remove them. Do not use a semicolon as a replacement for empty braces on for and while loops; this is a confusing practice.| |69|SpaceBeforeOpeningBrace - Check that there is at least one space (blank) or whitespace before each opening brace ("{") for method/class/interface declarations, closure expressions and block statements.| |68|SpaceAfterIf - Check that there is exactly one space (blank) after the if keyword and before the opening parenthesis.| |51|UnnecessaryGroovyImport - A Groovy file does not need to include an import for classes from java.lang, java.util, java.io, java.net, groovy.lang and groovy.util, as well as the classes java.math.BigDecimal and java.math.BigInteger.| |50|CyclomaticComplexity - Checks the cyclomatic complexity for methods/classes.A method (or "closure field") with a cyclomatic complexity value greater than the maxMethodComplexity property (20) causes a violation. Likewise, a class that has an (average method) cyclomatic complexityvalue greater than the maxClassAverageMethodComplexity property (20) causes a violation.| |46|BlockStartsWithBlankLine - Checks that code blocks such as method bodies, closures and control structure bodies do not start with an empty line.| |45|CatchException - Catching Exception is often too broad or general. It should usually be restricted to framework or infrastructure code, rather than application code.| |44|FileEndsWithoutNewline - Makes sure the source code file ends with a newline character.| |41|ExplicitCallToCompareToMethod - This rule detects when the compareTo(Object) method is called directly in code instead of using the <=>, >, >=, <, and <= operators. A groovier way to express this: a.compareTo(b) is this: a <=> b, or using the other operators.| |41|ParameterReassignment - Checks for a method or closure parameter being reassigned to a new value within the body of the method/closure, which is a confusing and questionable practice. Use a temporary variable instead.| |34|ExplicitCallToMultiplyMethod - This rule detects when the minus(Object) method is called directly in code instead of using the * operator. A groovier way to express this: a.multiply(b) is this: a * b| |34|UnusedVariable - Checks for variables that are never referenced. The ignoreVariableNames property (null) specifies one or more variable names that should be ignored, optionally containing wildcard characters ('*' or '?').| |33|DuplicateMapLiteral - Code containing duplicate Map literals can usually be improved by declaring the Map as a constant field.| |30|UnnecessaryPackageReference - Checks for explicit package reference for classes that Groovy imports by default, such as java.lang.String, java.util.Map and groovy.lang.Closure.| |28|JavaIoPackageAccess - This rule reports violations of the Enterprise JavaBeans specification by using the java.io package to access files or the file system.| |27|BlockEndsWithBlankLine - Checks that code blocks such as method bodies, closures and control structure bodies do not end with an empty line.| |27|NestedForLoop - Reports classes with nested for loops.| |25|MethodParameterTypeRequired - Checks that method parameters are not dynamically typed, that is they are explicitly stated and different than def.| |24|UnnecessaryElseStatement - When an if statement block ends with a return statement the else is unnecessary. The logic in the else branch can be run without being in a new scope.| |23|UnnecessaryParenthesesForMethodCallWithClosure - If a method is called and the only parameter to that method is an inline closure then the parentheses of the method call can be omitted.| |21|Instanceof - Checks for use of the instanceof operator. Use the ignoreTypeNames property to configure ignored type names.| |18|MethodSize - Checks if the size of a method exceeds the number of lines specified by the maxLines property (100).| |16|NoJavaUtilDate - Do not use java.util.Date. Prefer the classes in the java.time.* packages.| |14|ConfusingTernary - In a ternary expression avoid negation in the test. For example, rephrase: "(x != y) ? diff : same" as: "(x == y) ? same : diff". Consistent use of this rule makes the code easier to read. Also, this resolves trivial ordering problems, such as "does the error case go first?" or "does the common case go first?".| |13|JavadocMissingParamDescription - Checks for missing description within @param javadoc tags.| |11|UnnecessaryCast - Checks for unnecessary cast operations| |10|LongLiteralWithLowerCaseL - In Java and Groovy, you can specify long literals with the L or l character, for instance 55L or 24l. It is best practice to always use an uppercase L and never a lowercase l. This is because 11l rendered in some fonts may look like 111 instead of 11L.| |9|DuplicateListLiteral - Code containing duplicate List literals can usually be improved by declaring the List as a constant field.| |9|ExplicitArrayListInstantiation - This rule checks for the explicit instantiation of an ArrayList using the no-arg constructor. In Groovy, it is best to write new ArrayList() as [], which creates the same object.| |9|NoDouble - Checks for use of the double or Double types, in fields, variables, method parameters and method return types.| |8|AddEmptyString - Finds empty string literals which are being added. This is an inefficient way to convert any type to a String.| |8|SpaceAfterOpeningBrace - Check that there is at least one space (blank) or whitespace after each opening brace (" \{") for method/class/interface declarations, closure expressions and block statements. \| \| 7 \| CouldBeSwitchStatement - Checks for multiple if statements that could be converted to a switch \| \| 7 \| UnnecessaryBooleanInstantiation - Use Boolean.valueOf() for variable values or Boolean.TRUE and Boolean.FALSE for constant values instead of calling the Boolean() constructor directly or calling Boolean.valueOf(true) or Boolean.valueOf(false). \| \| 6 \| MissingBlankLineAfterImports - Makes sure there is a blank line after the imports of a source code file. \| \| 6 \| ParameterCount - Checks if the number of parameters in method/constructor exceeds the number of parameters specified by the maxParameters property. \| \| 6 \| ReturnNullFromCatchBlock - Returning null from a catch block often masks errors and requires the client to handle error codes. In some coding styles this is discouraged. \| \| 6 \| SpaceAfterCatch - Check that there is exactly one space (blank) after the catch keyword and before the opening parenthesis. \| \| 6 \| SpaceAfterClosingBrace - Check that there is at least one space (blank) or whitespace after each closing brace ("}") for method/class/interface declarations, closure expressions and block statements.| |5|ExplicitHashMapInstantiation - This rule checks for the explicit instantiation of a HashMap using the no-arg constructor. In Groovy, it is best to write new HashMap() as [:], which creates the same object.| |5|PublicMethodsBeforeNonPublicMethods - Enforce that all public methods are above protected and private methods.| |5|SpaceAfterFor - Check that there is exactly one space (blank) after the for keyword and before the opening parenthesis.| |4|NoTabCharacter - Checks that all source files do not contain the tab character| |4|SimpleDateFormatMissingLocale - Be sure to specify a Locale when creating a new instance of SimpleDateFormat; the class is locale-sensitive. If you instantiate SimpleDateFormat without a Locale parameter, it will format the date and time according to the default Locale. Both the pattern and the Locale determine the format. For the same pattern, SimpleDateFormat may format a date and time differently if the Locale varies.| |4|SpaceBeforeClosingBrace - Check that there is at least one space (blank) or whitespace before each closing brace ("}") for method/class/interface declarations, closure expressions and block statements.| |3|DuplicateImport - Duplicate import statements are unnecessary.| |3|GStringAsMapKey - A GString should not be used as a map key since its hashcode is not guaranteed to be stable. Consider calling key.toString().| |3|SpaceAroundClosureArrow - Checks that there is whitespace around the closure arrow (->) symbol| |3|TernaryCouldBeElvis - Checks for ternary expressions where the boolean and true expressions are the same. These can be simplified to an Elvis expression.| |3|UnnecessaryPublicModifier - The 'public' modifier is not required on methods or classes.| |2|ExplicitHashSetInstantiation - This rule checks for the explicit instantiation of a HashSet using the no-arg constructor. In Groovy, it is best to write new HashSet() as [] as Set, which creates the same object.| |2|ExplicitLinkedListInstantiation - This rule checks for the explicit instantiation of a LinkedList using the no-arg constructor. In Groovy, it is best to write new LinkedList() as [] as Queue, which creates the same object.| |2|ExplicitTreeSetInstantiation - This rule checks for the explicit instantiation of a TreeSet using the no-arg constructor. In Groovy, it is best to write new TreeSet() as [] as SortedSet, which creates the same object.| |2|IfStatementCouldBeTernary - Checks for if statements where both the if and else blocks contain only a single return statement with a constant or literal value| |2|JavadocEmptyFirstLine - Check for javadoc comments with an empty top line.| |2|SpaceAfterWhile - Check that there is exactly one space (blank) after the while keyword and before the opening parenthesis.| |2|UnnecessaryCallForLastElement - This rule checks for excessively verbose methods of accessing the last element of an array or list. For instance, it is possible to access the last element of an array by performing array[array.length - 1], in Groovy it is simpler to either call array.last() or array[-1]. The same is true for lists. This violation is triggered whenever a get, getAt, or array-style access is used with an object size check.| |2|UnnecessaryDotClass - To make a reference to a class, it is unnecessary to specify the '.class' identifier. For instance String.class can be shortened to String.| |2|VariableName - Verifies that the name of each method matches a regular expression. By default it checks that non-'final' variable names start with a lowercase letter and contains only letters or numbers, and 'final' variable names start with an uppercase letter and contain only uppercase letters, numbers and underscores. The regex property specifies the default regular expression used to validate a non-'final' variable name. The finalRegex property specifies the regular expression used to validate 'final' variable names. The ignoreVariableNames property (null) can specify variable names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|BracesForIfElse - Checks the location of the opening brace ({) for if statements. By default, requires them on the same line, but the sameLine property can be set to false to override this.| |1|CatchNullPointerException - Catching NullPointerException is never appropriate. It should be avoided in the first place with proper null checking, and it can mask underlying errors.| |1|ClassEndsWithBlankLine - Check whether the class ends with a blank line.By default, it enforces that there must be a blank line before the closing class brace, except if the class is empty and is written in a single line. A blank line is defined as any line that does not contain any visible characters.| |1|ClassName - Verifies that the name of a class matches a regular expression. By default it checks that the class name starts with an uppercase letter and is followed by zero or more word characters (letters, numbers or underscores). The regex property specifies the regular expression used to validate the class name.| |1|ClassNameSameAsFilename - Reports files containing only one top level class / enum / interface which is named differently than the file.| |1|ClassStartsWithBlankLine - Check whether the class starts with a blank line By default, it enforces that there must be a blank line after the opening class brace, except if the class is empty and is written in a single line. A blank line is defined as any line that does not contain any visible characters.| |1|ExplicitCallToPlusMethod - This rule detects when the plus(Object) method is called directly in code instead of using the + operator. A groovier way to express this: a.plus(b) is this: a + b| |1|GStringExpressionWithinString - Check for regular (single quote) strings containing a GString-type expression (${...}).| |1|ImplementationAsType - Checks for use of a predefined set of concrete classes (e.g. ArrayList, Hashtable, ConcurrentHashMap) when specifying the type of a method parameter, closure parameter, constructor parameter, method return type or field type. The associated interfaces should be used to specify the type instead.| |1|InsecureRandom - Reports usages of java.util.Random, which can produce very predictable results. If two instances of Random are created with the same seed and sequence of method calls, they will generate the exact same results. Use java.security.SecureRandom instead, which provides a cryptographically strong random number generator. SecureRandom uses PRNG, which means they are using a deterministic algorithm to produce a pseudo-random number from a true random seed. SecureRandom produces non-deterministic output.| |1|JavadocEmptyLastLine - Check for javadoc comments with an empty line at the bottom.| |1|JdbcResultSetReference - Check for direct use of java.sql.ResultSet, which is not necessary if using the Groovy Sql facility or an ORM framework such as Hibernate.| |1|MethodName - Verifies that the name of each method matches a regular expression. By default it checks that the method name starts with a lowercase letter. The regex property specifies the regular expression to check the method name against. The ignoreMethodNames property (null) can specify method names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|MissingBlankLineAfterPackage - Makes sure there is a blank line after the package statement of a source code file.| |1|NoFloat - Checks for use of the float or Float types, in fields, variables, method parameters and method return types.| |1|ParameterName - Verifies that the name of each parameter matches a regular expression. This rule applies to method parameters, constructor parameters and closure parameters. By default it checks that parameter names start with a lowercase letter and contains only letters or numbers. The regex property specifies the default regular expression used to validate the parameter name. The ignoreParameterNames property (null) can specify parameter names that should be ignored, optionally containing wildcard characters ('*' or '?').| |1|ReturnsNullInsteadOfEmptyCollection - Consider returning a zero length collection rather than null. It is often a better design to return a length zero collection rather than a null reference to indicate that there are no results (i.e., an empty list of results). This way, no explicit check for null is needed by clients of the method.| |1|UnnecessaryBigDecimalInstantiation - It is unnecessary to instantiate BigDecimal objects. Instead just use the decimal literal or the 'G' identifier to force the type, such as 123.45 or 123.45G.| |1|UnnecessaryBooleanExpression - Checks for unnecessary boolean expressions, including ANDing (&&) or ORing (\|\|) with true, false, null, or a Map/List/String/Number literal. Also checks for negation (!) of true, false, null, or a Map/List/String/Number literal.| |1|UnnecessaryCollectCall - Some method calls to Object.collect(Closure) can be replaced with the spread operator. For instance, list.collect \{ it.multiply(2) }can be replaced by list*.multiply(2). Warning: if a collection is null, collect will return an empty list, while *. will return null.| |1|UnnecessaryDefInMethodDeclaration - If a method has a visibility modifier or a type declaration, then the def keyword is unneeded. For instance 'def private method() {}' is redundant and can be simplified to 'private method() {}'.| |1|UnnecessaryLongInstantiation - It is unnecessary to instantiate Long objects. Instead just use the literal with the 'L' identifier to force the type, such as 8L or 42L.| |1|UnnecessaryToString - Checks for unnecessary calls to toString().| > Use Codenarc to test Groovy code > -------------------------------- > > Key: OFBIZ-11167 > URL: https://issues.apache.org/jira/browse/OFBIZ-11167 > Project: OFBiz > Issue Type: New Feature > Components: framework > Reporter: Jacques Le Roux > Assignee: Gil Portenseigne > Priority: Minor > Fix For: 22.01.01, Upcoming Branch > > Attachments: OFBIZ-11167.patch, main.html, test.html > > > Thread of community discussion about the rules to implement : > https://www.mail-archive.com/search?l=dev%40ofbiz.apache.org&q=subject:%22Codenarc+integration%2C+rules+to+use.%22&o=newest&f=1 > > Now that we use Groovy more and more, I think we should really have a look a > Codenarc > [https://docs.gradle.org/current/userguide/codenarc_plugin.html] > We already discussed it at [https://markmail.org/message/uigcpnxqgizhd2oi] > and [https://markmail.org/message/rp6njoiohkkiodbe] > We know it's a crucial task but not an easy but rather a long term one > Here are some interesting links (before I delete my FF tabs group about it) > [http://codenarc.sourceforge.net/codenarc-other-tools-frameworks.html] > [http://codenarc.sourceforge.net/codenarc-creating-ruleset.html] > [https://github.com/gradle/gradle/tree/master/config] > [https://stackoverflow.com/questions/14358471/how-to-generate-codenarc-report-for-main-and-test-classes-using-different-rule-s] > [https://mrhaki.blogspot.com/2011/01/gradle-goodness-use-groovy-ruleset-file.html] -- This message was sent by Atlassian Jira (v8.20.10#820010)