Scala Programming Language
  1. Scala Programming Language
  2. SI-4762

Inadvertent shadowing of base class fields in derived classes, Warning desirable

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: Scala 2.8.1, Scala 2.9.0, Scala 2.9.1
    • Fix Version/s: None
    • Component/s: Misc Compiler
    • Labels:
      None
    • Environment:

      any

      Description

      Issue arises whenever (a) a class parameter in a derived class uses the same
      symbol as a field or function in a base class and (b) that base class symbol
      is subsequently accessed in the derived class. The derived class parameter
      causes the compiler to auto-generate a field of the same name that shadows
      the base class symbol. Any reference to that symbol in the Derived class,
      intended to refer to the base class field, then inadvertently (a) causes
      duplicate definition of the field and (b) unexpectedly (but correctly) refers
      to the auto-generated field in the derived class.

      Code example:

      class Base( val x : String )
      class Derived( x : String ) extends Base( x )

      { override def toString = x }

      Since the 'Base' class has 'val' to generate the field and the derived class
      does not, the developer clearly intends to use the 'Derived' 'x' only for
      pass-through to Base, and therefore expects the reference in 'toString' to
      yield the Base value. Instead, the reference in 'toString' causes the compiler
      to automatically generate a field 'Derived.x' that shadows the 'Base.x' field,
      resulting in an error. The compiler behaves correctly, but the result is a
      programming error.

      Usage scenario (with var fields for clearer illustration of the risks):

      class Base( var x : Int ) { def increment()

      { x = x + 1 } }
      class Derived( x : Int ) extends Base( x ) { override def toString = x.toString }

      val derived = new Derived( 1 )
      println( derived.toString ) // yields '1', as expected
      derived.increment()
      println( derived.toString ) // still '1', probably unexpected

      Since this issue arises whenever anyone uses the default way to initialize
      base class fields from a derived class in Scala, the scenario would appear to
      be extremely common and result in lots of programming errors (easily fixed,
      but still) for newer users.

      An easy work-around for this issue exists (use differing names for the derived
      class parameter, like '_x', 'theX', 'initialX', etc.), but this introduces
      unwanted extra symbols.


      Solution A (Minimal): issue a warning whenever the compiler infers that a
      class parameter requires an auto-generated field in a derived class that
      would shadow a symbol already defined in a base class.

      Solution B: the work-around, still required with Solution A, is to come up
      with a new symbol name each time one initializes a base class field. This
      scenario comes up all the time and polluting the namespace with workaround
      field names like '_x' and 'theX' seems undesirable. Instead, it might be nice
      to have a way to suppress automatic field generation if the developer
      determines that the derived class symbols would otherwise end up hiding a base
      class symbol (e.g., following the warning of Solution A). Maybe a useful
      addition to Scala would be a modifier like 'noval' (or 'passthrough' or
      'temp', or whatever - in addition to 'val' and 'var'), like this:

      class Base( var x : Int ) { def increment() { x = x + 1 }

      }
      class Derived( noval x : Int ) extends Base( x )

      { override def toString = x.toString }

      val derived = new Derived( 1 )
      println( derived.toString ) // yields '1', as expected
      derived.increment()
      println( derived.toString ) // still '2', as expected

      BTW, this ticket was posted in response to a suggestion by @jsuereth in the related thread on StackOverflow.com, which can be found here: http://stackoverflow.com/questions/6515931/idiomatic-scala-way-to-deal-with-base-vs-derived-class-field-names

        Issue Links

          Activity

          Hide
          Paul Phillips added a comment -

          Allowing the reading of private[this] fields in different instances is in direct violation of the spec. If that part is wontfix, I would like to see the specification updated before the ticket is closed.

          Show
          Paul Phillips added a comment - Allowing the reading of private [this] fields in different instances is in direct violation of the spec. If that part is wontfix, I would like to see the specification updated before the ticket is closed.
          Hide
          DaveScala added a comment -

          Suggestion:
          Add warning when fields are shadowed (directly by programmer or indirectly by compiler) and a language feature shadowedFields to turn this warning off.

          Show
          DaveScala added a comment - Suggestion: Add warning when fields are shadowed (directly by programmer or indirectly by compiler) and a language feature shadowedFields to turn this warning off.
          Show
          DaveScala added a comment - Manifestation of this problem: https://groups.google.com/forum/?hl=en&fromgroups#!topic/scala-user/tjrnxpuzfNA%5B1-25%5D
          Hide
          Erik Allik added a comment -

          Would this be considered a manifestation of the same underlying problem: http://stackoverflow.com/questions/21580309/in-scala-what-is-the-reasoning-behind-allowing-parameter-shadowing ?

          Show
          Erik Allik added a comment - Would this be considered a manifestation of the same underlying problem: http://stackoverflow.com/questions/21580309/in-scala-what-is-the-reasoning-behind-allowing-parameter-shadowing ?
          Hide
          A. P. Marki added a comment -

          Erik asks about locals shadowing parameters. Might be nice to -Ywarn-unused if an unused param is also shadowed; and -Ywarn-unused-parameter unconditionally. That would need a ticket, I think.

          Show
          A. P. Marki added a comment - Erik asks about locals shadowing parameters. Might be nice to -Ywarn-unused if an unused param is also shadowed; and -Ywarn-unused-parameter unconditionally. That would need a ticket, I think.

            People

            • Assignee:
              Martin Odersky
              Reporter:
              Gregor Scheidt
            • Votes:
              8 Vote for this issue
              Watchers:
              19 Start watching this issue

              Dates

              • Created:
                Updated:

                Development