OpenJPA
  1. OpenJPA
  2. OPENJPA-2410

Build time detection of System.out/err.print(ln) in source files

    Details

    • Patch Info:
      Patch Available

      Description

      Source files may have inadvertent System.out/err.print statements left in source files on commit. Ideally we don't want any of these changes being checked in and a large majority of the time logging facilities should be utilized. That being said, there are a number of cases where we don't have access to a logger and must use System.out/err.

      This JIRA will be used to update our checkstyle to scan for unwanted print statements, and fail the build when/if they are encountered.

      1. OPENJPA-2410r2.patch
        1 kB
        Di Wu Lau
      2. OPENJPA-2410r1.patch
        0.9 kB
        Di Wu Lau
      3. OPENJPA-2410.patch
        7 kB
        Di Wu Lau
      4. OPENJPA-2410.2.2.x.patch
        26 kB
        Rick Curtis
      5. OPENJPA-2410.1.2.x.patch
        15 kB
        Rick Curtis

        Activity

        Hide
        ASF subversion and git services added a comment -

        Commit 1511071 from Rick Curtis in branch 'openjpa/branches/1.2.x'
        [ https://svn.apache.org/r1511071 ]

        OPENJPA-2410: Merge checkstyle changes from trunk.

        Show
        ASF subversion and git services added a comment - Commit 1511071 from Rick Curtis in branch 'openjpa/branches/1.2.x' [ https://svn.apache.org/r1511071 ] OPENJPA-2410 : Merge checkstyle changes from trunk.
        Hide
        ASF subversion and git services added a comment -

        Commit 1511055 from Rick Curtis in branch 'openjpa/branches/2.0.x'
        [ https://svn.apache.org/r1511055 ]

        OPENJPA-2410: Merge checkstyle changes from trunk.

        Show
        ASF subversion and git services added a comment - Commit 1511055 from Rick Curtis in branch 'openjpa/branches/2.0.x' [ https://svn.apache.org/r1511055 ] OPENJPA-2410 : Merge checkstyle changes from trunk.
        Hide
        ASF subversion and git services added a comment -

        Commit 1511008 from Rick Curtis in branch 'openjpa/branches/2.1.x'
        [ https://svn.apache.org/r1511008 ]

        OPENJPA-2410 : Merge checkstyle changes.

        Show
        ASF subversion and git services added a comment - Commit 1511008 from Rick Curtis in branch 'openjpa/branches/2.1.x' [ https://svn.apache.org/r1511008 ] OPENJPA-2410 : Merge checkstyle changes.
        Hide
        ASF subversion and git services added a comment -

        Commit 1510994 from Rick Curtis in branch 'openjpa/branches/2.2.x'
        [ https://svn.apache.org/r1510994 ]

        OPENJPA-2410: Merge checkstyle changes from trunk to 2.2.x .

        Show
        ASF subversion and git services added a comment - Commit 1510994 from Rick Curtis in branch 'openjpa/branches/2.2.x' [ https://svn.apache.org/r1510994 ] OPENJPA-2410 : Merge checkstyle changes from trunk to 2.2.x .
        Hide
        Rick Curtis added a comment -

        Attaching a patch for 1.2.x and 2.2.x branches. Heath – Can I have you take a look at the patches and give me a thumbs up if these are okay to commit to the service branches. There are very few .java source file changes, and moreso they should be safe changes to make.

        Show
        Rick Curtis added a comment - Attaching a patch for 1.2.x and 2.2.x branches. Heath – Can I have you take a look at the patches and give me a thumbs up if these are okay to commit to the service branches. There are very few .java source file changes, and moreso they should be safe changes to make.
        Hide
        Rick Curtis added a comment -

        I'll note that when running on maven 3.x or newer you'll get the following failure :

        [INFO] BUILD FAILURE
        [INFO] ------------------------------------------------------------------------
        [INFO] Total time: 4.673s
        [INFO] Finished at: Sat Aug 03 12:00:11 CDT 2013
        [INFO] Final Memory: 30M/60M
        [INFO] ------------------------------------------------------------------------
        [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.4:checkstyle (default) on project openjpa-lib: Execution default of goal org.apache.maven.plugins:mav
        en-checkstyle-plugin:2.4:checkstyle failed. NullPointerException -> [Help 1]
        [ERROR]
        [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
        [ERROR] Re-run Maven using the -X switch to enable full debug logging.
        [ERROR]
        [ERROR] For more information about the errors and possible solutions, please read the following articles:
        [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException
        [ERROR]
        [ERROR] After correcting the problems, you can resume the build with the command
        [ERROR] mvn <goals> -rf :openjpa-lib

        Show
        Rick Curtis added a comment - I'll note that when running on maven 3.x or newer you'll get the following failure : [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 4.673s [INFO] Finished at: Sat Aug 03 12:00:11 CDT 2013 [INFO] Final Memory: 30M/60M [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.4:checkstyle (default) on project openjpa-lib: Execution default of goal org.apache.maven.plugins:mav en-checkstyle-plugin:2.4:checkstyle failed. NullPointerException -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginExecutionException [ERROR] [ERROR] After correcting the problems, you can resume the build with the command [ERROR] mvn <goals> -rf :openjpa-lib
        Hide
        ASF subversion and git services added a comment -

        Commit 1505786 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1505786 ]

        OPENJPA-2410 : Yet another checkstyle change.

        Show
        ASF subversion and git services added a comment - Commit 1505786 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1505786 ] OPENJPA-2410 : Yet another checkstyle change.
        Hide
        Rick Curtis added a comment -

        I don't think we want to move to Maven 3.x for the sake of the checkstyle. I think I finally have everything working properly now, but apache svn is having some problems so I can't commit the change.

        Show
        Rick Curtis added a comment - I don't think we want to move to Maven 3.x for the sake of the checkstyle. I think I finally have everything working properly now, but apache svn is having some problems so I can't commit the change.
        Hide
        Di Wu Lau added a comment -

        To run on Maven 3.0.5, maven-checkstyle-plugin needs to be version 2.5. It can be changed in the root pom.xml. I haven't found a more compatible solution for both maven 2.2.1 and maven 3.0.5 yet.

        Show
        Di Wu Lau added a comment - To run on Maven 3.0.5, maven-checkstyle-plugin needs to be version 2.5. It can be changed in the root pom.xml. I haven't found a more compatible solution for both maven 2.2.1 and maven 3.0.5 yet.
        Hide
        ASF subversion and git services added a comment -

        Commit 1505051 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1505051 ]

        OPENJPA-2410 : Yet another checkstyle update. Should fix nix issues.

        Show
        ASF subversion and git services added a comment - Commit 1505051 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1505051 ] OPENJPA-2410 : Yet another checkstyle update. Should fix nix issues.
        Hide
        Rick Curtis added a comment -

        For some reason the changes I've made so far doesn't run correctly on *nix, and also has problems when running on Maven 3.0+.

        Unfortunately my linux machine is having problems right now so I can't get a full build to complete... I'm going to try one more commit this evening to try to get things mostly working and try to find some time later this weekend to get things properly cleaned up.

        Sorry for anyone that isn't able to build due to this little fiasco.

        Show
        Rick Curtis added a comment - For some reason the changes I've made so far doesn't run correctly on *nix, and also has problems when running on Maven 3.0+. Unfortunately my linux machine is having problems right now so I can't get a full build to complete... I'm going to try one more commit this evening to try to get things mostly working and try to find some time later this weekend to get things properly cleaned up. Sorry for anyone that isn't able to build due to this little fiasco.
        Hide
        ASF subversion and git services added a comment -

        Commit 1504984 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1504984 ]

        OPENJPA-2410 : Yet another checkstyle update.

        Show
        ASF subversion and git services added a comment - Commit 1504984 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1504984 ] OPENJPA-2410 : Yet another checkstyle update.
        Hide
        Di Wu Lau added a comment -

        patch for suppressions.xml only.

        Show
        Di Wu Lau added a comment - patch for suppressions.xml only.
        Hide
        Di Wu Lau added a comment -

        Hi Rick, if you take a look at my suppressions.xml about suppressing the src\test files, it looks like this: <suppress checks="RegexpSinglelineJava" files=".*src
        test.*java"/>. It's the backslash instead of forward slash. you also need to skip the backslash. I think this would solve the problem.

        Show
        Di Wu Lau added a comment - Hi Rick, if you take a look at my suppressions.xml about suppressing the src\test files, it looks like this: <suppress checks="RegexpSinglelineJava" files=".*src test.*java"/>. It's the backslash instead of forward slash. you also need to skip the backslash. I think this would solve the problem.
        Hide
        Di Wu Lau added a comment -

        a patch for checkstyle.xml. changing "END" to "STOP".

        Show
        Di Wu Lau added a comment - a patch for checkstyle.xml. changing "END" to "STOP".
        Hide
        ASF subversion and git services added a comment -

        Commit 1504883 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1504883 ]

        OPENJPA-2410 : Another minor change to checkstyle.xml

        Show
        ASF subversion and git services added a comment - Commit 1504883 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1504883 ] OPENJPA-2410 : Another minor change to checkstyle.xml
        Hide
        ASF subversion and git services added a comment -

        Commit 1504861 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1504861 ]

        OPENJPA-2410 : Minor change to checkstyle suppressions.xml

        Show
        ASF subversion and git services added a comment - Commit 1504861 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1504861 ] OPENJPA-2410 : Minor change to checkstyle suppressions.xml
        Hide
        Di Wu Lau added a comment - - edited

        Hi Rick, I think you meant " // STOP - ALLOW PRINT STATEMENTS " instead of " // END - ALLOW PRINT STATEMENTS " in the checkstyle.xml? It is still "END" for right now. In the java files, it is "STOP". This could cause build failure as we set severity=error for the print statements.

        Show
        Di Wu Lau added a comment - - edited Hi Rick, I think you meant " // STOP - ALLOW PRINT STATEMENTS " instead of " // END - ALLOW PRINT STATEMENTS " in the checkstyle.xml? It is still "END" for right now. In the java files, it is "STOP". This could cause build failure as we set severity=error for the print statements.
        Hide
        Di Wu Lau added a comment -

        Thank you Rick for going through all the print statements!

        Show
        Di Wu Lau added a comment - Thank you Rick for going through all the print statements!
        Hide
        Rick Curtis added a comment -

        I'll note that the code that I committed was somewhat different than your last patch. I decided to go back to allowing a comment to dis/enable checking to allow for exclusions.

        Going forward, if you want to have code that has a System.out/err in it you'll need to use the comment '// START - ALLOW PRINT STATEMENTS' to tell the checkstyle plugin to stop checking... and then us the comment '// STOP - ALLOW PRINT STATEMENTS' to re-enable checking. I'll cross post this to the users / dev list and try to get some info on the wiki also.

        ie:

        // START - ALLOW PRINT STATEMENTS
        System.err.println("Not handled " + fmd.getName() + " of type " + fmd.getDeclaredType());
        // STOP - ALLOW PRINT STATEMENTS

        Show
        Rick Curtis added a comment - I'll note that the code that I committed was somewhat different than your last patch. I decided to go back to allowing a comment to dis/enable checking to allow for exclusions. Going forward, if you want to have code that has a System.out/err in it you'll need to use the comment '// START - ALLOW PRINT STATEMENTS' to tell the checkstyle plugin to stop checking... and then us the comment '// STOP - ALLOW PRINT STATEMENTS' to re-enable checking. I'll cross post this to the users / dev list and try to get some info on the wiki also. ie: // START - ALLOW PRINT STATEMENTS System.err.println("Not handled " + fmd.getName() + " of type " + fmd.getDeclaredType()); // STOP - ALLOW PRINT STATEMENTS
        Hide
        Rick Curtis added a comment -

        Committed revision 1504673 to trunk. Thanks for the patch Di!

        Show
        Rick Curtis added a comment - Committed revision 1504673 to trunk. Thanks for the patch Di!
        Hide
        ASF subversion and git services added a comment -

        Commit 1504673 from Rick Curtis in branch 'openjpa/trunk'
        [ https://svn.apache.org/r1504673 ]

        OPENJPA-2410 : Detect SystemOut/Err in code at build time. Patch submitted by Di Wu Lau.

        Show
        ASF subversion and git services added a comment - Commit 1504673 from Rick Curtis in branch 'openjpa/trunk' [ https://svn.apache.org/r1504673 ] OPENJPA-2410 : Detect SystemOut/Err in code at build time. Patch submitted by Di Wu Lau.
        Hide
        Di Wu Lau added a comment -

        Some files have regular expressions instead of the full path because the regex is good enough in telling the difference between the files which have the same name.

        Show
        Di Wu Lau added a comment - Some files have regular expressions instead of the full path because the regex is good enough in telling the difference between the files which have the same name.
        Hide
        Di Wu Lau added a comment -

        The newly attached patch has full path to the file in order to distinguish different files with the same file names.

        Show
        Di Wu Lau added a comment - The newly attached patch has full path to the file in order to distinguish different files with the same file names.
        Hide
        Di Wu Lau added a comment - - edited

        Attached is a patch for maven 2.2.1 because maven 3.0.5 uses a different checkstyle-plugin version.

        Show
        Di Wu Lau added a comment - - edited Attached is a patch for maven 2.2.1 because maven 3.0.5 uses a different checkstyle-plugin version.
        Hide
        Di Wu Lau added a comment -

        Fixed patch according to the comment.

        Show
        Di Wu Lau added a comment - Fixed patch according to the comment.
        Hide
        Rick Curtis added a comment -

        A few comments on your patch :

        • Remove the changes to XMLFormatter.java.
        • Fix the the spacing on checkstyle.xml.
        • Can you move the suppressions.xml so that it is under the openjpa-project module?
        • Remove IdentifierUtilImpl from the supressions.xml file, and remove the System.out call from IdentifierUtilImpl:368

        Otherwise, the patch looks good.

        Show
        Rick Curtis added a comment - A few comments on your patch : Remove the changes to XMLFormatter.java. Fix the the spacing on checkstyle.xml. Can you move the suppressions.xml so that it is under the openjpa-project module? Remove IdentifierUtilImpl from the supressions.xml file, and remove the System.out call from IdentifierUtilImpl:368 Otherwise, the patch looks good.
        Hide
        Di Wu Lau added a comment -

        pom.xml and suppressions.xml are placed in the top level directory.

        checkstyle.xml is placed inside openjpa-project directory.

        Show
        Di Wu Lau added a comment - pom.xml and suppressions.xml are placed in the top level directory. checkstyle.xml is placed inside openjpa-project directory.

          People

          • Assignee:
            Rick Curtis
            Reporter:
            Di Wu Lau
          • Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development