Forgot the ref https://github.com/CodeNarc/CodeNarc/issues/331
Le 12 juillet 2022 21:11:13 GMT+02:00, Gil Portenseigne <gil.portensei...@nereide.fr> a écrit : >Hello, > >I agree with you, i find out here [1] that it is customisable. > >So we can agree upon this variation of the rule ! > >I have not tested yet, will see in the near future > >Thanks > >Gil > >Le 11 juillet 2022 17:57:45 GMT+02:00, Scott Gray <lekt...@gmail.com> a écrit : >>+1 in general from me although some of those rules might be challenging to >>resolve. For example DuplicateStringLiteral could be referring to an >>entity name being used in delegator queries more than once, I don't think >>we'd prefer to see that extracted to a constant. >> >>SpaceAroundMapEntryColon I'm not so sure about, my preference for >>legibility has always been [someKey: someValue]. I find >>[someKey:someValue] a bit too condensed and harder to differentiate key >>from value. >> >>Regards >>Scott >> >>On Mon, 4 Jul 2022 at 16:25, Gil Portenseigne <gil.portensei...@nereide.fr> >>wrote: >> >>> Hello Devs, >>> >>> I would like to continue Codenarc integration in OFBiz (OFBIZ-11167). >>> >>> For those who are not aware, Codenarc is an analysis tools for Groovy >>> for defects, bad practices, inconsistencies, style issues and more. >>> >>> For this purpose we need to agree about the ruleset to put in place. >>> >>> I took as a basis the ruleset : >>> https://codenarc.org/StarterRuleSet-AllRulesByCategory.groovy.txt >>> >>> And started to fix some in >>> https://github.com/apache/ofbiz-framework/pull/517 >>> >>> Before doing the complete work, a first rule made me wonder if that is a >>> choice that we will be doing as a community : >>> >>> IfStatementBraces - Use braces for if statements, even for a single >>> statement. >>> >>> There are 234 occurrences in the project, and I guess that some opinions >>> might diverge on this subject. >>> >>> Moreover, some rules needs lots of fixes in the project (those with more >>> than 200 occurrences) : >>> >>> >>> +------------+---------------------------------------------------------------------+ >>> | Number of | Rule name and details >>> | >>> | occurrence | >>> | >>> >>> +============+=====================================================================+ >>> | 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. >>> | >>> >>> +------------+---------------------------------------------------------------------+ >>> >>> I think that if someone want to express an opposition about applying one >>> of the above rule, i would be great to express it and discuss it here. >>> >>> My opinion is that it could be nice to adopt every rules, to respect >>> standard best practice. Even if it implies lot of code changes. >>> >>> Thanks in advance for your feedback. >>> >>> Regards, >>> >>> Gil >>> > >-- >Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma >brièveté. -- Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
