Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid to have an extension method in the package owning the class #14354

Merged
merged 6 commits into from
Jul 26, 2023

Conversation

jecisc
Copy link
Member

@jecisc jecisc commented Jul 24, 2023

It is currently possible to have a method that is an extension method that is in the same package as its class. This creates weird behaviors were some tools consider it an extension method and some other consider it not an extension method.

To make thing more coherent I propose that we forbid to have this by classifying such methods under the unclassified protocol instead.

Fixes ##14353

It is currently possible to have a method that is an extension method that is in the same package as its class. This creates weird behaviors were some tools consider it an extension method and some other consider it not an extension method.

To make thing more coherent I propose that we forbid to have this by classifying such methods under the unclassified protocol instead.
src/Kernel/ClassDescription.class.st Outdated Show resolved Hide resolved
src/Kernel/ClassDescription.class.st Outdated Show resolved Hide resolved
@jecisc
Copy link
Member Author

jecisc commented Jul 24, 2023

Apparently the warning is preventing the bootstrap to finish. Do you know what I should do so that it just log the problem and proceed?

@jecisc jecisc added the Status: Need more work The issue is nearly ready. Waiting some last bits. label Jul 24, 2023
@jecisc
Copy link
Member Author

jecisc commented Jul 25, 2023

Checking the code of the compiler it seems that the right way would be to use a Notification or even a SystemNotification instead of a warning

@jecisc jecisc removed the Status: Need more work The issue is nearly ready. Waiting some last bits. label Jul 25, 2023
@jecisc
Copy link
Member Author

jecisc commented Jul 25, 2023

The build now passes and things are working correctly. In the non interactive environment things get logged.
I just broke some tests that I'll fix now

@jecisc
Copy link
Member Author

jecisc commented Jul 25, 2023

The last failure is fixed by this: pillar-markup/Microdown#658

But I need someone with merge rights :'(

@jecisc
Copy link
Member Author

jecisc commented Jul 26, 2023

The requested changes were applied and the tests are now green

@Ducasse Ducasse merged commit aa57671 into pharo-project:Pharo12 Jul 26, 2023
1 check passed
@jecisc jecisc deleted the extension-of-the-same-package branch July 26, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants