Checker BadEq

belongs to group Basic
Identify possibly incorrect uses of == and equals()

Frameworks supported by this checker

  • java up to 11
  • android up to API level 28
  • dotnet

Warnings generated by this checker

  • AssigningInsteadOfComparingWarning: the assignment into a Boolean value is used in a condition, while == would be expected [ CWE481 ]
  • EqualityWarning: two objects are compared with == but equals() seems more appropriate instead [ CWE595 ]
  • EqualsOnArraysWarning: two arrays are compared with equals() [ CWE595 ]
  • EqualsOnDisjointTypesWarning: two objects are compared by equals() but they have always distinct types [ CWE596 ]
  • EqualsWarning: two objects are compared with equals() but == seems more appropriate instead [ CWE480 ]
  • ImpossibleEqualityWarning: two objects are compared by == but the comparison will always fail [ CWE595 ]
  • ImpossibleEqualsWarning: two objects are compared by equals() but the comparison will always fail [ CWE596 ]
  • InefficientStringEmptynessTestWarning: a string is compared to the empty string instead of using isEmpty() [ CWE597 ]

Options accepted by this checker

  • none

Annotations understood by this checker

  • none


Description

This checker identifies incorrect or inefficient comparisons through equals() or ==. In some cases, these comparisons are wrong: for instance, strings should be compared for equality through == rather then through equals(). Classes can be safely compared through == instead. In other cases, these comparisons can be replaced by more efficient code.

Action: Check if the equality is actually wrong or can be replaced by more optimized code.

This checker identifies incorrect or inefficient comparisons through Equals() or ==. In some cases, these comparisons are wrong: for instance, strings should be compared for equality through == rather then through equals(). Classes can be safely compared through == instead. In other cases, these comparisons can be replaced by more efficient code.

Action: Check if the equality is actually wrong or can be replaced by more optimized code.

Examples

Consider the following program:

public class Main {
  public static boolean verbose;
  private final static String[] arr = new String[] { "verbose" };

  public static void main(String[] args) {
    if (args.length > 0)
      assert !args[0].equals("");

    if (args.length > 0 && args[0] == "verbose")
      verbose = true;

    if (args.equals(arr))
      verbose = true;

    if (args.length > 0) {
      Main m = new Main();
      System.out.println(m.test(args[0]));
      System.out.println(m.weAreRedefined());
    }
  }

  private boolean test(Object o) {
    return o == this;
  }

  public boolean weAreRedefined() {
    return !getClass().equals(Main.class);
  }
}

This checker issues the following warnings:

Main.java:7: [BadEq: InefficientStringEmptynessTest] Inefficient comparison with the empty string. Use isEmpty() instead
Main.java:9: [BadEq: EqualityWarning] Suspicious use of == rather than equals() to compare two java.lang.String
Main.java:12: [BadEq: EqualsOnArraysWarning] Suspicious call to equals() on arrays
Main.java:23: [BadEq: ImpossibleEqualityWarning] Always distinct objects are compared here
Main.java:27: [BadEq: EqualsWarning] Inefficient use of equals() rather than == to compare two java.lang.Class

Each of these warnings corresponds to incorrect or inefficient comparisons. In particular, == compares strings wrt identity, not wrt their content. The same happens for arrays, as at line 12, that should be compared element-wise. Moreover, at line 23, the comparison will always fail since objects of incompatible types are always compared there (a string and an instance of Main).

In this example, the programmer should rewrite the code as follows:

public class Main {
  public static boolean verbose;
  private final static String[] arr = new String[] { "verbose" };

  public static void main(String[] args) {
    if (args.length > 0)
      assert !args[0].isEmpty();

    if (args.length > 0 && args[0].equals("verbose"))
      verbose = true;

    if (java.util.Arrays.equals(args, arr))
      verbose = true;

    if (args.length > 0) {
      Main m = new Main();
      //System.out.println(m.test(args[0]));
      System.out.println(m.weAreRedefined());
    }
  }

  public boolean weAreRedefined() {
    return getClass() != Main.class;
  }
}

Consider the following program:

using System;
using System.Diagnostics;

namespace DocumentationExamples
{

    public class BadEq
    {
        public static bool verbose;
        private readonly static string[] arr = new string[] { "verbose" };
        public static void Main(string[] args)
        {
            if (args.Length > 0)
                Debug.Assert(!args[0].Equals("")); // InefficientStringEmptynessTest
            if (args.Length > 0 && args[0] == "verbose")
                verbose = true;
            if (args.Equals(arr)) // EqualsOnArraysWarning
                verbose = true;
            if (args.Length > 0)
            {
                BadEq m = new BadEq();
                Console.WriteLine(m.Test(args[0]));
                Console.WriteLine(m.WeAreRedefined());
            }
        }
        private bool Test(object o)
        {
            return o == this; // ImpossibleEqualityWarning
        }
        public bool WeAreRedefined()
        {
            return !GetType().Equals(typeof(BadEq)); // EqualsWarning
        }
    }
}

This checker issues the following warnings:

DocumentationExamples.cs:14: [BadEq: InefficientStringEmptynessTest] Inefficient comparison against the empty string. Use method "String.IsNullOrEmpty" instead
DocumentationExamples.cs:17: [BadEq: EqualsOnArraysWarning] Suspicious call to method "Equals" on arrays
DocumentationExamples.cs:28: [BadEq: ImpossibleEqualityWarning] Always distinct objects are compared here
DocumentationExamples.cs:32: [BadEq: EqualsWarning] Inefficient use of method "Equals" rather than == to compare two "System.Type": they are unique objects

Each of these warnings corresponds to incorrect or inefficient comparisons. In particular, == compares strings wrt identity, not wrt their content. The same happens for arrays, as at line 17, that should be compared element-wise. Moreover, at line 28, the comparison will always fail since objects of incompatible types are always compared there (a string and an instance of Main).

In this example, the programmer should rewrite the code as follows:

public static bool verbose;
private readonly static string[] arr = new string[] { "verbose" };
public static void Main(string[] args)
{
	if (args.Length > 0)
		Debug.Assert(!string.IsNullOrEmpty(args[0]));
	if (args.Length > 0 && args[0].Equals("verbose"))
		verbose = true;
	if (args.SequenceEqual(arr))
		verbose = true;
	if (args.Length > 0)
	{
		BadEq m = new BadEq();
		//Console.WriteLine(m.Test(args[0]));
		Console.WriteLine(m.WeAreRedefined());
	}
}

public bool WeAreRedefined()
{
	return GetType() != typeof(BadEq);
}