Merge pull request #54 from jecisc/17-Cleaner-notNil--isNotNil-and-no…

…tEmpty--isNotEmpty

17-Cleaner-notNil--isNotNil-and-notEmpty--isNotEmpty

resources/doc/documentation.md

@@ -162,6 +162,40 @@ ifNotNil: aBlock
 *Warnings:*
 The only danger of this cleaning happens for projects working on multiple Smalltalks
 
+### Methods with alias unification
+
+Chanel unify the use of some methods with alias. Here is the list of rewrites:
+
+| Original | Transformation | Reason |
+| ------------- | ------------- | ------------- |
+| `x notEmpty` | `x isNotEmpty` | To be coherent with `isEmpty` |
+| `x notNil` | `x isNotNil` | To be coherent with `isNil` |
+| `x includesAnyOf: y` | `x includesAny: y` | Previous might be deprecated |
+| `x includesAllOf: y` | `x includesAll: y` | Previous is deprecated in P9 |
+| `x ifNotNilDo: y` | `x ifNotNil: y` | Missleading. `Do:` is usually for iterations |
+| `x ifNil: y ifNotNilDo: z` | `x ifNil: y ifNotNil: z` | Missleading. `Do:` is usually for iterations |
+| `x ifNotNilDo: y ifNil: z` | `x ifNotNil: y ifNil: z` | Missleading. `Do:` is usually for iterations |
+
+*Conditions for the cleanings to by applied:*
+- Can be applied on any classes and traits.
+- A pattern from the list above match.
+- Does not apply if the application of the pattern would cause an infinit loop. For example it will **not** rewrite:
+
+```Smalltalk
+ifNotNil: aBlock
+	^ self isNil ifFalse: aBlock
+```
+
+into:
+
+```Smalltalk
+ifNotNil: aBlock
+	^ self ifNotNil: aBlock
+```
+
+*Warnings:*
+The only danger of this cleaning happens for projects working on multiple Smalltalks or is a project implements ont of those methods and do something else than the ones present in Pharo.
+
 ### Test case names
 
 Chanel rename each test case ending with `Tests` te end with `Test` since this is `a XXTestCase`.

src/Chanel-Tests/ChanelMethodAliasesCleanerTest.class.st

@@ -0,0 +1,167 @@
+Class {
+	#name : #ChanelMethodAliasesCleanerTest,
+	#superclass : #ChanelAbstractCleanerTest,
+	#category : #'Chanel-Tests'
+}
+
+{ #category : #running }
+ChanelMethodAliasesCleanerTest >> setUp [
+	super setUp.
+	class := self createDefaultClass
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testDoesNotReplaceIfItIntroduceAnInfinitLoop [
+	| oldMethod |
+	class
+		compile:
+			'isNotEmpty
+  ^self notEmpty'.
+
+	oldMethod := class >> #isNotEmpty.
+
+	self runCleaner.
+
+	self
+		assert: (class >> #isNotEmpty) sourceCode
+		equals:
+			'isNotEmpty
+  ^self notEmpty'.
+
+	self assert: class >> #isNotEmpty identicalTo: oldMethod
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testDoesNotReplaceIfItIntroduceAnInfinitLoop2 [
+	| oldMethod |
+	class
+		compile:
+			'isNotEmpty
+  self notEmpty'.
+
+	oldMethod := class >> #isNotEmpty.
+
+	self runCleaner.
+
+	self
+		assert: (class >> #isNotEmpty) sourceCode
+		equals:
+			'isNotEmpty
+  self notEmpty'.
+
+	self assert: class >> #isNotEmpty identicalTo: oldMethod
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testIfNilIfNotNilDo [
+	self assert: 'nil ifNil: [ false ] ifNotNilDo: [ true ]' isRewrittenAs: 'nil ifNil: [ false ] ifNotNil: [ true ]'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testIfNotNilDo [
+	self assert: 'nil ifNotNilDo: [ true ]' isRewrittenAs: 'nil ifNotNil: [ true ]'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testIfNotNilDoIfNil [
+	self assert: 'nil ifNotNilDo: [ true ] ifNil: [ false ]' isRewrittenAs: 'nil ifNotNil: [ true ] ifNil: [ false ]'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testNotEmpty [
+	self assert: '#() notEmpty' isRewrittenAs: '#() isNotEmpty'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testNotNil [
+	self assert: 'nil notNil' isRewrittenAs: 'nil isNotNil'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testReplacementDoesNotRemoveExtensions [
+	class
+		compile:
+			('{1}
+  {2}' format: {self selector . '#() notEmpty'})
+		classified: self extensionProtocol.
+
+	self runCleaner.
+
+	self
+		assert: (class >> self selector) sourceCode
+		equals:
+			('{1}
+  {2}' format: {self selector . '#() isNotEmpty'}).
+
+	self assert: (class >> self selector) protocol equals: self extensionProtocol
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testReplacementInTraits [
+	| trait |
+	trait := self createDefaultTrait.
+
+	class setTraitComposition: trait.
+
+	trait
+		compile:
+			('{1}
+  {2}' format: {self selector . '#() notEmpty'}).
+
+	self runCleaner.
+
+	self
+		assert: (trait >> self selector) sourceCode
+		equals:
+			('{1}
+  {2}' format: {self selector . '#() isNotEmpty'}).
+
+	self assert: (trait localSelectors includes: self selector).
+	self deny: (class localSelectors includes: self selector)
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testReplacementOnClassSide [
+	class class
+		compile:
+			('{1}
+  {2}' format: {self selector . '#() notEmpty'}).
+
+	self runCleaner.
+
+	self
+		assert: (class class >> self selector) sourceCode
+		equals:
+			('{1}
+  {2}' format: {self selector . '#() isNotEmpty'})
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testWithNothingToReplace [
+	| oldMethod |
+	class
+		compile:
+			('{1}
+	{2}' format: {self selector . '#() isNotEmpty'}).
+
+	oldMethod := class >> self selector.
+	self runCleaner.
+
+	self
+		assert: (class >> self selector) sourceCode
+		equals:
+			('{1}
+	{2}' format: {self selector . '#() isNotEmpty'}).
+
+	self assert: class >> self selector identicalTo: oldMethod
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testincludesAllOf [
+	self assert: '#() includesAllOf: #()' isRewrittenAs: '#() includesAll: #()'
+]
+
+{ #category : #tests }
+ChanelMethodAliasesCleanerTest >> testincludesAnyOf [
+	self assert: '#() includesAnyOf: #()' isRewrittenAs: '#() includesAny: #()'
+]

src/Chanel-Tests/ChanelNilConditionalSimplifierCleanerTest.class.st

@@ -10,7 +10,7 @@ Class {
 { #category : #running }
 ChanelNilConditionalSimplifierCleanerTest >> setUp [
 	super setUp.
-	class := self createDefaultTestClass
+	class := self createDefaultClass
 ]
 
 { #category : #tests }

src/Chanel/ChanelAbstractCleaner.class.st

@@ -67,9 +67,3 @@ ChanelAbstractCleaner >> configuration [
 ChanelAbstractCleaner >> configuration: anObject [
 	configuration := anObject
 ]
-
-{ #category : #rewriting }
-ChanelAbstractCleaner >> rewriteMethodsOf: classes with: rewriter [
-	(classes flatCollect: [ :class | class localMethods , class class localMethods ])
-		do: [ :method | (rewriter executeTree: method ast) ifTrue: [ method installAST ] ]
-]

src/Chanel/ChanelMethodAliasesCleaner.class.st

@@ -0,0 +1,35 @@
+"
+Description
+--------------------
+
+I am a cleaner that will replace aliases with only one of the form. 
+
+For example I'll replace `notEmpty` by `isNotEmpty` and `notNil` by `isNotNil`. The reason is that it is more coherent with `isNil` that cannot be written `nil`.
+
+I'll also replace `ifNotNilDo:` because it is not an iteration and the name is missleading.
+"
+Class {
+	#name : #ChanelMethodAliasesCleaner,
+	#superclass : #ChanelMethodRewriterCleaner,
+	#category : #Chanel
+}
+
+{ #category : #accessing }
+ChanelMethodAliasesCleaner class >> priority [
+	"I need to be high in the priority since I'll simplify next cleaners."
+
+	^ 500
+]
+
+{ #category : #cleaning }
+ChanelMethodAliasesCleaner >> rewriter [
+	^ RBParseTreeRewriter new
+		replace: '`@receiver notEmpty' with: '`@receiver isNotEmpty';
+		replace: '`@receiver notNil' with: '`@receiver isNotNil';
+		replace: '`@receiver includesAnyOf: `@arg' with: '`@receiver includesAny: `@arg';
+		replace: '`@receiver includesAllOf: `@arg' with: '`@receiver includesAll: `@arg';
+		replace: '`@receiver ifNotNilDo: `@arg' with: '`@receiver ifNotNil: `@arg';
+		replace: '`@receiver ifNil: `@arg ifNotNilDo: `@arg2' with: '`@receiver ifNil: `@arg ifNotNil: `@arg2';
+		replace: '`@receiver ifNotNilDo: `@arg ifNil: `@arg2' with: '`@receiver ifNotNil: `@arg ifNil: `@arg2';
+		yourself
+]

src/Chanel/ChanelMethodRewriterCleaner.class.st

@@ -0,0 +1,38 @@
+"
+Description
+--------------------
+
+I am an abstract cleaner for cleanings using Pharo's parse tree rewriter to update the contents of some methods.
+"
+Class {
+	#name : #ChanelMethodRewriterCleaner,
+	#superclass : #ChanelAbstractCleaner,
+	#category : #Chanel
+}
+
+{ #category : #testing }
+ChanelMethodRewriterCleaner class >> isAbstract [
+	^ self = ChanelMethodRewriterCleaner
+]
+
+{ #category : #cleaning }
+ChanelMethodRewriterCleaner >> clean [
+	| rewriter |
+	rewriter := self rewriter.
+	(self scope flatCollect: [ :class | class localMethods , class class localMethods ])
+		do: [ :method | (rewriter executeTree: method ast) ifTrue: [ method installAST ] ]
+]
+
+{ #category : #cleaning }
+ChanelMethodRewriterCleaner >> rewriter [
+	"Return the rewriter to use to clean the methods."
+
+	^ self subclassResponsibility
+]
+
+{ #category : #cleaning }
+ChanelMethodRewriterCleaner >> scope [
+	"Return all the classes whose methods needs to be cleaned. By default, all of them."
+
+	^ self configuration definedClasses
+]

src/Chanel/ChanelNilConditionalSimplifierCleaner.class.st

@@ -23,7 +23,7 @@ x isNotNil ifFalse: y ifTrue: z 		==> x ifNil: y ifNotNil: z
 "
 Class {
 	#name : #ChanelNilConditionalSimplifierCleaner,
-	#superclass : #ChanelAbstractCleaner,
+	#superclass : #ChanelMethodRewriterCleaner,
 	#category : #Chanel
 }
 
@@ -33,12 +33,7 @@ ChanelNilConditionalSimplifierCleaner class >> priority [
 ]
 
 { #category : #cleaning }
-ChanelNilConditionalSimplifierCleaner >> clean [
-	self rewriteMethodsOf: self configuration definedClasses with: self conditionalsRewriter
-]
-
-{ #category : #cleaning }
-ChanelNilConditionalSimplifierCleaner >> conditionalsRewriter [
+ChanelNilConditionalSimplifierCleaner >> rewriter [
 	^ RBParseTreeRewriter new
 		replace: '`@receiver ifNil: [ nil ]' with: '`@receiver';
 		replace: '`@receiver ifNil: [ nil ] ifNotNil: `@arg' with: '`@receiver ifNotNil: `@arg';

src/Chanel/ChanelTestEqualityCleaner.class.st

@@ -32,7 +32,7 @@ x deny: y equals: false 			==> x assert: y
 "
 Class {
 	#name : #ChanelTestEqualityCleaner,
-	#superclass : #ChanelAbstractCleaner,
+	#superclass : #ChanelMethodRewriterCleaner,
 	#category : #Chanel
 }
 
@@ -42,12 +42,7 @@ ChanelTestEqualityCleaner class >> priority [
 ]
 
 { #category : #cleaning }
-ChanelTestEqualityCleaner >> clean [
-	self rewriteMethodsOf: self configuration definedTestCases with: self equalityRewriter
-]
-
-{ #category : #cleaning }
-ChanelTestEqualityCleaner >> equalityRewriter [
+ChanelTestEqualityCleaner >> rewriter [
 	^ RBParseTreeRewriter new
 		replace: '`@receiver assert: `@arg = true' with: '`@receiver assert: `@arg';
 		replace: '`@receiver deny: `@arg = true' with: '`@receiver deny: `@arg';
@@ -72,3 +67,8 @@ ChanelTestEqualityCleaner >> equalityRewriter [
 		replace: '`@receiver assert: `@arg = true' with: '`@receiver assert: `@arg';
 		yourself
 ]
+
+{ #category : #cleaning }
+ChanelTestEqualityCleaner >> scope [
+	^ self configuration definedTestCases
+]