Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions src/Refactoring-Core/RBSplitCascadeRefactoring.class.st
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,31 @@ RBSplitCascadeRefactoring >> split: anInterval from: aSelector in: aClass [

{ #category : 'transforming' }
RBSplitCascadeRefactoring >> splitCascade [
ancestorNode parent
addNode: (beforeNodes size > 1
ifTrue: [ OCCascadeNode messages: beforeNodes ]
ifFalse: [ beforeNodes first ])
before: ancestorNode.
afterNodes size > 1
ifTrue: [ cascadeNode messages: afterNodes ]
ifFalse: [ cascadeNode replaceWith: afterNodes first ].
class compileTree: ancestorNode methodNode
ancestorNode parent
addNode: (beforeNodes size > 1
ifTrue: [ OCCascadeNode messages: beforeNodes ]
ifFalse: [ beforeNodes first ])
before: ancestorNode.
ancestorNode isAssignment
ifTrue: [
"Flatten remaining part for assignment"
| allMessages |
allMessages := cascadeNode messages.
(allMessages copyFrom: beforeNodes size + 1 to: allMessages size - 1)
do: [ :each |
| copy |
copy := each copy.
copy receiver: cascadeNode receiver.
ancestorNode parent
addNode: copy
before: ancestorNode
].
cascadeNode replaceWith: allMessages last.
]
ifFalse: [
afterNodes size > 1
ifTrue: [ cascadeNode messages: afterNodes ]
ifFalse: [ cascadeNode replaceWith: afterNodes first ].
].
class compileTree: ancestorNode methodNode
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
Class {
#name : 'RBCreateCascadeRefactoringTest',
#superclass : 'RBAbstractTransformationTest',
#instVars : [
'class'
],
#category : 'Refactoring-Transformations-Tests-Test',
#package : 'Refactoring-Transformations-Tests',
#tag : 'Test'
}

{ #category : 'utilities' }
RBCreateCascadeRefactoringTest >> createCascadeFrom: sourceToCombine in: source [

| method start |
method := class compile: source classified: '#test data'.
start := method source findString: sourceToCombine.
^ RBCreateCascadeRefactoring
model: model
combine: (start to: (start + sourceToCombine size - 1))
from: #example
in: class
]

{ #category : 'running' }
RBCreateCascadeRefactoringTest >> setUp [

super setUp.
model := RBNamespace new.
model defineClass: [ :aBuilder |
aBuilder
superclass: Object;
name: #TestClass ].
class := model classNamed: #TestClass.
]

{ #category : 'tests' }
RBCreateCascadeRefactoringTest >> testAssignmentOnlyLast [

| ref |
ref := self
createCascadeFrom: 'obj foo.
x := obj bar.'
in: 'example
obj foo.
x := obj bar.'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
x := obj
foo;
bar.')
]

{ #category : 'tests' }
RBCreateCascadeRefactoringTest >> testCascadeFlattening [

| ref |
ref := self
createCascadeFrom: 'obj foo; bar.
obj baz.'
in: 'example
obj foo; bar.
obj baz.'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj
foo;
bar;
baz.')
]

{ #category : 'tests' }
RBCreateCascadeRefactoringTest >> testCreateCascadeBasic [

| ref |
ref := self
createCascadeFrom: 'obj foo.
obj bar.'
in: 'example
obj foo.
obj bar.'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj
foo;
bar.')
]

{ #category : 'failure tests' }
RBCreateCascadeRefactoringTest >> testDifferentReceiversFail [

| ref |
ref := self
createCascadeFrom: 'obj1 foo.
obj2 bar.'
in: 'example
obj1 foo.
obj2 bar.'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]

{ #category : 'tests' }
RBCreateCascadeRefactoringTest >> testMultipleAssignmentsFail [

| ref |
ref := self
createCascadeFrom: 'x := obj foo.
x := obj bar.'
in: 'example
x := obj foo.
x := obj bar.'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]

{ #category : 'failure tests' }
RBCreateCascadeRefactoringTest >> testSingleStatementFails [

| ref |
ref := self
createCascadeFrom: 'obj foo.'
in: 'example
obj foo.'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
Class {
#name : 'RBSplitCascadeRefactoringTest',
#superclass : 'RBAbstractTransformationTest',
#instVars : [
'class'
],
#category : 'Refactoring-Transformations-Tests-Test',
#package : 'Refactoring-Transformations-Tests',
#tag : 'Test'
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the impression that the from: argument is not that great

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i changed it to #example currently. if you may share the reasons behind, i will try to think of more.

{ #category : 'utilities' }
RBSplitCascadeRefactoringTest >> createCascadeFrom: sourceToCombine in: source [

| method start |
method := class compile: source classified: '#test data'.
start := method source findString: sourceToCombine.
^ RBCreateCascadeRefactoring
model: model
combine: (start to: (start + sourceToCombine size - 1))
from: #example
in: class
]

{ #category : 'running' }
RBSplitCascadeRefactoringTest >> setUp [

super setUp.
model := RBNamespace new.
model defineClass: [ :aBuilder |
aBuilder
superclass: Object;
name: #TestClass ].
class := model classNamed: #TestClass.
]

{ #category : 'utilities' }
RBSplitCascadeRefactoringTest >> splitCascadeFrom: sourceToSplit in: source [

| method start |
method := class compile: source classified: '#test data'.
start := method source findString: sourceToSplit.
^ RBSplitCascadeRefactoring
model: model
split: (start to: (start + sourceToSplit size - 1))
from: #example
in: class
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testAssignmentSplit [

| ref |
ref := self
splitCascadeFrom: ';'
in: 'example
x := obj foo;
bar;
baz'.
ref generateChanges.

self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj foo.
obj bar.
x := obj baz')
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the previous example is not really good.
I would expect

obj foo.
obj bar.
x := obj baz

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that should be the expected behaviour. but i see the method implementation does not support complete flattening and assigning the last, rather it depends on where the cursor is placed.


{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testBasicSplit [

| ref |
ref := self
splitCascadeFrom: ';'
in: 'example
obj foo;
bar;
baz'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj foo.
obj
bar;
baz.')
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testCreateThenSplitRoundTrip [

| createRef splitRef originalMethod |
originalMethod := self parseMethod: 'example
obj foo.
obj bar.'.
createRef := self
createCascadeFrom: 'obj foo.
obj bar.'
in: 'example
obj foo.
obj bar.'.
createRef generateChanges.
splitRef := self
splitCascadeFrom: ';'
in: ((class parseTreeForSelector: #example) source).
splitRef generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals: originalMethod
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testInvalidSplitPosition [

| ref |
ref := self
splitCascadeFrom: 'foo'
in: 'example
obj foo;
bar;
baz'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me this would pass, I'd split after first message here, better UX too.

I'm not saying we should implement this in this PR.


{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testNotInCascade [

| ref |
ref := self
splitCascadeFrom: 'foo'
in: 'example
obj foo.
obj bar.'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testReturnSplit [

| ref |
ref := self
splitCascadeFrom: ';'
in: 'example
^ obj foo;
bar;
baz'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj foo.
^ obj
bar;
baz.')
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testSplitSingleMessage [

| ref |
ref := self
splitCascadeFrom: ';'
in: 'example
obj foo;
bar'.
ref generateChanges.
self assert:
(class parseTreeForSelector: #example)
equals:
(self parseMethod: 'example
obj foo.
obj bar.')
]

{ #category : 'tests' }
RBSplitCascadeRefactoringTest >> testWholeCascadeFail [

| ref |
ref := self
splitCascadeFrom: 'foo; bar; baz'
in: 'example
obj foo;
bar;
baz'.
self should: [ ref generateChanges ]
raise: RBRefactoringError.
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also expect this to do:

obj foo.
obj bar.
obj baz.

But this might only be me.
I'm not saying we should do this in this PR, we should discuss this.