Issue 5722 in pharo: Be able to remove traits from class definition

Status: Accepted
Owner: marianopeck
Labels: Type-Bug

New issue 5722 by marianopeck: Be able to remove traits from class  
definition
http://code.google.com/p/pharo/issues/detail?id=5722

If we have a class with a trait, like:

Object subclass: #AA
        uses: {TAss}
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'AA'

And then we want to remove the trait from the compostion doing:

Object subclass: #AA
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'AA'

it does not work.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixToInclude
        Labels: Milestone-2.0

Comment #1 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

fix in inbox


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Integrated

Comment #2 on issue 5722 by [hidden email]: Be able to remove traits  
from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

in 2.0 026


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FailingTest

Comment #3 on issue 5722 by [hidden email]: Be able to remove traits  
from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Lots of tests are failing after this one... Now reverting is work.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: Milestone-1.4

Comment #4 on issue 5722 by [hidden email]: Be able to remove traits  
from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 5722 by [hidden email]: Be able to remove traits  
from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Issue 5679 has been merged into this issue.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 5722 by [hidden email]: Be able to remove traits  
from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Mariano, can you check? the following tests are now failing:


>>> KernelTests.Classes.ClassBuilderFormatTests.testDuplicateClassVariableError  
>>>
>>> 30 ms 3
>>> KernelTests.Classes.ClassBuilderFormatTests.testDuplicateInstanceVariableError  
>>>
>>> 0.15 sec 3
>>> KernelTests.Classes.ClassBuilderValidationTests.testCreateClassWithString  
>>>
>>> 0 ms 3

"fix to include" normally means that no tests will fail after adding the  
change...


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Grrr sorry Marcus. We did it with Guille in the least 20 minutes of the  
sprint.
I will take a look.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

The problem is that the method: #subclass: t instanceVariableNames: f  
classVariableNames: d poolDictionaries: s category: cat

can return NIL. Therefore, since we added a:

        class traitComposition traits
                do: [ :aTrait | class removeFromComposition: aTrait ].

we get a DNU.

Of course, we can fix it by doing:

        class ifNotNil:  [traitComposition traits
                do: [ :aTrait | class removeFromComposition: aTrait ]].

It is a pity that the ClassBuilder is so .... that answers nil rather than  
a prope exception


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #9 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Fix in inbox:

Name:  
SLICE-Issue-5722-Be-able-to-remove-traits-from-class-definition-V2-MarianoMartinezPeck.1
Author: MarianoMartinezPeck
Time: 30 April 2012, 12:08:26.693 pm
UUID: 6a248204-e8ab-4575-9e09-4bf3eb7b2ca6
Ancestors:
Dependencies: Kernel-MarianoMartinezPeck.1061

Fix for the failing tests


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixToInclude

Comment #10 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: -Milestone-2.0

Comment #11 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

in 2.0 030


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FailingTest

Comment #12 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Even worse. When running all tests we get a failing build.


   UndefinedObject(Object)>>doesNotUnderstand: #traits
    Receiver: nil
    Arguments and temporary variables:
    aMessage: traits
    exception: MessageNotUnderstood: receiver of "traits" is nil
    resumeValue: nil
    Receiver's instance variables:
   nil

   C1  
class(Class)>>subclass:instanceVariableNames:classVariableNames:poolDictionaries:category:
    Receiver: C1
    Arguments and temporary variables:
    t: #C2
    f: ''
    d: ''
    s: ''
    cat: #'Tests-Traits'
    class: C2
    Receiver's instance variables:
    superclass: Object
    methodDict: a MethodDictionary(#foo->(C1>>#foo "a  
CompiledMethod(922746880)") )...etc...
    format: 2
    layout: nil
    instanceVariables: nil
    organization: a ClassOrganizer
    subclasses: {C2}
    name: #C1
    classPool: nil
    sharedPools: nil
    environment: a SystemDictionary(lots of globals)
    category: nil
    traitComposition: nil
    localSelectors: nil




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #13 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Thanks Marcus. I am an idiot. I should less things at the same time, or  
drink a cup of coffee while running tests. I fixed and I am running the  
tests now....


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #14 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

well, the fix is easy,

subclass: t instanceVariableNames: f classVariableNames: d  
poolDictionaries: s category: cat
        "This is the standard initialization message for creating a new class as a
        subclass of an existing class (the receiver)."
        | class |
        class := (ClassBuilder new)
                superclass: self
                subclass: t
                instanceVariableNames: f
                classVariableNames: d
                poolDictionaries: s
                category: cat.
        class ifNotNil:  [class traitComposition traits
                do: [ :aTrait | class removeFromComposition: aTrait ]].
        ^ class


The problem is that now we broke only one test: testChangeSuperclass
and it looks that it is related to issue 5679.
So I can do a slice with the mentioned fix and let testChangeSuperclass  
broken until we fix issue 5679


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #15 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Actually, a more clean version should be:

class ifNotNil: [ class setTraitComposition: {} asTraitComposition ].

instead of:

class ifNotNil:  [class traitComposition traits
                do: [ :aTrait | class removeFromComposition: aTrait ]].



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #16 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

I now added the what I thought would be the change needed (It really would  
be nice to attach *machine readable* code...)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #17 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

So the tests now run again... with *lots* of new failing tests


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #18 on issue 5722 by [hidden email]: Be able to remove  
traits from class definition
http://code.google.com/p/pharo/issues/detail?id=5722

The log says they are because of RPackage  
(RPackageOrganizer>>systemMethodModifiedActionFrom:), so I'd close this  
issue :/.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #19 on issue 5722 by marianopeck: Be able to remove traits from  
class definition
http://code.google.com/p/pharo/issues/detail?id=5722

Unfortunatly, it is impossible to test this right now, it is full of tests  
red/yellow, test are run several times, lots of opened debuggers, etc. It  
is impossible to know whether the change breaks something or not.

As said by Guille, the solution is

subclass: t instanceVariableNames: f classVariableNames: d  
poolDictionaries: s category: cat
        "This is the standard initialization message for creating a new class as a
        subclass of an existing class (the receiver)."
        | class |
        class := (ClassBuilder new)
                superclass: self
                subclass: t
                instanceVariableNames: f
                classVariableNames: d
                poolDictionaries: s
                category: cat.
        class ifNotNil:  [ class setTraitComposition: {} asTraitComposition ].
        ^ class





_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker