Adobe-Consulting-Services/acs-aem-commons

View on GitHub

Showing 1,632 of 1,632 total issues

Remove this unused method parameter "session".
Open

    public void replicate(Session session, ReplicationActionType actionType, String path, ReplicationOptions replicationOptions) throws ReplicationException {

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

Remove this unused "WCM_AUTHORING_MODE_COOKIE" private field.
Open

    private static final String WCM_AUTHORING_MODE_COOKIE = "cq-authoring-mode";

If a private field is declared but not used in the program, it can be considered dead code and should therefore be removed. This will improve maintainability because developers will not wonder what the variable is used for.

Note that this rule does not take reflection into account, which means that issues will be raised on private fields that are only accessed using the reflection API.

Noncompliant Code Example

public class MyClass {
  private int foo = 42;

  public int compute(int a) {
    return a * 42;
  }

}

Compliant Solution

public class MyClass {
  public int compute(int a) {
    return a * 42;
  }
}

Exceptions

The Java serialization runtime associates with each serializable class a version number, called serialVersionUID, which is used during deserialization to verify that the sender and receiver of a serialized object have loaded classes for that object that are compatible with respect to serialization.

A serializable class can declare its own serialVersionUID explicitly by declaring a field named serialVersionUID that must be static, final, and of type long. By definition those serialVersionUID fields should not be reported by this rule:

public class MyClass implements java.io.Serializable {
  private static final long serialVersionUID = 42L;
}

Moreover, this rule doesn't raise any issue on annotated fields.

Remove this unused method parameter "requestToken".
Open

        public Token getAccessToken(Token requestToken, Verifier verifier) {

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

Provide the parametrized type for this generic.
Open

        Map<String, ? extends Object> tmp = new HashMap(newData);

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;

Provide the parametrized type for this generic.
Open

        final Collection<Resource> sortedCollection = new TreeSet(new WorkflowInstanceFolderComparator());

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;

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed.
Open

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

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

Provide the parametrized type for this generic.
Open

            Class injectionType = null;

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;

Remove this unused method parameter "properties".
Open

    protected void unbindNodeSorter(NodeSorter sorter, Map<String, Object> properties){

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

This branch's code block is the same as the block for the branch on line 125.
Open

        } else if (StringUtils.endsWith(request.getHeader("Referer"), "/editor.html" + request.getRequestURI())) {
            // Do not apply to pages loaded in the TouchUI editor.html
            return false;
        } else if (StringUtils.endsWith(request.getHeader("Referer"), "/cf")) {

Having two cases in a switch statement or two branches in an if chain with the same implementation is at best duplicate code, and at worst a coding error. If the same logic is truly needed for both instances, then in an if chain they should be combined, or for a switch, one should fall through to the other.

Noncompliant Code Example

switch (i) {
  case 1:
    doFirstThing();
    doSomething();
    break;
  case 2:
    doSomethingDifferent();
    break;
  case 3:  // Noncompliant; duplicates case 1's implementation
    doFirstThing();
    doSomething();
    break;
  default:
    doTheRest();
}

if (a >= 0 && a < 10) {
  doFirstThing();
  doTheThing();
}
else if (a >= 10 && a < 20) {
  doTheOtherThing();
}
else if (a >= 20 && a < 50) {
  doFirstThing();
  doTheThing();  // Noncompliant; duplicates first condition
}
else {
  doTheRest();
}

Exceptions

Blocks in an if chain that contain a single line of code are ignored, as are blocks in a switch statement that contain a single line of code with or without a following break.

if (a == 1) {
  doSomething();  //no issue, usually this is done on purpose to increase the readability
} else if (a == 2) {
  doSomethingElse();
} else {
  doSomething();
}

But this exception does not apply to if chains without else-s, or to switch-es without default clauses when all branches have the same single line of code. In case of if chains with else-s, or of switch-es with default clauses, rule {rule:java:S3923} raises a bug.

if (a == 1) {
  doSomething();  //Noncompliant, this might have been done on purpose but probably not
} else if (a == 2) {
  doSomething();
}

Provide the parametrized type for this generic.
Open

        List convertedValues = new ArrayList();

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;

"NullPointerException" will be thrown when invoking method "getPackageDescription()".
Open

                    String packageDescription = getPackageDescription(definition);

A reference to null should never be dereferenced/accessed. Doing so will cause a NullPointerException to be thrown. At best, such an exception will cause abrupt program termination. At worst, it could expose debugging information that would be useful to an attacker, or it could allow an attacker to bypass security measures.

Note that when they are present, this rule takes advantage of @CheckForNull and @Nonnull annotations defined in JSR-305 to understand which values are and are not nullable except when @Nonnull is used on the parameter to equals, which by contract should always work with null.

Noncompliant Code Example

@CheckForNull
String getName(){...}

public boolean isNameEmpty() {
  return getName().length() == 0; // Noncompliant; the result of getName() could be null, but isn't null-checked
}
Connection conn = null;
Statement stmt = null;
try{
  conn = DriverManager.getConnection(DB_URL,USER,PASS);
  stmt = conn.createStatement();
  // ...

}catch(Exception e){
  e.printStackTrace();
}finally{
  stmt.close();   // Noncompliant; stmt could be null if an exception was thrown in the try{} block
  conn.close();  // Noncompliant; conn could be null if an exception was thrown
}
private void merge(@Nonnull Color firstColor, @Nonnull Color secondColor){...}

public  void append(@CheckForNull Color color) {
    merge(currentColor, color);  // Noncompliant; color should be null-checked because merge(...) doesn't accept nullable parameters
}
void paint(Color color) {
  if(color == null) {
    System.out.println("Unable to apply color " + color.toString());  // Noncompliant; NullPointerException will be thrown
    return;
  }
  ...
}

See

Provide the parametrized type for this generic.
Open

    public static Stream<AccessibleObject> getAllAnnotatedObjectMembers(Class source, Class<? extends Annotation> annotation) {

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;

Remove this unused method parameter "options".
Open

                                     ReplicationContentFactory factory, Map<String, Object> options)

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

Provide the parametrized type for this generic.
Open

    public static Optional<Object> createObject(Class c) {

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;

Define a constant instead of duplicating this literal " }" 3 times.
Open

            out.println("    }");

Duplicated string literals make the process of refactoring error-prone, since you must be sure to update all occurrences.

On the other hand, constants can be referenced from many places, but only need to be updated in a single place.

Noncompliant Code Example

With the default threshold of 3:

public void run() {
  prepare("action1");                              // Noncompliant - "action1" is duplicated 3 times
  execute("action1");
  release("action1");
}

@SuppressWarning("all")                            // Compliant - annotations are excluded
private void method1() { /* ... */ }
@SuppressWarning("all")
private void method2() { /* ... */ }

public String method3(String a) {
  System.out.println("'" + a + "'");               // Compliant - literal "'" has less than 5 characters and is excluded
  return "";                                       // Compliant - literal "" has less than 5 characters and is excluded
}

Compliant Solution

private static final String ACTION_1 = "action1";  // Compliant

public void run() {
  prepare(ACTION_1);                               // Compliant
  execute(ACTION_1);
  release(ACTION_1);
}

Exceptions

To prevent generating some false-positives, literals having less than 5 characters are excluded.

Rename this method to not match a restricted identifier.
Open

    protected void record(String path, ItemStatus status, long timeTaken) {

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

Define and throw a dedicated exception instead of using a generic one.
Open

    void performCleanupActivity(ActionManager manager) throws Exception {

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

Remove this use of "Stream.peek".
Open

        folderList.stream().peek(folderList::remove).forEach(folder -> {

According to its JavaDocs, the intermediate Stream operation java.util.Stream.peek() “exists mainly to support debugging” purposes.

A key difference with other intermediate Stream operations is that the Stream implementation is free to skip calls to peek() for optimization purpose. This can lead to peek() being unexpectedly called only for some or none of the elements in the Stream.

As a consequence, relying on peek() without careful consideration can lead to error-prone code.

This rule raises an issue for each use of peek() to be sure that it is challenged and validated by the team to be meant for production debugging/logging purposes.

Noncompliant Code Example

Stream.of("one", "two", "three", "four")
         .filter(e -> e.length() > 3)
         .peek(e -> System.out.println("Filtered value: " + e)); // Noncompliant

Compliant Solution

Stream.of("one", "two", "three", "four")
         .filter(e -> e.length() > 3)
         .foreach(e -> System.out.println("Filtered value: " + e));

See

Change the visibility of this constructor to "protected".
Open

    public AssetIngestor(MimeTypeService mimeTypeService) {

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
    }
}

Provide the parametrized type for this generic.
Open

    private static final Collection<Class> IGNORED_CLASSES = Arrays.asList(

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;
Severity
Category
Status
Source
Language