user-customizable code critics

[peteruhnak]

In Roassal there's very common pattern

el := RTEllipse new
        size: 14;
        color: Color black;
        element

so I am in fact returning an instance of RTElement and not the receiver.
Of course QA says missing ";yourself".

This would require to be able to specify conditions for the code critic, such as
if receiver is subclass of RTShape then don't apply this (yourself) rule

[peteruhnak]

Another example is (also taken from Roassal)

container @ RTDraggable @ RTResizable.

with critic "Possible three element point".
Again the critic doesn't consider that someone else than a point can name a method @.

[Uko]

Thanks for another use-case. Maybe this rule should be removed altogether, as I doubt that it provides actual help.

As a side note:

if someone not knowing Roassal API will look at your code, he/she may think that you are sending something to the result of container @ RTDraggable. But if you will write your code as:

container
  @ RTDraggable;
  @ RTResizable.

it's 100% clear that you are sending messages to container.

[peteruhnak]

Well then the question is then who is the target user. I would say that people that work with the code regularly need it much more than people that occasionally look at the code and thus it should cater more to them.

So if someone doesn't know Roassal API then I don't think that I want him to touch my roassal-related code. And technically speaking I am sending it to the result, I just know what the result is.

[peteruhnak]

I have another case:

in PetitParser when creating delegate parsers (subclasses of PPCompositeParser) one would use instance variables to refer to them, however they are never explicitly written to (it is done via metaprogramming), so QA will complain about all the variables.

In addition the critic says "[...] Instance variables not read AND written", which is false, because they are read from.

[Uko]

@peteruhnak thanks for mentioning this. I've already noticed that PetitParser is a good place to work on customizability of the rules, as there are many of them that criticize PP as it uses plenty of metaprogramming. As for "Instance variables not read AND written" it's again a very bad naming of the rule that does not explain the real issue. I still have to figure out what the rule does and how to rename it :)

[peteruhnak]

I would also propose to ignore Refers to class name instead of "self class" rule for class-side example methods. It is in many instances (e.g. Roassal )common use case to copy the code, paste to playground and play with it, so one would have to always fix the name.

Plus the name of the rule doesn't make sense on the class-side, because self class is a metaclass.

[Uko]

@peteruhnak can you give me a roassal class for example? So I can get a better understanding

[peteruhnak]

Looking back through it.... most examples are actually moved to separate *Example, where the problem doesn't arise, so maybe it's just my habit and Alex moves it away once I commit it. :)

Look at the following methods

classes := Object withAllSubclasses select: [ :each | each className beginsWith: 'RT' ] thenCollect: #theMetaClass.
((classes flatCollect: #methods) select: [ :each |
        each selector beginsWith: 'example' ])
        select: [ :each |
    each critics detect: [ :re | re rule isMemberOf: RBRefersToClassRule ] ifFound: [ true ] ifNone: [ false ] ]

For example RTResizable class>>example