chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.8k stars 421 forks source link

[Feature Request]: linter rule for badly spaced `,` and `:` #26174

Open jabraham17 opened 3 weeks ago

jabraham17 commented 3 weeks ago

In lieu of a proper chapel syntax formatter (which still has some technical limitations), I think we can and should have more lint rules that catch some oddities in code.

Two that come to mind are , and :, where a user could have any of the following combinations

var y,x: int;
var x  , y : int;

proc foo(x:int ,y :int): int { }

I think an easy rule from this is to prefer x, y to x ,y/x , y. However, some users may prefer x,y. I think the important thing here is consistency, and so maybe the linter rule can establish what the user has done in the file x,y or x, y, and then suggest that.

: suffers from a similar issue, to my mind x: type and x : type are both valid and some users may prefer one or the other (or some other combination). So rather than dictating one way or another I think a lint rule for this should have a set of "acceptable" patterns, and then enforce consistency

There are some technical challenges with this, as its likely we will need to improve location tracking in the parser to get these rules to work.

We could also consider having warnings for expressions like x+y+z, instead preferring x + y + z. But the location tracking for this is going to be difficult, especially when parenthesis get involved (we have limited support for this today). We could also consider tracking locations for operators, attached to OpCall nodes.

bradcray commented 3 weeks ago

However, some users may prefer x,y. I think the important thing here is consistency, and so maybe the linter rule can establish what the user has done in the file x,y or x, y, and then suggest that.

I have to admit to being a problem child here. I typically use x, y, but sometimes use x,y in certain cases where the spaces seem more annoying than beneficial. E.g., [(i,j) in MyDomain] often reads nicer to me than [(i, j) in MyDomain] (not that I'd balk at someone using the latter). But I'd probably always write [(row, col) in MyDomain] just because the identifiers are longer So I'm intentionally/consciously inconsistent in my own code based on subtle style preferences and would probably need to disable this warning if it was either strict or clever. I'd always want to be warned about , though, I think.

For :, I definitely prefer x: type over x : type and don't think I ever intentionally use the latter. But I do sometimes intentionally use and prefer x:type particularly in casting situations.

Ditto binary operators. Sometimes I use spaces religiously; sometimes I use spacing to denote precedence. For example a*b + c is something I'll often write instead of a * b + c or (a*b) + c (again, not that I'd balk at either of the latter cases).