CodeNarc

CodeNarc is a static analysis tool specifically designed for Groovy source code. It allows developers to monitor and enforce coding standards and best practices by applying a set of predefined or custom Rules to Groovy files.

CodeNarc generates reports in HTML or XML formats, providing:

• A list of rules violated per source file.
• Violation counts per package.
• Total violation counts for the entire project.

CodeNarc provides several design-oriented rules to enforce best practices in Groovy/Java code. Key categories include:

• Class Structure: AbstractClassWithPublicConstructor, AbstractClassWithoutAbstractMethod, EmptyMethodInAbstractClass, FinalClassWithProtectedMember.
• Type Safety & Interfaces: CloneableWithoutClone, CompareToWithoutComparable, ConstantsOnlyInterface, ImplementationAsType, Instanceof.
• Method Integrity: BooleanMethodReturnsNull, BuilderMethodWithSideEffects.
• State & Side Effects: AssignmentToStaticFieldFromInstanceMethod, CloseWithoutCloseable, LocaleSetDefault.
• Complexity: NestedForLoop.

Several rules prevent catching exceptions that are too general or can mask critical system failures. Avoid the following patterns:

• CatchError: Catching java.lang.Error is dangerous as it can catch ThreadDeath or OutOfMemoryError.
• CatchThrowable: Catching java.lang.Throwable is too broad and can catch system-level errors.
• CatchException: Catching java.lang.Exception is often too general for application code; it should typically be restricted to framework or infrastructure code.
• CatchRuntimeException: Catching java.lang.RuntimeException is often too broad for application code.
• SwallowThreadDeath: Never catch java.lang.ThreadDeath without re-throwing it, as this prevents the thread from terminating correctly.

// Violation: Swallowing ThreadDeath
try {
    def a = 0
} catch (ThreadDeath td) {
    td.printStackTrace()
}

The UseCollectNested rule suggests using collectNested{} instead of nesting multiple collect{} calls to improve code clarity.

def list = [1, 2, [3, 4, 5, 6], [7]]

// Violation
list.collect { elem ->
    if (elem instanceof List)
        elem.collect {it *2}
    else elem * 2
}

// Better readability
list.collectNested { it * 2 }

The GrailsServletContextReference rule monitors and controls access to the servletContext object within Grails controllers and taglibs. Direct access to servletContext can inhibit scalability and requires manual synchronization for thread safety.

Usage Guidance:

• Use this rule as a governance tool in teams to limit servletContext access.
• If specific files require access, exclude them using doNotApplyToFilenames or doNotApplyToFilesMatching.

Default Behavior:

• Applies to files matching grails-app/controllers or grails-app/taglib.

The GStringAsMapKey rule flags the use of GStrings as keys in a Map because their hashCode is not guaranteed to be stable. You should call .toString() on the key instead.

// Violation
Map map = ["${someRef}" : 'invalid' ]

// Recommended
Map map = [someRef.toString() : 'invalid' ]

The GrailsMassAssignment rule flags code where untrusted input (like params) is used to set arbitrary object fields without restriction. This can allow attackers to modify sensitive fields (e.g., isAdmin) by including them in the request parameters.

Avoid patterns like:

• new Person(params)
• person.properties = params

// VIOLATION
def person = new Person(params)
person.save()

// VIOLATION
def person = Person.get(1)
person.properties = params
person.save()

The UnnecessaryNullCheck rule encourages using Groovy's safe dereference operator (?.) instead of explicit x == null checks in conditional statements.

Rules:

• Avoid if (obj != null && obj.method()). Use if (obj?.method()).
• Do not test this or super for null equality (e.g., if (this == null) is always pointless).
• Do not check for null before an instanceof check, as instanceof returns false for null arguments.

Example:

// Violation
if (x != null && x instanceof MyClass) { ... }

// Acceptable
if (x instanceof MyClass) { ... }

// Violation
if (obj != null && obj.prop) { }

// Acceptable
if (obj?.prop) { }

The ExceptionNotThrown rule checks that if you instantiate an exception inside a catch block, it is actually thrown as the last statement. Simply calling the constructor without throw is a violation.

// Violation: Constructor called but not thrown
void execute() {
    try { }
    catch(Exception e) {
        new Exception(e) 
    }
}

// OK: Exception is thrown
try {
    doStuff()
} catch(Exception e) {
    throw new DaoException(e)
}