Useless assignment to local variable¶
ID: cs/useless-assignment-to-local
Kind: problem
Security severity:
Severity: warning
Precision: very-high
Tags:
- maintainability
- external/cwe/cwe-563
Query suites:
- csharp-security-and-quality.qls
Click to see the query in the CodeQL repository
A value is assigned to a local variable, but either that variable is never read later on, or its value is always overwritten before being read. This means that the original assignment has no effect, and could indicate a logic error or incomplete code.
Recommendation¶
Ensure that you check the program logic carefully. If a value is really not needed, consider omitting the assignment. Be careful, though: if the right-hand side has a side effect (like performing a method call), it is important to keep this to preserve the overall behavior.
Example¶
The following example shows six different types of assignments to local variables whose value is not read:
In
ParseInt
, the result of the call toint.TryParse
is assigned directly to the unread local variablesuccess
.In
IsDouble
, theout
argument of the call toint.TryParse
is assigned to the unread local variablei
.In
ParseDouble
, the exception thrown by the call todouble.Parse
in case of parse failure is assigned to the unread local variablee
.In
Count
, the elements ofss
are assigned to the unread localforeach
variables
.In
IsInt
,o
is assigned (in caseo
is an integer) to the unread local type test variablei
.In
IsString
,o
is assigned (in caseo
is a string) to the unread local type case variables
.
using System;
class Bad
{
double ParseInt(string s)
{
var success = int.TryParse(s, out int i);
return i;
}
bool IsDouble(string s)
{
var success = double.TryParse(s, out double i);
return success;
}
double ParseDouble(string s)
{
try
{
return double.Parse(s);
}
catch (FormatException e)
{
return double.NaN;
}
}
int Count(string[] ss)
{
int count = 0;
foreach (var s in ss)
count++;
return count;
}
string IsInt(object o)
{
if (o is int i)
return "yes";
else
return "no";
}
string IsString(object o)
{
switch (o)
{
case string s:
return "yes";
default:
return "no";
}
}
}
The revised example eliminates the unread assignments.
using System;
class Good
{
double ParseInt(string s)
{
int.TryParse(s, out int i);
return i;
}
bool IsDouble(string s)
{
var success = double.TryParse(s, out _);
return success;
}
double ParseDouble(string s)
{
try
{
return double.Parse(s);
}
catch (FormatException)
{
return double.NaN;
}
}
int Count(string[] ss)
{
return ss.Length;
}
string IsInt(object o)
{
if (o is int)
return "yes";
else
return "no";
}
string IsString(object o)
{
switch (o)
{
case string _:
return "yes";
default:
return "no";
}
}
}
References¶
Wikipedia: Dead store.
MSDN, Code Analysis for Managed Code, CA1804: Remove unused locals.
Microsoft: What’s new in C# 7 - Discards.
Common Weakness Enumeration: CWE-563.