Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.1
    • Component/s: C# Squid Rules
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Title

      Assignments should not be used inside sub-expressions

      Description

      <p>
      Assignments within sub-expressions are hard to spot and therefore make the code less readable.
      It is also a common mistake to write <code>=</code> when <code>==</code> was meant.
      Ideally, every expression should have no more than one side-effect.
      Assignments inside lambda expressions are allowed.
      </p>
      
      <p>The following code:</p>
      
      <pre>
      foo(i = 42);  // Non-Compliant
      </pre>
      
      <p>should be refactored into:</p>
      
      <pre>
      i = 42;
      foo(i);       // Compliant
      </pre>
      
      or:
      
      <pre>
      foo(i == 42); // Compliant
      </pre>
      

      Message

      Extract this assignment outside of the sub-expression.

      Severity

      Major

      In Sonar way?

      Yes

        Activity

        Hide
        Dinesh Bolkensteyn added a comment -

        MISRA?

        Show
        Dinesh Bolkensteyn added a comment - MISRA?
        Show
        Dinesh Bolkensteyn added a comment - https://github.com/SonarSource/sonar-rules-specs/tree/master/assignments-in-condition
        Hide
        Dinesh Bolkensteyn added a comment -

        There is a delegate() syntax, that looks like lambdas, which causes many false positives to be generated currently.

        Show
        Dinesh Bolkensteyn added a comment - There is a delegate() syntax, that looks like lambdas, which causes many false positives to be generated currently.
        Hide
        Dinesh Bolkensteyn added a comment -

        False positive when using:

        if (!string.IsNullOrEmpty(result)) result = result + separator;
        
        Show
        Dinesh Bolkensteyn added a comment - False positive when using: if (!string.IsNullOrEmpty(result)) result = result + separator;
        Hide
        Dinesh Bolkensteyn added a comment -

        False positive:

        asyncManager.Finished += delegate { finishTrigger.Fire(); };
        
        Show
        Dinesh Bolkensteyn added a comment - False positive: asyncManager.Finished += delegate { finishTrigger.Fire(); };
        Hide
        Dinesh Bolkensteyn added a comment -

        The following use case is not covered by this rule:

        if (i = 0) { ... } // Compliant by this rule
        

        Indeed, "i = 0" is a top level expression, and not a sub-expression.

        That use case is to be covered by the assignment-in-condition rule.

        Show
        Dinesh Bolkensteyn added a comment - The following use case is not covered by this rule: if (i = 0) { ... } // Compliant by this rule Indeed, "i = 0" is a top level expression, and not a sub-expression. That use case is to be covered by the assignment-in-condition rule.
        Hide
        Dinesh Bolkensteyn added a comment -

        It seems to work great now

        Show
        Dinesh Bolkensteyn added a comment - It seems to work great now
        Hide
        Fabrice Bellingard added a comment -

        Tested!

        Show
        Fabrice Bellingard added a comment - Tested!

          People

          • Assignee:
            Dinesh Bolkensteyn
            Reporter:
            Fabrice Bellingard
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: