Skip to content

Commit ab4fce0

Browse files
shartteshartte
authored
Add a flag to disable parameter renames for parchment (needed for unobf) (#60)
1 parent 9da7e4c commit ab4fce0

File tree

7 files changed

+141
-35
lines changed

7 files changed

+141
-35
lines changed

‎README.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ Usage: jst [-hV] [--debug] [--in-format=<inputFormat>] [--libraries-list=<librar
6060
[--max-queue-depth=<maxQueueDepth>] [--out-format=<outputFormat>]
6161
[--problems-report=<problemsReport>] [--classpath=<addToClasspath>]...
6262
[--ignore-prefix=<ignoredPrefixes>]... [--enable-parchment
63-
--parchment-mappings=<mappingsPath> [--[no-]parchment-javadoc]
64-
[--parchment-conflict-prefix=<conflictPrefix>]] [--enable-accesstransformers
65-
--access-transformer=<atFiles> [--access-transformer=<atFiles>]...
66-
[--access-transformer-validation=<validation>]] [--enable-interface-injection
67-
[--interface-injection-stubs=<stubOut>]
63+
--parchment-mappings=<mappingsPath> [--[no-]parchment-javadoc] [--[no-]
64+
parchment-parameters] [--parchment-conflict-prefix=<conflictPrefix>]]
65+
[--enable-accesstransformers --access-transformer=<atFiles>
66+
[--access-transformer=<atFiles>]... [--access-transformer-validation=<validation>]]
67+
[--enable-interface-injection [--interface-injection-stubs=<stubOut>]
6868
[--interface-injection-marker=<annotationMarker>]
6969
[--interface-injection-data=<paths>]...] [--enable-unpick [--unpick-data=<paths>]...]
7070
INPUT OUTPUT
@@ -104,6 +104,8 @@ Plugin - parchment
104104
Whether Parchment javadocs should be applied
105105
--parchment-mappings=<mappingsPath>
106106
The location of the Parchment mappings file
107+
--[no-]parchment-parameters
108+
Whether Parchment parameters should be applied
107109
Plugin - accesstransformers
108110
--access-transformer=<atFiles>
109111

‎parchment/src/main/java/net/neoforged/jst/parchment/GatherReplacementsVisitor.java‎

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
import java.util.IdentityHashMap;
2424
import java.util.List;
2525
import java.util.Map;
26-
import java.util.Objects;
2726
import java.util.Set;
2827
import java.util.function.UnaryOperator;
2928

3029
class GatherReplacementsVisitor extends PsiRecursiveElementVisitor {
3130
private final NamesAndDocsDatabase namesAndDocs;
3231
private final boolean enableJavadoc;
32+
private final boolean enableParameters;
3333
@Nullable
3434
private final UnaryOperator<String> conflictResolver;
3535
private final Replacements replacements;
@@ -47,10 +47,12 @@ class GatherReplacementsVisitor extends PsiRecursiveElementVisitor {
4747

4848
public GatherReplacementsVisitor(NamesAndDocsDatabase namesAndDocs,
4949
boolean enableJavadoc,
50+
boolean enableParameters,
5051
@Nullable UnaryOperator<String> conflictResolver,
5152
Replacements replacements) {
5253
this.namesAndDocs = namesAndDocs;
5354
this.enableJavadoc = enableJavadoc;
55+
this.enableParameters = enableParameters;
5456
this.conflictResolver = conflictResolver;
5557
this.replacements = replacements;
5658
}
@@ -138,44 +140,49 @@ else if (PsiHelper.isNonStaticInnerClass(psiMethod.getContainingClass())) {
138140
// implications for the field names.
139141
if (paramData != null && paramData.getName() != null && !PsiHelper.isRecordConstructor(psiMethod)) {
140142
var paramName = namer.apply(paramData.getName());
141-
142-
// We cannot rename a parameter to name that was already taken in this scope
143-
if (activeNames.contains(paramName)) {
144-
// If we have no conflict resolver then we simply don't try to rename this parameter
145-
if (conflictResolver == null) {
146-
parameterOrder.add(psiParameter.getName());
147-
continue;
148-
}
149-
150-
// Keep applying the conflict resolver until the name is no longer used
151-
while (activeNames.contains(paramName)) {
152-
paramName = conflictResolver.apply(paramName);
143+
if (enableParameters) {
144+
// We cannot rename a parameter to name that was already taken in this scope
145+
if (activeNames.contains(paramName)) {
146+
// If we have no conflict resolver then we simply don't try to rename this parameter
147+
if (conflictResolver == null) {
148+
parameterOrder.add(psiParameter.getName());
149+
continue;
150+
}
151+
152+
// Keep applying the conflict resolver until the name is no longer used
153+
while (activeNames.contains(paramName)) {
154+
paramName = conflictResolver.apply(paramName);
155+
}
153156
}
154-
}
155157

156-
// Replace parameters within the method body
157-
activeParameters.put(psiParameter, paramName);
158-
activeNames.add(paramName);
158+
// Replace parameters within the method body
159+
activeParameters.put(psiParameter, paramName);
160+
activeNames.add(paramName);
159161

160-
// Find and replace the parameter identifier
161-
replacements.replace(psiParameter.getNameIdentifier(), paramName);
162+
// Find and replace the parameter identifier
163+
replacements.replace(psiParameter.getNameIdentifier(), paramName);
162164

163-
// Record the replacement for remapping existing Javadoc @param tags
164-
renamedParameters.put(psiParameter.getName(), paramName);
165+
// Record the replacement for remapping existing Javadoc @param tags
166+
renamedParameters.put(psiParameter.getName(), paramName);
165167

166-
hadReplacements = true;
168+
hadReplacements = true;
167169

168-
parameterOrder.add(paramName);
170+
parameterOrder.add(paramName);
171+
} else {
172+
parameterOrder.add(psiParameter.getName());
173+
}
169174
} else {
170175
parameterOrder.add(psiParameter.getName());
171176
}
172177

173178
// Optionally provide parameter javadocs
174179
if (paramData != null && paramData.getJavadoc() != null) {
175-
parameterJavadoc.put(
176-
Objects.requireNonNullElse(paramData.getName(), psiParameter.getName()),
177-
paramData.getJavadoc()
178-
);
180+
// If parameter renaming is disabled, always use the source name
181+
if (enableParameters && paramData.getName() != null) {
182+
parameterJavadoc.put(paramData.getName(), paramData.getJavadoc());
183+
} else {
184+
parameterJavadoc.put(psiParameter.getName(), paramData.getJavadoc());
185+
}
179186
}
180187
}
181188

@@ -207,7 +214,7 @@ else if (PsiHelper.isNonStaticInnerClass(psiMethod.getContainingClass())) {
207214
return;
208215
}
209216
}
210-
} else if (element instanceof PsiReferenceExpression refExpr && refExpr.getReferenceNameElement() != null) {
217+
} else if (enableParameters && element instanceof PsiReferenceExpression refExpr && refExpr.getReferenceNameElement() != null) {
211218
for (var entry : activeParameters.entrySet()) {
212219
if (refExpr.isReferenceTo(entry.getKey())) {
213220
replacements.replace(refExpr.getReferenceNameElement(), entry.getValue());

‎parchment/src/main/java/net/neoforged/jst/parchment/ParchmentTransformer.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ public class ParchmentTransformer implements SourceTransformer {
2626
)
2727
public boolean enableJavadoc = true;
2828

29+
@CommandLine.Option(
30+
names = "--parchment-parameters",
31+
description = "Whether Parchment parameter names should be applied",
32+
negatable = true,
33+
fallbackValue = "true"
34+
)
35+
public boolean enableParameters = true;
36+
2937
@CommandLine.Option(names = "--parchment-conflict-prefix", description = "Apply the prefix specified if a Parchment parameter name conflicts with existing variable names")
3038
public String conflictPrefix;
3139

@@ -56,7 +64,7 @@ public void beforeRun(TransformContext context) {
5664

5765
@Override
5866
public void visitFile(PsiFile psiFile, Replacements replacements) {
59-
var visitor = new GatherReplacementsVisitor(namesAndDocs, enableJavadoc, conflictResolver, replacements);
67+
var visitor = new GatherReplacementsVisitor(namesAndDocs, enableJavadoc, enableParameters, conflictResolver, replacements);
6068
visitor.visitElement(psiFile);
6169
}
6270

‎tests/data/parchment/javadoc-only/expected/ExistingJavadoc.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Existing class javadoc.
3+
* <p>
4+
* Parchment Class Line 1
5+
* Parchment Class Line 2
6+
* Parchment Class Line 3
7+
*/
8+
public class ExistingJavadoc {
9+
10+
/**
11+
* Existing Javadoc, isn't it great?
12+
* <p>
13+
* Parchment Method Line 1
14+
* Parchment Method Line 2
15+
* Parchment Method Line 3
16+
*
17+
* @param x Parchment Parameter Documentation. Long oh so very long Lorem ipsum
18+
* dolor sit amet Lorem ipsum dolor sit amet Lorem ipsum dolor sit amet
19+
* @param y Some description for x very long very long very long very long very
20+
* long very long very long very
21+
* @param z Javadoc only, no rename
22+
* @author SomeAuthor
23+
* @return Some return value
24+
*/
25+
int m(int x, int y, int z) {
26+
}
27+
28+
}

‎tests/data/parchment/javadoc-only/parchment.json‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
{
2+
"version": "1.1.0",
3+
"classes": [
4+
{
5+
"name": "ExistingJavadoc",
6+
"javadoc": [
7+
"Parchment Class Line 1",
8+
"Parchment Class Line 2",
9+
"Parchment Class Line 3"
10+
],
11+
"methods": [
12+
{
13+
"name": "m",
14+
"descriptor": "(III)I",
15+
"javadoc": [
16+
"Parchment Method Line 1",
17+
"Parchment Method Line 2",
18+
"Parchment Method Line 3"
19+
],
20+
"parameters": [
21+
{
22+
"index": 1,
23+
"name": "newX",
24+
"javadoc": "Parchment Parameter Documentation. Long oh so very long Lorem ipsum dolor sit amet Lorem ipsum dolor sit amet Lorem ipsum dolor sit amet"
25+
},
26+
{
27+
"index": 2,
28+
"name": "newY"
29+
},
30+
{
31+
"index": 3,
32+
"javadoc": "Javadoc only, no rename"
33+
}
34+
]
35+
}
36+
]
37+
}
38+
]
39+
}

‎tests/data/parchment/javadoc-only/source/ExistingJavadoc.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* Existing class javadoc.
3+
*/
4+
public class ExistingJavadoc {
5+
6+
/**
7+
* Existing Javadoc, isn't it great?
8+
*
9+
* @param y Some description for x very long very long very long very long
10+
* very long very long very long very
11+
* @param x This will be overwritten
12+
* @return Some return value
13+
* @author SomeAuthor
14+
*/
15+
int m(int x, int y, int z) {
16+
}
17+
18+
}

‎tests/src/test/java/net/neoforged/jst/tests/EmbeddedTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package net.neoforged.jst.tests;
22

3-
import com.google.gson.Gson;
43
import com.google.gson.GsonBuilder;
54
import com.google.gson.TypeAdapter;
65
import com.google.gson.stream.JsonReader;
@@ -236,6 +235,11 @@ void testAnonymousClasses() throws Exception {
236235
void testConflicts() throws Exception {
237236
runParchmentTest("conflicts", "mappings.tsrg", "--parchment-conflict-prefix=p_");
238237
}
238+
239+
@Test
240+
void testJavadocOnly() throws Exception {
241+
runParchmentTest("javadoc-only", "parchment.json", "--no-parchment-parameters");
242+
}
239243
}
240244

241245
@Nested

0 commit comments

Comments
 (0)