Minor cleanup ValidateOptions doc/code/test

Some nearby things in Main.py as well:
- docstrings polished a bit, minor linting
- move the list of predefined SConstruct file names into a constant
  defined at the top of the file, so it's a little less hidden,
  in the unlikely case of future changes.

Manpage text and example revised a bit.

Signed-off-by: Mats Wichmann <mats@linux.com>

5 files changed, 186 insertions, 113 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index c5949793d..6b3d8882b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -15,6 +15,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       embedded in a sequence, or by itself.  The conditional C scanner thus
       did not always properly apply the defines. The regular C scanner does
       not use these, so was not affected.  [fixes #4193]
+    - Minor cleanup for ValidateOptions - docs and docstring tweaked,
+      add missed versionadded indicator.
 
 
 RELEASE 4.5.2 -  Sun, 21 Mar 2023 14:08:29 -0700
diff --git a/SCons/Script/Main.py b/SCons/Script/Main.py
index d4228facb..e34301eb6 100644
--- a/SCons/Script/Main.py
+++ b/SCons/Script/Main.py
@@ -31,13 +31,8 @@ some other module.  If it's specific to the "scons" script invocation,
 it goes here.
 """
 
-# these define the range of versions SCons supports
-minimum_python_version = (3, 6, 0)
-deprecated_python_version = (3, 6, 0)
-
 import SCons.compat
 
-import atexit
 import importlib.util
 import os
 import re
@@ -46,6 +41,7 @@ import time
 import traceback
 import platform
 import threading
+from typing import Optional, List
 
 import SCons.CacheDir
 import SCons.Debug
@@ -66,6 +62,20 @@ import SCons.Script.Interactive
 
 from SCons import __version__ as SConsVersion
 
+# these define the range of versions SCons supports
+minimum_python_version = (3, 6, 0)
+deprecated_python_version = (3, 6, 0)
+
+# ordered list of SConsctruct names to look for if there is no -f flag
+KNOWN_SCONSTRUCT_NAMES = [
+    'SConstruct',
+    'Sconstruct',
+    'sconstruct',
+    'SConstruct.py',
+    'Sconstruct.py',
+    'sconstruct.py',
+]
+
 # Global variables
 first_command_start = None
 last_command_end = None
@@ -284,7 +294,7 @@ class BuildTask(SCons.Taskmaster.OutOfDateTask):
 
         node = buildError.node
         if not SCons.Util.is_List(node):
-                node = [ node ]
+            node = [node]
         nodename = ', '.join(map(str, node))
 
         errfmt = "scons: *** [%s] %s\n"
@@ -460,24 +470,29 @@ def python_version_deprecated(version=sys.version_info):
 
 
 class FakeOptionParser:
-    """
-    A do-nothing option parser, used for the initial OptionsParser variable.
+    """A do-nothing option parser, used for the initial OptionsParser value.
 
     During normal SCons operation, the OptionsParser is created right
-    away by the main() function.  Certain tests scripts however, can
+    away by the main() function.  Certain test scripts however, can
     introspect on different Tool modules, the initialization of which
     can try to add a new, local option to an otherwise uninitialized
     OptionsParser object.  This allows that introspection to happen
     without blowing up.
-
     """
+
     class FakeOptionValues:
         def __getattr__(self, attr):
             return None
+
     values = FakeOptionValues()
+
+    # TODO: to quiet checkers, FakeOptionParser should also define
+    #   raise_exception_on_error, preserve_unknown_options, largs and parse_args
+
     def add_local_option(self, *args, **kw):
         pass
 
+
 OptionsParser = FakeOptionParser()
 
 def AddOption(*args, **kw):
@@ -492,22 +507,26 @@ def GetOption(name):
 def SetOption(name, value):
     return OptionsParser.values.set_option(name, value)
 
-
-def ValidateOptions(throw_exception=False) -> None:
+def ValidateOptions(throw_exception: bool = False) -> None:
     """Validate options passed to SCons on the command line.
 
-    If you call this after you set all your command line options with AddOption(),
-    it will verify that all command line options are valid.
-    So if you added an option --xyz and you call SCons with --xyy you can cause
+    Checks that all options given on the command line are known to this
+    instance of SCons. Call after all of the cli options have been set
+    up through :func:`AddOption` calls.  For example, if you added an
+    option ``--xyz`` and you call SCons with ``--xyy`` you can cause
     SCons to issue an error message and exit by calling this function.
 
-    :param bool throw_exception: (Optional) Should this function raise an error if there's an invalid option on the command line, or issue a message and exit with error status.
+    Arguments:
+       throw_exception: if an invalid option is present on the command line,
+          raises an exception if this optional parameter evaluates true;
+          if false (the default), issue a message and exit with error status.
 
-    :raises SConsBadOptionError: If throw_exception is True and there are invalid options on command line.
+    Raises:
+       SConsBadOptionError: If *throw_exception* is true and there are invalid
+          options on the command line.
 
     .. versionadded:: 4.5.0
     """
-
     OptionsParser.raise_exception_on_error = throw_exception
     OptionsParser.preserve_unknown_options = False
     OptionsParser.parse_args(OptionsParser.largs, OptionsParser.values)
@@ -641,13 +660,23 @@ def _scons_internal_error():
     traceback.print_exc()
     sys.exit(2)
 
-def _SConstruct_exists(dirname='', repositories=[], filelist=None):
-    """This function checks that an SConstruct file exists in a directory.
-    If so, it returns the path of the file. By default, it checks the
-    current directory.
+def _SConstruct_exists(
+    dirname: str, repositories: List[str], filelist: List[str]
+) -> Optional[str]:
+    """Check that an SConstruct file exists in a directory.
+
+    Arguments:
+       dirname: the directory to search. If empty, look in cwd.
+       repositories: a list of repositories to search in addition to the
+          project directory tree.
+       filelist: names of SConstruct file(s) to search for.
+          If empty list, use the built-in list of names.
+
+    Returns:
+        The path to the located SConstruct file, or ``None``.
     """
     if not filelist:
-        filelist = ['SConstruct', 'Sconstruct', 'sconstruct', 'SConstruct.py', 'Sconstruct.py', 'sconstruct.py']
+        filelist = KNOWN_SCONSTRUCT_NAMES
     for file in filelist:
         sfile = os.path.join(dirname, file)
         if os.path.isfile(sfile):
@@ -679,14 +708,14 @@ def _set_debug_values(options):
             SCons.Warnings.warn(SCons.Warnings.NoObjectCountWarning, msg)
     if "dtree" in debug_values:
         options.tree_printers.append(TreePrinter(derived=True))
-    options.debug_explain = ("explain" in debug_values)
+    options.debug_explain = "explain" in debug_values
     if "findlibs" in debug_values:
         SCons.Scanner.Prog.print_find_libs = "findlibs"
-    options.debug_includes = ("includes" in debug_values)
-    print_memoizer = ("memoizer" in debug_values)
+    options.debug_includes = "includes" in debug_values
+    print_memoizer = "memoizer" in debug_values
     if "memory" in debug_values:
         memory_stats.enable(sys.stdout)
-    print_objects = ("objects" in debug_values)
+    print_objects = "objects" in debug_values
     if print_objects:
         SCons.Debug.track_instances = True
     if "presub" in debug_values:
@@ -919,9 +948,9 @@ def _main(parser):
     target_top = None
     if options.climb_up:
         target_top = '.'  # directory to prepend to targets
-        while script_dir and not _SConstruct_exists(script_dir,
-                                                    options.repository,
-                                                    options.file):
+        while script_dir and not _SConstruct_exists(
+            script_dir, options.repository, options.file
+        ):
             script_dir, last_part = os.path.split(script_dir)
             if last_part:
                 target_top = os.path.join(last_part, target_top)
@@ -951,8 +980,7 @@ def _main(parser):
     if options.file:
         scripts.extend(options.file)
     if not scripts:
-        sfile = _SConstruct_exists(repositories=options.repository,
-                                   filelist=options.file)
+        sfile = _SConstruct_exists("", options.repository, options.file)
         if sfile:
             scripts.append(sfile)
 
@@ -1011,8 +1039,8 @@ def _main(parser):
     # Next, set up the variables that hold command-line arguments,
     # so the SConscript files that we read and execute have access to them.
     # TODO: for options defined via AddOption which take space-separated
-    # option-args, the option-args will collect into targets here,
-    # because we don't yet know to do any different.
+    #   option-args, the option-args will collect into targets here,
+    #   because we don't yet know to do any different.
     targets = []
     xmit_args = []
     for a in parser.largs:
diff --git a/SCons/Script/Main.xml b/SCons/Script/Main.xml
index 2070c36b0..379d5347e 100644
--- a/SCons/Script/Main.xml
+++ b/SCons/Script/Main.xml
@@ -749,7 +749,7 @@ Sets &scons; option variable <parameter>name</parameter>
 to <parameter>value</parameter>.
 These options are all also settable via
 command-line options but the variable name
-may differ from the command-line option name - 
+may differ from the command-line option name -
 see the table for correspondences.
 A value set via command-line option will take
 precedence over one set with &f-SetOption;, which
@@ -946,64 +946,79 @@ SetOption('max_drift', 0)
 
 </scons_function>
 
-    <scons_function name="ValidateOptions">
-        <arguments signature="global">
-            ([throw_exception=False])
-        </arguments>
-
-        <summary>
-            <para>
-                Check that all the options specified on the command line are either defined by SCons itself
-                or defined by calls to &f-link-AddOption;.
-            </para>
-            <para>
-                This function should only be called after the last &f-link-AddOption; call in your &SConscript;
-                logic.
-            </para>
-            <para>
-                Be aware that some tools call &f-link-AddOption;, if you are getting error messages for arguments
-                that they add, you will need to ensure that you load those tools before you call &f-ValidateOptions;.
-            </para>
-            <para>
-                If there are any command line options not defined, calling this function will cause SCons to issue an
-                error message and then exit with an error exit
-                status.</para>
-            <para>If the optional <parameter>throw_exception</parameter> is <literal>True</literal>, &f-ValidateOptions; will raise a
-                <exceptionname>SConsBadOptionError</exceptionname>
-                exception. This would allow the calling
-                &SConscript; logic can catch that exception and handle invalid options itself.
-            </para>
-
-            <para>
-                Example:
-            </para>
-
-            <example_commands>
+  <scons_function name="ValidateOptions">
+    <arguments signature="global">([throw_exception=False])</arguments>
+
+    <summary>
+      <para>
+        Check that all the options specified on the command line are either
+        &SCons; built-in options or defined via calls to &f-link-AddOption;.
+        &SCons; will eventually fail on unknown options anyway, but calling
+        this function allows the build to "fail fast" before executing
+        expensive logic later in the build.
+      </para>
+
+      <para>
+        This function should only be called after the last &f-AddOption;
+        call in your &SConscript; logic.
+        Be aware that some tools call &f-AddOption;, if you are getting
+        error messages for arguments that they add, you will need to ensure
+        that those tools are loaded before calling &f-ValidateOptions;.
+      </para>
+
+      <para>
+        If there are any unknown command line options, &f-ValidateOptions;
+        prints an error message and exits with an error exit status.
+        If the optional <parameter>throw_exception</parameter> argument is
+        <literal>True</literal> (default is <literal>False</literal>),
+        a <exceptionname>SConsBadOptionError</exceptionname> is raised,
+        giving an opportunity for the &SConscript; logic to catch that
+        exception and handle invalid options appropriately. Note that
+        this exception name needs to be imported (see the example below).
+      </para>
+
+      <para>
+        A common build problem is typos (or thinkos) - a user enters an option
+        that is just a little off the expected value, or perhaps a different
+        word with a similar meaning.  It may be useful to abort the build
+        before going too far down the wrong path. For example:
+      </para>
+
+      <screen>
+$ <userinput>scons --compilers=mingw</userinput>  # the correct flag is --compiler
+      </screen>
+
+      <para>
+        Here &SCons; could go off and run a bunch of configure steps with
+        the default value of <literal>--compiler</literal>, since the
+        incorrect command line did not actually supply a value to it,
+        costing developer time to track down why the configure logic
+        made the "wrong" choices.  This example shows catching this:
+      </para>
+
+      <programlisting language="python">
+from SCons.Script.SConsOptions import SConsBadOptionError
+
+AddOption(
+    '--compiler',
+    dest='compiler',
+    action='store',
+    default='gcc',
+    type='string',
+)
+
+# ... other SConscript logic ...
+
 try:
     ValidateOptions(throw_exception=True)
 except SConsBadOptionError as e:
-    print("Parser is SConsOptionParser:%s" % (isinstance(e.parser, SConsOptionParser)))
-    print("Message is :%s" % e.opt_str)
+    print(f"ValidateOptions detects a fail: ", e.opt_str)
     Exit(3)
-            </example_commands>
-
-            <para>
-                This function is useful to force SCons to fail fast before you execute any expensive logic later in your
-                build logic.
-                For example if you specify build options via any flags, a simple typo could yield the incorrect build
-                option throughout your entire build.
-            </para>
-            <example_commands>
-scons --compilers=mingw (the correct flag is --compiler)
-            </example_commands>
-            <para>
-                Could cause SCons to run configure steps with the incorrect compiler. Costing developer time trying to
-                track down why the configure logic failed with a compiler which should work.
-            </para>
-            <para>
-                <emphasis>New in version 4.5.0</emphasis>
-            </para>
-        </summary>
-    </scons_function>
+      </programlisting>
+
+      <para><emphasis>New in version 4.5.0</emphasis></para>
+
+    </summary>
+  </scons_function>
 
 </sconsdoc>
diff --git a/test/ValidateOptions.py b/test/ValidateOptions.py
index 9b53c0949..5dff386e2 100644
--- a/test/ValidateOptions.py
+++ b/test/ValidateOptions.py
@@ -28,38 +28,60 @@ Test ValidateOptions().
 import TestSCons
 
 test = TestSCons.TestSCons()
+# ref: test/fixture/SConstruct-check-valid-options
 test.file_fixture('fixture/SConstruct-check-valid-options', 'SConstruct')
 
-# Should see "This is in SConstruct" because all options specified (none)  are valid and
-# so ValidatedOptions() won't exit before it's printed.
+# Should see "This is in SConstruct" because all options specified (none)
+# are valid and so ValidatedOptions() won't exit before it's printed.
 test.run()
 test.must_contain_single_instance_of(test.stdout(), ["This is in SConstruct"])
 
-# Should see "This is in SConstruct" because all options specified (--testing=abc)  are valid and
-# so ValidatedOptions() won't exit before it's printed.
+# Should see "This is in SConstruct" because all options specified
+# (--testing=abc)  are valid and so ValidatedOptions() won't exit before
+# it's printed.
 test.run(arguments="--testing=abc")
 test.must_contain_single_instance_of(test.stdout(), ["This is in SConstruct"])
 
-# Should not see "This is in SConstruct" because the option specified (--garbage=xyz)  is invalid and
-# so ValidatedOptions() will exit before it's printed.
-test.run(arguments="--garbage=xyz", status=2, stderr=".*SCons Error: no such option: --garbage.*",
-         match=TestSCons.match_re_dotall)
-test.fail_test(("This is in SConstruct" in test.stdout()),
-               message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed')
+# Should not see "This is in SConstruct" because the option specified
+# (--garbage=xyz)  is invalid and so ValidatedOptions() will exit
+# before it's printed.
+test.run(
+    arguments="--garbage=xyz",
+    status=2,
+    stderr=".*SCons Error: no such option: --garbage.*",
+    match=TestSCons.match_re_dotall,
+)
+test.fail_test(
+    ("This is in SConstruct" in test.stdout()),
+    message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed',
+)
 
 # Now we'll test having ValidateOptions raise a SConsBadOptionError exception
-test.run(arguments="--garbage=xyz raise=1", status=2,
-         stderr=".*SConsBadOptionError: no such option: no such option: --garbage.*",
-         match=TestSCons.match_re_dotall)
-test.fail_test(("This is in SConstruct" in test.stdout()),
-               message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed')
+test.run(
+    arguments="--garbage=xyz raise=1",
+    status=2,
+    stderr=".*SConsBadOptionError: no such option: no such option: --garbage.*",
+    match=TestSCons.match_re_dotall,
+)
+test.fail_test(
+    ("This is in SConstruct" in test.stdout()),
+    message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed',
+)
 
-# Now we'll test having ValidateOptions raise a SConsBadOptionError exception and catching that exception
-test.run(arguments="--garbage=xyz raise=2", status=3,
-         stdout=".*Parser is SConsOptionParser:True.*Message is .no such option. --garbage.*",
-         match=TestSCons.match_re_dotall)
-test.fail_test(("This is in SConstruct" in test.stdout()),
-               message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed')
+# Now we'll test having ValidateOptions raise a SConsBadOptionError
+# exception and catching that exception
+test.run(
+    arguments="--garbage=xyz raise=2",
+    status=3,
+    stdout=".*Parser is SConsOptionParser: True.*Message is. no such option. --garbage.*",
+    match=TestSCons.match_re_dotall,
+)
+test.fail_test(
+    ("This is in SConstruct" in test.stdout()),
+    message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed',
+)
+
+test.pass_test()
 
 # Local Variables:
 # tab-width:4
diff --git a/test/fixture/SConstruct-check-valid-options b/test/fixture/SConstruct-check-valid-options
index 2c935a23c..53bdf89d3 100644
--- a/test/fixture/SConstruct-check-valid-options
+++ b/test/fixture/SConstruct-check-valid-options
@@ -1,3 +1,7 @@
+# SPDX-License-Identifier: MIT
+#
+# Copyright The SCons Foundation
+
 import sys
 from SCons.Script.SConsOptions import SConsOptionParser, SConsBadOptionError
 
@@ -12,8 +16,10 @@ elif ARGUMENTS.get('raise', 0) == '2':
     try:
         ValidateOptions(throw_exception=True)
     except SConsBadOptionError as e:
-        print("Parser is SConsOptionParser:%s" % (isinstance(e.parser, SConsOptionParser)))
-        print("Message is :%s" % e.opt_str)
+        print(
+            f"Parser is SConsOptionParser: {isinstance(e.parser, SConsOptionParser)}"
+        )
+        print(f"Message is: {e.opt_str}")
         Exit(3)
 else:
     ValidateOptions()