[Refactorings] Implement Driver for PushDownSharedVariable

src/Calypso-SystemTools-FullBrowser/SycPushDownVariableCommand2.extension.st

@@ -0,0 +1,8 @@
+Extension { #name : 'SycPushDownVariableCommand2' }
+
+{ #category : '*Calypso-SystemTools-FullBrowser' }
+SycPushDownVariableCommand2 class >> fullBrowserMenuActivation [
+	<classAnnotation>
+
+	^CmdContextMenuActivation byRootGroupItemOrder: 2002 for: ClyFullBrowserVariableContext
+]

src/Refactoring-Core/ReClassesHaveAnySubclassCondition.class.st

@@ -0,0 +1,29 @@
+Class {
+	#name : 'ReClassesHaveAnySubclassCondition',
+	#superclass : 'ReClassesCondition',
+	#category : 'Refactoring-Core-Conditions',
+	#package : 'Refactoring-Core',
+	#tag : 'Conditions'
+}
+
+{ #category : 'testing' }
+ReClassesHaveAnySubclassCondition >> hasSubclasses: aClass [
+
+	^ aClass subclasses isNotEmpty
+]
+
+{ #category : 'displaying' }
+ReClassesHaveAnySubclassCondition >> violationMessageOn: aStream [
+
+	self violators do: [ :violator |
+		aStream
+			nextPutAll: violator name;
+			nextPutAll: ' has no subclasses.';
+			space ]
+]
+
+{ #category : 'accessing' }
+ReClassesHaveAnySubclassCondition >> violators [
+
+	^ violators ifNil: [ violators := classes reject: [ :aClass | self hasSubclasses: aClass ] ]
+]

src/Refactoring-Core/ReIsValidInstanceVariableName.class.st

@@ -13,21 +13,21 @@ Class {
 ReIsValidInstanceVariableName >> check [
 
 	| string |
-	name isString ifFalse: [
-		violator := name.
+	variable isString ifFalse: [
+		violator := variable.
 		^ false ].
-	string := name asString.
+	string := variable asString.
 	string isEmpty ifTrue: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	(Symbol reservedLiterals includes: string) ifTrue: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	string first isUppercase ifTrue: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	(OCScanner isVariable: string) ifFalse: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	^ true
 ]

src/Refactoring-Core/ReIsValidSharedVariableName.class.st

@@ -13,21 +13,21 @@ Class {
 ReIsValidSharedVariableName >> check [
 
 	| string |
-	name isString ifFalse: [
-		violator := name.
+	variable isString ifFalse: [
+		violator := variable.
 		^ false ].
-	string := name asString.
+	string := variable asString.
 	(Symbol reservedLiterals includes: string) ifTrue: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	string isEmpty ifTrue: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	string first isUppercase ifFalse: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	(OCScanner isVariable: string) ifFalse: [
-		violator := name.
+		violator := variable.
 		^ false ].
 	^ true
 ]

src/Refactoring-Core/ReIsVariableNotDefinedInHierarchy.class.st

@@ -4,9 +4,6 @@ I check if `name` is not already used by a variable in the `class` hierarchy
 Class {
 	#name : 'ReIsVariableNotDefinedInHierarchy',
 	#superclass : 'ReVariableNameCondition',
-	#instVars : [
-		'class'
-	],
 	#category : 'Refactoring-Core-Conditions',
 	#package : 'Refactoring-Core',
 	#tag : 'Conditions'
@@ -22,21 +19,21 @@ ReIsVariableNotDefinedInHierarchy class >> name: aString class: aClass [
 { #category : 'checking' }
 ReIsVariableNotDefinedInHierarchy >> check [
 
-	(class hierarchyDefinesVariable: name) ifTrue: [
-		violator := name.
+	(aClass hierarchyDefinesVariable: variable) ifTrue: [
+		violator := variable.
 		^ false ].
 	^ true
 ]
 
 { #category : 'accessing' }
-ReIsVariableNotDefinedInHierarchy >> class: aClass [
-	class := aClass
+ReIsVariableNotDefinedInHierarchy >> class: someClass [
+	aClass := someClass
 ]
 
-{ #category : 'accessing' }
+{ #category : 'displaying' }
 ReIsVariableNotDefinedInHierarchy >> violationMessageOn: aStream [
 
 	^ aStream
 		  nextPutAll: violator;
-		  nextPutAll: (' is already defined in the class {1} or its hierarchy.' format: { class name })
+		  nextPutAll: (' is already defined in the class {1} or its hierarchy.' format: { aClass name })
 ]

src/Refactoring-Core/ReMethodsDontReferToLocalSharedVarsCondition.class.st

@@ -7,7 +7,8 @@ Class {
 	#superclass : 'ReMethodsCondition',
 	#instVars : [
 		'class',
-		'selectors'
+		'selectors',
+		'variables'
 	],
 	#category : 'Refactoring-Core-Conditions',
 	#package : 'Refactoring-Core',
@@ -18,7 +19,16 @@ Class {
 ReMethodsDontReferToLocalSharedVarsCondition >> class: aRBClass selectors: aCollection [ 
 
 	class := aRBClass.
-	selectors := aCollection 
+	selectors := aCollection.
+	variables := nil
+]
+
+{ #category : 'initialization' }
+ReMethodsDontReferToLocalSharedVarsCondition >> class: aRBClass selectors: aCollection variables: aCollection2 [
+
+	class := aRBClass.
+	selectors := aCollection.
+	variables := aCollection2 
 ]
 
 { #category : 'accessing' }
@@ -27,6 +37,11 @@ ReMethodsDontReferToLocalSharedVarsCondition >> referencedSharedVariables [
 	^ self violators collect: [ :violator | violator at: 2 ]
 ]
 
+{ #category : 'accessing' }
+ReMethodsDontReferToLocalSharedVarsCondition >> variables [
+	^ variables ifNil: [ variables := class classVariableNames ]
+]
+
 { #category : 'displaying' }
 ReMethodsDontReferToLocalSharedVarsCondition >> violationMessageOn: aStream [
 
@@ -48,7 +63,7 @@ ReMethodsDontReferToLocalSharedVarsCondition >> violators [
 
 	violators := Set new.
 	selectors do: [ :selector |
-		class classVariableNames do: [ :classVar |
+		self variables do: [ :classVar |
 			((class methodFor: selector) refersToVariable: classVar)
 				ifTrue: [ violators add: { selector . classVar } ] ] ].
 	^ violators asOrderedCollection

src/Refactoring-Core/ReNewNegationCondition.class.st

@@ -24,12 +24,25 @@ ReNewNegationCondition >> condition: aCondition [
 	condition := aCondition.
 ]
 
+{ #category : 'accessing' }
+ReNewNegationCondition >> errorMacro [
+	"TODO: This was introduced for compatibility with RBAbstractCondition
+	and it should be removed"
+	^ self errorString
+]
+
 { #category : 'accessing' }
 ReNewNegationCondition >> errorString [ 
 
 	^ condition errorString
 ]
 
+{ #category : 'logical operations' }
+ReNewNegationCondition >> not [ 
+
+	^ condition
+]
+
 { #category : 'displaying' }
 ReNewNegationCondition >> violationMessageOn: aWriteStream [
 

src/Refactoring-Core/RePushDownSharedVariableRefactoring.class.st

@@ -0,0 +1,59 @@
+"
+I am a refactoring for moving a class variable down to my subclasses.
+
+My precondition verifies that the moved variable is not referenced in the methods of the original class.
+"
+Class {
+	#name : 'RePushDownSharedVariableRefactoring',
+	#superclass : 'RBVariableRefactoring',
+	#category : 'Refactoring-Core-Refactorings',
+	#package : 'Refactoring-Core',
+	#tag : 'Refactorings'
+}
+
+{ #category : 'preconditions' }
+RePushDownSharedVariableRefactoring >> applicabilityPreconditions [
+
+	^ {
+		  (RBCondition definesClassVariable: variableName in: class).
+		  (ReClassesHaveAnySubclassCondition new classes: { class }) }
+]
+
+{ #category : 'preconditions' }
+RePushDownSharedVariableRefactoring >> breakingChangePreconditions [
+
+	^ { self preconditionHasNoReferences }
+]
+
+{ #category : 'preconditions' }
+RePushDownSharedVariableRefactoring >> findDestinationClasses [
+
+	| classes |
+	classes := class allSubclasses reject: [ :each |
+		(each whichSelectorsReferToClassVariable: variableName) isEmpty
+			and: [ (each classSide whichSelectorsReferToClassVariable: variableName) isEmpty ] ].
+	^ classes
+]
+
+{ #category : 'preconditions' }
+RePushDownSharedVariableRefactoring >> preconditionHasNoReferences [
+
+	^ (ReSharedVariableHasReferencesInDefiningClass new class: class referencesToSharedVariable: variableName) not
+]
+
+{ #category : 'preconditions' }
+RePushDownSharedVariableRefactoring >> preconditions [
+
+	^ self applicabilityPreconditions & self breakingChangePreconditions
+]
+
+{ #category : 'transforming' }
+RePushDownSharedVariableRefactoring >> privateTransform [
+
+	| destinationClasses |
+
+	class removeClassVariable: variableName.
+	destinationClasses := self findDestinationClasses.
+	destinationClasses ifEmpty: [ ^ self ].
+	destinationClasses do: [ :aClass | aClass addClassVariable: variableName ]
+]

src/Refactoring-Core/ReSharedVariableHasReferences.class.st

@@ -1,28 +1,26 @@
+"
+I check if a Shared Variable is referenced from methods in aClass or any of its subclasses
+"
 Class {
 	#name : 'ReSharedVariableHasReferences',
-	#superclass : 'RBNewAbstractCondition',
-	#instVars : [
-		'aClass',
-		'violators',
-		'sharedVariable'
-	],
+	#superclass : 'ReVariableCondition',
 	#category : 'Refactoring-Core-Conditions',
 	#package : 'Refactoring-Core',
 	#tag : 'Conditions'
 }
 
-{ #category : 'accessing' }
+{ #category : 'checking' }
 ReSharedVariableHasReferences >> check [
 
 	aClass withAllSubclasses do: [ :each |
 			| res |
-			res := (each whichMethodsReferToSharedVariable: sharedVariable).
+			res := (each whichMethodsReferToSharedVariable: variable).
 			res isNotEmpty
 				ifTrue: [ violators addAll: res ].
 			].
 	aClass withAllSubclasses do: [ :each |
 			| res |
-			res := (each classSide whichMethodsReferToSharedVariable: sharedVariable).
+			res := (each classSide whichMethodsReferToSharedVariable: variable).
 			res isNotEmpty
 				ifTrue: [ violators addAll: res ].
 			].
@@ -32,14 +30,14 @@ ReSharedVariableHasReferences >> check [
 { #category : 'accessing' }
 ReSharedVariableHasReferences >> errorString [ 
 
-	^ ' Variable ', sharedVariable , ' is still referenced'
+	^ ' Variable ', variable , ' is still referenced'
 ]
 
 { #category : 'instance creation' }
 ReSharedVariableHasReferences >> hierarchyOf: aRBClass referencesSharedVariable: variableName [
 
 	aClass := aRBClass instanceSide.
-	sharedVariable := variableName 
+	variable := variableName 
 ]
 
 { #category : 'initialization' }
@@ -48,6 +46,12 @@ ReSharedVariableHasReferences >> initialize [
 	violators := OrderedCollection new
 ]
 
+{ #category : 'accessing' }
+ReSharedVariableHasReferences >> violatorCandidates [
+
+	^ aClass withAllSubclasses flatCollect: [ :cls | cls selectors , cls classSide selectors ]
+]
+
 { #category : 'accessing' }
 ReSharedVariableHasReferences >> violators [ 
 	^ violators

src/Refactoring-Core/ReSharedVariableHasReferencesInDefiningClass.class.st

@@ -0,0 +1,42 @@
+"
+I check if a Shared Variable is referenced from methods in the class that defines it.
+"
+Class {
+	#name : 'ReSharedVariableHasReferencesInDefiningClass',
+	#superclass : 'ReVariableCondition',
+	#category : 'Refactoring-Core-Conditions',
+	#package : 'Refactoring-Core',
+	#tag : 'Conditions'
+}
+
+{ #category : 'checking' }
+ReSharedVariableHasReferencesInDefiningClass >> check [
+
+	^ self violators isNotEmpty
+]
+
+{ #category : 'initialization' }
+ReSharedVariableHasReferencesInDefiningClass >> class: definingClass referencesToSharedVariable: aSharedVarName [
+	aClass := definingClass.
+	variable := aSharedVarName 
+]
+
+{ #category : 'accessing' }
+ReSharedVariableHasReferencesInDefiningClass >> violatorCandidates [
+
+	^ aClass selectors, aClass classSide selectors
+]
+
+{ #category : 'accessing' }
+ReSharedVariableHasReferencesInDefiningClass >> violators [
+	| res |
+	violators ifNotNil: [ ^ violators ].
+	violators := OrderedCollection new.
+	res := aClass whichMethodsReferToSharedVariable: variable.
+	res isNotEmpty	
+		ifTrue: [ violators addAll: res ].
+	res := (aClass classSide whichMethodsReferToSharedVariable: variable).
+	res isNotEmpty
+		ifTrue: [ violators addAll: res ].
+	^ violators
+]