Thanks Marcus
On Apr 14 2021, at 10:53 am, Marcus Denker <marcus.den...@inria.fr> wrote: > Just a status update: we merged all the code. > > - we now have for both the class and method view a Critique rule that warns > you if you use shadowing vars. > > - Pavel fixed all the cases of shadowing class variables. Thanks! > > - release tests are active, #testNoShadowedVariablesInMethods fo methods and > at the class level: > > > testClassesShadow > > | classes | > classes := Smalltalk globals allBehaviors select: [ :class | > class definedVariables anySatisfy: [ :var | > var isShadowing ] ]. > > self assert: classes isEmpty description: classes asArray asString > > > status cleanups: > > - DONE: Shadowed vars (as we saw) > => TODO: combine the snippets of writing into one blog post for the devblog > - DONE: unused Class Variables > - ongoing: unused ivars. Just some left in Spec+Newtools (and one in Kernel > tests) > - ongoing: all methods should be categorized. > - todo: we should have no method that is the same as a method in a superclass > > > > On 2 Apr 2021, at 15:33, Marcus Denker <marcus.den...@inria.fr > > (mailto:marcus.den...@inria.fr)> wrote: > > > > > > # Code Critique > > > > What we need next is a Code Critique rule. This for once warns developers > > early, but in addition it will > > make it much easier to fix the 6 problem cases that the release test > > exposed. > > > > Checking for current senders of #definedVariables leads us to > > ReVariableAssignedLiteralRule, we can use this as a template. > > The main method doing the check looks like this: > > > > ```Smalltalk > > check: aClass forCritiquesDo: aCriticBlock > > > > aClass definedVariables do: [ :variable | > > variable isShadowing ifTrue: [ > > aCriticBlock cull: (self critiqueFor: aClass about: variable name) ] ] > > ``` > > > > The full PR is here: https://github.com/pharo-project/pharo/pull/8940
