[scan-build-py] move argument parsing into separate module

[rizsotto.mailinglist]

Summary

It's a chunk from D26390

It gets rid of duplicate codes for command line parsing. Introduce validation for each command. And program entry points are no longer receives parameters from commands.

[dcoughlin]

Thanks for the upstreaming the patch! Comments inline.

Also: please make comments consistently into prose, including proper capitalization.

tools/scan-build-py/libscanbuild/analyze.py

67

This is pretty confusing because the 'analyze()' function doesn't actually analyze anything. (Same with 'scan') See my comments below for suggestions on how to make this more clear.

tools/scan-build-py/libscanbuild/arguments.py

7

I think "command line interface related duties" is pretty content free. Can you be more specific? How about: "This module parses and validates arguments for command-line interfaces."

45

As a general rule, if the name of a function doesn't have anything in common with the concepts mentioned in the description of the function then it should probably be renamed. How about 'parse_analyze_args'? (And a similar renaming for 'intercept' and 'scan'?)

60

As a general rule, if three functions do different things but have the same comment describing their functionality then the comment needs to be rewritten to be more precise. How about: "Parse and validate command-line arguments for scan-build." or something similar? (And the analogous rewriting for analyzer() and intercept()?)

79

I think you can make your intent more clear with something like:

if arg.plugins == None:
     arg.plugins = []

80

If you are really worried about future violations of this comment, it would be better to extract the checks in question into a well-named function so that any added checks are guaranteed to only be executed after the input is normalized.

When possible it is better to rely on well-structured code to avoid future pitfalls rather than leaving a land mine and hoping future contributors read and follow the comments.

131

The name of this function makes it sound like it is analyzing a parser, but if I understand correctly its purpose is to create a parser.

You could make this more clear by renaming the function to something like 'create_analyze_parser'.

With a name that mentions what the function does you could then avoid needing to use jargon like "factory method" in the description. How about "Creates a parser for command-line arguments to 'analyze'"? (And the analogous for 'intercept'?)

144

Is this correct? I think it is the opposite, since the intercept approach loses coverage for build systems that generate/modify source code in place.

What are the scenarios under which 'intercept' and then 'analyze' has better coverage than intertwined building and analysis?

[dcoughlin]

As Laszlo noted in in D30600, capitalizing while upstreaming is making diffing with his out-of-tree sources more noisy. We can hold off on consistent capitalization until later. But we do need comments in prose, otherwise we can't tell what the intent of the upstreamed patch is and so can't review it properly.

[rizsotto.mailinglist]

Thanks Devin for the comments. I'm happy to have your improvement suggestions crafted, scratched. It helps me to understand what part is debated... See my replies inlined.

tools/scan-build-py/libscanbuild/analyze.py

67

Good point, will fix that.

tools/scan-build-py/libscanbuild/arguments.py

7

Thanks for the suggestion, will use those words. I agree it's better to be more explicit.

45

Good point... Remember, I was hesitating between to parse_and_validate_analyze_build_args and something longer. Then I was choose the one word approach... How about analyze_build_arguments, scan_build_arguments and intercept_build_arguments? My point is, it not only parses, but parse-normalize-validate for command X. But at the end, it gives the parsed-normalized-validated arguments.

60

Sure, will fix it.

79

Sure.

80

To be honest, I would write test against this worry. But that is a lot of work. (Requires to write a dummy analyzer plugin.) And it would not only test this code, but clang too, so that would be a bit fragile test.

Might create a normalize_analyze_arguments function which is called before the validation. And this method would renamed to validate_analyze_arguments. How does that sound for you?

131

Good points. create_analyze_parser and create_intercept_parser sound good to me. Will do update the pydoc strings too.

144

If you check it, I was not change any argument help text. This is how it was before in the analyze module. If you want to fix the documentation shall we do it in a separate PR?

But as far as I can recall, I was trying to refer that intercept library covers more build system than the wrapper approach. Might be debatable that statement. Might be better write this information somewhere else... Is it a blocker for this PR or can we discuss in another thread?

[dcoughlin]

tools/scan-build-py/libscanbuild/arguments.py

45

The problem with analyze_build_arguments is that it starts with an action ("analyze") that is not what the function does. This means that at the call site someone reading that code may think that the function analyzes the build arguments -- but it doesn't.

In my opinion it is fine to use "parse" as the action and lump in validation and normalization into that action-- most parsers, including clang's, do some amount of validation and normalization as part of parsing so to me this would not be unexpected. But if you're worried about confusion then the longer, more descriptive parse_and_validate action seems fine to me as well. It is better to be clear at the call site than terse.

80

This sounds great.

144

It is not a blocker -- we can take to another thread.

[rizsotto.mailinglist]

fixed review comments.

[dcoughlin]

Looks great!

[rizsotto.mailinglist]

Thanks Devin. I forgot to add the arguments.py module at the commit, so this does not show it. But made another commit which has it. Sorry.

tools/scan-build-py/bin/analyze-build

 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
 import multiprocessing
 multiprocessing.freeze_support()
 
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
-from libscanbuild.analyze import analyze_build_main
-sys.exit(analyze_build_main(this_dir, False))
+from libscanbuild.analyze import analyze_build
+sys.exit(analyze_build())

tools/scan-build-py/bin/intercept-build

 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
 import multiprocessing
 multiprocessing.freeze_support()
 
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
-from libscanbuild.intercept import intercept_build_main
-sys.exit(intercept_build_main(this_dir))
+from libscanbuild.intercept import intercept_build
+sys.exit(intercept_build())

tools/scan-build-py/bin/scan-build

 #!/usr/bin/env python
 # -*- coding: utf-8 -*-
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
 import multiprocessing
 multiprocessing.freeze_support()
 
 import sys
 import os.path
 this_dir = os.path.dirname(os.path.realpath(__file__))
 sys.path.append(os.path.dirname(this_dir))
 
-from libscanbuild.analyze import analyze_build_main
-sys.exit(analyze_build_main(this_dir, True))
+from libscanbuild.analyze import scan_build
+sys.exit(scan_build())

tools/scan-build-py/libscanbuild/analyze.py

 # -*- coding: utf-8 -*-
 #                     The LLVM Compiler Infrastructure
 #
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 """ This module implements the 'scan-build' command API.
 
 To run the static analyzer against a build is done in multiple steps:
 
  -- Intercept: capture the compilation command during the build,
  -- Analyze:   run the analyzer against the captured commands,
  -- Report:    create a cover report from the analyzer outputs.  """
 
-import sys
 import re
 import os
 import os.path
 import json
-import argparse
 import logging
 import tempfile
 import multiprocessing
 import contextlib
 import datetime
 from libscanbuild import command_entry_point, compiler_wrapper, \
-    wrapper_environment, reconfigure_logging, run_build, tempdir
+    wrapper_environment, run_build
+from libscanbuild.arguments import parse_args_for_scan_build, \
+    parse_args_for_analyze_build
 from libscanbuild.runner import run
 from libscanbuild.intercept import capture
 from libscanbuild.report import document
-from libscanbuild.clang import get_checkers
 from libscanbuild.compilation import split_command
 
-__all__ = ['analyze_build_main', 'analyze_compiler_wrapper']
+__all__ = ['scan_build', 'analyze_build', 'analyze_compiler_wrapper']
 
 COMPILER_WRAPPER_CC = 'analyze-cc'
 COMPILER_WRAPPER_CXX = 'analyze-c++'
 
 
 @command_entry_point
-def analyze_build_main(bin_dir, from_build_command):
-    """ Entry point for 'analyze-build' and 'scan-build'. """
-
-    parser = create_parser(from_build_command)
-    args = parser.parse_args()
-    validate(parser, args, from_build_command)
-
-    # setup logging
-    reconfigure_logging(args.verbose)
-    logging.debug('Raw arguments %s', sys.argv)
+def scan_build():
+    """ Entry point for scan-build command. """
 
+    args = parse_args_for_scan_build()
     with report_directory(args.output, args.keep_empty) as target_dir:
-        if not from_build_command:
-            # run analyzer only and generate cover report
-            run_analyzer(args, target_dir)
-            number_of_bugs = document(args, target_dir, True)
-            return number_of_bugs if args.status_bugs else 0
-        elif args.intercept_first:
-            # run build command and capture compiler executions
-            exit_code = capture(args, bin_dir)
-            # next step to run the analyzer against the captured commands
+        # Run against a build command. there are cases, when analyzer run
+        # is not required. But we need to set up everything for the
+        # wrappers, because 'configure' needs to capture the CC/CXX values
+        # for the Makefile.
+        if args.intercept_first:
+            # Run build command with intercept module.
+            exit_code = capture(args)
+            # Run the analyzer against the captured commands.
             if need_analyzer(args.build):
                 run_analyzer(args, target_dir)
-                # cover report generation and bug counting
-                number_of_bugs = document(args, target_dir, True)
-                # remove the compilation database when it was not requested
-                if os.path.exists(args.cdb):
-                    os.unlink(args.cdb)
-                # set exit status as it was requested
-                return number_of_bugs if args.status_bugs else exit_code
-            else:
-                return exit_code
         else:
-            # run the build command with compiler wrappers which
-            # execute the analyzer too. (interposition)
-            environment = setup_environment(args, target_dir, bin_dir)
+            # Run build command and analyzer with compiler wrappers.
+            environment = setup_environment(args, target_dir)
             exit_code = run_build(args.build, env=environment)
-            # cover report generation and bug counting
-            number_of_bugs = document(args, target_dir, False)
-            # set exit status as it was requested
-            return number_of_bugs if args.status_bugs else exit_code
+        # Cover report generation and bug counting.
+        # Set exit status as it was requested.
 
 
+@command_entry_point
+def analyze_build():
+    """ Entry point for analyze-build command. """
+
+    args = parse_args_for_analyze_build()
+    with report_directory(args.output, args.keep_empty) as target_dir:
+        # Run the analyzer against a compilation db.
+        run_analyzer(args, target_dir)
+        # Cover report generation and bug counting.
+        number_of_bugs = document(args, target_dir, True)
+        # Set exit status as it was requested.
+        return number_of_bugs if args.status_bugs else 0
+
+
 def need_analyzer(args):
     """ Check the intent of the build command.
 
     When static analyzer run against project configure step, it should be
     silent and no need to run the analyzer or generate report.
 
     To run `scan-build` against the configure step might be neccessary,
     when compiler wrappers are used. That's the moment when build setup
with open(args.cdb, 'r') as handle:
             if current is not None:
                 # display error message from the static analyzer
                 for line in current['error_output']:
                     logging.info(line.rstrip())
         pool.close()
         pool.join()
 
 
-def setup_environment(args, destination, bin_dir):
+def setup_environment(args, destination):
     """ Set up environment for build command to interpose compiler wrapper. """
 
     environment = dict(os.environ)
     environment.update(wrapper_environment(args))
     environment.update({
-        'CC': os.path.join(bin_dir, COMPILER_WRAPPER_CC),
-        'CXX': os.path.join(bin_dir, COMPILER_WRAPPER_CXX),
+        'CC': COMPILER_WRAPPER_CC,
+        'CXX': COMPILER_WRAPPER_CXX,
         'ANALYZE_BUILD_CLANG': args.clang if need_analyzer(args.build) else '',
         'ANALYZE_BUILD_REPORT_DIR': destination,
         'ANALYZE_BUILD_REPORT_FORMAT': args.output_format,
         'ANALYZE_BUILD_REPORT_FAILURES': 'yes' if args.output_failures else '',
         'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args)),
         'ANALYZE_BUILD_FORCE_DEBUG': 'yes' if args.force_debug else ''
     })
     return environment
if args.enable_checker:
         result.extend(['-analyzer-checker', checkers])
     if args.disable_checker:
         checkers = ','.join(args.disable_checker)
         result.extend(['-analyzer-disable-checker', checkers])
     if os.getenv('UBIVIZ'):
         result.append('-analyzer-viz-egraph-ubigraph')
 
     return prefix_with('-Xclang', result)
-
-
-def print_active_checkers(checkers):
-    """ Print active checkers to stdout. """
-
-    for name in sorted(name for name, (_, active) in checkers.items()
-                       if active):
-        print(name)
-
-
-def print_checkers(checkers):
-    """ Print verbose checker help to stdout. """
-
-    print('')
-    print('available checkers:')
-    print('')
-    for name in sorted(checkers.keys()):
-        description, active = checkers[name]
-        prefix = '+' if active else ' '
-        if len(name) > 30:
-            print(' {0} {1}'.format(prefix, name))
-            print(' ' * 35 + description)
-        else:
-            print(' {0} {1: <30}  {2}'.format(prefix, name, description))
-    print('')
-    print('NOTE: "+" indicates that an analysis is enabled by default.')
-    print('')
-
-
-def validate(parser, args, from_build_command):
-    """ Validation done by the parser itself, but semantic check still
-    needs to be done. This method is doing that. """
-
-    # Make plugins always a list. (It might be None when not specified.)
-    args.plugins = args.plugins if args.plugins else []
-
-    if args.help_checkers_verbose:
-        print_checkers(get_checkers(args.clang, args.plugins))
-        parser.exit()
-    elif args.help_checkers:
-        print_active_checkers(get_checkers(args.clang, args.plugins))
-        parser.exit()
-
-    if from_build_command and not args.build:
-        parser.error('missing build command')
-
-
-def create_parser(from_build_command):
-    """ Command line argument parser factory method. """
-
-    parser = argparse.ArgumentParser(
-        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
-
-    parser.add_argument(
-        '--verbose', '-v',
-        action='count',
-        default=0,
-        help="""Enable verbose output from '%(prog)s'. A second and third
-                flag increases verbosity.""")
-    parser.add_argument(
-        '--override-compiler',
-        action='store_true',
-        help="""Always resort to the compiler wrapper even when better
-                interposition methods are available.""")
-    parser.add_argument(
-        '--intercept-first',
-        action='store_true',
-        help="""Run the build commands only, build a compilation database,
-                then run the static analyzer afterwards.
-                Generally speaking it has better coverage on build commands.
-                With '--override-compiler' it use compiler wrapper, but does
-                not run the analyzer till the build is finished. """)
-    parser.add_argument(
-        '--cdb',
-        metavar='<file>',
-        default="compile_commands.json",
-        help="""The JSON compilation database.""")
-
-    parser.add_argument(
-        '--output', '-o',
-        metavar='<path>',
-        default=tempdir(),
-        help="""Specifies the output directory for analyzer reports.
-                Subdirectory will be created if default directory is targeted.
-                """)
-    parser.add_argument(
-        '--status-bugs',
-        action='store_true',
-        help="""By default, the exit status of '%(prog)s' is the same as the
-                executed build command. Specifying this option causes the exit
-                status of '%(prog)s' to be non zero if it found potential bugs
-                and zero otherwise.""")
-    parser.add_argument(
-        '--html-title',
-        metavar='<title>',
-        help="""Specify the title used on generated HTML pages.
-                If not specified, a default title will be used.""")
-    parser.add_argument(
-        '--analyze-headers',
-        action='store_true',
-        help="""Also analyze functions in #included files. By default, such
-                functions are skipped unless they are called by functions
-                within the main source file.""")
-    format_group = parser.add_mutually_exclusive_group()
-    format_group.add_argument(
-        '--plist', '-plist',
-        dest='output_format',
-        const='plist',
-        default='html',
-        action='store_const',
-        help="""This option outputs the results as a set of .plist files.""")
-    format_group.add_argument(
-        '--plist-html', '-plist-html',
-        dest='output_format',
-        const='plist-html',
-        default='html',
-        action='store_const',
-        help="""This option outputs the results as a set of .html and .plist
-                files.""")
-    # TODO: implement '-view '
-
-    advanced = parser.add_argument_group('advanced options')
-    advanced.add_argument(
-        '--keep-empty',
-        action='store_true',
-        help="""Don't remove the build results directory even if no issues
-                were reported.""")
-    advanced.add_argument(
-        '--no-failure-reports', '-no-failure-reports',
-        dest='output_failures',
-        action='store_false',
-        help="""Do not create a 'failures' subdirectory that includes analyzer
-                crash reports and preprocessed source files.""")
-    advanced.add_argument(
-        '--stats', '-stats',
-        action='store_true',
-        help="""Generates visitation statistics for the project being analyzed.
-                """)
-    advanced.add_argument(
-        '--internal-stats',
-        action='store_true',
-        help="""Generate internal analyzer statistics.""")
-    advanced.add_argument(
-        '--maxloop', '-maxloop',
-        metavar='<loop count>',
-        type=int,
-        help="""Specifiy the number of times a block can be visited before
-                giving up. Increase for more comprehensive coverage at a cost
-                of speed.""")
-    advanced.add_argument(
-        '--store', '-store',
-        metavar='<model>',
-        dest='store_model',
-        choices=['region', 'basic'],
-        help="""Specify the store model used by the analyzer.
-                'region' specifies a field- sensitive store model.
-                'basic' which is far less precise but can more quickly
-                analyze code. 'basic' was the default store model for
-                checker-0.221 and earlier.""")
-    advanced.add_argument(
-        '--constraints', '-constraints',
-        metavar='<model>',
-        dest='constraints_model',
-        choices=['range', 'basic'],
-        help="""Specify the contraint engine used by the analyzer. Specifying
-                'basic' uses a simpler, less powerful constraint model used by
-                checker-0.160 and earlier.""")
-    advanced.add_argument(
-        '--use-analyzer',
-        metavar='<path>',
-        dest='clang',
-        default='clang',
-        help="""'%(prog)s' uses the 'clang' executable relative to itself for
-                static analysis. One can override this behavior with this
-                option by using the 'clang' packaged with Xcode (on OS X) or
-                from the PATH.""")
-    advanced.add_argument(
-        '--use-cc',
-        metavar='<path>',
-        dest='cc',
-        default='cc',
-        help="""When '%(prog)s' analyzes a project by interposing a "fake
-                compiler", which executes a real compiler for compilation and
-                do other tasks (to run the static analyzer or just record the
-                compiler invocation). Because of this interposing, '%(prog)s'
-                does not know what compiler your project normally uses.
-                Instead, it simply overrides the CC environment variable, and
-                guesses your default compiler.
-
-                If you need '%(prog)s' to use a specific compiler for
-                *compilation* then you can use this option to specify a path
-                to that compiler.""")
-    advanced.add_argument(
-        '--use-c++',
-        metavar='<path>',
-        dest='cxx',
-        default='c++',
-        help="""This is the same as "--use-cc" but for C++ code.""")
-    advanced.add_argument(
-        '--analyzer-config', '-analyzer-config',
-        metavar='<options>',
-        help="""Provide options to pass through to the analyzer's
-                -analyzer-config flag. Several options are separated with
-                comma: 'key1=val1,key2=val2'
-
-                Available options:
-                    stable-report-filename=true or false (default)
-
-                Switch the page naming to:
-                report-<filename>-<function/method name>-<id>.html
-                instead of report-XXXXXX.html""")
-    advanced.add_argument(
-        '--exclude',
-        metavar='<directory>',
-        dest='excludes',
-        action='append',
-        default=[],
-        help="""Do not run static analyzer against files found in this
-                directory. (You can specify this option multiple times.)
-                Could be usefull when project contains 3rd party libraries.
-                The directory path shall be absolute path as file names in
-                the compilation database.""")
-    advanced.add_argument(
-        '--force-analyze-debug-code',
-        dest='force_debug',
-        action='store_true',
-        help="""Tells analyzer to enable assertions in code even if they were
-                disabled during compilation, enabling more precise results.""")
-
-    plugins = parser.add_argument_group('checker options')
-    plugins.add_argument(
-        '--load-plugin', '-load-plugin',
-        metavar='<plugin library>',
-        dest='plugins',
-        action='append',
-        help="""Loading external checkers using the clang plugin interface.""")
-    plugins.add_argument(
-        '--enable-checker', '-enable-checker',
-        metavar='<checker name>',
-        action=AppendCommaSeparated,
-        help="""Enable specific checker.""")
-    plugins.add_argument(
-        '--disable-checker', '-disable-checker',
-        metavar='<checker name>',
-        action=AppendCommaSeparated,
-        help="""Disable specific checker.""")
-    plugins.add_argument(
-        '--help-checkers',
-        action='store_true',
-        help="""A default group of checkers is run unless explicitly disabled.
-                Exactly which checkers constitute the default group is a
-                function of the operating system in use. These can be printed
-                with this flag.""")
-    plugins.add_argument(
-        '--help-checkers-verbose',
-        action='store_true',
-        help="""Print all available checkers and mark the enabled ones.""")
-
-    if from_build_command:
-        parser.add_argument(
-            dest='build',
-            nargs=argparse.REMAINDER,
-            help="""Command to run.""")
-
-    return parser
-
-
-class AppendCommaSeparated(argparse.Action):
-    """ argparse Action class to support multiple comma separated lists. """
-
-    def __call__(self, __parser, namespace, values, __option_string):
-        # getattr(obj, attr, default) does not really returns default but none
-        if getattr(namespace, self.dest, None) is None:
-            setattr(namespace, self.dest, [])
-        # once it's fixed we can use as expected
-        actual = getattr(namespace, self.dest)
-        actual.extend(values.split(','))
-        setattr(namespace, self.dest, actual)

tools/scan-build-py/libscanbuild/arguments.py

+# -*- coding: utf-8 -*-
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+""" This module parses and validates arguments for command-line interfaces.
+
+It uses argparse module to create the command line parser. (This library is
+in the standard python library since 3.2 and backported to 2.7, but not
+earlier.)
+
+It also implements basic validation methods, related to the command.
+Validations are mostly calling specific help methods, or mangling values.
+"""
+
+import os
+import sys
+import argparse
+import logging
+from libscanbuild import reconfigure_logging, tempdir
+from libscanbuild.clang import get_checkers
+
+__all__ = ['parse_args_for_intercept_build', 'parse_args_for_analyze_build',
+           'parse_args_for_scan_build']
+
+
+def parse_args_for_intercept_build():
+    """ Parse and validate command-line arguments for intercept-build. """
+
+    parser = create_intercept_parser()
+    args = parser.parse_args()
+
+    reconfigure_logging(args.verbose)
+    logging.debug('Raw arguments %s', sys.argv)
+
+    # short validation logic
+    if not args.build:
+        parser.error(message='missing build command')
+
+    logging.debug('Parsed arguments: %s', args)
+    return args
+
+
+def parse_args_for_analyze_build():
+    """ Parse and validate command-line arguments for analyze-build. """
+
+    from_build_command = False
+    parser = create_analyze_parser(from_build_command)
+    args = parser.parse_args()
+
+    reconfigure_logging(args.verbose)
+    logging.debug('Raw arguments %s', sys.argv)
+
+    normalize_args_for_analyze(args, from_build_command)
+    validate_args_for_analyze(parser, args, from_build_command)
+    logging.debug('Parsed arguments: %s', args)
+    return args
+
+
+def parse_args_for_scan_build():
+    """ Parse and validate command-line arguments for scan-build. """
+
+    from_build_command = True
+    parser = create_analyze_parser(from_build_command)
+    args = parser.parse_args()
+
+    reconfigure_logging(args.verbose)
+    logging.debug('Raw arguments %s', sys.argv)
+
+    normalize_args_for_analyze(args, from_build_command)
+    validate_args_for_analyze(parser, args, from_build_command)
+    logging.debug('Parsed arguments: %s', args)
+    return args
+
+
+def normalize_args_for_analyze(args, from_build_command):
+    """ Normalize parsed arguments for analyze-build and scan-build.
+
+    :param args: Parsed argument object. (Will be mutated.)
+    :param from_build_command: Boolean value tells is the command suppose
+    to run the analyzer against a build command or a compilation db. """
+
+    # make plugins always a list. (it might be None when not specified.)
+    if args.plugins is None:
+        args.plugins = []
+
+    # make exclude directory list unique and absolute.
+    uniq_excludes = set(os.path.abspath(entry) for entry in args.excludes)
+    args.excludes = list(uniq_excludes)
+
+    # because shared codes for all tools, some common used methods are
+    # expecting some argument to be present. so, instead of query the args
+    # object about the presence of the flag, we fake it here. to make those
+    # methods more readable. (it's an arguable choice, took it only for those
+    # which have good default value.)
+    if from_build_command:
+        # add cdb parameter invisibly to make report module working.
+        args.cdb = 'compile_commands.json'
+
+
+def validate_args_for_analyze(parser, args, from_build_command):
+    """ Command line parsing is done by the argparse module, but semantic
+    validation still needs to be done. This method is doing it for
+    analyze-build and scan-build commands.
+
+    :param parser: The command line parser object.
+    :param args: Parsed argument object.
+    :param from_build_command: Boolean value tells is the command suppose
+    to run the analyzer against a build command or a compilation db.
+    :return: No return value, but this call might throw when validation
+    fails. """
+
+    if args.help_checkers_verbose:
+        print_checkers(get_checkers(args.clang, args.plugins))
+        parser.exit(status=0)
+    elif args.help_checkers:
+        print_active_checkers(get_checkers(args.clang, args.plugins))
+        parser.exit(status=0)
+    elif from_build_command and not args.build:
+        parser.error(message='missing build command')
+    elif not from_build_command and not os.path.exists(args.cdb):
+        parser.error(message='compilation database is missing')
+
+
+def create_intercept_parser():
+    """ Creates a parser for command-line arguments to 'intercept'. """
+
+    parser = create_default_parser()
+    parser_add_cdb(parser)
+
+    parser_add_prefer_wrapper(parser)
+    parser_add_compilers(parser)
+
+    advanced = parser.add_argument_group('advanced options')
+    group = advanced.add_mutually_exclusive_group()
+    group.add_argument(
+        '--append',
+        action='store_true',
+        help="""Extend existing compilation database with new entries.
+        Duplicate entries are detected and not present in the final output.
+        The output is not continuously updated, it's done when the build
+        command finished. """)
+
+    parser.add_argument(
+        dest='build', nargs=argparse.REMAINDER, help="""Command to run.""")
+    return parser
+
+
+def create_analyze_parser(from_build_command):
+    """ Creates a parser for command-line arguments to 'analyze'. """
+
+    parser = create_default_parser()
+
+    if from_build_command:
+        parser_add_prefer_wrapper(parser)
+        parser_add_compilers(parser)
+
+        parser.add_argument(
+            '--intercept-first',
+            action='store_true',
+            help="""Run the build commands first, intercept compiler
+            calls and then run the static analyzer afterwards.
+            Generally speaking it has better coverage on build commands.
+            With '--override-compiler' it use compiler wrapper, but does
+            not run the analyzer till the build is finished.""")
+    else:
+        parser_add_cdb(parser)
+
+    parser.add_argument(
+        '--status-bugs',
+        action='store_true',
+        help="""The exit status of '%(prog)s' is the same as the executed
+        build command. This option ignores the build exit status and sets to
+        be non zero if it found potential bugs or zero otherwise.""")
+    parser.add_argument(
+        '--exclude',
+        metavar='<directory>',
+        dest='excludes',
+        action='append',
+        default=[],
+        help="""Do not run static analyzer against files found in this
+        directory. (You can specify this option multiple times.)
+        Could be useful when project contains 3rd party libraries.""")
+
+    output = parser.add_argument_group('output control options')
+    output.add_argument(
+        '--output',
+        '-o',
+        metavar='<path>',
+        default=tempdir(),
+        help="""Specifies the output directory for analyzer reports.
+        Subdirectory will be created if default directory is targeted.""")
+    output.add_argument(
+        '--keep-empty',
+        action='store_true',
+        help="""Don't remove the build results directory even if no issues
+        were reported.""")
+    output.add_argument(
+        '--html-title',
+        metavar='<title>',
+        help="""Specify the title used on generated HTML pages.
+        If not specified, a default title will be used.""")
+    format_group = output.add_mutually_exclusive_group()
+    format_group.add_argument(
+        '--plist',
+        '-plist',
+        dest='output_format',
+        const='plist',
+        default='html',
+        action='store_const',
+        help="""Cause the results as a set of .plist files.""")
+    format_group.add_argument(
+        '--plist-html',
+        '-plist-html',
+        dest='output_format',
+        const='plist-html',
+        default='html',
+        action='store_const',
+        help="""Cause the results as a set of .html and .plist files.""")
+    # TODO: implement '-view '
+
+    advanced = parser.add_argument_group('advanced options')
+    advanced.add_argument(
+        '--use-analyzer',
+        metavar='<path>',
+        dest='clang',
+        default='clang',
+        help="""'%(prog)s' uses the 'clang' executable relative to itself for
+        static analysis. One can override this behavior with this option by
+        using the 'clang' packaged with Xcode (on OS X) or from the PATH.""")
+    advanced.add_argument(
+        '--no-failure-reports',
+        '-no-failure-reports',
+        dest='output_failures',
+        action='store_false',
+        help="""Do not create a 'failures' subdirectory that includes analyzer
+        crash reports and preprocessed source files.""")
+    parser.add_argument(
+        '--analyze-headers',
+        action='store_true',
+        help="""Also analyze functions in #included files. By default, such
+        functions are skipped unless they are called by functions within the
+        main source file.""")
+    advanced.add_argument(
+        '--stats',
+        '-stats',
+        action='store_true',
+        help="""Generates visitation statistics for the project.""")
+    advanced.add_argument(
+        '--internal-stats',
+        action='store_true',
+        help="""Generate internal analyzer statistics.""")
+    advanced.add_argument(
+        '--maxloop',
+        '-maxloop',
+        metavar='<loop count>',
+        type=int,
+        help="""Specifiy the number of times a block can be visited before
+        giving up. Increase for more comprehensive coverage at a cost of
+        speed.""")
+    advanced.add_argument(
+        '--store',
+        '-store',
+        metavar='<model>',
+        dest='store_model',
+        choices=['region', 'basic'],
+        help="""Specify the store model used by the analyzer. 'region'
+        specifies a field- sensitive store model. 'basic' which is far less
+        precise but can more quickly analyze code. 'basic' was the default
+        store model for checker-0.221 and earlier.""")
+    advanced.add_argument(
+        '--constraints',
+        '-constraints',
+        metavar='<model>',
+        dest='constraints_model',
+        choices=['range', 'basic'],
+        help="""Specify the constraint engine used by the analyzer. Specifying
+        'basic' uses a simpler, less powerful constraint model used by
+        checker-0.160 and earlier.""")
+    advanced.add_argument(
+        '--analyzer-config',
+        '-analyzer-config',
+        metavar='<options>',
+        help="""Provide options to pass through to the analyzer's
+        -analyzer-config flag. Several options are separated with comma:
+        'key1=val1,key2=val2'
+
+        Available options:
+            stable-report-filename=true or false (default)
+
+        Switch the page naming to:
+        report-<filename>-<function/method name>-<id>.html
+        instead of report-XXXXXX.html""")
+    advanced.add_argument(
+        '--force-analyze-debug-code',
+        dest='force_debug',
+        action='store_true',
+        help="""Tells analyzer to enable assertions in code even if they were
+        disabled during compilation, enabling more precise results.""")
+
+    plugins = parser.add_argument_group('checker options')
+    plugins.add_argument(
+        '--load-plugin',
+        '-load-plugin',
+        metavar='<plugin library>',
+        dest='plugins',
+        action='append',
+        help="""Loading external checkers using the clang plugin interface.""")
+    plugins.add_argument(
+        '--enable-checker',
+        '-enable-checker',
+        metavar='<checker name>',
+        action=AppendCommaSeparated,
+        help="""Enable specific checker.""")
+    plugins.add_argument(
+        '--disable-checker',
+        '-disable-checker',
+        metavar='<checker name>',
+        action=AppendCommaSeparated,
+        help="""Disable specific checker.""")
+    plugins.add_argument(
+        '--help-checkers',
+        action='store_true',
+        help="""A default group of checkers is run unless explicitly disabled.
+        Exactly which checkers constitute the default group is a function of
+        the operating system in use. These can be printed with this flag.""")
+    plugins.add_argument(
+        '--help-checkers-verbose',
+        action='store_true',
+        help="""Print all available checkers and mark the enabled ones.""")
+
+    if from_build_command:
+        parser.add_argument(
+            dest='build', nargs=argparse.REMAINDER, help="""Command to run.""")
+    return parser
+
+
+def create_default_parser():
+    """ Creates command line parser for all build wrapper commands. """
+
+    parser = argparse.ArgumentParser(
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
+
+    parser.add_argument(
+        '--verbose',
+        '-v',
+        action='count',
+        default=0,
+        help="""Enable verbose output from '%(prog)s'. A second, third and
+        fourth flags increases verbosity.""")
+    return parser
+
+
+def parser_add_cdb(parser):
+    parser.add_argument(
+        '--cdb',
+        metavar='<file>',
+        default="compile_commands.json",
+        help="""The JSON compilation database.""")
+
+
+def parser_add_prefer_wrapper(parser):
+    parser.add_argument(
+        '--override-compiler',
+        action='store_true',
+        help="""Always resort to the compiler wrapper even when better
+        intercept methods are available.""")
+
+
+def parser_add_compilers(parser):
+    parser.add_argument(
+        '--use-cc',
+        metavar='<path>',
+        dest='cc',
+        default=os.getenv('CC', 'cc'),
+        help="""When '%(prog)s' analyzes a project by interposing a compiler
+        wrapper, which executes a real compiler for compilation and do other
+        tasks (record the compiler invocation). Because of this interposing,
+        '%(prog)s' does not know what compiler your project normally uses.
+        Instead, it simply overrides the CC environment variable, and guesses
+        your default compiler.
+
+        If you need '%(prog)s' to use a specific compiler for *compilation*
+        then you can use this option to specify a path to that compiler.""")
+    parser.add_argument(
+        '--use-c++',
+        metavar='<path>',
+        dest='cxx',
+        default=os.getenv('CXX', 'c++'),
+        help="""This is the same as "--use-cc" but for C++ code.""")
+
+
+class AppendCommaSeparated(argparse.Action):
+    """ argparse Action class to support multiple comma separated lists. """
+
+    def __call__(self, __parser, namespace, values, __option_string):
+        # getattr(obj, attr, default) does not really returns default but none
+        if getattr(namespace, self.dest, None) is None:
+            setattr(namespace, self.dest, [])
+        # once it's fixed we can use as expected
+        actual = getattr(namespace, self.dest)
+        actual.extend(values.split(','))
+        setattr(namespace, self.dest, actual)
+
+
+def print_active_checkers(checkers):
+    """ Print active checkers to stdout. """
+
+    for name in sorted(name for name, (_, active) in checkers.items()
+                       if active):
+        print(name)
+
+
+def print_checkers(checkers):
+    """ Print verbose checker help to stdout. """
+
+    print('')
+    print('available checkers:')
+    print('')
+    for name in sorted(checkers.keys()):
+        description, active = checkers[name]
+        prefix = '+' if active else ' '
+        if len(name) > 30:
+            print(' {0} {1}'.format(prefix, name))
+            print(' ' * 35 + description)
+        else:
+            print(' {0} {1: <30}  {2}'.format(prefix, name, description))
+    print('')
+    print('NOTE: "+" indicates that an analysis is enabled by default.')
+    print('')

tools/scan-build-py/libscanbuild/intercept.py

 
 import sys
 import os
 import os.path
 import re
 import itertools
 import json
 import glob
-import argparse
 import logging
 from libear import build_libear, TemporaryDirectory
 from libscanbuild import command_entry_point, compiler_wrapper, \
-    wrapper_environment, run_command, run_build, reconfigure_logging
+    wrapper_environment, run_command, run_build
 from libscanbuild import duplicate_check, tempdir
 from libscanbuild.compilation import split_command
+from libscanbuild.arguments import parse_args_for_intercept_build
 from libscanbuild.shell import encode, decode
 
-__all__ = ['capture', 'intercept_build_main', 'intercept_compiler_wrapper']
+__all__ = ['capture', 'intercept_build', 'intercept_compiler_wrapper']
 
 GS = chr(0x1d)
 RS = chr(0x1e)
 US = chr(0x1f)
 
 COMPILER_WRAPPER_CC = 'intercept-cc'
 COMPILER_WRAPPER_CXX = 'intercept-c++'
 TRACE_FILE_EXTENSION = '.cmd'  # same as in ear.c
 WRAPPER_ONLY_PLATFORMS = frozenset({'win32', 'cygwin'})
 
 
 @command_entry_point
-def intercept_build_main(bin_dir):
+def intercept_build():
     """ Entry point for 'intercept-build' command. """
 
-    parser = create_parser()
-    args = parser.parse_args()
+    args = parse_args_for_intercept_build()
+    return capture(args)
 
-    reconfigure_logging(args.verbose)
-    logging.debug('Raw arguments %s', sys.argv)
 
-    if not args.build:
-        parser.print_help()
-        return 0
-
-    return capture(args, bin_dir)
-
-
-def capture(args, bin_dir):
+def capture(args):
     """ The entry point of build command interception. """
 
     def post_processing(commands):
         """ To make a compilation database, it needs to filter out commands
         which are not compiler calls. Needs to find the source file name
         from the arguments. And do shell escaping on the command.
 
         To support incremental builds, it is desired to read elements from
def post_processing(commands):
         # filter out duplicate entries from both
         duplicate = duplicate_check(entry_hash)
         return (entry
                 for entry in itertools.chain(previous, current)
                 if os.path.exists(entry['file']) and not duplicate(entry))
 
     with TemporaryDirectory(prefix='intercept-', dir=tempdir()) as tmp_dir:
         # run the build command
-        environment = setup_environment(args, tmp_dir, bin_dir)
+        environment = setup_environment(args, tmp_dir)
         exit_code = run_build(args.build, env=environment)
         # read the intercepted exec calls
         exec_traces = itertools.chain.from_iterable(
             parse_exec_trace(os.path.join(tmp_dir, filename))
             for filename in sorted(glob.iglob(os.path.join(tmp_dir, '*.cmd'))))
         # do post processing
         entries = post_processing(exec_traces)
         # dump the compilation database
         with open(args.cdb, 'w+') as handle:
             json.dump(list(entries), handle, sort_keys=True, indent=4)
         return exit_code
 
 
-def setup_environment(args, destination, bin_dir):
+def setup_environment(args, destination):
     """ Sets up the environment for the build command.
 
     It sets the required environment variables and execute the given command.
     The exec calls will be logged by the 'libear' preloaded library or by the
     'wrapper' programs. """
 
     c_compiler = args.cc if 'cc' in args else 'cc'
     cxx_compiler = args.cxx if 'cxx' in args else 'c++'
 
     libear_path = None if args.override_compiler or is_preload_disabled(
         sys.platform) else build_libear(c_compiler, destination)
 
     environment = dict(os.environ)
     environment.update({'INTERCEPT_BUILD_TARGET_DIR': destination})
 
     if not libear_path:
         logging.debug('intercept gonna use compiler wrappers')
         environment.update(wrapper_environment(args))
         environment.update({
-            'CC': os.path.join(bin_dir, COMPILER_WRAPPER_CC),
-            'CXX': os.path.join(bin_dir, COMPILER_WRAPPER_CXX)
+            'CC': COMPILER_WRAPPER_CC,
+            'CXX': COMPILER_WRAPPER_CXX
         })
     elif sys.platform == 'darwin':
         logging.debug('intercept gonna preload libear on OSX')
         environment.update({
             'DYLD_INSERT_LIBRARIES': libear_path,
             'DYLD_FORCE_FLAT_NAMESPACE': '1'
         })
     else:
def entry_hash(entry):
     directory = entry['directory'][::-1]
     # On OS X the 'cc' and 'c++' compilers are wrappers for
     # 'clang' therefore both call would be logged. To avoid
     # this the hash does not contain the first word of the
     # command.
     command = ' '.join(decode(entry['command'])[1:])
 
     return '<>'.join([filename, directory, command])
-
-
-def create_parser():
-    """ Command line argument parser factory method. """
-
-    parser = argparse.ArgumentParser(
-        formatter_class=argparse.ArgumentDefaultsHelpFormatter)
-
-    parser.add_argument(
-        '--verbose', '-v',
-        action='count',
-        default=0,
-        help="""Enable verbose output from '%(prog)s'. A second and third
-                flag increases verbosity.""")
-    parser.add_argument(
-        '--cdb',
-        metavar='<file>',
-        default="compile_commands.json",
-        help="""The JSON compilation database.""")
-    group = parser.add_mutually_exclusive_group()
-    group.add_argument(
-        '--append',
-        action='store_true',
-        help="""Append new entries to existing compilation database.""")
-
-    advanced = parser.add_argument_group('advanced options')
-    advanced.add_argument(
-        '--override-compiler',
-        action='store_true',
-        help="""Always resort to the compiler wrapper even when better
-                intercept methods are available.""")
-    advanced.add_argument(
-        '--use-cc',
-        metavar='<path>',
-        dest='cc',
-        default='cc',
-        help="""When '%(prog)s' analyzes a project by interposing a compiler
-                wrapper, which executes a real compiler for compilation and
-                do other tasks (record the compiler invocation). Because of
-                this interposing, '%(prog)s' does not know what compiler your
-                project normally uses. Instead, it simply overrides the CC
-                environment variable, and guesses your default compiler.
-
-                If you need '%(prog)s' to use a specific compiler for
-                *compilation* then you can use this option to specify a path
-                to that compiler.""")
-    advanced.add_argument(
-        '--use-c++',
-        metavar='<path>',
-        dest='cxx',
-        default='c++',
-        help="""This is the same as "--use-cc" but for C++ code.""")
-
-    parser.add_argument(
-        dest='build',
-        nargs=argparse.REMAINDER,
-        help="""Command to run.""")
-
-    return parser