Refactor: improve naming and add comments to EquivalentTreeChecker

src/Refactoring-Transformations-Tests/EquivalentTreeChecker.class.st

@@ -2,9 +2,9 @@ Class {
 	#name : 'EquivalentTreeChecker',
 	#superclass : 'Object',
 	#instVars : [
-		'selector',
 		'class',
-		'model'
+		'model',
+		'extractedFromSelector'
 	],
 	#category : 'Refactoring-Transformations-Tests-Test',
 	#package : 'Refactoring-Transformations-Tests',
@@ -29,23 +29,45 @@ EquivalentTreeChecker >> checkEquivalentTreeFor: aMethodNode [
 	^ nil
 ]
 
-{ #category : 'as yet unclassified' }
+{ #category : 'executing' }
 EquivalentTreeChecker >> checkIfMethodNode: aMethodNode isEquivalentMethodNode: anAnotherMethodNode [
 
+	| anotherMethodsClass |
+	"Methods should have the same number of arguments"
 	aMethodNode arguments size = anAnotherMethodNode arguments size ifFalse: [
 		^ false ].
+	self flag: #todo. "if I don't need return I can be equivalent to a method that has return; check code below"
+	"needsReturn ifFalse: [ tree := self removeReturnsOf: tree ]."
+	"Methods bodies should be the same with the exception of argument names"
 	(anAnotherMethodNode body
 		 equalTo: aMethodNode body
 		 exceptForVariables:
 		 (anAnotherMethodNode arguments collect: [ :each | each name ]))
 		ifFalse: [ ^ false ].
+	"Now we know methods are equivalent, we now check if it has super sends,
+	if it does, we cannot guarantee behavior preservation if that method is
+	from one of the superclasses (since super will not call the same method
+	as it would call when it was inlined in `aMethodNode`)"
+	self flag: #todo. "improve MRO preservation when super calls are present"
+	anotherMethodsClass := class whichClassIncludesSelector: anAnotherMethodNode selector.
+	(anAnotherMethodNode superMessages isNotEmpty 
+			and: [ class name ~= anotherMethodsClass name ])
+		ifTrue: [ ^ false ].
 	^ true
 ]
 
+{ #category : 'accessing' }
+EquivalentTreeChecker >> extractedFromSelector: aString [ 
+	"We have to remember the methods containing the original extracted expression,
+	so that it is excluded from the search."
+
+	extractedFromSelector := aString
+]
+
 { #category : 'query' }
 EquivalentTreeChecker >> methodsToBeChecked [
 
-	^ self allMethodsInHierarchy reject: [ :m | m selector = selector ]
+	^ self allMethodsInHierarchy reject: [ :m | m selector = extractedFromSelector ]
 ]
 
 { #category : 'instance creation' }
@@ -59,8 +81,3 @@ EquivalentTreeChecker >> on: aClass [
 
 	class := model classNamed: aClass name
 ]
-
-{ #category : 'accessing' }
-EquivalentTreeChecker >> selector: aString [ 
-	selector := aString
-]

src/Refactoring-Transformations-Tests/EquivalentTreeTest.class.st

@@ -34,35 +34,65 @@ EquivalentTreeTest >> testAllMethodsInHierarchy [
 			includesAll: ((model classNamed: Object name) methods collect: [:each | each selector])).
 ]
 
+{ #category : 'tests' }
+EquivalentTreeTest >> testEquivalentTreeWithMethodContainingSuperSendInSameClassExpectFound [
+
+	| checker methodNode equivalentTree |
+
+	checker := EquivalentTreeChecker new.
+	checker model: RBNamespace new.
+	checker on: RBEquivalentMethodSubclassHolder.
+	checker extractedFromSelector: #methodToExtractFrom.
+
+	methodNode := RBParser parseMethod: 'testtest ^ 7 raisedTo: super someNumber'.
+	equivalentTree := checker checkEquivalentTreeFor: methodNode.
+	self assert: equivalentTree isNotNil
+]
+
+{ #category : 'tests' }
+EquivalentTreeTest >> testEquivalentTreeWithMethodHasSuperSendInSuperClassExpectNil [
+
+	| checker methodNode equivalentTree |
+
+	checker := EquivalentTreeChecker new.
+	checker model: RBNamespace new.
+	checker on: RBEquivalentMethodSubclassHolder.
+	checker extractedFromSelector: #methodToExtractFromWithSuperAndSimilarMethodInSuper.
+
+	methodNode := RBParser parseMethod: 'testtest ^ 11 raisedTo: super someNumber factorial'.
+	equivalentTree := checker checkEquivalentTreeFor: methodNode.
+	self assert: equivalentTree isNil
+]
+
 { #category : 'tests' }
 EquivalentTreeTest >> testEquivalentTreeWithMethodNodeWhenUniqueMethodExpectNil [
 
-	| checker  methodNode equivlentTree |
+	| checker  methodNode equivalentTree |
 	checker := EquivalentTreeChecker new.
 	checker model: RBNamespace new.
 	checker on: RBEquivalentMethodSubclassHolder.
-	checker selector: #simpleLocalMethodReturn.
+	checker extractedFromSelector: #simpleLocalMethodReturn.
 
 	methodNode := RBParser parseMethod: 'testMethod: anArg ^ anArg + 7 raisedTo: 2'.
-	equivlentTree := checker checkEquivalentTreeFor: methodNode.
+	equivalentTree := checker checkEquivalentTreeFor: methodNode.
 
-	self assert: equivlentTree isNil
+	self assert: equivalentTree isNil
 
 ]
 
 { #category : 'tests' }
 EquivalentTreeTest >> testEquivalentTreeWithMethodNodeWithDuplicateMethodExpectFound [
 
-	| checker methodNode equivlentTree |
+	| checker methodNode equivalentTree |
 
 	checker := EquivalentTreeChecker new.
 	checker model: RBNamespace new.
 	checker on: RBEquivalentMethodSubclassHolder.
-	checker selector: #simpleLocalMethodReturn.
+	checker extractedFromSelector: #simpleLocalMethodReturn.
 
 	methodNode := RBParser parseMethod: 'testtest ^ 42'.
-	equivlentTree := checker checkEquivalentTreeFor: methodNode.
-	self assert: equivlentTree isNotNil
+	equivalentTree := checker checkEquivalentTreeFor: methodNode.
+	self assert: equivalentTree isNotNil
 ]
 
 { #category : 'tests' }
@@ -74,7 +104,7 @@ EquivalentTreeTest >> testMethodsToBeCheckedExceptSelector [
 	checker := EquivalentTreeChecker new.
 	checker model: model.
 	checker on: RBEquivalentMethodSubclassHolder.
-	checker selector: #simpleLocalMethodReturn.
+	checker extractedFromSelector: #simpleLocalMethodReturn.
 
 	methodsToBeChecked := checker methodsToBeChecked.