Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does ReSharper tell me this expression is always true?

Tags:

I have the following code which will tell me whether or not a certain property is used elsewhere in the code. The idea behind this is to verify whether a property with a private setter can be made readonly.

There are multiple gotchas here but the major ones are that an assignment to the property outside the constructor means it won't fire. Furthermore, a static property may only have an assignment in a static constructor to fire the diagnostic. Likewise, an instance property requires only an instance constructor.

Now, most scenarios I have so far are accounted for yet ReSharper gives me a warning in this piece of code and I just can't seem to figure out its logic. The above specification is translated in this bit of code:

var isStaticProperty = propertySymbol.IsStatic; bool hasInstanceUsage = false; bool hasStaticUsage = false;  foreach (var identifier in outerClass.DescendantNodes().OfType<IdentifierNameSyntax>()) {    var memberSymbol = context.SemanticModel.GetSymbolInfo(identifier);    if (memberSymbol.Symbol.Equals(propertySymbol))    {        var constructor = identifier.Ancestors().OfType<ConstructorDeclarationSyntax>()                                                .FirstOrDefault();        var isInConstructor = constructor != null;        var isAssignmentExpression = identifier.Ancestors()                                                .OfType<AssignmentExpressionSyntax>()                                                .FirstOrDefault() != null;         // Skip anything that isn't a setter        if (!isAssignmentExpression)        {            continue;        }         // if it is a setter but outside the constructor, we don't report any diagnostic        if (!isInConstructor)        {            return;        }         var isStaticConstructor = context.SemanticModel                                         .GetDeclaredSymbol(constructor).IsStatic;        if (isStaticConstructor && isStaticProperty)        {            hasStaticUsage = true;        }         if (!isStaticConstructor && !isStaticProperty)        {            hasInstanceUsage = true;        }    } }  // We can't set it to readonly if it's set in both the instance  //   and the static constructor // We need a NAND operation: either it's never set,  //   it's set in ctor 1 or it's set in ctor 2 if (!(hasStaticUsage & hasInstanceUsage)) {    context.ReportDiagnostic(Diagnostic.Create(                  Rule, property.Identifier.GetLocation(), propertySymbol.Name)); } 

With the warning being

Expression is always true

on the line

if (!(hasStaticUsage & hasInstanceUsage)) 

Why does it show this warning? There are an unknown amount of descendants so there's an unknown amount of loops. Each loop can set hasStaticUsage or hasInstanceUsage to true, which means that after 2 loops (at the earliest), both values could become true and the if condition should fail: a NAND returns true, true, true, false.

This is the boolean logic I intend to accomplish:

+----------------+------------------+--------+ | hasStaticUsage | hasInstanceUsage | result | +----------------+------------------+--------+ | false          | false            | true   | | false          | true             | true   | | true           | false            | true   | | true           | true             | false  | +----------------+------------------+--------+ 
like image 827
Jeroen Vannevel Avatar asked Jun 08 '15 03:06

Jeroen Vannevel


2 Answers

isStaticProperty is initialized outside the loop:

var isStaticProperty = propertySymbol.IsStatic; 

If isStaticProperty is false, then this expression:

 (isStaticConstructor && isStaticProperty) 

is always false, hence hasStaticUsage is false.

If isStaticProperty is true, then this expression:

 (!isStaticConstructor && !isStaticProperty) 

is always false, hence hasInstanceUsage is false.

In any case hasStaticUsage and hasInstanceUsage can't both be true at the same time.

like image 74
default locale Avatar answered Jan 03 '23 00:01

default locale


You can find out the answer by creating truth table for this expressions. isStaticConstructor && isStaticProperty and !isStaticConstructor && !isStaticProperty. Lets do it together.

isStaticConstructor && isStaticProperty

+---------------------+------------------+--------+ | isStaticConstructor | isStaticProperty | result | +---------------------+------------------+--------+ | false               | false            | false  | | false               | true             | false  | | true                | false            | false  | | true                | true             | true   | +---------------------+------------------+--------+ 

!isStaticConstructor && !isStaticProperty

+---------------------+------------------+--------+ | isStaticConstructor | isStaticProperty | result | +---------------------+------------------+--------+ | false               | false            | true   | | false               | true             | false  | | true                | false            | false  | | true                | true             | false  | +---------------------+------------------+--------+ 

So you can see, that there is no any possibility that the both isStaticConstructor && isStaticProperty and !isStaticConstructor && !isStaticProperty to be true.

So, depending on the truth table you had provided, the only possibility that !(hasStaticUsage & hasInstanceUsage) becomes false is when both expressions are true in same time, which is impossible.

like image 36
Hamlet Hakobyan Avatar answered Jan 03 '23 00:01

Hamlet Hakobyan