Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
19 changes: 19 additions & 0 deletions tests/org.eclipse.n4js.ide.tests/PDTs/pnpmDependency1.pdt
Original file line number Diff line number Diff line change
@@ -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
25 changes: 25 additions & 0 deletions tests/org.eclipse.n4js.ide.tests/PDTs/pnpmDependency2.pdt
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions tests/org.eclipse.n4js.ide.tests/PDTs/pnpmSimple1.pdt
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions tests/org.eclipse.n4js.ide.tests/PDTs/pnpmSimple2.pdt
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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'.)"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading