Skip to content

Escape each element of the IntelliJ module name - #354

Open
adityaanikam wants to merge 1 commit into
neoforged:mainfrom
adityaanikam:fix-intellij-module-name-escaping-353
Open

adityaanikam wants to merge 1 commit into
neoforged:mainfrom
adityaanikam:fix-intellij-module-name-escaping-353

Conversation

@adityaanikam

Copy link
Copy Markdown

Fixes #353

Problem

getIntellijModuleName builds the module name by joining the Gradle path as-is:

moduleName.append(project.getRootProject().getName().replace(" ", "_"));
if (project != project.getRootProject()) {
    moduleName.append(project.getPath().replaceAll(":", "."));
}
moduleName.append(".").append(sourceSet.getName());

IntelliJ escapes each element of the path individually and then joins the escaped elements with ., so an element that itself contains a . is not preserved. For a subproject :1.21.1-neoforge under root xaeronav, IntelliJ's module is xaeronav.1_21_1-neoforge.main while the above produces xaeronav.1.21.1-neoforge.main.

That name matches no module, so setModuleName(..) leaves the generated run configuration without one. IntelliJ then builds -classpath from the whole project, which in a multi-loader setup pulls in the other loader's Minecraft and dev launch aborts:

Found multiple copies of net/minecraft/server/MinecraftServer.class on the classpath

About the comment above that code

The comment currently points at IntelliJ's GradleProjectResolverUtil#getInternalModuleName / PathUtilRt#suggestFileName at a pinned revision. That code does not do this escaping — it splits on :, joins with ., and finishes with suggestFileName(name, /* allowDots = */ true, false), which keeps dots. Read on its own it suggests the current behaviour is correct, which is presumably how this survived.

The escaping happens on the newer sync path, which is what recent IDEA uses:

return Arrays.stream(moduleName.split(":"))
  .map(it -> escapeModuleNameElement(it))
  .collect(Collectors.joining("."));

private static String escapeModuleNameElement(String moduleNameElement) {
  return moduleNameElement
    .replace(" ", "_").replace("/", "_").replace("\\", "_").replace(".", "_");
}

and source set modules are holderModuleName + "." + escapeModuleNameElement(sourceSetName).

So I have repointed the comment at those two methods with a pinned permalink, since the stale link is part of why this looked right.

Fix

Mirror escapeModuleNameElement and apply it per element — to the root project name, each path element, and the source set name:

moduleName.append(escapeModuleNameElement(project.getRootProject().getName()));
if (project != project.getRootProject()) {
    for (var element : project.getPath().split(":")) {
        if (!element.isEmpty()) {
            moduleName.append(".").append(escapeModuleNameElement(element));
        }
    }
}
moduleName.append(".").append(escapeModuleNameElement(sourceSet.getName()));

This is slightly wider than just handling dots: IntelliJ escapes / and \ too, and applies the same escaping to the root name and the source set name, where the current code only replaced spaces in the root name and did not touch the source set name at all. getIntellijModuleName is now package-private so it can be tested.

Testing

IntelliJIntegrationTest — 7 cases: four for escapeModuleNameElement (plain, space, dots, slash) and three building names through ProjectBuilder (root project, a subproject with dots, and a nested subproject).

Verified with a negative control. Restoring the old assembly (keeping the new helper so the test still compiles) fails exactly the two name tests, with the broken names from the issue:

expected: <xaeronav.1_21_1-neoforge.main> but was: <xaeronav.1.21.1-neoforge.main>
expected: <root.versions.1_21_1.main>     but was: <root.versions.1.21.1.main>

Restoring the fix returns all 7 to passing.

IntelliJ escapes every element of the Gradle path on its own and joins the
escaped elements with '.', so an element containing a '.' does not survive.
Joining the raw path meant a subproject named "1.21.1-neoforge" produced
"root.1.21.1-neoforge.main" where IntelliJ has "root.1_21_1-neoforge.main".
That module does not exist, so the generated run configuration was written
without one and IntelliJ built the classpath from the whole project.

Mirror IntelliJ's escapeModuleNameElement and apply it to the root project
name, each path element and the source set name. The comment above this
code pointed at an older code path that keeps dots, which is repointed at
the current one.
@neoforged-pr-publishing

Copy link
Copy Markdown
  • Publish PR to GitHub Packages

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generated IntelliJ run configuration has no resolvable module when a subproject name contains '.' (e.g. Stonecutter nodes)

1 participant