Bug Report T528993
Visible to All Users

Code Analysis - False positive of CRR0025 for lambda expressions

created 8 years ago

Not sure how easy this is to fix, but I thought I'd report it anyways since I just encountered this in one of our bigger solutions.

In there, we have some logic that receives an object and caches the result of one of the properties. But since the object might not be in a valid state by that point, we have to split it up into "the property is valid, do it now" and "the property is not valid, wait for PropertyChanged" code paths.

The latter hooks into PropertyChanged and waits for the property to get a value.

A reduced sample looks like this (including  warning disable pragmas to illustrate the issue):

C#
class ThisHasReferences : INotifyPropertyChanged { // assume this raises PropertyChanged public OtherClass Other { get; set; } public event PropertyChangedEventHandler PropertyChanged; } class OtherClass { } class Program { static void Main(string[] args) { // passed in from somewhere else var hasRefs = new ThisHasReferences(); // [...] if (hasRefs.Other == null) { hasRefs.PropertyChanged += (s, e) => { #pragma warning disable CRR0025 // Unreachable conditional code block (the inversed condition is already satisfied) if (e.PropertyName == nameof(ThisHasReferences.Other) && hasRefs.Other != null) #pragma warning restore CRR0025 // Unreachable conditional code block (the inversed condition is already satisfied) RememberTheOtherRef(hasRefs.Other); }; } else { RememberTheOtherRef(hasRefs.Other); } } private static void RememberTheOtherRef(OtherClass other) { // [...] } }

CRR0025 is raised for "hasRefs.Other != null" because it was checked for null just a few lines before. However, since this is an event handler, the message is unlikely to be correct.

I'm not sure if this could be generalized for all delegates (depends on whether a dependency/call analysis takes place inside the scope of that analyzer).
I also don't know if it even makes sense to check for those conditions, given the chances of false positives/negatives or ignored violations are fairly high (and as such, I don't mind if you decide to not implement this - I mostly reported this so you know this is a thing)

Comments (1)
DevExpress Support Team 8 years ago

    Hello Emanuel,

    Thank you for detailed explanations and providing the code sample.
    You are definitely right - CodeRush should not detect unreachable code in this case.
    We will fix this false positive and let you know as soon as we have any results.

    Answers approved by DevExpress Support

    created 8 years ago

    We have fixed the issue described in this ticket and will include the fix in our next maintenance update. To apply this solution before the official update, request a hotfix by clicking the corresponding link for product versions you require.

    Note: Hotfixes may be unavailable for beta versions and updates that are about to be released.

      Disclaimer: The information provided on DevExpress.com and affiliated web properties (including the DevExpress Support Center) is provided "as is" without warranty of any kind. Developer Express Inc disclaims all warranties, either express or implied, including the warranties of merchantability and fitness for a particular purpose. Please refer to the DevExpress.com Website Terms of Use for more information in this regard.

      Confidential Information: Developer Express Inc does not wish to receive, will not act to procure, nor will it solicit, confidential or proprietary materials and information from you through the DevExpress Support Center or its web properties. Any and all materials or information divulged during chats, email communications, online discussions, Support Center tickets, or made available to Developer Express Inc in any manner will be deemed NOT to be confidential by Developer Express Inc. Please refer to the DevExpress.com Website Terms of Use for more information in this regard.