[RB] Revisit ReRenameMethodRefactoring and Driver tests

[Ducasse]

Once I have a green bar and a driver that is working we should revisit add tests to ReRenameMethodRefactoring and create tests for the ReRenameMethodRefactoring

I added tests to cover overriding scenario when changing the names

[Ducasse]

Imagine that we have

Root >> flop

SubRoot >> flip

and we rename flip in flop

We check the implementors of flip and we get SubRoot. The condition checks if SubRoot or its subclasses defines flop. It does not.

My impression is that starting from SubRoot the superclasses should be checked and not the subclasses.

[Ducasse]

Ok the superclasses are checked!

definesMethod: aSelector 
	(self directlyDefinesMethod: aSelector) ifTrue: [^true].
	^self superclass notNil and: [self superclass definesMethod: aSelector]

[Ducasse]

Now the logic is strange because it checks the full hierarchy and subclasses do not really care. In ReRenameMethodRefactoring we have now the following logic

doesNotOverrideExistingMethodPreconditions
	"Check that the new selector is not already defined in class or superclasses of the implementors of the oldSelector."
	
	self implementors asOrderedCollection do: [ :each | 
			| cond |
			cond := (ReUpToRootDefinesMethod new class: each ; selector: newSelector).
			cond check
				ifTrue: [ self violations addAll: cond violators ] ].
	^ self violations

[Ducasse]

I sketch some scenarios for the semantics of rename. In the future I would like to see them turned into tests.

Scenario 1

Imagine that we have the following independent hierarchies

	A
	|
	B 
		flup
			^ ‘flup'

	X
		flop
			^ ‘flop'
	|

	Y
		flup
			^ ‘flup'

When we rename in B flup into flop

• check all the flup implementations here there are two B and Y
• for each of them check if there is flop method in the class or superclass if any ask the user because we will have an override.
• So here the system should tell us that there is X>>flop

Scenario 2

Imagine that we have the following independent hierarchies

	X
		
	|

	Y
		flup
			^ ‘flup’
		flop
			^ ‘flop’

Here the system should tell us that flop is will be overridden if we proceed we should get only one method

	X
		
	|

	Y
		flop
			^ ‘ flup’

So far this is what the new driver I’m working on is doing.

[Ducasse]

Scenario 3:

Imagine that we have the following independent hierarchies

	A
		raise: x to: y
		   ^ x raisedTo: y
		
		returns9
		   ^ (self raise: 3 to: 2) = 9
```		

A new returns9 is true.

- If I select raise:to: and rename by just permuting the argument
- I get a prompt telling me that I will ‘overrides' the method raise:to: and I do not really like that


I obtain

raise: y to: x
	   ^ x raisedTo: y
returns9
	   ^ (self raise: 2 to: 3) = 9

A new returns9 is true.

so far so good.

I did some tests with methods having the same name but using the argument in different ways. 
The refactoring works because it will permute all the methods and all the call sites.
So the different ways will be kept (we will still have two methods using x and y at different positions and the callers will be updated) so it works.