96-Reapply-ChanelExtractReturnFromAllBranchesCleanerTest-on-methods-who-were-cleaned

src/Chanel-Tests/ChanelExtractReturnFromAllBranchesCleanerTest.class.st

@@ -136,6 +136,17 @@ ChanelExtractReturnFromAllBranchesCleanerTest >> testDoesNotExtractReturnIfRetur
 	self assert: oldMethod identicalTo: class >> #method
 ]
 
+{ #category : #tests }
+ChanelExtractReturnFromAllBranchesCleanerTest >> testExtractReturnDoesNotFailForConditonsInConditions [
+	class compile: 'method
+	self toto1 ifTrue: [ self toto2 ifTrue: [ ^ 1 ] ifFalse: [ ^ 2 ] ] ifFalse: [ 3 ]'.
+
+	self runCleaner.
+
+	self assert: (class >> #method) sourceCode equals: 'method
+  self toto1 ifTrue: [ ^self toto2 ifTrue: [ 1 ] ifFalse: [ 2 ] ] ifFalse: [ 3 ]'
+]
+
 { #category : #tests }
 ChanelExtractReturnFromAllBranchesCleanerTest >> testExtractReturnDoesNotFailIfThereIsAlreadyAReturn [
 	class compile: 'method
@@ -249,6 +260,17 @@ ChanelExtractReturnFromAllBranchesCleanerTest >> testExtractReturnIfTheLastState
   self bar'
 ]
 
+{ #category : #tests }
+ChanelExtractReturnFromAllBranchesCleanerTest >> testExtractReturnInNestedConditions [
+	class compile: 'method
+	self toto1 ifTrue: [ self toto2 ifTrue: [ ^1 ] ifFalse: [ ^2 ] ] ifFalse: [ ^3 ]'.
+
+	self runCleaner.
+
+	self assert: (class >> #method) sourceCode equals: 'method
+  ^self toto1 ifTrue: [ self toto2 ifTrue: [ 1 ] ifFalse: [ 2 ] ] ifFalse: [ 3 ]'
+]
+
 { #category : #tests }
 ChanelExtractReturnFromAllBranchesCleanerTest >> testExtractReturnInTrait [
 	| trait |

src/Chanel/ChanelExtractReturnFromAllBranchesCleaner.class.st

@@ -17,17 +17,23 @@ ChanelExtractReturnFromAllBranchesCleaner class >> priority [
 
 { #category : #cleaning }
 ChanelExtractReturnFromAllBranchesCleaner >> clean [
-	(self configuration localMethods iterator
-		| #ast collectIt
+  (self cleanASTs: (self configuration localMethods collect: #ast)) do: #install
+]
+
+{ #category : #cleaning }
+ChanelExtractReturnFromAllBranchesCleaner >> cleanASTs: aCollectionOfMethods [
+	"In the end we run again the cleaning on ASTs because we can have case of nested conditionals each of the having returns in their branches."
+
+	^ (aCollectionOfMethods iterator
 		| #allChildren flatCollectIt
 		| #isMessage selectIt
 		| #isCascaded rejectIt
 		| [ :node | node parent isLast: node ] selectIt
 		| #isConditionNecessarilyExecutingABranch selectIt
 		| [ :node | node arguments allSatisfy: #isBlock ] selectIt
-		| [ :node | node arguments allSatisfy: #hasBlockReturn ] selectIt
+		| [ :node | node arguments allSatisfy: #lastStatementIsReturn ] selectIt
 		| [ :node | node arguments do: #inlineLastReturn ] doIt
 		| #wrapsInReturn doIt
 		| #methodNode collectIt
-		> Set) do: #install
+		> Set) ifNotEmpty: [ :updatedASTs | self cleanASTs: updatedASTs. updatedASTs ]
 ]

src/Chanel/RBBlockNode.extension.st

@@ -4,3 +4,8 @@ Extension { #name : #RBBlockNode }
 RBBlockNode >> inlineLastReturn [
 	self statements last inline
 ]
+
+{ #category : #'*Chanel' }
+RBBlockNode >> lastStatementIsReturn [
+	^ self statements last isReturn
+]