Reference to the original bug submission is here.

The purpose of this page is to summarize the initial findings into a global replace of all log statements on the entire RI Java code base, as follows:
Before
if (Logging.LOGGING) log.warn("Warning message.");

After
if (Logging.LOGGING && log.isDebugEnabledFor(Level.WARN)) log.warn("Warning message.");

The initial approach consisted of attempting to create a SED script that goes through every .java file and identifies log statements that need to be modified. I've created a separate page here that shows a sample of log statements that would need to be correctly parsed and modified.

SED allows to perform lexical substitutions and modifications to a sequence of characters. However, this is not sufficient, as we need to be able to perform the following syntactic analysis of the log statements:

  1. Identify and parse out an arbitrary boolean expression within an if statement: Sample 6, Sample 9, Sample 10, Sample 11, Sample 22.
  2. Correctly identify the "LOGGER" variable: Sample 16, Sample 25.
  3. Correctly identify the logger object in order to extract the logging level: Sample 12, Sample 14, Sample 15, Sample 19, Sample 20, Sample 21.
  4. Follow method calls within log statements: Sample 7, Sample 12, Sample 19, Sample 20, Sample 21, Sample 24.
  5. When modifying the original log statement, preserve the original source code formatting, even if it is incompliant with our coding standards: Sample 1, Sample 8, Sample 17.

The above requires parsing out snippets of Java code into AST (abstract syntax tree) representation, modifying the tree appropriately, and writing it back into the source code text. I briefly prototyped an open-source Java parsing library called javaparser, located here. Unfortunately, the library does not remember the original whitespace formatting of the source file, so when it is written back into the text form, there are plenty of non-syntactic white space changes introduced.

I briefly looked at the Eclipse JDT and its Java AST parser implementation. This prospect looks very good as it allows for full control of white-space formatting control. However, it would require a lot of work to use the JDT framework in a standalone Java application, so the logging fix would need to be written as an Eclipse plugin. The following article also provides a guideline on how the basic implementation could be achieved.

Oustanding issues

  1. How do we deal with log statements inside comments? Sample 13, Sample 18
  2. Is there a way to automate/script out the execution of the logging fixer application when implemented as an Eclipse plugin?
  • No labels

5 Comments

  1. Unknown User (sdeboy)

    There is also IntelliJ's structural search and replace feature, which gives you access to the AST.  It's not available in the free edition, but the ultimate edition can be evaluated (I have a copy of ultimate).

    See:

    http://www.jetbrains.com/idea/documentation/ssr.html

    http://www.onboard.jetbrains.com/articles/04/10/ssr/

    1. Unknown User (mkorzen)

      The basic idea of how this could be achieved with ASTs goes like this:

      1. In the main class definition, locate the "Category" variable. Note the variable name. Example:
        static final Category log = Logging.LOGGING ? Category.getInstance("ManagerManager") : null;
      2. Look for any boolean class variables that use LOGGING, for example:
        private static final boolean FILTER_LOGGING = LOGGING && true;
        If none are found, assume LOGGING if the class implements Logging or LOGGING.Logging if it does not.
      3. Locate all if statements within the class that contain the variable name identified in step 2.
      4. Within the body of each if statement, identify all instructions that contain method invocation call on the variable name identified in step 1. Follow any enclosed method invocations, if any.
      5. Verify that all statements identified at step 4 call the same method name, i.e. log.warn("..."); or log.info("...");
      6. Insert additional boolean statement into the if expression from step 3, of the form && log.isDebugEnabledFor(Level.WARN)

      Would IntelliJ be able to perform such operation?

  2. Unknown User (sdeboy)

    Is the primary reason for this change to avoid string concatenation prior to evaluation of the logging severity of the logger?

    If so, I believe the need for the change goes away if helper methods are added to the logging API which allow us to avoid string concatenation prior to logger level threshold checks.

    1. Unknown User (mkorzen)

      Can you give an example?

  3. Unknown User (mkorzen)

    Some statistics: there are approximately 5,894 logging statements in the RI source code base:
    $ grep -r LOGGING * | grep -v svn | wc -l
    5894