pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.21k stars 356 forks source link

Class method defined in a trait overloading existing superclass method #16079

Closed ELePors closed 9 months ago

ELePors commented 9 months ago

Bug description I have a ZoneToolbox class subclass of VisualToolbox. VisualToolbox has a setVisual: message implemented...

i wanted to add a specific Trait to the ZoneToolbox which is a specialization of the VisualToolbox api. but the traitShall contain or use the Trait of VisualToolbox...

at VisualToolbox level i have a setVisual: with the code... The VisualToolboxService trait has setVisual defined

my ZoneToolbox uses a ZoneToolboxServices which uses VisualToolboxServices as a part of its own behavior so ZoneToolbox has an empty setVisual: method added by the trait even if the superclass defines it. (because the VisualToolboxServices setVisual: is empty, just to specify something todo)

and i dont want to add a "super setVisual:" because VisualToolboxServices is an Object subclass...

A trait should not overload a super class message ... by construction a Trait is a behavior we want to add to a class. if the trait method code is empty (or with a specific pragma ... or... ) a trait should not overload the subclass function.

I know there is other solutions but in some of our objects ... this Trait behavior is not desirable...

Expected behavior When a trait is used in a subclass the trait method is used if not empty or if a pragma specify to explicitely overload...

Version information:

Ducasse commented 9 months ago

Could you provide some code because I'm not sure that I understand? A trait is just like a macro. When you use it in a class it define locally the methods of the trait in the user. Trait does not care about inheritance. Trait semantics is that if you copy paste the method of the trait in the class (modulo the methods you defined in the class) you can remove the trait. So it means that a trait will shadow superclass methods. and normally in a trait should not call super to jump over local methods.

ELePors commented 9 months ago

Yes but the should have a way to not shadow the superclass methods ... in certain conditions... A trait is a behavior or an interface we want for our objet to respect... not obviously to block inherited behavior... but only to add it when it is not already present...

Imagine a class animal with a message "shout" with a Transcript show:'Waaa !' call in it ... when you define a subclass of animal named cat, and you add a trait, MouthAnimalTrait which proposes to have a shout method (with empty source), if the shout method exists in the cat class, it will be preserved... if it is in the superclass no... why does Trait should not handle superclass inherited messages as current class ones...

It is a question that blocks me some use cases...

guillep commented 9 months ago

Hi Eric, traits already propose a mechanism for this. Although it's probable not "as automatic" as you would like.

When you apply a trait you can refine its application and remove/alias methods from the trait. So you could do:

Superclass << #ClassName
    traits: {(MyTrait - {#theSelectorIDoNotWantToImport})};
...

Should that solve your problem?

Ducasse commented 9 months ago

'why does Trait should not handle superclass inherited messages as current class ones...'

It does. When you apply a trait to a class. It is like if the traits does not exist. The trait method override the superclass methods as any subclass would do it. So I do not understand your remark.

Please give me an example even in pseudo code. I do not want to make wrong hypothesis reading your comments.

ELePors commented 9 months ago

Hi Eric, traits already propose a mechanism for this. Although it's probable not "as automatic" as you would like.

When you apply a trait you can refine its application and remove/alias methods from the trait. So you could do:

Superclass << #ClassName
  traits: {(MyTrait - {#theSelectorIDoNotWantToImport})};
...

Should that solve your problem?

It is a workaround yes... but it would be usefull to have something in the Trait method that should accept to not override a superclass method.

ELePors commented 9 months ago

'why does Trait should not handle superclass inherited messages as current class ones...'

It does. When you apply a trait to a class. It is like if the traits does not exist. The trait method override the superclass methods as any subclass would do it. So I do not understand your remark.

Please give me an example even in pseudo code. I do not want to make wrong hypothesis reading your comments.

The first class: SmockHuman ...

Object subclass: #SmockHuman
    instanceVariableNames: ''
    classVariableNames: ''
    package: 'Smock-Tests'!

!SmockHuman methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:44'!
wakeUp
Transcript show:'Haaaa !!'.! !

!SmockHuman methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:44'!
sleep
Transcript show:'I sleep'.! !

!SmockHuman methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:39'!
hello
Transcript show: 'hello !!' ! !

!SmockHuman methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:39'!
bye
Transcript show: 'bye'! !

The second class : SmockHumanWorker

SmockHuman subclass: #SmockHumanWorker
    uses: TSmockPublicRelationWorker
    instanceVariableNames: ''
    classVariableNames: ''
    package: 'Smock-Tests'!

!SmockHumanWorker methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:43'!
stopWork
Transcript show: 'i dont work now'! !

!SmockHumanWorker methodsFor: 'as yet unclassified' stamp: 'EricLePors 2/2/2024 09:43'!
work
Transcript show: 'i work now'! !

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

SmockHumanWorker class
    uses: TSmockPublicRelationWorker classTrait
    instanceVariableNames: ''!

The Trait used by SmockHumanWorker to add a specific behavior to the SmockHumanWorker (it is an example... but i have a similar problem in our architecture)

Trait named: #TSmockPublicRelationWorker
    instanceVariableNames: ''
    package: 'Smock-Tests'!

!TSmockPublicRelationWorker methodsFor: 'command line' stamp: 'EricLePors 2/2/2024 09:41'!
hello
self shouldBeImplemented ! !

!TSmockPublicRelationWorker methodsFor: 'command line' stamp: 'EricLePors 2/2/2024 09:41'!
explainSubject
self shouldBeImplemented ! !

!TSmockPublicRelationWorker methodsFor: 'command line' stamp: 'EricLePors 2/2/2024 09:41'!
manageCommunity
self shouldBeImplemented ! !

!TSmockPublicRelationWorker methodsFor: 'command line' stamp: 'EricLePors 2/2/2024 09:42'!
bye
self shouldBeImplemented ! !

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

TSmockPublicRelationWorker classTrait
    instanceVariableNames: ''!

a call to

SmockHumanWorker new hello

produces a 'shouldBeImplemented' error... it is logic... but... i would like to have a possibility to say to the Trait... "if the method is in class hierarchy... please dont override it" (a pragma ... i dont know... )

The same behavior if the SmockHumanWorker has the "bye" method implemented ... the bye method is preserved... The Trait definition should be able to embed the way of adding its list of behavior... the current way of doing is ok for me... i'm just asking about another way of doing ... ? Specific Trait ? Pragmas in methods ?

Thanks by advance... Eric.

Ducasse commented 9 months ago

Trait should not force to scan a complete hierarchy. Else each time you add a method in the trait, in the trait users and in the trait user hierarchy you would have to check and some methods would magically disappear! It would be super strange. It would mean that if a tool extend Object with a method suddenly methods would disappear in far subclasses while the same method would be visible in a Trait. I call this the slowAndNightmare semantics. The semantics of trait is nice and it has good properties. We should improve the handling of explicit requirement (or remove them so better do not use them).

Ducasse commented 9 months ago

If you do not want that a trait method is not added to the class then you should explicitly remove it using the - operator.

ELePors commented 9 months ago

This is possible ? ok this is a good solution for me.

De : StéphaneDucasse @.> Envoyé : vendredi 2 février 2024 13:57 À : pharo-project/pharo @.> Cc : LE PORS Eric @.>; Author @.> Objet : Re: [pharo-project/pharo] Class method defined in a trait overloading existing superclass method (Issue #16079)

If you do not want that a trait method is not added to the class then you should explicitly remove it using the - operator.

— Reply to this email directly, view it on GitHubhttps://github.com/pharo-project/pharo/issues/16079#issuecomment-1923751811, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AI67RWJERYWTTXGWRZNY4ATYRTPCTAVCNFSM6AAAAABCTGGTCWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRTG42TCOBRGE. You are receiving this because you authored the thread.Message ID: @.**@.>>

Ducasse commented 9 months ago

Yes As guille said


Superclass << #ClassName
    traits: {(MyTrait - {#theSelectorIDoNotWantToImport})};
...