Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7022f3c

Browse files
jeciscjecisc
authoredApr 25, 2020
Merge pull request #54 from jecisc/17-Cleaner-notNil--isNotNil-and-notEmpty--isNotEmpty
17-Cleaner-notNil--isNotNil-and-notEmpty--isNotEmpty
2 parents 57a535f + 8d2066b commit 7022f3c

8 files changed

+284
-21
lines changed
 

‎resources/doc/documentation.md

+34
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,40 @@ ifNotNil: aBlock
162162
*Warnings:*
163163
The only danger of this cleaning happens for projects working on multiple Smalltalks
164164

165+
### Methods with alias unification
166+
167+
Chanel unify the use of some methods with alias. Here is the list of rewrites:
168+
169+
| Original | Transformation | Reason |
170+
| ------------- | ------------- | ------------- |
171+
| `x notEmpty` | `x isNotEmpty` | To be coherent with `isEmpty` |
172+
| `x notNil` | `x isNotNil` | To be coherent with `isNil` |
173+
| `x includesAnyOf: y` | `x includesAny: y` | Previous might be deprecated |
174+
| `x includesAllOf: y` | `x includesAll: y` | Previous is deprecated in P9 |
175+
| `x ifNotNilDo: y` | `x ifNotNil: y` | Missleading. `Do:` is usually for iterations |
176+
| `x ifNil: y ifNotNilDo: z` | `x ifNil: y ifNotNil: z` | Missleading. `Do:` is usually for iterations |
177+
| `x ifNotNilDo: y ifNil: z` | `x ifNotNil: y ifNil: z` | Missleading. `Do:` is usually for iterations |
178+
179+
*Conditions for the cleanings to by applied:*
180+
- Can be applied on any classes and traits.
181+
- A pattern from the list above match.
182+
- Does not apply if the application of the pattern would cause an infinit loop. For example it will **not** rewrite:
183+
184+
```Smalltalk
185+
ifNotNil: aBlock
186+
^ self isNil ifFalse: aBlock
187+
```
188+
189+
into:
190+
191+
```Smalltalk
192+
ifNotNil: aBlock
193+
^ self ifNotNil: aBlock
194+
```
195+
196+
*Warnings:*
197+
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.
198+
165199
### Test case names
166200

167201
Chanel rename each test case ending with `Tests` te end with `Test` since this is `a XXTestCase`.

‎src/Chanel-Tests/ChanelMethodAliasesCleanerTest.class.st

+167
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
Class {
2+
#name : #ChanelMethodAliasesCleanerTest,
3+
#superclass : #ChanelAbstractCleanerTest,
4+
#category : #'Chanel-Tests'
5+
}
6+
7+
{ #category : #running }
8+
ChanelMethodAliasesCleanerTest >> setUp [
9+
super setUp.
10+
class := self createDefaultClass
11+
]
12+
13+
{ #category : #tests }
14+
ChanelMethodAliasesCleanerTest >> testDoesNotReplaceIfItIntroduceAnInfinitLoop [
15+
| oldMethod |
16+
class
17+
compile:
18+
'isNotEmpty
19+
^self notEmpty'.
20+
21+
oldMethod := class >> #isNotEmpty.
22+
23+
self runCleaner.
24+
25+
self
26+
assert: (class >> #isNotEmpty) sourceCode
27+
equals:
28+
'isNotEmpty
29+
^self notEmpty'.
30+
31+
self assert: class >> #isNotEmpty identicalTo: oldMethod
32+
]
33+
34+
{ #category : #tests }
35+
ChanelMethodAliasesCleanerTest >> testDoesNotReplaceIfItIntroduceAnInfinitLoop2 [
36+
| oldMethod |
37+
class
38+
compile:
39+
'isNotEmpty
40+
self notEmpty'.
41+
42+
oldMethod := class >> #isNotEmpty.
43+
44+
self runCleaner.
45+
46+
self
47+
assert: (class >> #isNotEmpty) sourceCode
48+
equals:
49+
'isNotEmpty
50+
self notEmpty'.
51+
52+
self assert: class >> #isNotEmpty identicalTo: oldMethod
53+
]
54+
55+
{ #category : #tests }
56+
ChanelMethodAliasesCleanerTest >> testIfNilIfNotNilDo [
57+
self assert: 'nil ifNil: [ false ] ifNotNilDo: [ true ]' isRewrittenAs: 'nil ifNil: [ false ] ifNotNil: [ true ]'
58+
]
59+
60+
{ #category : #tests }
61+
ChanelMethodAliasesCleanerTest >> testIfNotNilDo [
62+
self assert: 'nil ifNotNilDo: [ true ]' isRewrittenAs: 'nil ifNotNil: [ true ]'
63+
]
64+
65+
{ #category : #tests }
66+
ChanelMethodAliasesCleanerTest >> testIfNotNilDoIfNil [
67+
self assert: 'nil ifNotNilDo: [ true ] ifNil: [ false ]' isRewrittenAs: 'nil ifNotNil: [ true ] ifNil: [ false ]'
68+
]
69+
70+
{ #category : #tests }
71+
ChanelMethodAliasesCleanerTest >> testNotEmpty [
72+
self assert: '#() notEmpty' isRewrittenAs: '#() isNotEmpty'
73+
]
74+
75+
{ #category : #tests }
76+
ChanelMethodAliasesCleanerTest >> testNotNil [
77+
self assert: 'nil notNil' isRewrittenAs: 'nil isNotNil'
78+
]
79+
80+
{ #category : #tests }
81+
ChanelMethodAliasesCleanerTest >> testReplacementDoesNotRemoveExtensions [
82+
class
83+
compile:
84+
('{1}
85+
{2}' format: {self selector . '#() notEmpty'})
86+
classified: self extensionProtocol.
87+
88+
self runCleaner.
89+
90+
self
91+
assert: (class >> self selector) sourceCode
92+
equals:
93+
('{1}
94+
{2}' format: {self selector . '#() isNotEmpty'}).
95+
96+
self assert: (class >> self selector) protocol equals: self extensionProtocol
97+
]
98+
99+
{ #category : #tests }
100+
ChanelMethodAliasesCleanerTest >> testReplacementInTraits [
101+
| trait |
102+
trait := self createDefaultTrait.
103+
104+
class setTraitComposition: trait.
105+
106+
trait
107+
compile:
108+
('{1}
109+
{2}' format: {self selector . '#() notEmpty'}).
110+
111+
self runCleaner.
112+
113+
self
114+
assert: (trait >> self selector) sourceCode
115+
equals:
116+
('{1}
117+
{2}' format: {self selector . '#() isNotEmpty'}).
118+
119+
self assert: (trait localSelectors includes: self selector).
120+
self deny: (class localSelectors includes: self selector)
121+
]
122+
123+
{ #category : #tests }
124+
ChanelMethodAliasesCleanerTest >> testReplacementOnClassSide [
125+
class class
126+
compile:
127+
('{1}
128+
{2}' format: {self selector . '#() notEmpty'}).
129+
130+
self runCleaner.
131+
132+
self
133+
assert: (class class >> self selector) sourceCode
134+
equals:
135+
('{1}
136+
{2}' format: {self selector . '#() isNotEmpty'})
137+
]
138+
139+
{ #category : #tests }
140+
ChanelMethodAliasesCleanerTest >> testWithNothingToReplace [
141+
| oldMethod |
142+
class
143+
compile:
144+
('{1}
145+
{2}' format: {self selector . '#() isNotEmpty'}).
146+
147+
oldMethod := class >> self selector.
148+
self runCleaner.
149+
150+
self
151+
assert: (class >> self selector) sourceCode
152+
equals:
153+
('{1}
154+
{2}' format: {self selector . '#() isNotEmpty'}).
155+
156+
self assert: class >> self selector identicalTo: oldMethod
157+
]
158+
159+
{ #category : #tests }
160+
ChanelMethodAliasesCleanerTest >> testincludesAllOf [
161+
self assert: '#() includesAllOf: #()' isRewrittenAs: '#() includesAll: #()'
162+
]
163+
164+
{ #category : #tests }
165+
ChanelMethodAliasesCleanerTest >> testincludesAnyOf [
166+
self assert: '#() includesAnyOf: #()' isRewrittenAs: '#() includesAny: #()'
167+
]

‎src/Chanel-Tests/ChanelNilConditionalSimplifierCleanerTest.class.st

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Class {
1010
{ #category : #running }
1111
ChanelNilConditionalSimplifierCleanerTest >> setUp [
1212
super setUp.
13-
class := self createDefaultTestClass
13+
class := self createDefaultClass
1414
]
1515

1616
{ #category : #tests }

‎src/Chanel/ChanelAbstractCleaner.class.st

-6
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,3 @@ ChanelAbstractCleaner >> configuration [
6767
ChanelAbstractCleaner >> configuration: anObject [
6868
configuration := anObject
6969
]
70-
71-
{ #category : #rewriting }
72-
ChanelAbstractCleaner >> rewriteMethodsOf: classes with: rewriter [
73-
(classes flatCollect: [ :class | class localMethods , class class localMethods ])
74-
do: [ :method | (rewriter executeTree: method ast) ifTrue: [ method installAST ] ]
75-
]

‎src/Chanel/ChanelMethodAliasesCleaner.class.st

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
"
2+
Description
3+
--------------------
4+
5+
I am a cleaner that will replace aliases with only one of the form.
6+
7+
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`.
8+
9+
I'll also replace `ifNotNilDo:` because it is not an iteration and the name is missleading.
10+
"
11+
Class {
12+
#name : #ChanelMethodAliasesCleaner,
13+
#superclass : #ChanelMethodRewriterCleaner,
14+
#category : #Chanel
15+
}
16+
17+
{ #category : #accessing }
18+
ChanelMethodAliasesCleaner class >> priority [
19+
"I need to be high in the priority since I'll simplify next cleaners."
20+
21+
^ 500
22+
]
23+
24+
{ #category : #cleaning }
25+
ChanelMethodAliasesCleaner >> rewriter [
26+
^ RBParseTreeRewriter new
27+
replace: '`@receiver notEmpty' with: '`@receiver isNotEmpty';
28+
replace: '`@receiver notNil' with: '`@receiver isNotNil';
29+
replace: '`@receiver includesAnyOf: `@arg' with: '`@receiver includesAny: `@arg';
30+
replace: '`@receiver includesAllOf: `@arg' with: '`@receiver includesAll: `@arg';
31+
replace: '`@receiver ifNotNilDo: `@arg' with: '`@receiver ifNotNil: `@arg';
32+
replace: '`@receiver ifNil: `@arg ifNotNilDo: `@arg2' with: '`@receiver ifNil: `@arg ifNotNil: `@arg2';
33+
replace: '`@receiver ifNotNilDo: `@arg ifNil: `@arg2' with: '`@receiver ifNotNil: `@arg ifNil: `@arg2';
34+
yourself
35+
]

‎src/Chanel/ChanelMethodRewriterCleaner.class.st

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
"
2+
Description
3+
--------------------
4+
5+
I am an abstract cleaner for cleanings using Pharo's parse tree rewriter to update the contents of some methods.
6+
"
7+
Class {
8+
#name : #ChanelMethodRewriterCleaner,
9+
#superclass : #ChanelAbstractCleaner,
10+
#category : #Chanel
11+
}
12+
13+
{ #category : #testing }
14+
ChanelMethodRewriterCleaner class >> isAbstract [
15+
^ self = ChanelMethodRewriterCleaner
16+
]
17+
18+
{ #category : #cleaning }
19+
ChanelMethodRewriterCleaner >> clean [
20+
| rewriter |
21+
rewriter := self rewriter.
22+
(self scope flatCollect: [ :class | class localMethods , class class localMethods ])
23+
do: [ :method | (rewriter executeTree: method ast) ifTrue: [ method installAST ] ]
24+
]
25+
26+
{ #category : #cleaning }
27+
ChanelMethodRewriterCleaner >> rewriter [
28+
"Return the rewriter to use to clean the methods."
29+
30+
^ self subclassResponsibility
31+
]
32+
33+
{ #category : #cleaning }
34+
ChanelMethodRewriterCleaner >> scope [
35+
"Return all the classes whose methods needs to be cleaned. By default, all of them."
36+
37+
^ self configuration definedClasses
38+
]

‎src/Chanel/ChanelNilConditionalSimplifierCleaner.class.st

+2-7
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ x isNotNil ifFalse: y ifTrue: z ==> x ifNil: y ifNotNil: z
2323
"
2424
Class {
2525
#name : #ChanelNilConditionalSimplifierCleaner,
26-
#superclass : #ChanelAbstractCleaner,
26+
#superclass : #ChanelMethodRewriterCleaner,
2727
#category : #Chanel
2828
}
2929

@@ -33,12 +33,7 @@ ChanelNilConditionalSimplifierCleaner class >> priority [
3333
]
3434

3535
{ #category : #cleaning }
36-
ChanelNilConditionalSimplifierCleaner >> clean [
37-
self rewriteMethodsOf: self configuration definedClasses with: self conditionalsRewriter
38-
]
39-
40-
{ #category : #cleaning }
41-
ChanelNilConditionalSimplifierCleaner >> conditionalsRewriter [
36+
ChanelNilConditionalSimplifierCleaner >> rewriter [
4237
^ RBParseTreeRewriter new
4338
replace: '`@receiver ifNil: [ nil ]' with: '`@receiver';
4439
replace: '`@receiver ifNil: [ nil ] ifNotNil: `@arg' with: '`@receiver ifNotNil: `@arg';

‎src/Chanel/ChanelTestEqualityCleaner.class.st

+7-7
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ x deny: y equals: false ==> x assert: y
3232
"
3333
Class {
3434
#name : #ChanelTestEqualityCleaner,
35-
#superclass : #ChanelAbstractCleaner,
35+
#superclass : #ChanelMethodRewriterCleaner,
3636
#category : #Chanel
3737
}
3838

@@ -42,12 +42,7 @@ ChanelTestEqualityCleaner class >> priority [
4242
]
4343

4444
{ #category : #cleaning }
45-
ChanelTestEqualityCleaner >> clean [
46-
self rewriteMethodsOf: self configuration definedTestCases with: self equalityRewriter
47-
]
48-
49-
{ #category : #cleaning }
50-
ChanelTestEqualityCleaner >> equalityRewriter [
45+
ChanelTestEqualityCleaner >> rewriter [
5146
^ RBParseTreeRewriter new
5247
replace: '`@receiver assert: `@arg = true' with: '`@receiver assert: `@arg';
5348
replace: '`@receiver deny: `@arg = true' with: '`@receiver deny: `@arg';
@@ -72,3 +67,8 @@ ChanelTestEqualityCleaner >> equalityRewriter [
7267
replace: '`@receiver assert: `@arg = true' with: '`@receiver assert: `@arg';
7368
yourself
7469
]
70+
71+
{ #category : #cleaning }
72+
ChanelTestEqualityCleaner >> scope [
73+
^ self configuration definedTestCases
74+
]

0 commit comments

Comments
 (0)
Failed to load comments.