Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.0.3
    • Fix Version/s: None
    • Component/s: core/store
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      https://svn.apache.org/repos/asf/lucene/dev/trunk/lucene/src/java/org/apache/lucene/store/NativeFSLockFactory.java shows a private top-level class NativeFSLock. This is a JDK 1.0-ism which should be considered deprecated. Either use a nested class, or create a separate source file.

      Can produce problems for tools. Cause of: https://netbeans.org/bugzilla/show_bug.cgi?id=204039

        Activity

        Hide
        Uwe Schindler added a comment -

        I fixed that for DocSet and DocSetBase in Solr.

        Show
        Uwe Schindler added a comment - I fixed that for DocSet and DocSetBase in Solr.
        Hide
        Jesse Glick added a comment -

        BTW I missed TransactionLog (in FSUpdateLog.java) before.

        "surely it's a transient bug" - probably existed for years. There is just not that much code out there using JDK 1.0 private classes for people to notice it earlier.

        "will be fixed soon" - no idea. For the same reason, it is not a high priority. Anyway there are surely other things affected. While hyperlinking stack traces with such classes is easy enough once you know about it (for \tat p.B(A.java:123) ignore B and open p/A.java), other tool features can be harder to fix since you need a bytecode parser.

        "why would NullUpdateLog be scoped within FSUpdateLong" - no good reason that I can see. Why was it in FSUpdateLong.java? A candidate for refactoring of some kind, in this case probably to be moved into UpdateHandler.java.

        "Class.forName used by Solr configuration" - perhaps, though remember that these classes are necessarily package-private so not usable via reflection unless setAccessible(true) is called. For such cases (is there a list?) you would want to move the class into its own source file. For a committer this would be simple enough; for purposes of a submitted patch I did not want to make any such changes because the diff would be unreviewable and unlikely to apply cleanly after other trunk changes.

        Show
        Jesse Glick added a comment - BTW I missed TransactionLog (in FSUpdateLog.java ) before. "surely it's a transient bug" - probably existed for years. There is just not that much code out there using JDK 1.0 private classes for people to notice it earlier. "will be fixed soon" - no idea. For the same reason, it is not a high priority. Anyway there are surely other things affected. While hyperlinking stack traces with such classes is easy enough once you know about it (for \tat p.B(A.java:123) ignore B and open p/A.java ), other tool features can be harder to fix since you need a bytecode parser. "why would NullUpdateLog be scoped within FSUpdateLong " - no good reason that I can see. Why was it in FSUpdateLong.java ? A candidate for refactoring of some kind, in this case probably to be moved into UpdateHandler.java . " Class.forName used by Solr configuration" - perhaps, though remember that these classes are necessarily package-private so not usable via reflection unless setAccessible(true) is called. For such cases (is there a list?) you would want to move the class into its own source file. For a committer this would be simple enough; for purposes of a submitted patch I did not want to make any such changes because the diff would be unreviewable and unlikely to apply cleanly after other trunk changes.
        Hide
        Yonik Seeley added a comment -

        This netbeans bug was just reported a few weeks ago - surely it's a transient bug and will be fixed soon?

        We shouldn't blindly change package private classes to static inner clases - first of all, it sometimes makes no sense... why would NullUpdateLog be scoped within FSUpdateLong? Second, it changes the class names (i.e. breaks things Class.forName that are used by solr configuration).

        Show
        Yonik Seeley added a comment - This netbeans bug was just reported a few weeks ago - surely it's a transient bug and will be fixed soon? We shouldn't blindly change package private classes to static inner clases - first of all, it sometimes makes no sense... why would NullUpdateLog be scoped within FSUpdateLong? Second, it changes the class names (i.e. breaks things Class.forName that are used by solr configuration).
        Hide
        Jesse Glick added a comment -

        Patch to remove all private top-level classes from Lucene/Solr trunk sources including tests.

        Show
        Jesse Glick added a comment - Patch to remove all private top-level classes from Lucene/Solr trunk sources including tests.
        Hide
        Uwe Schindler added a comment -

        Can you provide a patch for Lucene/Solr trunk, that transforms all those classes to static inner ones? This would be nice to have, but I see no real need for doing that now, so I am a little bit reluctant to do it.

        Show
        Uwe Schindler added a comment - Can you provide a patch for Lucene/Solr trunk, that transforms all those classes to static inner ones? This would be nice to have, but I see no real need for doing that now, so I am a little bit reluctant to do it.
        Hide
        Jesse Glick added a comment -

        The bug in NetBeans was already given in the link above. But this construction is long obsolete and particularly tricky for tools to support in general , so it should be replaced whenever possible.

        Since to find an associated source file you need to inspect a bytecode attribute. For JDK 1.1+ sources it suffices to just look at the class FQN to determine the source path.

        Show
        Jesse Glick added a comment - The bug in NetBeans was already given in the link above. But this construction is long obsolete and particularly tricky for tools to support in general , so it should be replaced whenever possible. Since to find an associated source file you need to inspect a bytecode attribute. For JDK 1.1+ sources it suffices to just look at the class FQN to determine the source path.
        Hide
        Uwe Schindler added a comment -

        I agree with Robert: It's a bug with Netbeans.

        But we should change those stupid side-by-side package private classes and make them subclasses. I have seen a lot of them in old code. Whenever I updated a class using this, I changed it.

        Show
        Uwe Schindler added a comment - I agree with Robert: It's a bug with Netbeans. But we should change those stupid side-by-side package private classes and make them subclasses. I have seen a lot of them in old code. Whenever I updated a class using this, I changed it.
        Hide
        Robert Muir added a comment -

        How is this not a bug in Netbeans?

        Seriously, this code compiles just fine, its legal java.

        Show
        Robert Muir added a comment - How is this not a bug in Netbeans? Seriously, this code compiles just fine, its legal java.

          People

          • Assignee:
            Unassigned
            Reporter:
            Jesse Glick
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development