Extend "unused local assignment" (strict mode) check to include some .. in bindings

[anderseknert]

While manually reviewing Regal policies recently, I found one case where a some .. in construct was put in the start of a rule body, but neither code in the head or body of that rule referenced the var(s) bound there. This was most likely a result of the code having been rewritten at some point, and that single line having been missed. That mistake can be hard to spot as the unused "loop" unlikely impacts the output of the rule, or will make any tests fail. The rule will however perform terrible in comparison., and in Regal (for a rule that did a lot things in the rule body) this mistake accounted for almost a million heap allocations made for no good reason.

oops if {
    some item in input.large.array
    
    # a bunch of code here that never references item
}

We can make a Regal rule to flag this, and if you think that's a better place for this check, we can close this issue and start work on that. We try to avoid overlapping features with OPA though, and since this seems like a good candidate for strict mode, I'll raise it here first for consideration.

[anderseknert]

@johanfylling @sspaink @srenatus thoughts on extending the strict mode rule vs. making a Regal linter rule?

[johanfylling]

This seems fairly close to the existing unused-assignment strict check, and I wouldn't mind adding it to strict-mode.
OTOH, not everyone uses strict mode, and the rich explanations Regal can provide for issues has great advantages, I think. Apart from amounting to some double work, are there good reasons for not implementing this in both OPA and Regal?

[srenatus]

"some ... in ... " is syntactic sugar, I wonder it might merely be a matter of reordering compiler stages to have this covered by strict mode 🤔

[anderseknert]

OTOH, not everyone uses strict mode, and the rich explanations Regal can provide for issues has great advantages

100% so, yes. I think the long term plan is to integrate compilation and strict mode into Regal, as we'll want to catch compilation errors in the language server in particular. So the reason for not having this in both would be that.