Fix detecting additional Java inner classes following use of the "new" keyword inside an inner class. (Adam MacBeth)

3 files changed, 79 insertions, 12 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 9e6cb16..2e24314 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -157,6 +157,11 @@ RELEASE 0.97 - XXX
     $WINDOWSSHLIBMANIFESTPREFIX and $WINDOWSSHLIBMANIFESTSUFFIX
     construction variables.
 
+  From Adam MacBeth:
+
+  - Fix detection of additional Java inner classes following use of a
+    "new" keyword inside an inner class.
+
   From Sanjoy Mahajan:
 
   - Correct TeX-related command lines to just $SOURCE, not $SOURCES
diff --git a/src/engine/SCons/Tool/JavaCommon.py b/src/engine/SCons/Tool/JavaCommon.py
index 06a4199..77363f0 100644
--- a/src/engine/SCons/Tool/JavaCommon.py
+++ b/src/engine/SCons/Tool/JavaCommon.py
@@ -42,14 +42,18 @@ if java_parsing:
     # This is a really cool parser from Charles Crain
     # that finds appropriate class names in Java source.
 
-    # A regular expression that will find, in a java file:  newlines;
-    # any alphanumeric token (keyword, class name, specifier); open or
-    # close brackets; a single-line comment "//"; the multi-line comment
-    # begin and end tokens /* and */; single or double quotes;
-    # single or double quotes preceeded by a backslash; array
-    # declarations "[]".
-    _reToken = re.compile(r'(\n|\\\\|//|\\[\'"]|[\'"\{\}]|[A-Za-z_][\w\.]*|' +
-                          r'/\*|\*/|\[\])')
+    # A regular expression that will find, in a java file:
+    #     newlines;
+    #     double-backslashes;
+    #     a single-line comment "//";
+    #     single or double quotes preceeded by a backslash;
+    #     single quotes, double quotes, open or close braces, semi-colons;
+    #     any alphanumeric token (keyword, class name, specifier);
+    #     the multi-line comment begin and end tokens /* and */;
+    #     array declarations "[]";
+    #     semi-colons.
+    _reToken = re.compile(r'(\n|\\\\|//|\\[\'"]|[\'"\{\}\;]|' +
+                          r'[A-Za-z_][\w\.]*|/\*|\*/|\[\])')
 
     class OuterState:
         """The initial state for parsing a Java file for classes,
@@ -62,6 +66,9 @@ if java_parsing:
             self.nextAnon = 1
             self.package = None
 
+        def trace(self):
+            pass
+
         def __getClassState(self):
             try:
                 return self.classState
@@ -108,7 +115,7 @@ if java_parsing:
                     self.listOutputs.append(string.join(self.listClasses, '$'))
                     self.listClasses.pop()
                     self.stackBrackets.pop()
-            elif token == '"' or token == "'":
+            elif token in [ '"', "'" ]:
                 return IgnoreState(token, self)
             elif token == "new":
                 # anonymous inner class
@@ -197,7 +204,7 @@ if java_parsing:
     def parse_java_file(fn):
         return parse_java(open(fn, 'r').read())
 
-    def parse_java(contents):
+    def parse_java(contents, trace=None):
         """Parse a .java file and return a double of package directory,
         plus a list of .class files that compiling that .java file will
         produce"""
@@ -208,6 +215,7 @@ if java_parsing:
             # The regex produces a bunch of groups, but only one will
             # have anything in it.
             currstate = currstate.parseToken(token)
+            if trace: trace(token, currstate)
         if initial.package:
             package = string.replace(initial.package, '.', os.sep)
         return (package, initial.listOutputs)
diff --git a/src/engine/SCons/Tool/JavaCommonTests.py b/src/engine/SCons/Tool/JavaCommonTests.py
index 484fbf8..6d9fc43 100644
--- a/src/engine/SCons/Tool/JavaCommonTests.py
+++ b/src/engine/SCons/Tool/JavaCommonTests.py
@@ -29,6 +29,16 @@ import unittest
 
 import SCons.Tool.JavaCommon
 
+
+# Adding this trace to any of the calls below to the parse_java() method
+# will cause the parser to spit out trace messages of the tokens it sees
+# and state transitions.
+
+def trace(token, newstate):
+    from SCons.Debug import Trace
+    statename = newstate.__class__.__name__
+    Trace('token = %s, state = %s\n' % (repr(token), statename))
+
 class parse_javaTestCase(unittest.TestCase):
 
     def test_bare_bones(self):
@@ -53,6 +63,7 @@ public class Foo
         assert pkg_dir == os.path.join('com', 'sub', 'bar'), pkg_dir
         assert classes == ['Foo'], classes
 
+
     def test_inner_classes(self):
         """Test parsing various forms of inner classes"""
 
@@ -66,7 +77,7 @@ interface Listener {
 
 public
 class
-Test {
+Test implements Listener {
   class Inner {
     void go() {
       use(new Listener() {
@@ -81,6 +92,15 @@ Test {
     /* new Listener() { } */
   }
 
+  class Inner2 {
+     Inner2() { Listener l = new Listener(); }
+  }
+
+  /* Make sure this class doesn't get interpreted as an inner class of the previous one, when "new" is used in the previous class. */
+  class Inner3 {
+
+  }
+
   public static void main(String[] args) {
     new Test().run();
   }
@@ -113,13 +133,15 @@ class Private {
   }
 }
 """)
-
+    
         assert pkg_dir is None, pkg_dir
         expect = [
                    'Empty',
                    'Listener',
                    'Test$1',
                    'Test$Inner',
+                   'Test$Inner2',
+                   'Test$Inner3',
                    'Test$2',
                    'Test$3',
                    'Test',
@@ -128,6 +150,7 @@ class Private {
                  ]
         assert classes == expect, classes
 
+
     def test_comments(self):
         """Test a class with comments"""
 
@@ -171,6 +194,7 @@ public class Example1 extends UnicastRemoteObject implements Hello {
         assert pkg_dir == os.path.join('com', 'sub', 'foo'), pkg_dir
         assert classes == ['Example1'], classes
 
+
     def test_arrays(self):
         """Test arrays of class instances"""
 
@@ -187,6 +211,33 @@ public class Test {
         assert pkg_dir == None, pkg_dir
         assert classes == ['Test'], classes
 
+
+# This test comes from bug report #1197470:
+#
+#    http://sourceforge.net/tracker/index.php?func=detail&aid=1194740&group_id=30337&atid=398971
+#
+# I've captured it here so that someone with a better grasp of Java syntax
+# and the parse_java() state machine can uncomment it and fix it some day.
+#
+#    def test_arrays_in_decls(self):
+#        """Test how arrays in method declarations affect class detection"""
+#
+#        pkg_dir, classes = SCons.Tool.JavaCommon.parse_java("""\
+#public class A {
+#    public class B{
+#        public void F(Object[] o) {
+#            F(new Object[] {Object[].class});
+#        }
+#        public void G(Object[] o) {
+#            F(new Object[] {});
+#        }
+#    }
+#}
+#""")
+#        assert pkg_dir == None, pkg_dir
+#        assert classes == ['A$B', 'A'], classes
+
+
     def test_backslash(self):
         """Test backslash handling"""
 
@@ -202,6 +253,7 @@ public class MyTabs
         assert pkg_dir == None, pkg_dir
         assert classes == ['MyTabs$MyInternal', 'MyTabs'], classes
 
+
     def test_enum(self):
         """Test the Java 1.5 enum keyword"""
 
@@ -212,6 +264,8 @@ public enum a {}
         assert pkg_dir == 'p', pkg_dir
         assert classes == ['a'], classes
 
+
+
 if __name__ == "__main__":
     suite = unittest.TestSuite()
     tclasses = [ parse_javaTestCase ]