GH-2588: Exported d.ts files contain non-public elements (#2591)

* add tests

* support pnpm for project discovery tests

* fix bug in node_modules identification

* fix project discovery and ide tests

plugins/org.eclipse.n4js/src/org/eclipse/n4js/utils/NodeModulesDiscoveryHelper.java

@@ -85,13 +85,21 @@ public boolean isYarnWorkspace() {
 			return this.workspaceNodeModulesFolder != null;
 		}
 
-		/** @return the related root of this project */
-		public Path getRelatedRoot() {
+		/** @return the local root of this project */
+		public Path getLocalRoot() {
 			if (isYarnWorkspaceRoot || isYarnWorkspaceMember || isYarnWorkspaceDependency) {
 				return workspaceNodeModulesFolder.getParentFile().toPath();
 			}
 			return localNodeModulesFolder.getParentFile().toPath();
 		}
+
+		/** @return the workspace root of this project */
+		public Path getWorkspaceRoot() {
+			if (workspaceNodeModulesFolder != null) {
+				return workspaceNodeModulesFolder.getParentFile().toPath();
+			}
+			return localNodeModulesFolder.getParentFile().toPath();
+		}
 	}
 
 	/** @return the node_modules folder of the given project including a flag if this is a yarn workspace. */

plugins/org.eclipse.n4js/src/org/eclipse/n4js/utils/ProjectDiscoveryHelper.java

@@ -431,24 +431,27 @@ private Set<Path> collectNecessaryDependencies(Set<Path> allProjectDirs, Map<Pat
 	private void collectProjectDependencies(Set<Path> allProjectDirs, Path projectDir,
 			Map<Path, ProjectDescription> pdCache, Set<Path> dependencies) {
 
-		NodeModulesFolder nodeModulesFolder = nodeModulesDiscoveryHelper.getNodeModulesFolder(projectDir, pdCache);
+		Path wsRoot = nodeModulesDiscoveryHelper.getNodeModulesFolder(projectDir, pdCache).getWorkspaceRoot();
+
 		LinkedHashSet<Path> workList = new LinkedHashSet<>();
 		workList.add(projectDir);
 		while (!workList.isEmpty()) {
 			Iterator<Path> iterator = workList.iterator();
 			Path nextProject = iterator.next();
 			iterator.remove();
 
-			findDependencies(allProjectDirs, nextProject, nodeModulesFolder, pdCache, workList, dependencies);
+			NodeModulesFolder nodeModulesFolder = nodeModulesDiscoveryHelper.getNodeModulesFolder(nextProject, pdCache);
+			findDependencies(allProjectDirs, nextProject, wsRoot, nodeModulesFolder, pdCache, workList, dependencies);
 		}
 	}
 
-	private void findDependencies(Set<Path> allProjectDirs, Path prjDir, NodeModulesFolder nodeModulesFolder,
-			Map<Path, ProjectDescription> pdCache, Set<Path> workList, Set<Path> dependencies) {
+	private void findDependencies(Set<Path> allProjectDirs, Path prjDir, Path wsRoot,
+			NodeModulesFolder nodeModulesFolder, Map<Path, ProjectDescription> pdCache, Set<Path> workList,
+			Set<Path> dependencies) {
 
-		Path relatedRoot = nodeModulesFolder.getRelatedRoot();
+		// Path wsRoot = nodeModulesFolder.getWorkspaceRoot();
 		Path prjNodeModules = prjDir.resolve(N4JSGlobals.NODE_MODULES);
-		ProjectDescription prjDescr = getOrCreateProjectDescription(prjDir, relatedRoot, pdCache);
+		ProjectDescription prjDescr = getOrCreateProjectDescription(prjDir, wsRoot, pdCache);
 		if (prjDescr == null) {
 			return;
 		}
@@ -457,8 +460,8 @@ private void findDependencies(Set<Path> allProjectDirs, Path prjDir, NodeModules
 			String depName = dependency.getPackageName();
 
 			Path depLocation = findDependencyLocation(nodeModulesFolder, prjNodeModules, depName);
-			if (isNewDependency(allProjectDirs, prjDir, pdCache, depLocation, relatedRoot)) {
-				addDependency(depLocation, relatedRoot, pdCache, workList, dependencies);
+			if (isNewDependency(allProjectDirs, prjDir, pdCache, depLocation, wsRoot)) {
+				addDependency(depLocation, wsRoot, pdCache, workList, dependencies);
 			}
 		}
 	}

testhelpers/org.eclipse.n4js.ide.tests.helper/src/org/eclipse/n4js/ide/tests/helper/server/TestWorkspaceManager.java

@@ -582,6 +582,7 @@ private Project createSimpleProject(String projectName, Map<String, ? extends Ch
 					Project nmProject = project.getNodeModuleProject(nmName);
 					if (nmProject == null) {
 						nmProject = new Project(nmName, VENDOR, VENDOR + "_name", prjType);
+						addDependencyToN4JSRuntime(nmProject);
 						nmProject.createSourceFolder(DEFAULT_SOURCE_FOLDER);
 						project.addNodeModuleProject(nmProject);
 						project.addProjectDependency(nmProject.getName());
@@ -602,6 +603,7 @@ private Project createSimpleProject(String projectName, Map<String, ? extends Ch
 					int startIdx = moduleName.indexOf(CFG_NODE_MODULES, length) + CFG_NODE_MODULES.length();
 					String nmnmName = moduleName.substring(startIdx);
 					Project nmnmProject = new Project(nmnmName, VENDOR, VENDOR + "_name", prjType);
+					addDependencyToN4JSRuntime(nmnmProject);
 					nmnmProject.createSourceFolder(DEFAULT_SOURCE_FOLDER);
 					nmProject.addNodeModuleProject(nmnmProject);
 					nmProject.addProjectDependency(nmnmProject.getName());
@@ -633,9 +635,8 @@ private Project createSimpleProject(String projectName, Map<String, ? extends Ch
 		}
 
 		// apply default values
+		addDependencyToN4JSRuntime(project);
 		if (N4JSGlobals.PROJECT_TYPES_REQUIRING_N4JS_RUNTIME.contains(project.getType())) {
-			// add dependency to n4js-runtime (if not already present)
-			project.addProjectDependency(N4JS_RUNTIME);
 			// add fake n4js-runtime to node_modules folder (if not already present)
 			if (projectKind == ProjectKind.TopLevel) {
 				if (project.getNodeModuleProject(N4JS_RUNTIME) == null) {
@@ -647,6 +648,16 @@ private Project createSimpleProject(String projectName, Map<String, ? extends Ch
 		return project;
 	}
 
+	private void addDependencyToN4JSRuntime(Project project) {
+		// apply default values
+		if (N4JSGlobals.PROJECT_TYPES_REQUIRING_N4JS_RUNTIME.contains(project.getType())) {
+			if (!N4JSGlobals.N4JS_RUNTIME.getRawName().equals(project.getName())) {
+				// add dependency to n4js-runtime (if not already present)
+				project.addProjectDependency(N4JS_RUNTIME);
+			}
+		}
+	}
+
 	private void createAndAddModule(String contents, String moduleName, Folder nmSourceFolder) {
 		NameAndExtension nae = getN4JSNameAndExtension(moduleName);
 		Module module = nae.extension == null ? new Module(moduleName) : new Module(nae.name, nae.extension, null);

testhelpers/org.eclipse.n4js.tests.helper/src/org/eclipse/n4js/tests/helper/mock/MockWorkspaceSupplier.java

@@ -124,7 +124,7 @@ protected N4JSWorkspaceConfigSnapshot createWorkspaceConfig() {
 		N4JSProjectConfigSnapshot projectConfig = createProjectConfig();
 		URI workspacePath = URIUtils.trimTrailingPathSeparator(projectConfig.getPath()).trimSegments(1);
 		ProjectSet projects = new ProjectSet(Collections.singleton(projectConfig));
-		BuildOrderInfo buildOrderInfo = new BuildOrderInfo(Collections.emptyList(), Collections.emptySet());
+		BuildOrderInfo buildOrderInfo = BuildOrderInfo.NULL;
 		return new N4JSWorkspaceConfigSnapshot(workspacePath, projects, buildOrderInfo);
 	}
 

tests/org.eclipse.n4js.ide.tests/PDTs/pnpmDependency1.pdt

@@ -0,0 +1,19 @@
+# Project Discovery Test
+# working dir marked with '*'
+
+
+FOLDERS
+-*Root [PROJECT workspaces-pnpm=["packages/*"]]
+-- packages
+--- P1 [PROJECT dependencies={"D1":"*"}]
+---- node_modules
+----- D1 [SYMLINK_TO_PATH Root/node_modules/.pnpm/D1]
+-- node_modules
+--- .pnpm
+---- P1 [SYMLINK_TO_PATH Root/packages/P1]
+---- D1 [PROJECT]
+
+EXPECT
+- Root
+- Root/packages/P1
+- Root/node_modules/.pnpm/D1

tests/org.eclipse.n4js.ide.tests/PDTs/pnpmDependency2.pdt

@@ -0,0 +1,25 @@
+# Project Discovery Test
+# working dir marked with '*'
+
+
+FOLDERS
+-*Root [PROJECT workspaces-pnpm=["packages/*"]]
+-- packages
+--- P1 [PROJECT dependencies={"D1":"*"}]
+---- node_modules
+----- D1 [SYMLINK_TO_PATH Root/node_modules/.pnpm/[email protected]/node_modules/D1]
+-- node_modules
+--- .pnpm
+---- P1 [SYMLINK_TO_PATH Root/packages/P1]
+---- [email protected]
+----- node_modules
+------ D1 [PROJECT dependencies={"D2":"*"}]
+------ D2 [SYMLINK_TO_PATH Root/node_modules/.pnpm/D2]
+---- D2 [PROJECT]
+
+
+EXPECT
+- Root
+- Root/packages/P1
+- Root/node_modules/.pnpm/[email protected]/node_modules/D1
+- Root/node_modules/.pnpm/D2

tests/org.eclipse.n4js.ide.tests/PDTs/pnpmSimple1.pdt

@@ -0,0 +1,15 @@
+# Project Discovery Test
+# working dir marked with '*'
+
+
+FOLDERS
+-*P1 [PROJECT workspaces-pnpm=["packages/*"]]
+-- packages
+--- D1 [PROJECT]
+-- node_modules
+--- D1 [SYMLINK_TO_PATH P1/packages/D1]
+
+
+EXPECT
+- P1
+- P1/packages/D1

tests/org.eclipse.n4js.ide.tests/PDTs/pnpmSimple2.pdt

@@ -0,0 +1,18 @@
+# Project Discovery Test
+# working dir marked with '*'
+
+
+FOLDERS
+-*P1 [PROJECT workspaces=["packages/*"]]
+-- packages
+--- D1 [PROJECT dependencies={"D2":"*"}]
+--- D2 [PROJECT]
+-- node_modules
+--- D1 [SYMLINK_TO_PATH P1/packages/D1]
+--- D2 [SYMLINK_TO_PATH P1/packages/D2]
+
+
+EXPECT
+- P1
+- P1/packages/D1
+- P1/packages/D2

tests/org.eclipse.n4js.ide.tests/src/org/eclipse/n4js/ide/tests/builder/IncrementalBuilderCopiedProjectTest.java

@@ -50,7 +50,7 @@ public void testCreateDuplicateProject() throws IOException {
 
 		startAndWaitForLspServer();
 		assertProjectBuildOrder(
-				"[test-project/node_modules/lib, test-project/node_modules/n4js-runtime, test-project]");
+				"[test-project/node_modules/n4js-runtime, test-project/node_modules/lib, test-project]");
 		assertIssues2(Pair.of("MyModule",
 				List.of("(Error, [0:1 - 0:12], Couldn't resolve reference to IdentifiableElement '_globalThis'.)")));
 		shutdownLspServer();
@@ -65,7 +65,7 @@ public void testCreateDuplicateProject() throws IOException {
 		FileURI myModule1 = new FileURI(new File(getRoot(), DEFAULT_PROJECT_NAME + "/src/MyModule.n4js"));
 		FileURI myModule2 = new FileURI(new File(getRoot(), DEFAULT_PROJECT_NAME + "2/src/MyModule.n4js"));
 		assertProjectBuildOrder(
-				"[test-project/node_modules/lib, test-project/node_modules/n4js-runtime, test-project, test-project2/node_modules/lib, test-project2/node_modules/n4js-runtime, test-project2]");
+				"[test-project/node_modules/n4js-runtime, test-project/node_modules/lib, test-project, test-project2/node_modules/n4js-runtime, test-project2/node_modules/lib, test-project2]");
 		assertIssues(Map.of(
 				myModule1,
 				List.of("(Error, [0:1 - 0:12], Couldn't resolve reference to IdentifiableElement '_globalThis'.)"),

tests/org.eclipse.n4js.ide.tests/src/org/eclipse/n4js/ide/tests/buildorder/AbstractBuildOrderTest.java

@@ -28,6 +28,9 @@
 import org.eclipse.xtext.xbase.lib.IteratorExtensions;
 import org.eclipse.xtext.xbase.lib.Pair;
 
+import com.google.common.collect.ImmutableCollection;
+import com.google.common.collect.ImmutableList;
+
 /**
  * Test for build order
  */
@@ -123,9 +126,10 @@ final void assertBuildOrder(String buildOrder, Collection<Collection<String>> cy
 			throw new RuntimeException("Never happens since toString never throws an exception. Bogus xtext warning",
 					exc);
 		}
-		assertEquals(cycles.size(), projectBuildOrderInfo.getProjectCycles().size());
+		ImmutableCollection<ImmutableList<String>> projectCycles = projectBuildOrderInfo.getProjectCycles();
+		assertEquals("Cycle: " + Strings.join(", ", projectCycles), cycles.size(), projectCycles.size());
 		Set<String> expectedCycles = cycles.stream().map(it -> Strings.join(", ", it)).collect(Collectors.toSet());
-		for (List<String> cycle : projectBuildOrderInfo.getProjectCycles()) {
+		for (List<String> cycle : projectCycles) {
 			String detectedCycle = Strings.join(", ", cycle);
 			assertTrue("Cycle " + detectedCycle + " not found in " + expectedCycles,
 					expectedCycles.contains(detectedCycle));

tests/org.eclipse.n4js.ide.tests/src/org/eclipse/n4js/ide/tests/buildorder/BuildOrderDependenciesTest.java

@@ -17,13 +17,13 @@
 /**
  * Test for build order
  */
-
 public class BuildOrderDependenciesTest extends AbstractBuildOrderTest {
 
 	@Test
 	public void testSingleDependency1() {
 		testProject(
 				"test-project/node_modules/n4js-runtime, " +
+						"test-project/node_modules/D2/node_modules/n4js-runtime, " +
 						"test-project/node_modules/D2, " +
 						"test-project/node_modules/D1, " +
 						"test-project",
@@ -45,8 +45,9 @@ public void testSingleDependency1() {
 	@Test
 	public void testSingleDependency2() {
 		testProject(
-				"test-project/node_modules/n4js-runtime, " +
+				"test-project/node_modules/@scope/D2/node_modules/n4js-runtime, " +
 						"test-project/node_modules/@scope/D2, " +
+						"test-project/node_modules/n4js-runtime, " +
 						"test-project/node_modules/D1, " +
 						"test-project",
 				Map.of(

tests/org.eclipse.n4js.ide.tests/src/org/eclipse/n4js/ide/tests/codeActions/OrganizeImportsAddMissingTest.java

@@ -18,7 +18,6 @@
 /**
  * Like {@link OrganizeImportsTest}, but covers cases in which a missing import has to be added during organize imports.
  */
-
 public class OrganizeImportsAddMissingTest extends AbstractOrganizeImportsTest {
 
 	@Test