wdl icon indicating copy to clipboard operation
wdl copied to clipboard

Fix how exceptions work

Open claymcleod opened this issue 7 months ago • 1 comments

In #130, we discovered a bug in how excepting rules work. Here's the description of the problem as posted in Slack and also the suggested fix:

The rule actually works if you remove the except:. As has been hinted, adding the except statement means that the callback for command_text is never called. That, in turn, means that only newlines before and after the command segment are respected, and this behavior of skipping the processing of callbacks for excepted rules is a problem for rules that need to keep track of state—even if they don't report any diagnostics in that area.

One solution is to simply make available the information for every callback whether or not the current callback is excepted. I think this is less optimal for a variety of reasons.

A better approach would probably be locking of the Diagnostics object so that no new diagnostics can be added during a rule's excepted period. In that way, designers of rules wouldn't have to always be thinking about whether things are excepted: they'd just call state.add(_) and that would simply fail to add the diagnostic. Of course, that method may need to be renamed given that it would no longer be guaranteed, and we might have some result indicating back to the rule writer if the add was successful or not.

@a-frantz also suggested tracking whether or not expected exceptions had an effect and flagging exceptions that had no effect.

Once this is finished, the tests introduced in #130 should change, and only the offending line should be flagged as too long (instead of the entire command block).

claymcleod avatar Jul 19 '24 14:07 claymcleod