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.denker@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 <https://github.com/pharo-project/pharo/pull/8940>