nmoroze / tclint

EDA-centric utility for linting and analyzing Tcl code.
MIT License
27 stars 1 forks source link

Fix indentation check for continuations of commands starting on same line as script #35

Open nmoroze opened 3 months ago

nmoroze commented 3 months ago

Examples:

if {1} {puts $chan \
    "foo"}

Actual:

test.tcl:2:1: expected indent of 0 spaces, got 4 [indent]

Expected: no violations

eval cmd \
a \
    b

Actual:

test.tcl:3:1: expected indent of 0 spaces, got 4 [indent]

Expected:

test.tcl:2:1: expected indent of 4 spaces, got 0 [indent]
maliberty commented 2 months ago

I'm seeing the same problem in OR (eg https://github.com/The-OpenROAD-Project/OpenROAD/actions/runs/9783206598/job/27011254166?pr=5272) image

image

nmoroze commented 2 months ago

@maliberty thanks for the pointer, I'll take a look ASAP.

In the meantime, you can add comments to locally disable indent checking and get your CI back to green.

# tclint-disable indent
[ code ]
# tclint-enable indent

See docs for more info.

BTW, I think tclint will want a different style than you have, e.g. it'll ask for:

mpl2::set_debug_cmd $block \
  $coarse \
  $fine \
  ...

If you prefer the aligned style I can look into supporting it, but I think that'll be lower priority.

maliberty commented 2 months ago

We have lots of other code like this that doesn't get flagged. Any idea why just here?

nmoroze commented 2 months ago

The violation on line 313 is related to the # checker comment. Dropping the closing brace to a lower line seems to be how this is done in other parts of the codebase, e.g. https://github.com/The-OpenROAD-Project/OpenROAD/blob/efe05a9c9aee2955753945eca8a9e7aaee40b8cd/src/pad/src/pad.tcl#L142. That fixes the problem, although this is a bug in tclint that I'll address (#48).

I think the remaining violations are actually just tclint working as intended. Do you have any examples of it allowing that style elsewhere? There are two exceptions where it won't check the indentation of multiline commands (in expressions [#20] and command substitutions [#23]), so it's possible the other examples you've seen have been one of those two cases (e.g. the mpl2::rtl_macro_placer_cmd in mpl.tcl is inside an if statement expression, so it's not currently being checked).

maliberty commented 2 months ago

Perhaps it is just that those two exceptions dominate. I would like to see the indented style supported. For now I've used your disable suggestion.