Add tests, improve find occurrences when the method has return

src/AST-Core/RBProgramNode.class.st

@@ -784,7 +784,9 @@ RBProgramNode >> matchList: matchNodes index: matchIndex against: programNodes i
 			[currentDictionary := aDictionary copy.
 			programNodes size < currentIndex or: 
 					[nodes := programNodes copyFrom: programIndex to: currentIndex.
-					(nodes allSatisfy: [:n | (currentDictionary at: node ifAbsentPut: [n]) = n]) and: 
+					(nodes size = 1 ifTrue: [ (currentDictionary at: node ifAbsentPut: [ nodes first ]) = nodes first ]
+					ifFalse:[
+					(currentDictionary at: node ifAbsentPut: [nodes]) = nodes]) and: 
 							[(self 
 								matchList: matchNodes
 								index: matchIndex + 1

src/Refactoring-Core/RBExtractMethodRefactoring.class.st

@@ -272,6 +272,7 @@ RBExtractMethodRefactoring >> isParseTreeEquivalentTo: aSelector [
 	tree := definingClass parseTreeFor: aSelector.
 	tree ifNil: [^false].
 	tree isPrimitive ifTrue: [^false].
+	needsReturn ifFalse: [ tree := self removeReturnsOf: tree ].
 	(tree body equalTo: extractedParseTree body
 		exceptForVariables: (tree arguments collect: [:each | each name])) 
 			ifFalse: [^false].
@@ -353,7 +354,7 @@ RBExtractMethodRefactoring >> placeholderNode [
 RBExtractMethodRefactoring >> preconditions [
 	^(RBCondition definesSelector: selector in: class) 
 		& (RBCondition withBlock: 
-					[self extractMethod.
+					[ self extractMethod.
 					self checkSpecialExtractions.
 					self checkReturn.
 					needsReturn ifTrue: [extractedParseTree addReturn].
@@ -391,6 +392,7 @@ RBExtractMethodRefactoring >> renameParameterWith: oldName to: newName [
 RBExtractMethodRefactoring >> reorderParametersToMatch: aSelector [ 
 	| tree dictionary |
 	tree := class parseTreeFor: aSelector.
+	needsReturn ifFalse: [ tree := self removeReturnsOf: tree ].
 	dictionary := Dictionary new.
 	tree body equalTo: extractedParseTree body withMapping: dictionary.
 	parameters := tree arguments collect: 

src/Refactoring-Core/RBFindAndReplaceRefactoring.class.st

@@ -97,7 +97,7 @@ RBFindAndReplaceRefactoring >> extract: occurrence of: rbMethod [
 	refactoring setOption: #existingSelector toUse:  [ :ref | 
 			ref parameters: (self argumentsOf: occurrence value).
 			selector].
-	self performCompositeRefactoring: refactoring ] on: Exception do: [ :e | e ]
+	self performCompositeRefactoring: refactoring ] on: Exception do: [ :e | ]
 ]
 
 { #category : #accessing }

src/Refactoring-Core/RBInlineMethodRefactoring.class.st

@@ -183,7 +183,7 @@ RBInlineMethodRefactoring >> insertInlinedMethod [
 			node := node parent].
 	node parent isReturn 
 		ifTrue: [node := node parent]
-		ifFalse: [self removeReturns].
+		ifFalse: [inlineParseTree := self removeReturnsOf: inlineParseTree].
 	self replaceArguments.
 	self inlineSourceReplacing: node.
 	sourceParseTree removeDeadCode.
@@ -334,15 +334,6 @@ RBInlineMethodRefactoring >> removeImmediateBlocks [
 		ifTrue: [sourceParseTree := rewriter tree]
 ]
 
-{ #category : #transforming }
-RBInlineMethodRefactoring >> removeReturns [
-	| rewriter |
-	rewriter := self parseTreeRewriter.
-	rewriter replace: '^``@object' with: '``@object'.
-	(rewriter executeTree: inlineParseTree) 
-		ifTrue: [inlineParseTree := rewriter tree]
-]
-
 { #category : #transforming }
 RBInlineMethodRefactoring >> renameConflictingTemporaries [
 	inlineParseTree allDefinedVariables 
@@ -411,7 +402,7 @@ RBInlineMethodRefactoring >> rewriteCascadedMessage [
 				detect: [:i | sourceMessage == (messages at: i)]
 				ifNone: [0].
 	inlineParseTree body addNodesFirst: (messages copyFrom: 1 to: index - 1).
-	self removeReturns.
+	inlineParseTree := self removeReturnsOf: inlineParseTree.
 	inlineParseTree body 
 		addNodes: (messages copyFrom: index + 1 to: messages size).
 	inlineParseTree addReturn

src/Refactoring-Core/RBRefactoring.class.st

@@ -390,6 +390,16 @@ RBRefactoring >> refactoringWarning: aString [
 	^ RBRefactoringWarning signal: aString
 ]
 
+{ #category : #removing }
+RBRefactoring >> removeReturnsOf: parseTree [
+	| rewriter |
+	rewriter := self parseTreeRewriter.
+	rewriter replace: '^``@object' with: '``@object'.
+	(rewriter executeTree: parseTree) 
+		ifTrue: [ ^ rewriter tree].
+	^ parseTree
+]
+
 { #category : #requests }
 RBRefactoring >> requestImplementorToInline: implementorsCollection [ 
 	^(self options at: #implementorToInline) value: self

src/Refactoring-Tests-Core/RBExtractMethodAndOccurrencesTest.class.st

@@ -24,7 +24,28 @@ RBExtractMethodAndOccurrencesTest >> testBadInterval [
 ]
 
 { #category : #tests }
-RBExtractMethodAndOccurrencesTest >> testExtractMethodWithArgsAndOcurrences [
+RBExtractMethodAndOccurrencesTest >> testExtractMethodWithOneArgAndOcurrences [
+	|class refactoring|
+	class := model classNamed: #MyClassC.
+	refactoring := RBExtractMethodAndOccurrences
+				model: model 
+				extract: (28 to: 77)
+				from: #visitMonospace:
+				in: class.
+	self setupShouldUseExistingMethodFor: refactoring toReturn: true.
+	self setupSearchInAllHierarchyFor: refactoring toReturn: true.
+	self executeRefactoring: refactoring.
+	self assert: (class parseTreeFor: #visitMonospace:) equals: (self parseMethod: 'visitMonospace: aFormat
+	self visitParagraph: aFormat.
+	self fixBlockWithoutMicText: aFormat').
+	self assert: (class parseTreeFor: #visitStrike:) equals: (self parseMethod: 'visitStrike: aFormat
+	self visitParagraph: aFormat.
+	self fixBlockWithoutMicText: aFormat.
+	#( 4 5 2 ) do: [ :val | self visitParagraph: val ]').
+]
+
+{ #category : #tests }
+RBExtractMethodAndOccurrencesTest >> testExtractMethodWithTwoArgsAndOcurrences [
 	|class refactoring|
 	class := model classNamed: #MyClassA.
 	refactoring := RBExtractMethodAndOccurrences

src/Refactoring-Tests-Core/RBRefactoringTest.class.st

@@ -139,7 +139,7 @@ m
 { #category : #private }
 RBRefactoringTest >> extractMethodTestData [
 	| newModel classEnvironment classes |
-	classes := #(#MyClassA #MyClassB ) 
+	classes := #(#MyClassA #MyClassB #MyClassC ) 
 		inject: OrderedCollection new 
 		into: [ :sum :each | 
 			testingEnvironment at: each ifPresent: [ :class | 
@@ -220,7 +220,17 @@ RBRefactoringTest >> extractMethodTestData [
 		#('methodWithArg: anArg
 			(currentChar isLetter and: [anArg isDecimal])
 			ifTrue: [^ self].
-			^ nil' #tests)))) 
+			^ nil' #tests)
+		#('visitMonospace: aFormat
+			aFormat children do: [ :each | each accept: self ].
+			self fixBlockWithoutMicText: aFormat' #tests)
+		#('visitStrike: aFormat
+			aFormat children do: [ :each | each accept: self ].
+			self fixBlockWithoutMicText: aFormat.
+			#( 4 5 2 ) do: [ :val | val children do: [ :each | each accept: self ] ]' #tests)
+		#('visitParagraph: aParagraph
+			^ aParagraph children do: [:each | each accept: self ]' #tests)
+		)))
 		do: [:each | 
 			| class |
 			class := newModel classNamed: each first.
@@ -515,6 +525,14 @@ RBRefactoringTest >> setupSelfArgumentNameFor: aRefactoring toReturn: aString [
 	aRefactoring options: options
 ]
 
+{ #category : #'set up' }
+RBRefactoringTest >> setupShouldUseExistingMethodFor: aRefactoring toReturn: aBoolean [ 
+	| options |
+	options := aRefactoring options copy.
+	options at: #useExistingMethod put: [ :ref :cls | aBoolean ].
+	aRefactoring options: options
+]
+
 { #category : #'set up' }
 RBRefactoringTest >> setupVariableToMoveToFor: aRefactoring toReturn: aString [ 
 	| options |

src/SystemCommands-MessageCommands/SycFindAndReplaceMethodCommand.class.st

@@ -46,7 +46,7 @@ SycFindAndReplaceMethodCommand >> defaultMenuItemName [
 
 { #category : #testing }
 SycFindAndReplaceMethodCommand >> isComplexRefactoring [
-	^false
+	^ true
 ]
 
 { #category : #execution }