Adobe-Consulting-Services/acs-aem-commons

            Pattern.compile("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\\.[0-9]{3}[-+]{1}[0-9]{2}[:]{0,1}[0-9]{2}$");

Close

Overly complicated regular expressions are hard to read and to maintain and can easily cause hard-to-find bugs. If a regex is too complicated, you should consider replacing it or parts of it with regular code or splitting it apart into multiple patterns at least.

The complexity of a regular expression is determined as follows:

Each of the following operators increases the complexity by an amount equal to the current nesting level and also increases the current nesting level by one for its arguments:

• | - when multiple | operators are used together, the subsequent ones only increase the complexity by 1
• && (inside character classes) - when multiple && operators are used together, the subsequent ones only increase the complexity by 1
• Quantifiers (*, +, ?, {n,m}, {n,} or {n})
• Non-capturing groups that set flags (such as (?i:some_pattern) or (?i)some_pattern)
• Lookahead and lookbehind assertions

Additionally, each use of the following features increase the complexity by 1 regardless of nesting:

• character classes
• back references

If a regular expression is split among multiple variables, the complexity is calculated for each variable individually, not for the whole regular expression. If a regular expression is split over multiple lines, each line is treated individually if it is accompanied by a comment (either a Java comment or a comment within the regular expression), otherwise the regular expression is analyzed as a whole.

Noncompliant Code Example

if (dateString.matches("^(?:(?:31(\\/|-|\\.)(?:0?[13578]|1[02]))\\1|(?:(?:29|30)(\\/|-|\\.)(?:0?[13-9]|1[0-2])\\2))(?:(?:1[6-9]|[2-9]\\d)?\\d{2})$|^(?:29(\\/|-|\\.)0?2\\3(?:(?:(?:1[6-9]|[2-9]\\d)?(?:0[48]|[2468][048]|[13579][26])|(?:(?:16|[2468][048]|[3579][26])00))))$|^(?:0?[1-9]|1\\d|2[0-8])(\\/|-|\\.)(?:(?:0?[1-9])|(?:1[0-2]))\\4(?:(?:1[6-9]|[2-9]\\d)?\\d{2})$")) {
    handleDate(dateString);
}

Compliant Solution

    if (dateString.matches("^\\d{1,2}([-/.])\\d{1,2}\\1\\d{1,4}$")) {
        String dateParts[] = dateString.split("[-/.]");
        int day = Integer.parseInt(dateParts[0]);
        int month = Integer.parseInt(dateParts[1]);
        int year = Integer.parseInt(dateParts[2]);
        // Put logic to validate and process the date based on its integer parts here
    }

Exceptions

Regular expressions are only analyzed if all parts of the regular expression are either string literals, effectively final local variables or static final fields, all of which can be combined using the '+' operator.

When a regular expression is split among multiple variables or commented lines, each part is only analyzed if it is syntactically valid by itself.

            Class collectionType = (Class) type.getRawType();

Close

Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.

Noncompliant Code Example

List myList; // Noncompliant
Set mySet; // Noncompliant

Compliant Solution

List<String> myList;
Set<? extends Number> mySet;

    HttpClientFactoryImpl(boolean useSSL, String hostname, int port, int soTimeout, 

Close

A long parameter list can indicate that a new structure should be created to wrap the numerous parameters or that the function is doing too many things.

Noncompliant Code Example

With a maximum number of 4 parameters:

public void doSomething(int param1, int param2, int param3, String param4, long param5) {
...
}

Compliant Solution

public void doSomething(int param1, int param2, int param3, String param4) {
...
}

Exceptions

Methods annotated with :

• Spring's @RequestMapping (and related shortcut annotations, like @GetRequest)
• JAX-RS API annotations (like @javax.ws.rs.GET)
• Bean constructor injection with @org.springframework.beans.factory.annotation.Autowired
• CDI constructor injection with @javax.inject.Inject
• @com.fasterxml.jackson.annotation.JsonCreator

may have a lot of parameters, encapsulation being possible. Such methods are therefore ignored.

public class TimedRunnableFuture extends FutureTask implements Comparable<TimedRunnableFuture> {

Close

Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.

Noncompliant Code Example

List myList; // Noncompliant
Set mySet; // Noncompliant

Compliant Solution

List<String> myList;
Set<? extends Number> mySet;

    private final transient ThreadLocal<String> currentPath;

Close

ThreadLocal variables are supposed to be garbage collected once the holding thread is no longer alive. Memory leaks can occur when holding threads are re-used which is the case on application servers using pool of threads.

To avoid such problems, it is recommended to always clean up ThreadLocal variables using the remove() method to remove the current thread’s value for the ThreadLocal variable.

In addition, calling set(null) to remove the value might keep the reference to this pointer in the map, which can cause memory leak in some scenarios. Using remove is safer to avoid this issue.

Noncompliant Code Example

public class ThreadLocalUserSession implements UserSession {

  private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();

  public UserSession get() {
    UserSession session = DELEGATE.get();
    if (session != null) {
      return session;
    }
    throw new UnauthorizedException("User is not authenticated");
  }

  public void set(UserSession session) {
    DELEGATE.set(session);
  }

   public void incorrectCleanup() {
     DELEGATE.set(null); // Noncompliant
   }

  // some other methods without a call to DELEGATE.remove()
}

Compliant Solution

public class ThreadLocalUserSession implements UserSession {

  private static final ThreadLocal<UserSession> DELEGATE = new ThreadLocal<>();

  public UserSession get() {
    UserSession session = DELEGATE.get();
    if (session != null) {
      return session;
    }
    throw new UnauthorizedException("User is not authenticated");
  }

  public void set(UserSession session) {
    DELEGATE.set(session);
  }

  public void unload() {
    DELEGATE.remove(); // Compliant
  }

  // ...
}

Exceptions

Rule will not detect non-private ThreadLocal variables, because remove() can be called from another class.

See

• Understanding Memory Leaks in Java

                if (shouldOverride || !map.containsKey(propertyName)) {

Close

It's a common pattern to test the result of a java.util.Map.get() against null or calling java.util.Map.containsKey() before proceeding with adding or changing the value in the map. However the java.util.Map API offers a significantly better alternative in the form of the computeIfPresent() and computeIfAbsent() methods. Using these instead leads to cleaner and more readable code.

Note that this rule is automatically disabled when the project's sonar.java.source is not 8.

Noncompliant Code Example

V value = map.get(key);
if (value == null) {  // Noncompliant
  value = V.createFor(key);
  if (value != null) {
    map.put(key, value);
  }
}
if (!map.containsKey(key)) {  // Noncompliant
  value = V.createFor(key);
  if (value != null) {
    map.put(key, value);
  }
}
return value;

Compliant Solution

return map.computeIfAbsent(key, k -> V.createFor(k));

Exceptions

This rule will not raise an issue when trying to add null to a map, because computeIfAbsent will not add the entry if the value returned by the function is null.

See also

• {rule:java:S6104} - Map "computeIfAbsent()" and "computeIfPresent()" should not be used to add "null" values.

    protected void activate(final McpLocalizationConfiguration config) throws Exception {

Close

Using such generic exceptions as Error, RuntimeException, Throwable, and Exception prevents calling methods from handling true, system-generated exceptions differently than application-generated errors.

Noncompliant Code Example

public void foo(String bar) throws Throwable {  // Noncompliant
  throw new RuntimeException("My Message");     // Noncompliant
}

Compliant Solution

public void foo(String bar) {
  throw new MyOwnRuntimeException("My Message");
}

Exceptions

Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the analysis).

@Override
public void myMethod() throws Exception {...}

Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.

public void myOtherMethod throws Exception {
  doTheThing();  // this method throws Exception
}

See

• MITRE, CWE-397 - Declaration of Throws for Generic Exception
• CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable

  private void record(ItemStatus status, String tagId, String title, Collection<String> references) {

Close

Even if it is technically possible, Restricted Identifiers should not be used as identifiers. This is only possible for compatibility reasons, using it in Java code is confusing and should be avoided.

Note that this applies to any version of Java, including the one where these identifiers are not yet restricted, to avoid future confusion.

This rule reports an issue when restricted identifiers:

• var
• yield
• record

are used as identifiers.

Noncompliant Code Example

var var = "var"; // Noncompliant: compiles but this code is confusing
var = "what is this?";

int yield(int i) { // Noncompliant
  return switch (i) {
    case 1: yield(0); // This is a yield from switch expression, not a recursive call.
    default: yield(i-1);
  };
}

String record = "record"; // Noncompliant

Compliant Solution

var myVariable = "var";

int minusOne(int i) {
  return switch (i) {
    case 1: yield(0);
    default: yield(i-1);
  };
}

String myRecord = "record";

See

• JLS16, 3.8: Identifiers

    private void writeFromBinaryProperty(SlingHttpServletRequest request, SlingHttpServletResponse response) throws IOException {

Close

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

        }, new Hashtable());

Close

Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.

Noncompliant Code Example

List myList; // Noncompliant
Set mySet; // Noncompliant

Compliant Solution

List<String> myList;
Set<? extends Number> mySet;

                            && !StringUtils.startsWith(folder.getName(), new SimpleDateFormat(WORKFLOW_FOLDER_FORMAT).format(new Date()))) {

Close

Few developers are aware of the difference between Y for "Week year" and y for Year when formatting and parsing a date with SimpleDateFormat or DateTimeFormatter. That's likely because for most dates, Week year and Year are the same, so testing at any time other than the first or last week of the year will yield the same value for both y and Y. But in the last week of December and the first week of January, you may get unexpected results.

According to the Javadoc:

A week year is in sync with a WEEK_OF_YEAR cycle. All weeks between the first and last weeks (inclusive) have the same week year value. Therefore, the first and last days of a week year may have different calendar year values.

For example, January 1, 1998 is a Thursday. If getFirstDayOfWeek() is MONDAY and getMinimalDaysInFirstWeek() is 4 (ISO 8601 standard compatible setting), then week 1 of 1998 starts on December 29, 1997, and ends on January 4, 1998. The week year is 1998 for the last three days of calendar year 1997. If, however, getFirstDayOfWeek() is SUNDAY, then week 1 of 1998 starts on January 4, 1998, and ends on January 10, 1998; the first three days of 1998 then are part of week 53 of 1997 and their week year is 1997.

Noncompliant Code Example

Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = new SimpleDateFormat("YYYY/MM/dd").format(date);   //Noncompliant; yields '2016/12/31'
result = DateTimeFormatter.ofPattern("YYYY/MM/dd").format(date); //Noncompliant; yields '2016/12/31'

Compliant Solution

Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = new SimpleDateFormat("yyyy/MM/dd").format(date);   //Yields '2015/12/31' as expected
result = DateTimeFormatter.ofPattern("yyyy/MM/dd").format(date); //Yields '2015/12/31' as expected

Exceptions

Date date = new SimpleDateFormat("yyyy/MM/dd").parse("2015/12/31");
String result = new SimpleDateFormat("YYYY-ww").format(date);  //compliant, 'Week year' is used along with 'Week of year'. result = '2016-01'
DateTimeFormatter.ofPattern("YYYY-ww").format(date); //compliant; yields '2016-01' as expected

    public Object getValue(Object adaptable, String name, Type declaredType, AnnotatedElement element, DisposalCallbackRegistry callbackRegistry) {

Close

Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.

Noncompliant Code Example

void doSomething(int a, int b) {     // "b" is unused
  compute(a);
}

Compliant Solution

void doSomething(int a) {
  compute(a);
}

Exceptions

The rule will not raise issues for unused parameters:

• that are annotated with @javax.enterprise.event.Observes
• in overrides and implementation methods
• in interface default methods
• in non-private methods that only throw or that have empty bodies
• in annotated methods, unless the annotation is @SuppressWarning("unchecked") or @SuppressWarning("rawtypes"), in which case the annotation will be ignored
• in overridable methods (non-final, or not member of a final class, non-static, non-private), if the parameter is documented with a proper javadoc.

@Override
void doSomething(int a, int b) {     // no issue reported on b
  compute(a);
}

public void foo(String s) {
  // designed to be extended but noop in standard case
}

protected void bar(String s) {
  //open-closed principle
}

public void qix(String s) {
  throw new UnsupportedOperationException("This method should be implemented in subclasses");
}

/**
 * @param s This string may be use for further computation in overriding classes
 */
protected void foobar(int a, String s) { // no issue, method is overridable and unused parameter has proper javadoc
  compute(a);
}

See

• CERT, MSC12-C. - Detect and remove code that has no effect or is never executed

    protected abstract void doActivate(ComponentContext context) throws Exception;

Close

Using such generic exceptions as Error, RuntimeException, Throwable, and Exception prevents calling methods from handling true, system-generated exceptions differently than application-generated errors.

Noncompliant Code Example

public void foo(String bar) throws Throwable {  // Noncompliant
  throw new RuntimeException("My Message");     // Noncompliant
}

Compliant Solution

public void foo(String bar) {
  throw new MyOwnRuntimeException("My Message");
}

Exceptions

Generic exceptions in the signatures of overriding methods are ignored, because overriding method has to follow signature of the throw declaration in the superclass. The issue will be raised on superclass declaration of the method (or won't be raised at all if superclass is not part of the analysis).

@Override
public void myMethod() throws Exception {...}

Generic exceptions are also ignored in the signatures of methods that make calls to methods that throw generic exceptions.

public void myOtherMethod throws Exception {
  doTheThing();  // this method throws Exception
}

See

• MITRE, CWE-397 - Declaration of Throws for Generic Exception
• CERT, ERR07-J. - Do not throw RuntimeException, Exception, or Throwable

    public AbstractAuthorizable(Map<String, Object> config) throws EnsureAuthorizableException {

Close

Abstract classes should not have public constructors. Constructors of abstract classes can only be called in constructors of their subclasses. So there is no point in making them public. The protected modifier should be enough.

Noncompliant Code Example

public abstract class AbstractClass1 {
    public AbstractClass1 () { // Noncompliant, has public modifier
        // do something here
    }
}

Compliant Solution

public abstract class AbstractClass2 {
    protected AbstractClass2 () {
        // do something here
    }
}

            resourceResolverField.set(any, resolver);

Close

This rule raises an issue when reflection is used to change the visibility of a class, method or field, and when it is used to directly update a field value. Altering or bypassing the accessibility of classes, methods, or fields violates the encapsulation principle and could lead to run-time errors.

Noncompliant Code Example

public void makeItPublic(String methodName) throws NoSuchMethodException {

  this.getClass().getMethod(methodName).setAccessible(true); // Noncompliant
}

public void setItAnyway(String fieldName, int value) {
  this.getClass().getDeclaredField(fieldName).setInt(this, value); // Noncompliant; bypasses controls in setter
}

See

• CERT, SEC05-J. - Do not use reflection to increase accessibility of classes, methods, or fields

    public GeneratedDialogWrapper(Class c, SlingScriptHelper slingHelper) {

Close

Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.

Noncompliant Code Example

List myList; // Noncompliant
Set mySet; // Noncompliant

Compliant Solution

List<String> myList;
Set<? extends Number> mySet;

            } catch (Throwable e) {

Close

Throwable is the superclass of all errors and exceptions in Java. Error is the superclass of all errors, which are not meant to be caught by applications.

Catching either Throwable or Error will also catch OutOfMemoryError and InternalError, from which an application should not attempt to recover.

Noncompliant Code Example

try { /* ... */ } catch (Throwable t) { /* ... */ }
try { /* ... */ } catch (Error e) { /* ... */ }

Compliant Solution

try { /* ... */ } catch (RuntimeException e) { /* ... */ }
try { /* ... */ } catch (MyException e) { /* ... */ }

See

• MITRE, CWE-396 - Declaration of Catch for Generic Exception
• C++ Core Guidelines E.14 - Use purpose-designed user-defined types as exceptions (not built-in types)

    public void handleEvent(Event event) {

Close

Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same.

Noncompliant Code Example

void doSomething(int a, int b) {     // "b" is unused
  compute(a);
}

Compliant Solution

void doSomething(int a) {
  compute(a);
}

Exceptions

The rule will not raise issues for unused parameters:

• that are annotated with @javax.enterprise.event.Observes
• in overrides and implementation methods
• in interface default methods
• in non-private methods that only throw or that have empty bodies
• in annotated methods, unless the annotation is @SuppressWarning("unchecked") or @SuppressWarning("rawtypes"), in which case the annotation will be ignored
• in overridable methods (non-final, or not member of a final class, non-static, non-private), if the parameter is documented with a proper javadoc.

@Override
void doSomething(int a, int b) {     // no issue reported on b
  compute(a);
}

public void foo(String s) {
  // designed to be extended but noop in standard case
}

protected void bar(String s) {
  //open-closed principle
}

public void qix(String s) {
  throw new UnsupportedOperationException("This method should be implemented in subclasses");
}

/**
 * @param s This string may be use for further computation in overriding classes
 */
protected void foobar(int a, String s) { // no issue, method is overridable and unused parameter has proper javadoc
  compute(a);
}

See

• CERT, MSC12-C. - Detect and remove code that has no effect or is never executed

        public static ObjectType fromClassAndName(Class<?> classOrGenericParam, String name) {

Close

Cognitive Complexity is a measure of how hard the control flow of a method is to understand. Methods with high Cognitive Complexity will be difficult to maintain.

See

• Cognitive Complexity

    private Map getJwtClaims() {

Close

Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime.

Noncompliant Code Example

List myList; // Noncompliant
Set mySet; // Noncompliant

Compliant Solution

List<String> myList;
Set<? extends Number> mySet;