Bug 181474 - API Review: Move all java related ClassPath types into a single class
API Review: Move all java related ClassPath types into a single class
Status: NEW
Product: java
Classification: Unclassified
Component: Classpath
6.x
All All
: P3 (vote)
: 7.2
Assigned To: Tomas Zezula
issues@java
: API, API_REVIEW_FAST, PLAN
Depends on:
Blocks: 207717
  Show dependency treegraph
 
Reported: 2010-03-03 07:02 UTC by Tomas Zezula
Modified: 2012-02-13 17:48 UTC (History)
5 users (show)

See Also:
Issue Type: TASK
:


Attachments
Patch (8.99 KB, patch)
2010-03-03 07:10 UTC, Tomas Zezula
Details | Diff
Controversial usages of ClassPath.SOURCE (23.55 KB, text/plain)
2010-03-05 02:36 UTC, Petr Jiricka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Zezula 2010-03-03 07:02:41 UTC
Currently the java ClassPath types are in three different classes in three different modules. The  client of this API needs to depend in classpath, java.api and java.common.api. These types should be moved into a single class (JavaClassPathConstants).
Comment 1 Tomas Zezula 2010-03-03 07:10:28 UTC
Created attachment 94715 [details]
Patch
Comment 2 Jan Lahoda 2010-03-03 07:44:01 UTC
Seems fine to me.
Comment 3 Jesse Glick 2010-03-03 10:44:30 UTC
[JG01] Typo: "rahter"


[JG02] Wrong Javadoc link for EXECUTE.


[JG03] Javadoc links from api.java.classpath to api.java cannot work (reverse dep direction); you need to use explicit hrefs with @org-netbeans-api-java@ syntax.


[JG04] Annotations do not require NOI18N.


[JG05] If deprecating constants, remove their Javadoc bodies; just leave the link to the new constant. Otherwise have duplicated text.


[JG06] If ClassPath.* constants are deprecated in favor of api.java, how are modules in non-Java clusters supposed to use them? I thought the whole point of making api.java.classpath was so that it could be used with only a dep on the ide cluster. Don't these other modules use at least SOURCE?


[JG07] PROCESSOR_PATH Javadoc should now use {@link #COMPILE}.


[JG08] Will you update modules referring to the old constants?
Comment 4 Tomas Zezula 2010-03-03 14:31:12 UTC
JG06: 
>how are modules in non-Java clusters supposed to use them?
They should not use them. The parsing.api PathRecognizer registers ClassPath types for language. The types should differ otherwise all indexers are used for all roots.
>I thought the whole point of making api.java.classpath was so that it could be used with only a dep on the ide cluster. 
Yes, the ClassPath API + SPI is a dependency of parsing.api which supports all CSL languages and java. But these languages are having their own constants, for example PHP sources are classpath/php-source. 
>Don't these other modules use at least SOURCE?
They should not use it. If they are doing so, they should fix it.

>JG08: Will you update modules referring to the old constants
I will try a Jackpot to do it, otherwise not.

Typos & javadoc: I will fix it.
Comment 5 Vladimir Voskresensky 2010-03-04 15:10:43 UTC
VV1: May be out of topic. But we (cnd) were very surprised that we had to use java ClassPaths in our cluster. They are tightly connected with sources.
Have a look at 
cnd.makeproject/src/org/netbeans/modules/cnd/makeproject/MakeProject.java
and it's usage of 
org.netbeans.api.java.classpath.ClassPath;
Can this issue be addressed as well?
we really don't want to import org.netbeans.api.java.* packages.

Thanks,
Vladimir.
Comment 6 Petr Jiricka 2010-03-05 02:36:11 UTC
Created attachment 94784 [details]
Controversial usages of ClassPath.SOURCE

Re [JG06]: I did a grep of ClassPath.SOURCE across the codebase, and I am attaching the result after removing the non-controversial usages (clearly related to Java). It is used a lot, including some places in the infrastructure (csl.api, parsing.api, kenai.ui, versioning.util). Not sure if usages in JVM-based languages like Groovy and Scala are ok. And not sure what to do with JavaScript, which does not have its project type and needs to work in a diverse set of project types.
Comment 7 Tomas Zezula 2010-03-05 04:43:49 UTC
VV1: No, this does not resolve the problem of ClassPath being packaged in *.java.*.
These APIs are now in the IDE cluster and I agree they are wrongly named and packaged.
The reason is backward compatibility as these packages and classes were already used from NB 3.6.
Originally I wanted to create Path in the IDE cluster and ClassPath in java which extended the Path. But it was problematic to forbid Path to be subclassed only by ClassPath, etc.
The usage on ClassPath.SOURCE in MakeProject: The Make project uses the SOURCE to be indexed by file indexer, right? It should probably create also PathRecognizer otherwise it slows down the java indexer. When I will be back from vacation I will try it.
Comment 8 Tomas Zezula 2010-03-05 04:52:17 UTC
To pjiricka. Thanks for the list!
There are several cases:
1) parsing.api usage is a bug and it needs to be fixed anyway. The same in PHP which has it's own PHP_SOURCES for sources, probably caused copy paste programming.
2) csl - also copy paste programming, not used just it was in JavaDataLoader.
3) JVM languages are valid use case and they should use JavaClassPathConstants.
4) Ruby, as far as I understand it's using the SOURCE because of jruby integration == JVM language. I will ask about it after vacation.
5) All java modules - OK
6) Erlang, Ada - it's wrong and slows down the java indexing as it brakes the root=>indexer cache in parsing.api.
Comment 9 Vladimir Voskresensky 2010-03-05 06:24:59 UTC
(In reply to comment #7)
> VV1: No, this does not resolve the problem of ClassPath being packaged in
> *.java.*.
> These APIs are now in the IDE cluster and I agree they are wrongly named and
> packaged.
> The reason is backward compatibility as these packages and classes were already
> used from NB 3.6.
> Originally I wanted to create Path in the IDE cluster and ClassPath in java
> which extended the Path. 
That would be great and very appreciated!
> But it was problematic to forbid Path to be subclassed
> only by ClassPath, etc.
    protected Path() {
        if (!(this.getClass().getName.equals("org.netbeans.api.java.ClassPath)) {
            throw new IllegalStateException("Custom Path implementations prohibited."); // NOI18N
        }
    }

> The usage on ClassPath.SOURCE in MakeProject: The Make project uses the SOURCE
> to be indexed by file indexer, right? 
Yes, exactly by indexer only, otherwise we have lost Go To File functionality for C++ projects.

> It should probably create also
> PathRecognizer otherwise it slows down the java indexer. 
We have few bugs how indexer slows down our parsing phase, but not vice versa :-)
So for now to solve our problems we use a lot of hacks (thanks to Jan Lahoda) like using:
import org.netbeans.api.java.classpath.ClassPath;
import org.netbeans.api.java.classpath.GlobalPathRegistry;
import org.netbeans.spi.java.classpath.ClassPathFactory;
import org.netbeans.spi.java.classpath.ClassPathImplementation;
import org.netbeans.spi.java.classpath.ClassPathProvider;
import org.netbeans.spi.java.classpath.FilteringPathResourceImplementation;
import org.netbeans.spi.java.classpath.PathResourceImplementation;
import org.netbeans.spi.java.classpath.support.ClassPathSupport;

> When I will be back from vacation I will try it.
Thank you!
Comment 10 Vladimir Voskresensky 2010-03-05 06:29:12 UTC
(In reply to comment #7)
> The usage on ClassPath.SOURCE in MakeProject: The Make project uses the SOURCE
> to be indexed by file indexer, right? 
I'm sorry, I didn't answer your direct question.
Answer is "no" we do not use ClassPath.SOURCE (although use a lot of other ClassPath staff)
Comment 11 Tomas Zezula 2010-03-05 13:32:54 UTC
Thanks Vladimir.
Comment 12 Jesse Glick 2012-01-27 20:32:06 UTC
JG08 cont'd: specifically please remove dep on java.api.common from maven.

Also introduce a META-INF/upgrade/ClassPath.hint - should make it easy to use Inspect & Transform to fix old usages.


[JG09] ENDORSED Javadoc should be updated (do not refer to ClassPath).
Comment 13 Jesse Glick 2012-02-13 17:35:40 UTC
Are this and bug #207717 under consideration for 7.2?
Comment 14 Tomas Zezula 2012-02-13 17:48:01 UTC
Yes.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo