priv-kweihmann / oelint-adv

Advanced oelint
BSD 2-Clause "Simplified" License
54 stars 27 forks source link

oelint.vars.multilineident seems to contradict Yocto Recipe Style Guide #515

Closed amuetzel closed 6 months ago

amuetzel commented 6 months ago

The rule oelint.vars.multilident seems to warn about formatting that is correct according to the Yocto Recipe Style Guide: On multiline strings, it always produces a warning if the first line has actual content apart from the backslash character.

Is this the intended behaviour? If not, would a fix actually be desired, since it would change the behaviour for existing recipes?

Steps to reproduce: Add this to an existing recipe (literal example from the linked style guide) or new .bb file:

FOO = "this line is \
       long \
       "

This will result in two warnings regarding the indentation:

[...]/test.bb:9:info:oelint.vars.multilineident:On a multiline assignment, line indent is desirable. 0 set, 4 desirable
[...]/test.bb:10:info:oelint.vars.multilineident:On a multiline assignment, line indent is desirable. 7 set, 4 desirable

However, this will not print any warning:

FOO = "\
       this line is \
       long \
       "

And this should probably result in a warning, but only a warning of type info:oelint.vars.notneededspace:Space at the beginning of the var is not needed happens:

FOO = "       this line is \
       long \
       "

Version: $ oelint-adv --version oelint-adv 4.3.1

priv-kweihmann commented 6 months ago

I tend to disagree with the Yocto style guide. It it much better to write

# 4 spaces indent
FOO = "\
    this \
    line \
    is \
    long \
"

than

# mixed level of indent
FOO = "this line is \
       long \
       "

in terms of readability. The rule is less sharp about 4 spaces in the beginning if the indent is at least consistent over all lines, thus

 # same number of spaces on each line
 FOO = "\
       this line is \
       long \
       "

doesn't produce a warning. Anyway, I find the Yocto styleguide falling short on what the actual purpose of such a rule should be.

So the compromise so far was just notify the user (info level) and be less sharp on the 4 spaces of indent. BTW you can find the original styleguide here https://www.openembedded.org/index.php?title=Styleguide&oldid=10281, which does make much more sense to me to be honest - funny that upstream changed that

priv-kweihmann commented 6 months ago

So in conclusion over different yocto versions contradicting things are proposed by upstream - I tend to not fix that, but PRs are welcome

amuetzel commented 6 months ago

Thanks for the explanation! That does make sense - I'll see if I can think of some consistent logic for all these cases and send a PR, if I find the time to do so :)