pkgcore / pkgcheck

pkgcore-based QA utility for ebuild repos
https://pkgcore.github.io/pkgcheck
BSD 3-Clause "New" or "Revised" License
34 stars 29 forks source link

[New Check]: Multiple assignments to a variable (maybe just eclass vars?) in global scope #622

Closed thesamesam closed 9 months ago

thesamesam commented 9 months ago

Is there an existing such new check request for this?

Explain

Assigning twice to a variable in global scope is a serious code smell and is likely a typo:

SRC_URI="https://example.com/foo.tar.gz"
S="${WORKDIR}/x"

LICENSE="..."

S="${WORKDIR}"
src_compile() {
:;
}

We could also restrict the check to just eclass variables, although I'd like to try it without that restriction first.

C more or less calls this -Wshadow (not quite as there's some interaction w/ scoping which bash lacks).

Examples:

Examples

pecl-pttp (https://github.com/gentoo/gentoo/pull/32762/files#diff-c95f1d4880dbd4fd60024792709466e4113c4a06fe6d46b5b660c00e55f5440a, https://github.com/gentoo/gentoo/blob/4463a8bd59a49bdba73880461f3cedccf6b8cdb6/dev-php/pecl-http/pecl-http-3.3.0.ebuild#L16C5-L16C5) has USE_PHP defined twice

Output message

VariableShadowed

Documentation

Variable is shadowed / repeatedly declared. This is a possible typo.

Result level

warning

thesamesam commented 9 months ago

There's a chance that we need to restrict this a bit, either to:

The latter two would be to cover cases where you set something like USE_RUBY globally but you need a hack to restrict the dependencies added by a function, like:

USE_RUBY="ruby31 ruby32"
inherit ruby-ng
# ...
USE_RUBY="ruby31" ruby_add_bdepend "dev-ruby/foo" # only needed for ruby31, as it's built in to ruby32 (think of this like a hack for Python's gen_cond_dep)