Skip to content

Commit

Permalink
rules: improve VarOp handling
Browse files Browse the repository at this point in the history
as described in the linked issue, previously false positives were
reported for various rules checking on VarOp.
Instead of looking for hardcoded ` <op> ` pattern, we
just strip the result from the parser and compare it to
equally stripped results.
This of course doesn't apply to
oelint.vars.spacesassignment rule.

Signed-off-by: Konrad Weihmann <[email protected]>
  • Loading branch information
priv-kweihmann committed Aug 23, 2023
1 parent d9cdb7d commit c05e695
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 15 deletions.
2 changes: 1 addition & 1 deletion oelint_adv/rule_base/rule_append_protvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def check(self, _file, stash):
items = stash.GetItemsFor(
filename=_file, classifier=Variable.CLASSIFIER)
for i in [x for x in items if x.VarName in CONSTANTS.VariablesProtectedAppend]:
if i.VarOp not in [' ??= ', ' ?= '] and i.Flag not in ['vardeps', 'vardepsexclude', 'vardepvalue', 'vardepvalueexclude']:
if i.VarOp.strip() not in ['??=', '?='] and i.Flag not in ['vardeps', 'vardepsexclude', 'vardepvalue', 'vardepvalueexclude']:
res += self.finding(i.Origin, i.InFileLine, override_msg=self.Msg.replace(
'{VAR}', i.VarName), appendix=i.VarName)
return res
8 changes: 4 additions & 4 deletions oelint_adv/rule_base/rule_var_appendop.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ def check(self, _file, stash):
filename=_file, classifier=Variable.CLASSIFIER)
_names = {x.VarName for x in items if x.VarName != 'inherit'}
for name in _names:
_weakest = [x for x in items if x.VarName == name and x.VarOp == ' ??= ']
_weak = [x for x in items if x.VarName == name and x.VarOp == ' ?= ']
_items = [x for x in items if x.VarName == name and x not in _weakest + _weak and x.VarOp in [' .= ', ' += ']]
_weakest = [x for x in items if x.VarName == name and x.VarOp.strip() == '??=']
_weak = [x for x in items if x.VarName == name and x.VarOp.strip() == '?=']
_items = [x for x in items if x.VarName == name and x not in _weakest + _weak and x.VarOp.strip() in ['.=', '+=']]
for i in _items:
override_delimiter = i.OverrideDelimiter
if any(_weakest):
Expand All @@ -25,7 +25,7 @@ def check(self, _file, stash):
elif any(x.Line > i.Line for x in _weak):
res += self.finding(i.Origin, i.InFileLine, override_msg=self.Msg.format(
a='{od}append'.format(od=override_delimiter), b=i.VarOp, c=_weak[0].Raw))
_items = [x for x in items if x.VarName == name and x not in _weakest + _weak and x.VarOp in [' =. ', ' =+ ']]
_items = [x for x in items if x.VarName == name and x not in _weakest + _weak and x.VarOp.strip() in ['=.', '=+']]
for i in _items:
override_delimiter = i.OverrideDelimiter
if any(_weakest):
Expand Down
2 changes: 1 addition & 1 deletion oelint_adv/rule_base/rule_var_bbvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def check(self, _file, stash):
items = stash.GetItemsFor(
filename=_file, classifier=Variable.CLASSIFIER)
for i in [x for x in items if x.VarName in CONSTANTS.VariablesProtected]:
if i.VarOp not in [' ??= ', ' ?= ']:
if i.VarOp.strip() not in ['??=', '?=']:
res += self.finding(i.Origin, i.InFileLine, override_msg=self.Msg.replace(
'{VAR}', i.VarName), appendix=i.VarName)
return res
2 changes: 1 addition & 1 deletion oelint_adv/rule_base/rule_var_filesextrapathsop.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ def check(self, _file, stash):
attribute=Variable.ATTR_VAR)
for i in items:
if i.VarName in ['FILESEXTRAPATHS']:
if i.VarOp != ' := ':
if i.VarOp.strip() != ':=':
res += self.finding(i.Origin, i.InFileLine)
return res
2 changes: 1 addition & 1 deletion oelint_adv/rule_base/rule_var_src_uri_append.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def check(self, _file, stash):
if any(item.Flag.endswith(x) for x in ['md5sum', 'sha256sum']):
# These are just the hashes
continue
if item.VarOp in [' += ']:
if item.VarOp.strip() in ['+=']:
override_delimiter = item.OverrideDelimiter
res += self.finding(item.Origin, item.InFileLine,
'Use SRC_URI{od}append otherwise this will override weak defaults by inherit'.format(
Expand Down
2 changes: 1 addition & 1 deletion oelint_adv/rule_base/rule_vars_variable_override.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def check(self, _file, stash):
# as these will be handled during parse time
# and apply to different rules
_items = [x for x in items if x.SubItem == sub and not x.IsAppend(
) and x.VarOp not in [' := ', ' ?= ', ' ??= '] and not x.Flag]
) and x.VarOp.strip() not in [':=', '?=', '??='] and not x.Flag]
if len(_items) > 1:
_files = {os.path.basename(x.Origin) for x in _items}
res += self.finding(_items[0].Origin, _items[0].InFileLine,
Expand Down
8 changes: 4 additions & 4 deletions tests/test_class_oelint_append_protvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def __generate_sample_code(self, var, operation):
'LICENSE',
'LIC_FILES_CHKSUM',
])
@pytest.mark.parametrize('operation', ['=', ':='])
@pytest.mark.parametrize('operation', ['=', ':=', ' :='])
def test_bad(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bbappend': self.__generate_sample_code(var, operation),
Expand All @@ -34,7 +34,7 @@ def test_bad(self, id_, occurrence, var, operation):
'LICENSE',
'LIC_FILES_CHKSUM',
])
@pytest.mark.parametrize('operation', ['?=', '??='])
@pytest.mark.parametrize('operation', ['?=', '??=', ' ??='])
def test_good_weak(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bbappend': self.__generate_sample_code(var, operation),
Expand All @@ -51,7 +51,7 @@ def test_good_weak(self, id_, occurrence, var, operation):
'LICENSE',
'LIC_FILES_CHKSUM',
])
@pytest.mark.parametrize('operation', ['=', ':='])
@pytest.mark.parametrize('operation', ['=', ':=', ' :='])
def test_good_bb(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bb': self.__generate_sample_code(var, operation),
Expand All @@ -67,7 +67,7 @@ def test_good_bb(self, id_, occurrence, var, operation):
'PV[vardepvalue]',
'PV[vardepvalueexclude]',
])
@pytest.mark.parametrize('operation', ['=', ':='])
@pytest.mark.parametrize('operation', ['=', ':=', ' :='])
def test_good_flags(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bbappend': self.__generate_sample_code(var, operation),
Expand Down
7 changes: 7 additions & 0 deletions tests/test_class_oelint_var_override.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ def test_bad(self, input_, id_, occurrence):
A ??= "1"
''',
},
{
'oelint_adv_test.bb':
'''
A = "1"
A ??= "1"
''',
},
{
'oelint_adv_test.bb':
'''
Expand Down
28 changes: 28 additions & 0 deletions tests/test_class_oelint_vars_appendop.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ class TestClassOelintVarAppendOp(TestBaseClass):
I ?= "1"
''',
},
{
'oelint_adv_test.bb':
'''
I =. "2"
I ?= "1"
''',
},
{
'oelint_adv_test.bb':
'''
G =+ "B"
G ?= "A"
''',
},
],
)
def test_bad(self, input_, id_, occurrence):
Expand Down Expand Up @@ -186,6 +200,20 @@ def test_bad(self, input_, id_, occurrence):
I ?= "1"
''',
},
{
'oelint_adv_test.bb':
'''
I:prepend = "2"
I ?= "1"
''',
},
{
'oelint_adv_test.bb':
'''
D:append = "2"
D ?= "1"
''',
},
],
)
def test_good(self, input_, id_, occurrence):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_class_oelint_vars_bbvars.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def __generate_sample_code(self, var, operation):
'STAMP',
'TOPDIR',
])
@pytest.mark.parametrize('operation', ['=', ':=', '.=', '=.', '+=', '=+'])
@pytest.mark.parametrize('operation', ['=', ':=', '.=', '=.', '+=', '=+', ' =+'])
def test_bad(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bb': self.__generate_sample_code(var, operation),
Expand Down Expand Up @@ -171,7 +171,7 @@ def test_bad(self, id_, occurrence, var, operation):
'STAMP',
'TOPDIR',
])
@pytest.mark.parametrize('operation', ['?=', '??='])
@pytest.mark.parametrize('operation', ['?=', '??=', ' ??='])
def test_good_weak(self, id_, occurrence, var, operation):
input_ = {
'oelint_adv_test.bb': self.__generate_sample_code(var, operation),
Expand Down
8 changes: 8 additions & 0 deletions tests/test_class_oelint_vars_fileextrapathsop.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class TestClassOelintVarsFilextrapaths(TestBaseClass):
'oelint_adv_test.bb':
'FILESEXTRAPATHS =+ "${THISDIR}/file"',
},
{
'oelint_adv_test.bb':
'FILESEXTRAPATHS =+ "${THISDIR}/file"',
},
],
)
def test_bad(self, input_, id_, occurrence):
Expand Down Expand Up @@ -70,6 +74,10 @@ def test_bad(self, input_, id_, occurrence):
'oelint_adv_test.bbappend':
'FILESEXTRAPATHS := "${THISDIR}/file"',
},
{
'oelint_adv_test.bbappend':
'FILESEXTRAPATHS := "${THISDIR}/file"',
},
],
)
def test_good(self, input_, id_, occurrence):
Expand Down
11 changes: 11 additions & 0 deletions tests/test_class_oelint_vars_srcuriappend.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ class TestClassOelintVarsSRCURIappend(TestBaseClass):
inherit abc
''',
},
{
'oelint_adv_test.bb':
'''
SRC_URI += "file://abc"
inherit abc
''',
},
],
)
def test_bad(self, input_, id_, occurrence):
Expand All @@ -29,6 +36,10 @@ def test_bad(self, input_, id_, occurrence):
'oelint_adv_test.bb':
'SRC_URI += "file://abc"',
},
{
'oelint_adv_test.bb':
'SRC_URI += "file://abc"',
},
{
'oelint_adv_test.bb':
'''
Expand Down

0 comments on commit c05e695

Please sign in to comment.