This isn't really a call to action just sort of an observation wondering
what opinions folks have...
There are quite a few common-ish practices of SCons code that some (or
more) of the many Python checkers have heartburn over. I certainly
haven't tried every checker in the world, that would likely be
impossible (and definitely unproductive), and this isn't either a formal
report, just from memory...
These things are all legal Python - "it works, after all" - but the
authors of checkers get to express opinions about "good" Python.
* Methods (and data attributes) defined in derived classes which have no
mention in the parent class.
* Methods in derived classes which have different signatures than in the
parent class, or in other classes derived from the same parent.
* Attributes (data) which are assigned to in some method in the class,
but do not appear in the class initializer or as class attributes.
* "alternates" assigned to instance attributes which don't match the
mainline (i.e. "if foo: something; else: bar" where bar is the
(presumably rare) error case, and doesn't quite match types. Looks like
many of these are the workarounds for "we're not on Windows" situations,
* Cases where None is used as an error return where it doesn't need to
be because it confuses typing (i.e. you could return empty string ''
because that's false and not a valid success, so you don't need to use
None) (too nitpicky?)
* "possibly unbound"... I've looked at a few of these that come from one
particular checker who shall remain nameless... oh what the heck,
PyRight. Here's a sample:
csig = self.get_max_drift_csig()
if csig is None:
try:
size = self.get_size()
if size == -1:
contents = SCons.Util.NOFILE
elif size < File.hash_chunksize:
contents = self.get_contents()
else:
csig = self.get_content_hash()
except IOError:
# This can happen if there's actually a directory on-disk,
# which can be the case if they've disabled disk checks,
# or if an action with a File target actually happens to
# create a same-named directory by mistake.
csig = ''
else:
if not csig:
csig = SCons.Util.hash_signature(contents)
You can consider a path where contents is not defined when you get to
the last line of this chunk. If you squint sideways... I guess they're
saying "not defensive enough".
And then there are things that make you want to pull your hair out...
this line from sconsign (thus, not even mainline scons);
return imp.load_module(mname, fp, pathname, description)
Which a checker reported as:
/home/mats/github/scons/SCons/Utilities/sconsign.py
/home/mats/github/scons/SCons/Utilities/sconsign.py:63:35 - error:
Argument of type "IO[Any]" cannot be assigned to parameter "file" of
type "_FileLike | None" in function "load_module"
Type "IO[Any]" cannot be assigned to type "_FileLike | None"
Cannot assign to "None"
"closed" is an incompatible type
"property" is incompatible with "bool"
"mode" is an incompatible type
"property" is incompatible with "str"
"__exit__" is an incompatible type
Type "(t: Type[BaseException] | None, value: BaseException |
None, traceback: TracebackType | None) -> bool | None" cannot be
assigned to type "(*args: Any) -> Any" (reportGeneralTypeIssues)
There's more, of course... I'm trying to spend only drips of time trying
to understand objections and if they matter.
_______________________________________________
Scons-dev mailing list
[email protected]
https://pairlist2.pair.net/mailman/listinfo/scons-dev
