Checker UselessAssignment

belongs to group Basic
Identify useless assignments

Frameworks supported by this checker

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

Warnings generated by this checker

  • AssignmentToUnreadParameterWarning: a parameter is assigned before being read [ CWE563 ]
  • AssignmentToUnusedParameterWarning: a parameter is assigned but the written value is never used later [ CWE563 ]
  • AssignmentToUnusedVariableWarning: a variable is assigned but the written value is never used later [ CWE563 ]
  • TautologicalAssignmentWarning: a field is assigned to itself [ CWE665 ]
  • UselessAssignmentToDefaultValueWarning: a field is assigned in a constructor or finaliser to its default value [ CWE665 ]

Options accepted by this checker

  • none

Annotations understood by this checker

  • none


Description

This checker finds assignments to local variables that are useless and could be consequently removed from code, hence obtaining a more efficient program. In some cases, these assignments hide actual bugs in the logic of the code.

Action: Remove the assignment and check if it actually hided a more serious algorithmical issue.

This checker finds assignments to local variables that are useless and could be consequently removed from code, hence obtaining a more efficient program. In some cases, these assignments hide actual bugs in the logic of the code.

Action: Remove the assignment and check if it actually hided a more serious algorithmical issue.

Examples

Consider the following program:

import java.util.HashMap;
import java.util.Map;

public class Test {
  private int f;
	
  public Test(int f) {
    f = process(f);
		
    Map factory = new HashMap<>();
    factory = buildFactory();
    go(factory);
  }

  private int process(int x) {
    return x * 17 + 13;
  }

  private Map buildFactory() {
    Map result = new HashMap<>();
    result.put("value", f);
	
    return result;
  }

  private void go(Map factory) {
    for (String s: factory.keySet())
      System.out.println(s);
  }
}

This checker issues the following warnings:

Test.java:8: [UselessAssignment: AssignmentToUnusedParameterWarning] The value written here into formal parameter "f" is never read again later. Did you want to write into the synonym field instead?
Test.java:10: [UselessAssignment: AssignmentToUnusedVariableWarning] The value written here into "factory" is never read again later

since the values written at lines 8 and 10 are not read again later. Note that both useless assignments hide more serious issues. The assignment at line 8 was probably meant to write into field f rather than into the parameter f. Hence, that field remains uninitialized after the execution of the constructor. The assignment at line 10 initializes factory to a value that gets immediately discarded after, at line 11. This is a waste of time and memory.

In this example, the programmer should probably write into field f and initialize factory to the return value of buildFactory(), immediately, as follows:

import java.util.HashMap;
import java.util.Map;

public class Test {
  private int f;
	
  public Test(int f) {
    this.f = process(f);
		
    Map factory = buildFactory();
    go(factory);
  }

  ...
}

Consider the following program:

using System;
using System.Collections.Generic;

namespace DocumentationExamples
{

    public class UselessAssignment
    {
        public static void Main(string[] args)
        { }

        private int f;
        public UselessAssignment(int f)
        {
            f = Process(f);
            Dictionary factory = new Dictionary();
            factory = BuildFactory();
            Go(factory);
        }
        private int Process(int x)
        {
            return x * 17 + 13;
        }
        private Dictionary BuildFactory()
        {
            Dictionary result = new Dictionary
            {
                { "value", f }
            };
            return result;
        }
        private void Go(Dictionary factory)
        {
            foreach (string s in factory.Keys)
                Console.WriteLine(s);
        }
    }
}

This checker issues the following warnings:

DocumentationExamples.cs:15: [UselessAssignment: AssignmentToUnusedParameterWarning] The value written here into formal parameter "f" is never read again later. Did you want to write into the synonym field instead?
DocumentationExamples.cs:16: [UselessAssignment: AssignmentToUnusedVariableWarning] The value written here is never read again later

since the values written at lines 15 and 16 are not read again later. Note that both useless assignments hide more serious issues. The assignment at line 15 was probably meant to write into field f rather than into the parameter f. Hence, that field remains uninitialized after the execution of the constructor. The assignment at line 16 initializes factory to a value that gets immediately discarded after, at line 17. This is a waste of time and memory.

In this example, the programmer should probably write into field f and initialize factory to the return value of BuildFactory(), immediately, as follows:

import java.util.HashMap;
import java.util.Map;

public class Test {
  private int f;
	
  public Test(int f) {
    this.f = process(f);
		
    Map factory = buildFactory();
    go(factory);
  }

using System;
using System.Collections.Generic;

namespace DocumentationExamples
{

    public class UselessAssignment
    {
        public static void Main(string[] args)
        { }

        private int f;
        public UselessAssignment(int f)
        {
            f = Process(f);
            Dictionary factory = BuildFactory();
            Go(factory);
        }
        
         ...
}