Skip to content

Commit

Permalink
Merge pull request #80 from jecisc/78-Introduce-a-cleaner-removing-us…
Browse files Browse the repository at this point in the history
…eless-conditional-branches

78-Introduce-a-cleaner-removing-useless-conditional-branches
  • Loading branch information
jecisc committed May 6, 2020
2 parents 65e9daa + 15c77d2 commit 96b851e
Show file tree
Hide file tree
Showing 9 changed files with 328 additions and 104 deletions.
43 changes: 29 additions & 14 deletions resources/doc/documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,20 +145,6 @@ Chanel simplifies conditionals. For example it will rewrite:
| `x isNil ifFalse: y ifTrue: z` | `x ifNil: z ifNotNil: y` |
| `x isNotNil ifTrue: y ifFalse: z` | `x ifNil: z ifNotNil: y` |
| `x isNotNil ifFalse: y ifTrue: z` | `x ifNil: y ifNotNil: z` |
| `x ifNil: [ nil ]` | `x` |
| `x ifNil: [ nil ] ifNotNil: y` | `x ifNotNil: y` |
| `x ifNotNil: y ifNil: [ nil ]` | `x ifNotNil: y` |
| `x ifNil: nil` | `x` |
| `x ifNil: nil ifNotNil: y` | `x ifNotNil: y` |
| `x ifNotNil: y ifNil: nil` | `x ifNotNil: y` |
| `x isNil ifTrue: [ nil ] ifFalse: y` | `x ifNotNil: y` |
| `x isNil ifTrue: nil ifFalse: y` | `x ifNotNil: y` |
| `x isNil ifFalse: y ifTrue: [ nil ]` | `x ifNotNil: y` |
| `x isNil ifFalse: y ifTrue: nil` | `x ifNotNil: y` |
| `x isNotNil ifTrue: y ifFalse: [ nil ]` | `x ifNotNil: y` |
| `x isNotNil ifTrue: y ifFalse: nil` | `x ifNotNil: y` |
| `x isNotNil ifFalse: [ nil ] ifTrue: y ` | `x ifNotNil: y` |
| `x isNotNil ifFalse: nil ifTrue: y` | `x ifNotNil: y` |

*Conditions for the cleanings to by applied:*
- Can be applied on any classes and traits.
Expand Down Expand Up @@ -229,6 +215,35 @@ This cleaner might introduce problems in two cases:
- You project works on multiple smalltalks with different APIs
- You have your own objects implementing #isEmpty or #isNotEmpty but not the conditionals.

### Cut conditionals branches

Chanel simplifies some conditionals by cutting branches. For example it will rewrite:

| Original | Transformation |
| ------------- | ------------- |
| `x ifNil: [ nil ]` | `x` |
| `x ifNil: [ nil ] ifNotNil: y` | `x ifNotNil: y` |
| `x ifNotNil: y ifNil: [ nil ]` | `x ifNotNil: y` |
| `x ifNil: nil` | `x` |
| `x ifNil: nil ifNotNil: y` | `x ifNotNil: y` |
| `x ifNotNil: y ifNil: nil` | `x ifNotNil: y` |
| `x ifTrue: [ true ] ifFalse: [ false ]` | `x` |
| `x ifTrue: [ false ] ifFalse: [ true ]` | `x not` |
| `x ifFalse: [ false ] ifTrue: [ true ]` | `x` |
| `x ifFalse: [ true ] ifTrue: [ false ]` | `x not` |
| `x ifNotNil: [ x ]` | `x` |
| `x ifNotNil: [ x ] ifNil: y` | `x ifNil: y` |
| `x ifNil: y ifNotNil: [ x ]` | `x ifNil: y` |

*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:

*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:
Expand Down
58 changes: 58 additions & 0 deletions src/Chanel-Tests/ChanelCleanersOrderTest.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,64 @@ ChanelCleanersOrderTest >> testAliasBeforeNilConditionals [
{2}' format: {self selector . '10 ifNotNil: [ 1 ]'})
]

{ #category : #tests }
ChanelCleanersOrderTest >> testAliasBeforeNilConditionalsBeforeCutConditionalBranches [
"The alias cleaner needs to run before the nil conditionals cleaner which need to run before the cut conditional branches."

class
compile:
('{1}
{2}' format: {self selector . '10 notNil ifFalse: [ nil ]'}).

Chanel perfume: {package} using: {ChanelNilConditionalSimplifierCleaner . ChanelCutConditionalBranchesCleaner . ChanelMethodAliasesCleaner}.

self
assert: (class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10'})
]

{ #category : #tests }
ChanelCleanersOrderTest >> testAliasBeforeNilConditionalsBeforeCutConditionalBranches2 [
"The alias cleaner needs to run before the nil conditionals cleaner which need to run before the cut conditional branches."

class
compile:
('{1}
{2}' format: {self selector . '10 notNil ifFalse: nil'}).

Chanel perfume: {package} using: {ChanelNilConditionalSimplifierCleaner . ChanelCutConditionalBranchesCleaner . ChanelMethodAliasesCleaner}.

self
assert: (class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10'})
]

{ #category : #tests }
ChanelCleanersOrderTest >> testAliasBeforeNilConditionalsBeforeCutConditionalBranchesBeforeUnreadTemporaries [
"The alias cleaner needs to run before the nil conditionals cleaner which need to run before the cut conditional branches which needs to run before the remove unused temporaries."

class
compile:
('{1}
{2}' format: {self selector . '| x |
x := self patate.
x notNil ifFalse: [ nil ].
^3'}).

Chanel perfume: {package} using: {ChanelNilConditionalSimplifierCleaner . ChanelUnreadTemporaryCleaner .ChanelCutConditionalBranchesCleaner . ChanelMethodAliasesCleaner}.

self
assert: (class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . 'self patate.
^3'})
]

{ #category : #tests }
ChanelCleanersOrderTest >> testUnreadTemporariesBeforeUselessASTNodes [
"The cleaner removing the useless AST nodes need to run after the cleaner removing unread temporaries because it will remove more nodes that way."
Expand Down
179 changes: 179 additions & 0 deletions src/Chanel-Tests/ChanelCutConditionalBranchesCleanerTest.class.st
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
"
A ChanelCutConditionalBranchesCleanerTest is a test class for testing the behavior of ChanelCutConditionalBranchesCleaner
"
Class {
#name : #ChanelCutConditionalBranchesCleanerTest,
#superclass : #ChanelAbstractCleanerTest,
#category : #'Chanel-Tests'
}

{ #category : #running }
ChanelCutConditionalBranchesCleanerTest >> setUp [
super setUp.
class := self createDefaultClass
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testDoesNotReplaceIfItIntroduceAnInfinitLoop [
| oldMethod |
class
compile:
'ifNotNil: aBlock
^self ifNil: [ nil ] ifNotNil: aBlock'.

oldMethod := class >> #ifNotNil:.

self runCleaner.

self
assert: (class >> #ifNotNil:) sourceCode
equals:
'ifNotNil: aBlock
^self ifNil: [ nil ] ifNotNil: aBlock'.

self assert: class >> #ifNotNil: identicalTo: oldMethod
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfFalseIfTrue [
self assert: 'true ifFalse: [ false ] ifTrue: [ true ]' isRewrittenAs: 'true'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfFalseIfTrue2 [
self assert: 'true ifFalse: [ true ] ifTrue: [ false ]' isRewrittenAs: 'true not'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNil [
self assert: '10 ifNil: [ nil ]' isRewrittenAs: '10'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNil2 [
self assert: '10 ifNil: nil' isRewrittenAs: '10'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNilIfNotNil [
self assert: '10 ifNil: [ nil ] ifNotNil: #even' isRewrittenAs: '10 ifNotNil: #even'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNilIfNotNil2 [
self assert: '10 ifNil: nil ifNotNil: #even' isRewrittenAs: '10 ifNotNil: #even'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNilIfNotNilReturningReceiver [
self assert: 'self toto ifNil: [ self tata ] ifNotNil: [ self toto ]' isRewrittenAs: 'self toto ifNil: [ self tata ]'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNotNilIfNil [
self assert: '10 ifNotNil: #even ifNil: [ nil ]' isRewrittenAs: '10 ifNotNil: #even'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNotNilIfNil2 [
self assert: '10 ifNotNil: #even ifNil: nil' isRewrittenAs: '10 ifNotNil: #even'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNotNilIfNilReturningReceiver [
self assert: 'self toto ifNotNil: [ self toto ] ifNil: [ self tata ]' isRewrittenAs: 'self toto ifNil: [ self tata ]'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfNotNilReturningReceiver [
self assert: 'self toto ifNotNil: [ self toto ]' isRewrittenAs: 'self toto'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfTrueIfFalse [
self assert: 'true ifTrue: [ true ] ifFalse: [ false ]' isRewrittenAs: 'true'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testIfTrueIfFalse2 [
self assert: 'true ifTrue: [ false ] ifFalse: [ true ]' isRewrittenAs: 'true not'
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testReplacementDoesNotRemoveExtensions [
class
compile:
('{1}
{2}' format: {self selector . '10 ifNil: [ nil ]'})
classified: self extensionProtocol.

self runCleaner.

self
assert: (class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10'}).

self assert: (class >> self selector) protocol equals: self extensionProtocol
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testReplacementInTraits [
| trait |
trait := self createDefaultTrait.

class setTraitComposition: trait.

trait
compile:
('{1}
{2}' format: {self selector . '10 ifNil: [ nil ]'}).

self runCleaner.

self
assert: (trait >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10'}).

self assert: (trait localSelectors includes: self selector).
self deny: (class localSelectors includes: self selector)
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testReplacementOnClassSide [
class class
compile:
('{1}
{2}' format: {self selector . '10 ifNil: [ nil ]'}).

self runCleaner.

self
assert: (class class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10'})
]

{ #category : #tests }
ChanelCutConditionalBranchesCleanerTest >> testWithNothingToReplace [
| oldMethod |
class
compile:
('{1}
{2}' format: {self selector . '10 ifNotNil: [ false ]'}).

oldMethod := class >> self selector.
self runCleaner.

self
assert: (class >> self selector) sourceCode
equals:
('{1}
{2}' format: {self selector . '10 ifNotNil: [ false ]'}).

self assert: class >> self selector identicalTo: oldMethod
]

0 comments on commit 96b851e

Please sign in to comment.