January 2023 - Pharo-dev - lists.pharo.org

Re: Code quality: tagging unsent selectors (e.g. in tests)

by Marcus Denker

Thanks, I will take some notes for a future âbetter code meta dataâ project. For now I propose to merge the PR as a first step. Marcus > On 26 Jan 2023, at 18:52, Daniel Slomovits <daniels220(a)gmail.com> wrote: > > A bit of perspective on other approaches to this problem: > > In Dolphin Smalltalk, the FFI mechanism uses #doesNotUnderstand: to implement getters and setters on external structs, so it already has a mechanism for ensuring they aren't flagged as unimplemented. In addition to the standard ClassDescription>>selectors, which is more-or-less the actual keys of the methodDictionary, there is #understoodSelectors, which is explicitly a hook to include others understood via #doesNotUnderstand:. These are then treated exactly as actually-implemented selectors, that is, they are available for autocomplete as well as not being flagged as unimplemented. > > Also, I believe the unimplemented-selectors rule applies only to non-self-sends, and there is a separate rule for self-sends which instead just asks the class if it #canUnderstand: the selector, which creates a middle ground where the selector is not "known" to the system at large, but won't be warned about if self-sent. > > The pragma approach has the advantage of being more specific, which is useful if a certain selector is valid only in a very narrow context and should still be warned about in general. Conversely, the disadvantage is that it needs to be repeated everywhere the message is sent, rather than centralized on the class where it is implemented/understood. I assume it also does not influence autocomplete. So, for cases that are valid in a broader scope, it creates the extra work of adding the pragma, and you have to type out the actual send manually, multiplied for each time it's usedâlots of repetition. Fundamentally we're running up against the limitations of a dynamically-typed language here. > > Long story short, I think both options may be useful. In my own usage I find it's about 50/50âsome cases of one-offs in a test, other cases of a widely-used class with some dynamic behavior. I do like the pragma as an improvement over rule banning for sure. Looking towards improving rule banning, I think it's very important for that to be (a) highly visible (and nothing is more visible than something that's part of the source code) and (b) manageable via source control (which metadata certainly can beâcategories, for instance, already are). Call me old-fashioned, but for all the wonders of the image, there's something to be said for having a limited set of concepts in a "dead text" format that fully describe everything about the codeâand if there are too many separate pieces of UI presenting information, it's easy to forget that some of them exist. But this is a discussion for another thread I think. > > On Thu, Jan 26, 2023 at 5:48 AM Marcus Denker <marcus.denker(a)inria.fr <mailto:marcus.denker@inria.fr>> wrote: >> >> >> > On 26 Jan 2023, at 11:27, Marcus Denker <marcus.denker(a)inria.fr <mailto:marcus.denker@inria.fr>> wrote: >> > >> > >> > >> >> On 26 Jan 2023, at 11:21, Nicolas Anquetil <nicolas.anquetil(a)inria.fr <mailto:nicolas.anquetil@inria.fr>> wrote: >> >> >> >> correct me if I am wrong, but one can already deactivate a rule in Nautilus on a perpackage/class/method basis. >> >> >> >> So why not use this mechanism instead of having yet another pragma >> >> >> > >> > Because disabeling the rule disables it for *all* selectors >> > >> >> (there is a whole universe of pragmas hidden in the code) >> > >> > But the pragma is less hidden than the banned rule, which is completely invisible. >> > >> >> The whole rule banning meta data storage is very very limited right nowâ¦ e.g it does never get cleaned, is never updated >> if the code it annotates changes and has many problems. >> >> The long-term solution I want to do is to improve meta-data for code storage and use it for the rule banning >> and for things we might abuse pragmas now. >> >> But that will no be soon, and there are lots of things to be taken into account for a nice design. >> >> For me the pragma is a nice intermediate solution: per selector, survives rename of method, rename of >> class, move method, is visibleâ¦ >> >> Marcus >>

Jan. 30, 2023

This week (4/2023) on the Pharo Issue Tracker

by Marcus Denker

This week, we merged 28 Pull requests, fixing 31 issues Fixes ===== - Update BaselineOfPharo with Roassal3 version v0.9.9L #12430 https://github.com/pharo-project/pharo/pull/12430 - checkIfMustRefresh in OmBlockFileStore>>blockForLocalNameAsInteger: #12197 https://github.com/pharo-project/pharo/pull/12197 - Fix for multiple Epicea issues #12375 https://github.com/pharo-project/pharo/pull/12375 - FIX CI issues - FFIExternalString>>#basicHandle:at: calls deprecated message #12405 https://github.com/pharo-project/pharo/pull/12405 - Fix for "Browse this version in Calypso" Error after clicking an unloaded package #12423 https://github.com/pharo-project/pharo/pull/12423 - 12410-Minimal-Image-does-not-open-ChangesFile-correctly #12411 https://github.com/pharo-project/pharo/pull/12411 - fix testTallyInstructions #12393 https://github.com/pharo-project/pharo/pull/12393 - Spotter is broken> DNU #tools #12397 https://github.com/pharo-project/pharo/pull/12397 Cleanups ======== - RubScrolledTextMorph>>#mouseWheel: sends unimplemented message #scrolâ¦ #12402 https://github.com/pharo-project/pharo/pull/12402 - 12416-Fixing-Classes-Reported-by-NoUnusedVariablesLeftTest #12417 https://github.com/pharo-project/pharo/pull/12417 - 12413-Improving-the-dependency-of-Formatter-Settings-with-EFFormatter #12414 https://github.com/pharo-project/pharo/pull/12414 - Move #asNumber from AST-Core to Kernel (where it is also already tested) #12388 https://github.com/pharo-project/pharo/pull/12388 - extract return from ifTrue:ifFalse: in Integer>>#printStringRadix: #12433 https://github.com/pharo-project/pharo/pull/12433 - 11x Cleanups of the kind Cleanup: Condense Calypso packages #12284 https://github.com/pharo-project/pharo/pull/12284 this Changelog is archived at https://github.com/pharo-project/pharo-changelogs/blob/master/weekly/2023/4…

Jan. 27, 2023

Re: Code quality: tagging unsent selectors (e.g. in tests)

by Daniel Slomovits

A bit of perspective on other approaches to this problem: In Dolphin Smalltalk, the FFI mechanism uses #doesNotUnderstand: to implement getters and setters on external structs, so it already has a mechanism for ensuring they aren't flagged as unimplemented. In addition to the standard ClassDescription>>selectors, which is more-or-less the actual keys of the methodDictionary, there is #understoodSelectors, which is explicitly a hook to include others understood via #doesNotUnderstand:. These are then treated exactly as actually-implemented selectors, that is, they are available for autocomplete as well as not being flagged as unimplemented. Also, I believe the unimplemented-selectors rule applies only to non-self-sends, and there is a separate rule for self-sends which instead just asks the class if it #canUnderstand: the selector, which creates a middle ground where the selector is not "known" to the system at large, but won't be warned about if self-sent. The pragma approach has the advantage of being more specific, which is useful if a certain selector is valid only in a very narrow context and should still be warned about in general. Conversely, the disadvantage is that it needs to be repeated everywhere the message is sent, rather than centralized on the class where it is implemented/understood. I assume it also does not influence autocomplete. So, for cases that are valid in a broader scope, it creates the extra work of adding the pragma, *and* you have to type out the actual send manually, multiplied for each time it's usedâlots of repetition. Fundamentally we're running up against the limitations of a dynamically-typed language here. Long story short, I think both options *may* be useful. In my own usage I find it's about 50/50âsome cases of one-offs in a test, other cases of a widely-used class with some dynamic behavior. I do like the pragma as an improvement over rule banning for sure. Looking towards improving rule banning, I think it's very important for that to be (a) highly visible (and nothing is more visible than something that's part of the source code) and (b) manageable via source control (which metadata certainly *can* beâcategories, for instance, already are). Call me old-fashioned, but for all the wonders of the image, there's something to be said for having a limited set of concepts in a "dead text" format that fully describe everything about the codeâand if there are too many separate pieces of UI presenting information, it's easy to forget that some of them exist. But this is a discussion for another thread I think. On Thu, Jan 26, 2023 at 5:48 AM Marcus Denker <marcus.denker(a)inria.fr> wrote: > > > > On 26 Jan 2023, at 11:27, Marcus Denker <marcus.denker(a)inria.fr> wrote: > > > > > > > >> On 26 Jan 2023, at 11:21, Nicolas Anquetil <nicolas.anquetil(a)inria.fr> > wrote: > >> > >> correct me if I am wrong, but one can already deactivate a rule in > Nautilus on a perpackage/class/method basis. > >> > >> So why not use this mechanism instead of having yet another pragma > >> > > > > Because disabeling the rule disables it for *all* selectors > > > >> (there is a whole universe of pragmas hidden in the code) > > > > But the pragma is less hidden than the banned rule, which is completely > invisible. > > > > The whole rule banning meta data storage is very very limited right nowâ¦ > e.g it does never get cleaned, is never updated > if the code it annotates changes and has many problems. > > The long-term solution I want to do is to improve meta-data for code > storage and use it for the rule banning > and for things we might abuse pragmas now. > > But that will no be soon, and there are lots of things to be taken into > account for a nice design. > > For me the pragma is a nice intermediate solution: per selector, survives > rename of method, rename of > class, move method, is visibleâ¦ > > Marcus > >

Jan. 26, 2023

Re: Code quality: tagging unsent selectors (e.g. in tests)

by Marcus Denker

> On 26 Jan 2023, at 11:27, Marcus Denker <marcus.denker(a)inria.fr> wrote: > > > >> On 26 Jan 2023, at 11:21, Nicolas Anquetil <nicolas.anquetil(a)inria.fr> wrote: >> >> correct me if I am wrong, but one can already deactivate a rule in Nautilus on a perpackage/class/method basis. >> >> So why not use this mechanism instead of having yet another pragma >> > > Because disabeling the rule disables it for *all* selectors > >> (there is a whole universe of pragmas hidden in the code) > > But the pragma is less hidden than the banned rule, which is completely invisible. > The whole rule banning meta data storage is very very limited right nowâ¦ e.g it does never get cleaned, is never updated if the code it annotates changes and has many problems. The long-term solution I want to do is to improve meta-data for code storage and use it for the rule banning and for things we might abuse pragmas now. But that will no be soon, and there are lots of things to be taken into account for a nice design. For me the pragma is a nice intermediate solution: per selector, survives rename of method, rename of class, move method, is visibleâ¦ Marcus

Jan. 26, 2023

Re: Code quality: tagging unsent selectors (e.g. in tests)

by Marcus Denker

> On 26 Jan 2023, at 11:21, Nicolas Anquetil <nicolas.anquetil(a)inria.fr> wrote: > > correct me if I am wrong, but one can already deactivate a rule in Nautilus on a perpackage/class/method basis. > > So why not use this mechanism instead of having yet another pragma > Because disabeling the rule disables it for *all* selectors > (there is a whole universe of pragmas hidden in the code) But the pragma is less hidden than the banned rule, which is completely invisible. Marcus

Jan. 26, 2023

Re: Code quality: tagging unsent selectors (e.g. in tests)

by Nicolas Anquetil

correct me if I am wrong, but one can already deactivate a rule in Nautilus on a perpackage/class/method basis. So why not use this mechanism instead of having yet another pragma (there is a whole universe of pragmas hidden in the code) nicolas On 26/01/2023 10:46, Marcus Denker wrote: > Hi, > > If you have in your code a message send with a selector that is not implemented anywhere, it likely will result in a DNU > > - but not if you compile a method on the fly (used in tests often) > - if the DNU is catched (e.g. for a proxy pattern) > - if the code is compatibility code, never executed in this version of the system > > > We can ban the rule, but that just stopps checking for the whole method for all selectors, what we want is to communicate that *this selector* is ok in this > special use (per method granularity is ok). > > SystemNaviation>>#allUnimplementedCalls did have the feature that one can tag selectors that should be ignored. This uses the pragma #ignoreUnimplementedCalls: > > see > Context class >> initializePrimitiveSimulators > > > <ignoreUnimplementedCalls: #(#registerPrimitiveSimulators)> > > This was used in just one method, I think because a) the rule did not take it into account and b) the terminology is just wrong (âCallâ). > > I think > > - ReSentNotImplementedRule should takes this pragma into account > - change the bad terminology âallUnimplementedCalls" > - use pragma: <ignoreNotImplementedSelectors: > > > > PR is here: https://github.com/pharo-project/pharo/pull/12295 > > > (what the PR does not yet do is to change syntax highlight to not color the selector red, this could be done later, too) -- Nicolas Anquetil RMod team -- Inria Lille

Jan. 26, 2023

Code quality: tagging unsent selectors (e.g. in tests)

by Marcus Denker

Hi, If you have in your code a message send with a selector that is not implemented anywhere, it likely will result in a DNU - but not if you compile a method on the fly (used in tests often) - if the DNU is catched (e.g. for a proxy pattern) - if the code is compatibility code, never executed in this version of the system We can ban the rule, but that just stopps checking for the whole method for all selectors, what we want is to communicate that *this selector* is ok in this special use (per method granularity is ok). SystemNaviation>>#allUnimplementedCalls did have the feature that one can tag selectors that should be ignored. This uses the pragma #ignoreUnimplementedCalls: see Context class >> initializePrimitiveSimulators <ignoreUnimplementedCalls: #(#registerPrimitiveSimulators)> This was used in just one method, I think because a) the rule did not take it into account and b) the terminology is just wrong (âCallâ). I think - ReSentNotImplementedRule should takes this pragma into account - change the bad terminology âallUnimplementedCalls" - use pragma: <ignoreNotImplementedSelectors: > PR is here: https://github.com/pharo-project/pharo/pull/12295 (what the PR does not yet do is to change syntax highlight to not color the selector red, this could be done later, too)

Jan. 26, 2023

[Issue Tracker] reviews needed

by Marcus Denker

Hi, I wanted again to make some noise about the issue tracker. Yes, we merge 20-40 PRs per week, but even with that, there are many PRs in the queueâ¦ e.g. - Complete symbols #11918 https://github.com/pharo-project/pharo/pull/11918 - 12128-Poor-performance-with-large-WriteStreams #12205 https://github.com/pharo-project/pharo/pull/12205 - Fix Calypso browsing unloaded package artifacts from Iceberg #12425 https://github.com/pharo-project/pharo/pull/12425 - Introducing a Color Palette to cache the allocation of Color objects by tht UITheme #12419 https://github.com/pharo-project/pharo/pull/12419 We have open 35 PRs, the complete list is here: https://github.com/pharo-project/pharo/pulls some random notes: - reviews are always helpful, and even if there is already a positive review, a second one helps. - one part of a review is to check that the code is not doing obvious bad things (like un-doing older changes, dumb mistakesâ¦.) if you feel unsure what you can do as you do not really understand the change, that one you can check (and just say that in the review). - if you do PRs e.g with a pair, ask the person that did not commit to add a real review so it is clear that there where already two eyes on the code - if you work in a team, ask your team mates to review your code especially if you use that fix already internally - if you use the fix in your project already, add a review to just state that - if you see a fix where you think âI hope this will be merged soonâ, if you review, you will make it much easierâ¦ - if you see a fix that is reviewed, green and not merged and you do not understand why: do a review - consider just reviewing / doing *something* once every week. or even once every day, instead of letting it all pile up. Marcus

Jan. 26, 2023

Re: Pharo11 32 bits images missing?

by Guillermo Polito

Nope, this is most certainly a bug, thanks for raising the flag! > El 22 ene. 2023, a las 18:10, Pierre Misse Chanabier <pierre.misse-chanabier(a)inria.fr> escribiÃ³: > > Hello, > > Apparently Pharo 11 does not build 32 bit images [1]. > Did I miss an announcement? > > Pierre > > http://files.pharo.org/image/110/

Jan. 23, 2023

Pharo11 32 bits images missing?

by Pierre Misse Chanabier

Hello, Apparently Pharo 11 does not build 32 bit images [1]. Did I miss an announcement? Pierre http://files.pharo.org/image/110/

Jan. 22, 2023