SiLeBAT/FSK-Lab

View on GitHub

Showing 14,752 of 14,752 total issues

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

    if (flowVariables.containsKey("secondModelParameters.json")) {

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.

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

    public ParametricModel(KnimeTuple row, int level, Integer newTsID) throws PmmException {

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

Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.
Open

    public void savePortObjectSpec(final CombinedFskPortObjectSpec portObjectSpec,

There are several reasons for a method not to have a method body:

  • It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
  • It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
  • The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.
Open

    public void mouseEntered(MouseEvent e) {

There are several reasons for a method not to have a method body:

  • It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
  • It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
  • The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
Open

        MD, Model1, Model2

Shared coding conventions allow teams to collaborate efficiently. This rule checks that all constant names match a provided regular expression.

Noncompliant Code Example

With the default regular expression ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$:

public class MyClass {
  public static final int first = 1;
}

public enum MyEnum {
  first;
}

Compliant Solution

public class MyClass {
  public static final int FIRST = 1;
}

public enum MyEnum {
  FIRST;
}

Make "configListeners" transient or serializable.
Open

    private List<ConfigListener> configListeners;

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

Rename this class.
Open

public class Model2Schema extends de.bund.bfr.knime.pmm.common.pmmtablemodel.Model2Schema {

While it's perfectly legal to give a class the same simple name as a class in another package that it extends or interface it implements, it's confusing and could cause problems in the future.

Noncompliant Code Example

package my.mypackage;

public class Foo implements a.b.Foo { // Noncompliant

Compliant Solution

package my.mypackage;

public class FooJr implements a.b.Foo {

Rename field "function" to prevent any misunderstanding/clash with field "FUNCTION" defined on line 51.
Open

    private String function;

Looking at the set of methods in a class, including superclass methods, and finding two methods or fields that differ only by capitalization is confusing to users of the class. It is similarly confusing to have a method and a field which differ only in capitalization or a method and a field with exactly the same name and visibility.

In the case of methods, it may have been a mistake on the part of the original developer, who intended to override a superclass method, but instead added a new method with nearly the same name.

Otherwise, this situation simply indicates poor naming. Method names should be action-oriented, and thus contain a verb, which is unlikely in the case where both a method and a member have the same name (with or without capitalization differences). However, renaming a public method could be disruptive to callers. Therefore renaming the member is the recommended action.

Noncompliant Code Example

public class Car{

  public DriveTrain drive;

  public void tearDown(){...}

  public void drive() {...}  // Noncompliant; duplicates field name
}

public class MyCar extends Car{
  public void teardown(){...}  // Noncompliant; not an override. It it really what's intended?

  public void drivefast(){...}

  public void driveFast(){...} //Huh?
}

Compliant Solution

public class Car{

  private DriveTrain drive;

  public void tearDown(){...}

  public void drive() {...}  // field visibility reduced
}

public class MyCar extends Car{
  @Override
  public void tearDown(){...}

  public void drivefast(){...}

  public void driveReallyFast(){...}

}

Replace this call to "replaceAll()" by a call to the "replace()" method.
Open

        this.formula = formula.replaceAll("~", "=");

The underlying implementation of String::replaceAll calls the java.util.regex.Pattern.compile() method each time it is called even if the first argument is not a regular expression. This has a significant performance cost and therefore should be used with care.

When String::replaceAll is used, the first argument should be a real regular expression. If it’s not the case, String::replace does exactly the same thing as String::replaceAll without the performance drawback of the regex.

This rule raises an issue for each String::replaceAll used with a String as first parameter which doesn’t contains special regex character or pattern.

Noncompliant Code Example

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("Bob is", "It's"); // Noncompliant
changed = changed.replaceAll("\\.\\.\\.", ";"); // Noncompliant

Compliant Solution

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replace("Bob is", "It's");
changed = changed.replace("...", ";");

Or, with a regex:

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("\\w*\\sis", "It's");
changed = changed.replaceAll("\\.{3}", ";");

See

  • {rule:java:S4248} - Regex patterns should not be created needlessly

Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation.
Open

    public void mouseClicked(MouseEvent e) {

There are several reasons for a method not to have a method body:

  • It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
  • It is not yet, or never will be, supported. In this case an UnsupportedOperationException should be thrown.
  • The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.

Noncompliant Code Example

public void doSomething() {
}

public void doSomethingElse() {
}

Compliant Solution

@Override
public void doSomething() {
  // Do nothing because of X and Y.
}

@Override
public void doSomethingElse() {
  throw new UnsupportedOperationException();
}

Exceptions

Default (no-argument) constructors are ignored when there are other constructors in the class, as are empty methods in abstract classes.

public abstract class Animal {
  void speak() {  // default implementation ignored
  }
}

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

    public boolean isPlotable() {

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

Make "listeners" transient or serializable.
Open

    private List<SelectionListener> listeners;

Fields in a Serializable class must themselves be either Serializable or transient even if the class is never explicitly serialized or deserialized. For instance, under load, most J2EE application frameworks flush objects to disk, and an allegedly Serializable object with non-transient, non-serializable data members could cause program crashes, and open the door to attackers. In general a Serializable class is expected to fulfil its contract and not have an unexpected behaviour when an instance is serialized.

This rule raises an issue on non-Serializable fields, and on collection fields when they are not private (because they could be assigned non-Serializable values externally), and when they are assigned non-Serializable types within the class.

Noncompliant Code Example

public class Address {
  //...
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;  // Noncompliant; Address isn't serializable
}

Compliant Solution

public class Address implements Serializable {
  private static final long serialVersionUID = 2405172041950251807L;
}

public class Person implements Serializable {
  private static final long serialVersionUID = 1905122041950251207L;

  private String name;
  private Address address;
}

Exceptions

The alternative to making all members serializable or transient is to implement special methods which take on the responsibility of properly serializing and de-serializing the object. This rule ignores classes which implement the following methods:

 private void writeObject(java.io.ObjectOutputStream out)
     throws IOException
 private void readObject(java.io.ObjectInputStream in)
     throws IOException, ClassNotFoundException;

See

Use already-defined constant 'WORKSPACE' instead of duplicating its value here.
Open

      Path workspacePath = FileUtil.createTempFile("workspace", ".r").toPath();

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.

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

    public void saveFSKPortObject(FskPortObject portObject, final PortObjectZipOutputStream out,

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

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

    public FskPortObject loadFSKPortObject(PortObjectZipInputStream in, PortObjectSpec spec,

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

Use try-with-resources or close this "ObjectOutputStream" in a "finally" clause.
Open

          final ObjectOutputStream oos = new ObjectOutputStream(out);

Connections, streams, files, and other classes that implement the Closeable interface or its super-interface, AutoCloseable, needs to be closed after use. Further, that close call must be made in a finally block otherwise an exception could keep the call from being made. Preferably, when class implements AutoCloseable, resource should be created using "try-with-resources" pattern and will be closed automatically.

Failure to properly close resources will result in a resource leak which could bring first the application and then perhaps the box the application is on to their knees.

Noncompliant Code Example

private void readTheFile() throws IOException {
  Path path = Paths.get(this.fileName);
  BufferedReader reader = Files.newBufferedReader(path, this.charset);
  // ...
  reader.close();  // Noncompliant
  // ...
  Files.lines("input.txt").forEach(System.out::println); // Noncompliant: The stream needs to be closed
}

private void doSomething() {
  OutputStream stream = null;
  try {
    for (String property : propertyList) {
      stream = new FileOutputStream("myfile.txt");  // Noncompliant
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();  // Multiple streams were opened. Only the last is closed.
  }
}

Compliant Solution

private void readTheFile(String fileName) throws IOException {
    Path path = Paths.get(fileName);
    try (BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8)) {
      reader.readLine();
      // ...
    }
    // ..
    try (Stream<String> input = Files.lines("input.txt"))  {
      input.forEach(System.out::println);
    }
}

private void doSomething() {
  OutputStream stream = null;
  try {
    stream = new FileOutputStream("myfile.txt");
    for (String property : propertyList) {
      // ...
    }
  } catch (Exception e) {
    // ...
  } finally {
    stream.close();
  }
}

Exceptions

Instances of the following classes are ignored by this rule because close has no effect:

  • java.io.ByteArrayOutputStream
  • java.io.ByteArrayInputStream
  • java.io.CharArrayReader
  • java.io.CharArrayWriter
  • java.io.StringReader
  • java.io.StringWriter

Java 7 introduced the try-with-resources statement, which implicitly closes Closeables. All resources opened in a try-with-resources statement are ignored by this rule.

try (BufferedReader br = new BufferedReader(new FileReader(fileName))) {
  //...
}
catch ( ... ) {
  //...
}

See

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

    public FskPortObject loadPortObject(PortObjectZipInputStream in, PortObjectSpec spec,

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

Remove this throw statement from this finally block.
Open

          throw new ModelScriptException(getStdErr());  

Using return, break, throw, and so on from a finally block suppresses the propagation of any unhandled Throwable which was thrown in the try or catch block.

This rule raises an issue when a jump statement (break, continue, return, throw, and goto) would force control flow to leave a finally block.

Noncompliant Code Example

public static void main(String[] args) {
  try {
    doSomethingWhichThrowsException();
    System.out.println("OK");   // incorrect "OK" message is printed
  } catch (RuntimeException e) {
    System.out.println("ERROR");  // this message is not shown
  }
}

public static void doSomethingWhichThrowsException() {
  try {
    throw new RuntimeException();
  } finally {
    for (int i = 0; i < 10; i ++) {
      //...
      if (q == i) {
        break; // ignored
      }
    }

    /* ... */
    return;      // Noncompliant - prevents the RuntimeException from being propagated
  }
}

Compliant Solution

public static void main(String[] args) {
  try {
    doSomethingWhichThrowsException();
    System.out.println("OK");
  } catch (RuntimeException e) {
    System.out.println("ERROR");  // "ERROR" is printed as expected
  }
}

public static void doSomethingWhichThrowsException() {
  try {
    throw new RuntimeException();
  } finally {
    for (int i = 0; i < 10; i ++) {
      //...
      if (q == i) {
        break; // ignored
      }
    }

    /* ... */
  }
}

See

Replace this call to "replaceAll()" by a call to the "replace()" method.
Open

    paramScript = paramScript.replaceAll("<-", "=");

The underlying implementation of String::replaceAll calls the java.util.regex.Pattern.compile() method each time it is called even if the first argument is not a regular expression. This has a significant performance cost and therefore should be used with care.

When String::replaceAll is used, the first argument should be a real regular expression. If it’s not the case, String::replace does exactly the same thing as String::replaceAll without the performance drawback of the regex.

This rule raises an issue for each String::replaceAll used with a String as first parameter which doesn’t contains special regex character or pattern.

Noncompliant Code Example

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("Bob is", "It's"); // Noncompliant
changed = changed.replaceAll("\\.\\.\\.", ";"); // Noncompliant

Compliant Solution

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replace("Bob is", "It's");
changed = changed.replace("...", ";");

Or, with a regex:

String init = "Bob is a Bird... Bob is a Plane... Bob is Superman!";
String changed = init.replaceAll("\\w*\\sis", "It's");
changed = changed.replaceAll("\\.{3}", ";");

See

  • {rule:java:S4248} - Regex patterns should not be created needlessly

Rename this constant name to match the regular expression '^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$'.
Open

        MD, Model1, Model2

Shared coding conventions allow teams to collaborate efficiently. This rule checks that all constant names match a provided regular expression.

Noncompliant Code Example

With the default regular expression ^[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$:

public class MyClass {
  public static final int first = 1;
}

public enum MyEnum {
  first;
}

Compliant Solution

public class MyClass {
  public static final int FIRST = 1;
}

public enum MyEnum {
  FIRST;
}
Severity
Category
Status
Source
Language