Container org.eclipse.jdt.MODULE_PATH hides problems in CompletionTests9

[stephan-herrmann]

Work on #2169 / #2358 raised some suspicion regarding:

1. class path container "org.eclipse.jdt.MODULE_PATH"
2. the interplay of different strategies within SearchableEnvironment.findPackages(char[], ISearchRequestor, IPackageFragmentRoot[], boolean)

(1) is a strawman from the early module days. Its contents is backed by ModuleSourcePathManager which simply enumerates all modular projects in the workspace. There is no support for binary modules. This was to be an equivalent of PDE's container "org.eclipse.pde.core.requiredPlugins", but the module path container never really took off. I don't recall that it was ever publicly documented.

(2) combines two searches:

• the first is driven just by what the current JavaProject can see, i.e., all IPackageFragmentRoots etc.
• second a "semantic" traversal of module descriptions to find all exported packages of directly or transitively required modules

Tests in CompletionTests9, in particular those from bug 519417, use the module path container. If I change the project structure from module path container to regular project dependencies, then already nameLookup.seekPackageFragments(..) finds more packages including some that should not be seen.

It's the addition from bug 519417 which caused grief in #2169

I think it's time to retire the container "org.eclipse.jdt.MODULE_PATH". When comparing to the PDE container, we never came up with an equivalent of a target platform. Just searching workspace projects doesn't suffice.

But when we require "regular" project dependencies between modular projects, then CompletionTests9 reveals more problems.

[stephan-herrmann]

There's also a suspicious navigation in SearchableEnvironment.findPackagesFromRequires(..):

		for (IPackageFragmentRoot root : fragmentRoots) {
			IJavaProject requiredProject = root.getJavaProject();
			try {
				IModuleDescription module = requiredProject.getModuleDescription();

The detour via requireProject looks wrong. I'm quite sure it should read like this:

		for (IPackageFragmentRoot root : fragmentRoots) {
			try {
				IModuleDescription module = root.getModuleDescription();

[stephan-herrmann]

@jarthana @mpalat do you see any future for the class path container "org.eclipse.jdt.MODULE_PATH"?
See class org.eclipse.jdt.internal.core.ModulePathContainer.

Judging by history this was developed by Sasi, with a little help from @jarthana ?

[jarthana]

@jarthana @mpalat do you see any future for the class path container "org.eclipse.jdt.MODULE_PATH"? See class org.eclipse.jdt.internal.core.ModulePathContainer.

Judging by history this was developed by Sasi, with a little help from @jarthana ?

I have nothing against retiring it.

[stephan-herrmann]

I have nothing against retiring it.

thanks,

just now, genie reminds me about our previous discussion in bugzilla - what a coincidence!
In bug 518751 c8 @noopur2507 called out a main deficiency of the container: lack of versioning.

And folks already agreed on removing, so I'll do it soonish, finally :)