SonarQube C#
  1. SonarQube C#
  2. SONARCS-220

New rule: Avoid reassigning parameters

    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

      Parameter variable should not be assigned to

      Description

      <p>
      Parameters variables that are not marked as <code>out</code> or <code>ref</code> should not be assigned to from within a method.
      Those modifications will not impact the caller of the method, which can be confusing.
      Instead, it is better to assign the parameter to a temporary variable.
      </p>
      
      <p>The following code snippet:</p>
      
      <pre>
      void foo(int a)
      {
        a = 42;           // Non-Compliant
      }
      </pre>
      
      <p>should be refactored into:</p>
      
      <pre>
      void foo(int a)
      {
        int tmp = a;
        tmp = 42;         // Compliant
      }
      </pre>
      
      <p>or:</p>
      
      <pre>
      void foo(out int a) // 'ref' is also allowed
      {
        a = 42;           // Compliant
      }
      </pre>
      

      Message

      Remove this assignment to the method parameter '{}'.

      Severity

      Major

      In Sonar way?

      Yes

        Activity

        Hide
        Dinesh Bolkensteyn added a comment -

        I guess that there are false negatives when delegates or lambdas are being used.

        Show
        Dinesh Bolkensteyn added a comment - I guess that there are false negatives when delegates or lambdas are being used.
        Hide
        Dinesh Bolkensteyn added a comment - - edited

        Indeed, we don't currently detect violations when parameters to delegates or lambdas are used, as in:

        class Foo
        {
            public delegate void FooEventHandler(object sender);
        
            public event FooEventHandler OnFoo;
        }
        
        class Bar
        {
          void test(int param)
          {
            Foo a = new Foo();
            a.OnFoo += delegate(object b)
            {
              b = null; // Non-Compliant < This is not detected for now
            };
        
            param = 42; // Non-Compliant
          }
        }
        
        Show
        Dinesh Bolkensteyn added a comment - - edited Indeed, we don't currently detect violations when parameters to delegates or lambdas are used, as in: class Foo { public delegate void FooEventHandler(object sender); public event FooEventHandler OnFoo; } class Bar { void test( int param) { Foo a = new Foo(); a.OnFoo += delegate(object b) { b = null ; // Non-Compliant < This is not detected for now }; param = 42; // Non-Compliant } }
        Hide
        Dinesh Bolkensteyn added a comment -

        And it now should perfectly work for delegates and lambdas too!

        Show
        Dinesh Bolkensteyn added a comment - And it now should perfectly work for delegates and lambdas too!
        Hide
        Fabrice Bellingard added a comment -

        This does not cover the following cases:

        • indexed properties
            public int this[int index]
            {
                get
                {
                    index = 1; // Violation
                    return 0;
                }
                set
                {
                    index = 1; // Violation
                    value = 45; // Violation ("value" is the implicit parameter)
                }
            }
        
        • operator overloading
            public static MyClass operator +(MyClass first, MyClass second)
            {
        	first = new MyClass(); // Violation
                return ...;
            }
        
        • property setters
            public int Amount
            {
                get { return fAmount; }
                set 
                { 
                    value = 45; // Violation ("value" is the implicit parameter)
                    this.fAmount = value;
                }
            }
        
        Show
        Fabrice Bellingard added a comment - This does not cover the following cases: indexed properties public int this [ int index] { get { index = 1; // Violation return 0; } set { index = 1; // Violation value = 45; // Violation ( "value" is the implicit parameter) } } operator overloading public static MyClass operator +(MyClass first, MyClass second) { first = new MyClass(); // Violation return ...; } property setters public int Amount { get { return fAmount; } set { value = 45; // Violation ( "value" is the implicit parameter) this .fAmount = value; } }
        Hide
        Dinesh Bolkensteyn added a comment -

        Should be fixed

        Show
        Dinesh Bolkensteyn added a comment - Should be fixed
        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: