Sundarrajk's Weblog

Archive for the ‘Static Code Checks’ Category

In addition to all the Rule validation done by PMD, PMD also checks for copy paste occurrences in the code. This is by far one of the most important checks that PMD does. More the occurrences of Copy-Paste more the likelihood of errors in the code. More the occurrences of Copy Paste the more the possibility of creating errors. One would change in one place and forget in another. Also it leads to code bloat and inefficient memory usage.

One should pay close attention to these results and read Refactoring by Martin Fowler to solve the copy paste problems.

This is the twenty fourth installment of explanation of PMD rules covering some Android rules.

Android Rules

These rules deal with the Android SDK, mostly related to best practices. To get better results, make sure that the auxclasspath is defined for type resolution to work.

CallSuperFirst

Super should be called at the start of the method

Example

public class DummyActivity extends Activity {
    public void onCreate(Bundle bundle) {
     // missing call to super.onCreate(bundle)
     foo();
    }
   }

CallSuperLast

Super should be called at the end of the method

Example

public class DummyActivity extends Activity {
    public void onPause() {
     foo();
     // missing call to super.onPause()
    }
   }

ProtectLogD

Log.d calls should be protected by checking Config.LOGD first

Example

public class DummyActivity extends Activity {
    public void foo() {
     Log.d("TAG", "msg1"); // Bad
 
     bar();
 
     if (Config.LOGD) Log.d("TAG", "msg1"); // Good
    }
   }

ProtectLogV

Log.v calls should be protected by checking Config.LOGV first

Example

public class DummyActivity extends Activity {
    public void foo() {
     Log.v("TAG", "msg1"); // Bad
 
     bar();
 
     if (Config.LOGV) Log.v("TAG", "msg1"); // Good
    }
This is the twenty third installment of explanation of PMD rules covering rules related to JSP and JSF

Basic JSF rules

Rules concerning basic JSF guidelines.

DontNestJsfInJstlIteration

Do not nest JSF component custom actions inside a custom action that iterates over its body.

Example

                           
                                       
  •                        

Basic JSP rules

Rules concerning basic JSP guidelines.

NoLongScripts

Scripts should be part of Tag Libraries, rather than part of JSP pages.

Example

<!–

function calcDays(){

  var date1 = document.getElementById(‘d1’).lastChild.data;

  var date2 = document.getElementById(‘d2’).lastChild.data;

  date1 = date1.split(“-“);

  date2 = date2.split(“-“);

  var sDate = new Date(date1[0]+”/”+date1[1]+”/”+date1[2]);

  var eDate = new Date(date2[0]+”/”+date2[1]+”/”+date2[2]);

  var daysApart = Math.abs(Math.round((sDate-eDate)/86400000));

  document.getElementById(‘diffDays’).lastChild.data = daysApart;

}

 

onload=calcDays;

//–>

NoScriptlets

Scriptlets should be factored into Tag Libraries or JSP declarations, rather than being part of JSP pages.

Example

<%
response.setHeader(“Pragma”, “No-cache”);
%>
           
                        String title = “Hello world!”;
           

NoInlineStyleInformation

Style information should be put in CSS files, not in JSPs. Therefore, don’t use or tags, or attributes like “align=’center'”.

Example

text

NoClassAttribute

Do not use an attribute called ‘class’. Use “styleclass” for CSS styles.

Example

 
Some text
 

NoJspForward

Do not do a forward from within a JSP file.

Example

IframeMissingSrcAttribute

IFrames which are missing a src element can cause security information popups in IE if you are accessing the page through SSL. See http://support.microsoft.com/default.aspx?scid=kb;EN-US;Q261188

Example

bad example>

 

good example>

http://foo

NoHtmlComments

In a production system, HTML comments increase the payload between the application server to the client, and serve little other purpose. Consider switching to JSP comments.

Example

bad example>

 

good example>

DuplicateJspImports

Avoid duplicate import statements inside JSP’s.

Example

<img src=\"/foo\”>xxtext

JspEncoding

A missing ‘meta’ tag or page directive will trigger this rule, as well as a non-UTF-8 charset.

Example

Most browsers should be able to interpret the following headers:

               

               

                   

                

This is the twenty second installment of explanation of PMD rules covering Cloning of objects.

Clone Implementation Rules

The Clone Implementation ruleset contains a collection of rules that find questionable usages of the clone() method.
Clone is a method used to create a copy of objects in Java. The resulting object must be an exact replica of the object from which it was cloned. It is important to note that in the process of cloning if we use a statement like
this.attribute = classcloned.attribute
will only achieve a shallow copy as only the reference of the object attribute will be copied into the cloned class. Any changes to attribute in the clone will impact the original object. Instead we should use
this.attribute = classcloned.attribute,clone()
This will ensure that the cloned object gets a cloned copy of the attribute object too.

ProperCloneImplementation

Object clone() should be implemented with super.clone().

Example

class Foo{
    public Object clone(){
        return new Foo(); // This is bad
    }
}

CloneThrowsCloneNotSupportedException

The method clone() should throw a CloneNotSupportedException.

Example

public class MyClass implements Cloneable{
     public Object clone() { // will cause an error
          MyClass clone = (MyClass)super.clone();
          return clone;
     }
}

CloneMethodMustImplementCloneable

The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException.

Example

public class MyClass {
public Object clone() throws CloneNotSupportedException {
  return foo;
}
}
This is the twenty first installment of explanation of PMD rules covering J2EE Rules.

J2EE Rules

These are rules for J2EE

UseProperClassLoader

In J2EE getClassLoader() might not work as expected. Use Thread.currentThread().getContextClassLoader() instead.

Example

public class Foo {
ClassLoader cl = Bar.class.getClassLoader();
}

MDBAndSessionBeanNamingConvention

The EJB Specification state that any MessageDrivenBean or SessionBean should be suffixed by Bean.

Example

/* Proper name */
public class SomeBean implements SessionBean{}
/* Bad name */
public class MissingTheProperSuffix implements SessionBean {}

RemoteSessionInterfaceNamingConvention

Remote Home interface of a Session EJB should be suffixed by ‘Home’.

Example

/* Proper name */
public interface MyBeautifulHome extends javax.ejb.EJBHome {}
/* Bad name */
public interface MissingProperSuffix extends javax.ejb.EJBHome {}

LocalInterfaceSessionNamingConvention

The Local Interface of a Session EJB should be suffixed by ‘Local’.

Example

/* Proper name */
public interface MyLocal extends javax.ejb.EJBLocalObject {}
/* Bad name */
public interface MissingProperSuffix extends javax.ejb.EJBLocalObject {}

LocalHomeNamingConvention

The Local Home interface of a Session EJB should be suffixed by ‘LocalHome’.

Example

/* Proper name */
public interface MyBeautifulLocalHome extends javax.ejb.EJBLocalHome {}
/* Bad name */
public interface MissingProperSuffix extends javax.ejb.EJBLocalHome {}

RemoteInterfaceNamingConvention

Remote Interface of a Session EJB should NOT be suffixed.

Example

/* Bad Session suffix */
public interface BadSuffixSession extends javax.ejb.EJBObject {}
/* Bad EJB suffix */
public interface BadSuffixEJB extends javax.ejb.EJBObject {}
/* Bad Bean suffix */
public interface BadSuffixBean extends javax.ejb.EJBObject {}

DoNotCallSystemExit

Web applications should not call System.exit(), since only the web container or the application server should stop the JVM.

Example

public class Foo {
    public void bar() {
        // NEVER DO THIS IN A APP SERVER !!!
        System.exit(0);
    }
}

StaticEJBFieldShouldBeFinal

According to the J2EE specification (p.494), an EJB should not have any static fields with write access. However, static read only fields are allowed. This ensures proper behavior especially when instances are distributed by the container on several JREs.

Example

public class SomeEJB extends EJBObject implements EJBLocalHome {
        private static int BAD_STATIC_FIELD;
 
        private static final int GOOD_STATIC_FIELD;
}

DoNotUseThreads

The J2EE specification explicitly forbid use of threads.

Example

// This is not allowed
public class UsingThread extends Thread {
 
}
// Neither this,
public class OtherThread implements Runnable {
        // Nor this ...
        public void methode() {
                Runnable thread = new Thread(); thread.run();
        }
}
This is the twentieth installment of explanation of PMD rules covering Unused Code Checks.

Unused Code Rules

The Unused Code Ruleset contains a collection of rules that find unused code.

UnusedPrivateField

Detects when a private field is declared and/or assigned a value, but not used.

Example

public class Something {
  private static int FOO = 2; // Unused
  private int i = 5; // Unused
  private int j = 6;
  public int addOne() {
    return j++;
  }
}

UnusedLocalVariable

Detects when a local variable is declared and/or assigned, but not used.

Example

public class Foo {
public void doSomething() {
  int i = 5; // Unused
}
}

UnusedPrivateMethod

Unused Private Method detects when a private method is declared but is unused.

Example

public class Something {
private void foo() {} // unused
}

UnusedFormalParameter

Avoid passing parameters to methods or constructors and then not using those parameters.

Example

public class Foo {
private void bar(String howdy) {
  // howdy is not used
}
This is the nineteenth installment of explanation of PMD rules covering Type Resolution Rules.

Type Resolution Rules

These are rules which resolve java Class files for comparisson, as opposed to a String

LooseCoupling

Avoid using implementation types (i.e., HashSet); use the interface (i.e, Set) instead

Example

import java.util.ArrayList;
import java.util.HashSet;
public class Bar {
// Use List instead
private ArrayList list = new ArrayList();
// Use Set instead
public HashSet getFoo() {
  return new HashSet();
}
}

CloneMethodMustImplementCloneable

The method clone() should only be implemented if the class implements the Cloneable interface with the exception of a final method that only throws CloneNotSupportedException. This version uses PMD’s type resolution facilities, and can detect if the class implements or extends a Cloneable class

Example

public class MyClass {
public Object clone() throws CloneNotSupportedException {
  return foo;
}
}

UnusedImports

Avoid unused import statements. This rule will find unused on demand imports, i.e. import com.foo.*.

Example

// this is bad
import java.io.*;
public class Foo {}

SignatureDeclareThrowsException

It is unclear which exceptions that can be thrown from the methods. It might be difficult to document and understand the vague interfaces. Use either a class derived from RuntimeException or a checked exception. Junit classes are excluded.

Example

public void methodThrowingException() throws Exception {
}
This is the eighteenth installment of explanation of PMD rules covering some Security Guidelines.

Security Code Guidelines

These rules check the security guidelines from Sun, published at http://java.sun.com/security/seccodeguide.html#gcg

MethodReturnsInternalArray

Exposing internal arrays directly allows the user to modify some code that could be critical. It is safer to return a copy of the array.

Example

public class SecureSystem {
  UserData [] ud;
  public UserData [] getUserData() {
      // Don't return directly the internal array, return a copy
      return ud;
  }
}

ArrayIsStoredDirectly

Constructors and methods receiving arrays should clone objects and store the copy. This prevents that future changes from the user affect the internal functionality.

Example

public class Foo {
private String [] x;
  public void foo (String [] param) {
      // Don't do this, make a copy of the array at least
      this.x=param;
  }
}
This is the seventeenth installment giving details of PMD rules covering String and StringBuffer rules.

String and StringBuffer Rules

These rules deal with different problems that can occur with manipulation of the class String or StringBuffer.

AvoidDuplicateLiterals

Code containing duplicate String literals can usually be improved by declaring the String as a constant field.

Example

public class Foo {
private void bar() {
    buz("Howdy");
    buz("Howdy");
    buz("Howdy");
    buz("Howdy");
}
private void buz(String x) {}
}

StringInstantiation

Avoid instantiating String objects; this is usually unnecessary.

Example

public class Foo {
private String bar = new String("bar"); // just do a String bar = "bar";
}

StringToString

Avoid calling toString() on String objects; this is unnecessary.

Example

public class Foo {
private String baz() {
  String bar = "howdy";
  return bar.toString();
}
}

InefficientStringBuffering

Avoid concatenating non literals in a StringBuffer constructor or append().

Example

public class Foo {
void bar() {
  // Avoid this
  StringBuffer sb=new StringBuffer("tmp = "+System.getProperty("java.io.tmpdir"));
  // use instead something like this
  StringBuffer sb = new StringBuffer("tmp = ");
  sb.append(System.getProperty("java.io.tmpdir"));
}
}

UnnecessaryCaseChange

Using equalsIgnoreCase() is faster than using toUpperCase/toLowerCase().equals()

Example

public class Foo {
  public boolean bar(String buz) {
    // should be buz.equalsIgnoreCase("baz")
    return buz.toUpperCase().equals("baz");
    // another unnecessary toUpperCase()
    // return buz.toUpperCase().equalsIgnoreCase("baz");
  }
}

UseStringBufferLength

Use StringBuffer.length() to determine StringBuffer length rather than using StringBuffer.toString().equals(“”) or StringBuffer.toString().length() ==.

Example

public class Foo {
void bar() {
  StringBuffer sb = new StringBuffer();
  // this is bad
  if(sb.toString().equals("")) {}
  // this is good
  if(sb.length() == 0) {}
}
}

AppendCharacterWithChar

Avoid concatenating characters as strings in StringBuffer.append.

Example

public class Foo {
void bar() {
  StringBuffer sb=new StringBuffer();
  // Avoid this
  sb.append("a");
 
  // use instead something like this
  StringBuffer sb=new StringBuffer();
  sb.append('a');
}
}

ConsecutiveLiteralAppends

Consecutively calling StringBuffer.append with String literals

Example

public class Foo {
private void bar() {
   StringBuffer buf = new StringBuffer();
   buf.append("Hello").append(" ").append("World"); //bad
   buf.append("Hello World");//good
}
}

UseIndexOfChar

Use String.indexOf(char) when checking for the index of a single character; it executes faster.

Example

public class Foo {
void bar() {
  String s = "hello world";
  // avoid this
  if (s.indexOf("d") {}
  // instead do this
  if (s.indexOf('d') {}
}
}

InefficientEmptyStringCheck

String.trim().length() is an inefficient way to check if a String is really empty, as it creates a new String object just to check its size. Consider creating a static function that loops through a string, checking Character.isWhitespace() on each character and returning false if a non-whitespace character is found.

Example

public class Foo {
    void bar(String string) {
        if (string != null && string.trim().size() > 0) {
                   doSomething();
       }
    }
}

InsufficientStringBufferDeclaration

Failing to pre-size a StringBuffer properly could cause it to re-size many times during runtime. This rule checks the characters that are actually passed into StringBuffer.append(), but represents a best guess “worst case” scenario. An empty StringBuffer constructor initializes the object to 16 characters. This default is assumed if the length of the constructor can not be determined.

Example

public class Foo {
    void bar() {
        StringBuffer bad = new StringBuffer();
        bad.append("This is a long string, will exceed the default 16 characters");//bad
        StringBuffer good = new StringBuffer(41);
        good.append("This is a long string, which is pre-sized");//good
    }
}

UselessStringValueOf

No need to call String.valueOf to append to a string; just use the valueOf() argument directly.

Example

public String convert(int i) {
  String s;
  s = "a" + String.valueOf(i); // Bad
  s = "a" + i; // Better
  return s;
}

StringBufferInstantiationWithChar

StringBuffer sb = new StringBuffer(‘c’); The char will be converted into int to intialize StringBuffer size.

Example

class Foo {
  StringBuffer sb1 = new StringBuffer('c'); //Bad. This is wrong, not bad.
  StringBuffer sb2 = new StringBuffer("c"); //Better
}

UseEqualsToCompareStrings

Using ‘==’ or ‘!=’ to compare strings only works if intern version is used on both sides

Example

class Foo {
  boolean test(String s) {
    if (s == "one") return true; //Bad
    if ("two".equals(s)) return true; //Better
    return false;
  }
}

AvoidStringBufferField

StringBuffers can grow quite a lot, and so may become a source of memory leak (if the owning class has a long life time).

Example

class Foo {
        private StringBuffer memoryLeak;
}
The sixteenth installment of explanation of PMD rules covering Strict Exception Rules

Strict Exception Rules

These rules provide some strict guidelines about throwing and catching exceptions.

AvoidCatchingThrowable

This is dangerous because it casts too wide a net; it can catch things like OutOfMemoryError.

Example

public class Foo {
public void bar() {
  try {
   // do something
  } catch (Throwable th) {  //Should not catch throwable
   th.printStackTrace();
  }
}
}

SignatureDeclareThrowsException

It is unclear which exceptions that can be thrown from the methods. It might be difficult to document and understand the vague interfaces. Use either a class derived from RuntimeException or a checked exception.

Example

public void methodThrowingException() throws Exception {
}

ExceptionAsFlowControl

Using Exceptions as flow control leads to GOTOish code and obscures true exceptions when debugging.

Example

public class Foo {
void bar() {
  try {
   try {
   } catch (Exception e) {
    throw new WrapperException(e);
    // this is essentially a GOTO to the WrapperException catch block
   }
  } catch (WrapperException e) {
   // do some more stuff
  }
}
}

AvoidCatchingNPE

Code should never throw NPE under normal circumstances. A catch block may hide the original error, causing other more subtle errors in its wake.

Example

public class Foo {
void bar() {
  try {
   // do something
   }  catch (NullPointerException npe) {
  }
}
}

AvoidThrowingRawExceptionTypes

Avoid throwing certain exception types. Rather than throw a raw RuntimeException, Throwable, Exception, or Error, use a subclassed exception or error instead.

Example

public class Foo {
public void bar() throws Exception {
  throw new Exception();
}
}

AvoidThrowingNullPointerException

Avoid throwing a NullPointerException – it’s confusing because most people will assume that the virtual machine threw it. Consider using an IllegalArgumentException instead; this will be clearly seen as a programmer-initiated exception.

Example

public class Foo {
void bar() {
  throw new NullPointerException();
}
}

AvoidRethrowingException

Catch blocks that merely rethrow a caught exception only add to code size and runtime complexity.

Example

public class Foo {
   void bar() {
    try {
    // do something
    }  catch (SomeException se) {
       throw se;
    }
   }
  }

DoNotExtendJavaLangError

Errors are system exceptions. Do not extend them.

Example

public class Foo extends Error { }

DoNotThrowExceptionInFinally

Throwing exception in a finally block is confusing. It may mask exception or a defect of the code, it also render code cleanup uninstable. Note: This is a PMD implementation of the Lint4j rule “A throw in a finally block”

Example

public class Foo {
        public void bar() {
               try {
                       // Here do some stuff
               }
               catch( Exception e) {
                       // Handling the issue
               }
               finally {
                       // is this really a good idea?
                       throw new Exception();
               }
        }
}

AvoidThrowingNewInstanceOfSameException

Catch blocks that merely rethrow a caught exception wrapped inside a new instance of the same type only add to code size and runtime complexity.

Example

public class Foo {
     void bar() {
      try {
       // do something
      }  catch (SomeException se) {
         // harmless comment      
           throw new SomeException(se);
      }
     }
    }

Categories