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 <[email protected]> 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>
>
>
