Showing 105 of 110 total issues
Make the enclosing method "static" or remove this set. Open
plugin = this;
- Read upRead up
- Exclude checks
Correctly updating a static
field from a non-static method is tricky to get right and could easily lead to bugs if there are multiple
class instances and/or multiple threads in play. Ideally, static
fields are only updated from synchronized static
methods.
This rule raises an issue each time a static
field is updated from a non-static method.
Noncompliant Code Example
public class MyClass { private static int count = 0; public void doSomething() { //... count++; // Noncompliant } }
Add the missing @Deprecated annotation. Open
public void addPlayer(OfflinePlayer offlinePlayer) {
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Add the missing @deprecated Javadoc tag. Open
public boolean addPotionEffect(PotionEffect effect, boolean force)
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Change the visibility of this constructor to "protected". Open
public InventoryMock(InventoryHolder holder, int size, InventoryType type)
- Read upRead up
- Exclude checks
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 } }
Define and throw a dedicated exception instead of using a generic one. Open
throw new RuntimeException(e);
- Read upRead up
- Exclude checks
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
Rename "commands" which hides the field declared at line 61. Open
Map<String, Map<String, Object>> commands = plugin.getDescription().getCommands();
- Read upRead up
- Exclude checks
Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.
Noncompliant Code Example
class Foo { public int myField; public void doSomething() { int myField = 0; ... } }
See
- CERT, DCL01-C. - Do not reuse variable names in subscopes
- CERT, DCL51-J. - Do not shadow or obscure identifiers in subscopes
This branch's code block is the same as the block for the branch on line 632. Open
permissions.add(permission);
- Read upRead up
- Exclude checks
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(); }
Rename "permissions" which hides the field declared at line 67. Open
Set<String> permissions = entry.getValue();
- Read upRead up
- Exclude checks
Overriding or shadowing a variable declared in an outer scope can strongly impact the readability, and therefore the maintainability, of a piece of code. Further, it could lead maintainers to introduce bugs because they think they're using one variable but are really using another.
Noncompliant Code Example
class Foo { public int myField; public void doSomething() { int myField = 0; ... } }
See
- CERT, DCL01-C. - Do not reuse variable names in subscopes
- CERT, DCL51-J. - Do not shadow or obscure identifiers in subscopes
Add a nested comment explaining why this method is empty, throw an UnsupportedOperationException or complete the implementation. Open
public void recalculatePermissionDefaults(Permission perm)
- Read upRead up
- Exclude checks
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 the missing @deprecated Javadoc tag. Open
public double getDurationModifier()
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Save and re-use this "Random". Open
Random random = new Random();
- Read upRead up
- Exclude checks
Creating a new Random
object each time a random value is needed is inefficient and may produce numbers which are not random depending
on the JDK. For better efficiency and randomness, create a single Random
, then store, and reuse it.
The Random()
constructor tries to set the seed with a distinct value every time. However there is no guarantee that the seed will be
random or even uniformly distributed. Some JDK will use the current time as seed, which makes the generated numbers not random at all.
This rule finds cases where a new Random
is created each time a method is invoked and assigned to a local random variable.
Noncompliant Code Example
public void doSomethingCommon() { Random rand = new Random(); // Noncompliant; new instance created with each invocation int rValue = rand.nextInt(); //...
Compliant Solution
private Random rand = SecureRandom.getInstanceStrong(); // SecureRandom is preferred to Random public void doSomethingCommon() { int rValue = this.rand.nextInt(); //...
Exceptions
A class which uses a Random
in its constructor or in a static main
function and nowhere else will be ignored by this
rule.
See
- OWASP Top 10 2017 Category A6 - Security Misconfiguration
String contains no format specifiers. Open
String.format("Could not find file plugin.yml. Maybe forgot to add the 'main' property?"));
- Read upRead up
- Exclude checks
Because printf
-style format strings are interpreted at runtime, rather than validated by the compiler, they can contain errors that
result in the wrong strings being created. This rule statically validates the correlation of printf
-style format strings to their
arguments when calling the format(...)
methods of java.util.Formatter
, java.lang.String
,
java.io.PrintStream
, MessageFormat
, and java.io.PrintWriter
classes and the printf(...)
methods of
java.io.PrintStream
or java.io.PrintWriter
classes.
Noncompliant Code Example
String.format("First {0} and then {1}", "foo", "bar"); //Noncompliant. Looks like there is a confusion with the use of {{java.text.MessageFormat}}, parameters "foo" and "bar" will be simply ignored here String.format("Display %3$d and then %d", 1, 2, 3); //Noncompliant; the second argument '2' is unused String.format("Too many arguments %d and %d", 1, 2, 3); //Noncompliant; the third argument '3' is unused String.format("First Line\n"); //Noncompliant; %n should be used in place of \n to produce the platform-specific line separator String.format("Is myObject null ? %b", myObject); //Noncompliant; when a non-boolean argument is formatted with %b, it prints true for any nonnull value, and false for null. Even if intended, this is misleading. It's better to directly inject the boolean value (myObject == null in this case) String.format("value is " + value); // Noncompliant String s = String.format("string without arguments"); // Noncompliant MessageFormat.format("Result '{0}'.", value); // Noncompliant; String contains no format specifiers. (quote are discarding format specifiers) MessageFormat.format("Result {0}.", value, value); // Noncompliant; 2nd argument is not used MessageFormat.format("Result {0}.", myObject.toString()); // Noncompliant; no need to call toString() on objects java.util.Logger logger; logger.log(java.util.logging.Level.SEVERE, "Result {0}.", myObject.toString()); // Noncompliant; no need to call toString() on objects logger.log(java.util.logging.Level.SEVERE, "Result.", new Exception()); // compliant, parameter is an exception logger.log(java.util.logging.Level.SEVERE, "Result '{0}'", 14); // Noncompliant - String contains no format specifiers. logger.log(java.util.logging.Level.SEVERE, "Result " + param, exception); // Noncompliant; Lambda should be used to differ string concatenation. org.slf4j.Logger slf4jLog; org.slf4j.Marker marker; slf4jLog.debug(marker, "message {}"); slf4jLog.debug(marker, "message", 1); // Noncompliant - String contains no format specifiers. org.apache.logging.log4j.Logger log4jLog; log4jLog.debug("message", 1); // Noncompliant - String contains no format specifiers.
Compliant Solution
String.format("First %s and then %s", "foo", "bar"); String.format("Display %2$d and then %d", 1, 3); String.format("Too many arguments %d %d", 1, 2); String.format("First Line%n"); String.format("Is myObject null ? %b", myObject == null); String.format("value is %d", value); String s = "string without arguments"; MessageFormat.format("Result {0}.", value); MessageFormat.format("Result '{0}' = {0}", value); MessageFormat.format("Result {0}.", myObject); java.util.Logger logger; logger.log(java.util.logging.Level.SEVERE, "Result {0}.", myObject); logger.log(java.util.logging.Level.SEVERE, "Result {0}'", 14); logger.log(java.util.logging.Level.SEVERE, exception, () -> "Result " + param); org.slf4j.Logger slf4jLog; org.slf4j.Marker marker; slf4jLog.debug(marker, "message {}"); slf4jLog.debug(marker, "message {}", 1); org.apache.logging.log4j.Logger log4jLog; log4jLog.debug("message {}", 1);
See
- CERT, FIO47-C. - Use valid format strings
Add the missing @deprecated Javadoc tag. Open
public void setRawData(byte data)
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Add the missing @deprecated Javadoc tag. Open
public boolean refreshChunk(int x, int z)
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Add the missing @deprecated Javadoc tag. Open
public MaterialData getData()
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Change the visibility of this constructor to "protected". Open
public MobMock(ServerMock server, UUID uuid)
- Read upRead up
- Exclude checks
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 } }
Add the missing @deprecated Javadoc tag. Open
public <T extends Entity> Collection<T> getEntitiesByClass(Class<T>... classes)
- Read upRead up
- Exclude checks
Deprecation should be marked with both the @Deprecated
annotation and @deprecated Javadoc tag. The annotation enables tools such as
IDEs to warn about referencing deprecated elements, and the tag can be used to explain when it was deprecated, why, and how references should be
refactored.
Further, Java 9 adds two additional arguments to the annotation:
-
since
allows you to describe when the deprecation took place -
forRemoval
, indicates whether the deprecated element will be removed at some future date
If your compile level is Java 9 or higher, you should be using one or both of these arguments.
Noncompliant Code Example
class MyClass { @Deprecated public void foo1() { } /** * @deprecated */ public void foo2() { // Noncompliant } }
Compliant Solution
class MyClass { /** * @deprecated (when, why, refactoring advice...) */ @Deprecated public void foo1() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="5.1") public void foo2() { } /** * Java >= 9 * @deprecated (when, why, refactoring advice...) */ @Deprecated(since="4.2", forRemoval=true) public void foo3() { } }
Exceptions
The members and methods of a deprecated class or interface are ignored by this rule. The classes and interfaces themselves are still subject to it.
/** * @deprecated (when, why, etc...) */ @Deprecated class Qix { public void foo() {} // Compliant; class is deprecated } /** * @deprecated (when, why, etc...) */ @Deprecated interface Plop { void bar(); }
Change the visibility of this constructor to "protected". Open
public LivingEntityMock(ServerMock server, UUID uuid)
- Read upRead up
- Exclude checks
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 } }
Change the visibility of this constructor to "protected". Open
public EntityMock(@NotNull ServerMock server, @NotNull UUID uuid)
- Read upRead up
- Exclude checks
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 } }
Define and throw a dedicated exception instead of using a generic one. Open
throw new RuntimeException(e);
- Read upRead up
- Exclude checks
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