[Bug]: Description Generation - Don't Write Null Accessors (#359)

* [Bug]: Description Generation - Don't Write Null Accessors

* [Test]: Change Write Test - Don't Write Default

source/Magritte-Developer/MADeveloperTests.class.st

@@ -0,0 +1,20 @@
+Class {
+	#name : #MADeveloperTests,
+	#superclass : #TestCase,
+	#category : #'Magritte-Developer-Tests'
+}
+
+{ #category : #accessing }
+MADeveloperTests >> testGenerateDescriptionSource [
+
+	| desc |
+	desc := MANumberDescription new
+		default: 5;
+		yourself.
+
+	self
+		assert: desc storeString
+		equals: 'MANumberDescription new
+	default: 5;
+	yourself'
+]

source/Magritte-Developer/MASelectorAccessor.extension.st

@@ -10,6 +10,8 @@ MASelectorAccessor >> maCompile: templateString asAccessor: aSymbol forinstVarNa
 
 { #category : #'*Magritte-Developer-private' }
 MASelectorAccessor >> maSetUp: aClass for: anMAElementDescription [
+	"Ensure inst var, setter and getter exist"
+
 	| variableName needsInstVar defaultArgumentType setterArgumentName |
 	variableName := self readSelector.
 	needsInstVar := (aClass hasInstVarNamed: variableName) not.
@@ -31,7 +33,6 @@ MASelectorAccessor >> maSetUp: aClass for: anMAElementDescription [
 { #category : #'*Magritte-Developer-private' }
 MASelectorAccessor >> store: anObject inDescriptionOn: aStream [ 
 	anObject storeVia: self inDescriptionOn: aStream.
-
 ]
 
 { #category : #'*Magritte-Developer-private' }

source/Magritte-Developer/Object.extension.st

@@ -76,6 +76,7 @@ Object class >> maAddField: aSymbol with: customizationBlock [
 
 { #category : #'*Magritte-Developer' }
 Object >> storeVia: anAccessor inDescriptionOn: aStream [
+
 	aStream
 		nextPutAll: anAccessor writeSelector , ' (';
 		store: self;

source/Magritte-Model/MAElementDescription.class.st

@@ -95,6 +95,21 @@ MAElementDescription >> initialAnswer: aValuable [
 	self propertyAt: #initialAnswer put: aValuable
 ]
 
+{ #category : #utility }
+MAElementDescription >> isIgnorableValue: anObject [
+
+	| ignorableValues isIgnorableDefault |
+
+	ignorableValues := OrderedCollection
+		with: nil
+		with: self undefinedValue.
+
+	isIgnorableDefault := self shouldCacheDefault not and: [ anObject = self default ].
+	isIgnorableDefault ifTrue: [ ignorableValues add: self default ].
+
+	^ ignorableValues includes: anObject
+]
+
 { #category : #'lazy initialization' }
 MAElementDescription >> lazilyInitializeFrom: currentValue for: anObject [
 	"- The default value is cached if the description's #shouldCacheDefault property is true. An example when caching is necessary is for to-many relations because the user may modify the collection, which will then be thrown away if not cached
@@ -124,21 +139,12 @@ MAElementDescription >> printFor: anObject on: aWriteStream [
 { #category : #utility }
 MAElementDescription >> shouldWrite: anObject over: anotherObject [
 
-	| defaultValues areBothDefaultValues isIgnorableDefault |
-
 	self isVisible ifFalse: [ ^ false ].
 	self isReadOnly ifTrue: [ ^ false ].
 
-	defaultValues := OrderedCollection
-		with: nil
-		with: self undefinedValue.
-
-	isIgnorableDefault := self shouldCacheDefault not and: [ anObject = self default ].
-	isIgnorableDefault ifTrue: [ defaultValues add: self default ].
-
-	areBothDefaultValues := { anObject. anotherObject } allSatisfy: [ :obj | defaultValues includes: obj ].
+	((self isIgnorableValue: anObject) and: [ self isIgnorableValue: anotherObject ])
+		ifTrue: [ ^ false ].
 
-	areBothDefaultValues ifTrue: [ ^ false ].
 	(anObject == anotherObject or: [ anObject = anotherObject ])
 		ifTrue: [ ^ false ].
 
@@ -151,13 +157,20 @@ MAElementDescription >> shouldWrite: anObject over: anotherObject [
 
 { #category : #printing }
 MAElementDescription >> storeOn: aStream [
-	aStream 
-		nextPutAll: self className;
-		nextPutAll: ' new'; cr.
-	(self magritteDescription reject: #isReadOnly) do: [ :desc |
-		| value |
-		value := desc read: self.
-		(value ~= desc default and: [ value isNotNil ]) ifTrue: [ 
-			desc accessor store: value inDescriptionOn: aStream ] ].
-	aStream nextPutAll: 'yourself'
+
+	| source formattedSource |
+	source := String streamContents: [ :str |
+		str
+			nextPutAll: self className;
+			nextPutAll: ' new'; cr.
+		(self magritteDescription reject: #isReadOnly) do: [ :desc |
+			| value |
+			value := desc read: self.
+			(desc isIgnorableValue: value) ifFalse: [ 
+				desc accessor store: value inDescriptionOn: str ] ].
+		str nextPutAll: 'yourself' ].
+
+	formattedSource := (RBParser parseExpression: source) formattedCode.
+
+	aStream << formattedSource
 ]

source/Magritte-Model/MANullAccessor.class.st

@@ -7,7 +7,7 @@ Class {
 	#instVars : [
 		'uuid'
 	],
-	#category : 'Magritte-Model-Accessor'
+	#category : #'Magritte-Model-Accessor'
 }
 
 { #category : #testing }
@@ -42,6 +42,11 @@ MANullAccessor >> hash [
 	^ super hash bitXor: self uuid hash
 ]
 
+{ #category : #accessing }
+MANullAccessor >> maSetUp: aClass for: anMAElementDescription [
+	"do nothing"
+]
+
 { #category : #model }
 MANullAccessor >> read: aModel [ 
 	MAReadError signal: 'This message is not appropriate for this object'
@@ -57,6 +62,10 @@ MANullAccessor >> storeOn: aStream [
 		nextPut: $)
 ]
 
+{ #category : #accessing }
+MANullAccessor >> storeVia: anAccessor inDescriptionOn: aStream [
+]
+
 { #category : #accessing }
 MANullAccessor >> uuid [
 	^ uuid

source/Magritte-Model/Object.extension.st

@@ -34,11 +34,11 @@ Object >> isSameAs: rhs [
 ]
 
 { #category : #'*Magritte-Model' }
-Object >> isSameAs: rhs using: valuableDescription [
+Object >> isSameAs: rhs using: descriptionValuable [
 	"Use #isSameAs: unless you need to cache the description for efficiency.
 	CAUTION: this may not work if the description depends on the instance e.g. uses `self` in a block
 	(See comment below for more details)"
-	^ valuableDescription value allSatisfy: [ :desc |
+	^ descriptionValuable value allSatisfy: [ :desc |
 			| myVal rhsVal |
 			(desc accessor canRead: rhs)
 				ifFalse: [ false ]

source/Magritte-Tests-Model/MAStraightMementoTest.class.st

@@ -1,7 +1,7 @@
 Class {
 	#name : #MAStraightMementoTest,
 	#superclass : #MAMementoTest,
-	#category : 'Magritte-Tests-Model-Memento'
+	#category : #'Magritte-Tests-Model-Memento'
 }
 
 { #category : #testing }
@@ -49,12 +49,14 @@ MAStraightMementoTest >> testValidateRequired [
 
 { #category : #'tests-basic' }
 MAStraightMementoTest >> testWrite [
-	self write: self includedInstance.
-	self assert: self value = self includedInstance.
 
+	"No need to save the default value"
 	self write: self defaultInstance.
-	self assert: self value = self defaultInstance.
-
+	self assert: self value equals: self nullInstance.
+	
 	self write: self nullInstance.
-	self assert: self value = self nullInstance
+	self assert: self value equals: self nullInstance.
+
+	self write: self includedInstance.
+	self assert: self value equals: self includedInstance.
 ]