[Scons-dev] scons things checkers don't like

[Mats Wichmann]

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.