GRECLIPSE
  1. GRECLIPSE
  2. GRECLIPSE-907

Extract local variable extracts the variable outside the scope of the closure which is incorrect

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.0Release
    • Fix Version/s: 2.6.0.Release
    • Component/s: Refactoring
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Given the following example:

      class Simple {
      
      	def test() {
      		def map = [one:"two"]
      		def foo = {
      			println map.one	
      		}
      	}
      }
      

      If you run extract variable on map.one you get:

      
      class Simple {
      
      	def test() {
      		def map = [one:"two"]
      		def mapOne = map.one
      
      		def foo = {
      			println mapOne	
      		}
      	}
      }
      
      

      Which is incorrect. The variable should be within the scope of the closure above the println statement

        Activity

        Hide
        Andrew Eisenberg added a comment -

        This one is a little tricky because the refactoring engine assumes that the extracted local should be as close as possible to the declaration. But you are saying that it should be close to its first use.

        There are semantic differences between these two options, but different situations may call for different uses.

        I did try a similar scenario in Java and the result is putting the local variable inside the closure. We should probably be using the same behavior as JDT, so I'll keep this issue open.

        Show
        Andrew Eisenberg added a comment - This one is a little tricky because the refactoring engine assumes that the extracted local should be as close as possible to the declaration. But you are saying that it should be close to its first use. There are semantic differences between these two options, but different situations may call for different uses. I did try a similar scenario in Java and the result is putting the local variable inside the closure. We should probably be using the same behavior as JDT, so I'll keep this issue open.
        Hide
        Andrew Eisenberg added a comment -

        We should really get this working for 2.6.0.

        Show
        Andrew Eisenberg added a comment - We should really get this working for 2.6.0.
        Hide
        Andrew Eisenberg added a comment - - edited

        I have a fix locally. Need to write some tests and then I will commit.

        Show
        Andrew Eisenberg added a comment - - edited I have a fix locally. Need to write some tests and then I will commit.
        Hide
        Andrew Eisenberg added a comment -

        Now commmitted.

        Show
        Andrew Eisenberg added a comment - Now commmitted.

          People

          • Assignee:
            Andrew Eisenberg
            Reporter:
            Graeme Rocher
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: