Skip to content

Commit 29c9add

Browse files
Report for missing method parameter names (#25)
1 parent 495823a commit 29c9add

7 files changed

Lines changed: 261 additions & 8 deletions

File tree

codebook-lvt/src/main/java/io/papermc/codebook/lvt/LvtNamer.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
package io.papermc.codebook.lvt;
2424

25+
import com.google.inject.Guice;
26+
import com.google.inject.Injector;
2527
import dev.denwav.hypo.asm.AsmClassData;
2628
import dev.denwav.hypo.asm.AsmMethodData;
2729
import dev.denwav.hypo.core.HypoContext;
@@ -34,7 +36,9 @@
3436
import dev.denwav.hypo.model.data.MethodData;
3537
import dev.denwav.hypo.model.data.types.JvmType;
3638
import dev.denwav.hypo.model.data.types.PrimitiveType;
39+
import io.papermc.codebook.report.ReportType;
3740
import io.papermc.codebook.report.Reports;
41+
import io.papermc.codebook.report.type.MissingMethodParam;
3842
import java.io.IOException;
3943
import java.util.ArrayList;
4044
import java.util.Arrays;
@@ -57,12 +61,16 @@ public class LvtNamer {
5761

5862
private final MappingSet mappings;
5963
private final LvtTypeSuggester lvtTypeSuggester;
64+
private final Reports reports;
65+
private final Injector reportsInjector;
6066
private final RootLvtSuggester lvtAssignSuggester;
6167

6268
public LvtNamer(final HypoContext context, final MappingSet mappings, final Reports reports) throws IOException {
6369
this.mappings = mappings;
6470
this.lvtTypeSuggester = new LvtTypeSuggester(context);
65-
this.lvtAssignSuggester = new RootLvtSuggester(context, this.lvtTypeSuggester, reports);
71+
this.reports = reports;
72+
this.reportsInjector = Guice.createInjector(reports);
73+
this.lvtAssignSuggester = new RootLvtSuggester(context, this.lvtTypeSuggester, this.reportsInjector);
6674
}
6775

6876
public void processClass(final AsmClassData classData) throws IOException {
@@ -96,6 +104,7 @@ private void fillNames0(final MethodData method) throws IOException {
96104
// If it does, we need to keep track of the LVTs we inherit
97105
@Nullable AsmMethodData outerMethod = null;
98106
int @Nullable [] outerMethodParamLvtIndices = null;
107+
@Nullable LambdaClosure lambdaClosure = null;
99108
final @Nullable List<LambdaClosure> lambdaCalls = method.get(HypoHydration.LAMBDA_CALLS);
100109
// Only track synthetic, non-synthetic means a method reference which does not behave as a closure (does not
101110
// capture LVT)
@@ -111,6 +120,7 @@ private void fillNames0(final MethodData method) throws IOException {
111120
continue;
112121
}
113122
outerMethodParamLvtIndices = lambdaCall.getParamLvtIndices();
123+
lambdaClosure = lambdaCall;
114124
// there can only be 1 outer method
115125
break;
116126
}
@@ -178,6 +188,22 @@ private void fillNames0(final MethodData method) throws IOException {
178188
.getClassMapping(parentClass.name())
179189
.flatMap(c -> c.getMethodMapping(method.name(), method.descriptorText()));
180190

191+
final @Nullable ClassData superClass = parentClass.superClass();
192+
193+
if (this.reports.shouldGenerate(ReportType.MISSING_METHOD_PARAM)) {
194+
this.reportsInjector
195+
.getInstance(MissingMethodParam.class)
196+
.handleCheckingMappings(
197+
method,
198+
parentClass,
199+
superClass,
200+
lambdaCalls,
201+
methodMapping.orElse(null),
202+
outerMethodParamLvtIndices,
203+
lambdaClosure,
204+
localClassClosure);
205+
}
206+
181207
// If there's no LVT table there's nothing for us to process
182208
if (node.localVariables == null) {
183209
// interface / abstract methods don't have LVT

codebook-lvt/src/main/java/io/papermc/codebook/lvt/RootLvtSuggester.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import static io.papermc.codebook.lvt.LvtUtil.toJvmType;
2727

2828
import com.google.inject.AbstractModule;
29-
import com.google.inject.Guice;
3029
import com.google.inject.Injector;
3130
import dev.denwav.hypo.core.HypoContext;
3231
import dev.denwav.hypo.model.data.ClassData;
@@ -52,7 +51,6 @@
5251
import io.papermc.codebook.lvt.suggestion.context.method.MethodInsnContext;
5352
import io.papermc.codebook.lvt.suggestion.numbers.MthRandomSuggester;
5453
import io.papermc.codebook.lvt.suggestion.numbers.RandomSourceSuggester;
55-
import io.papermc.codebook.report.Reports;
5654
import io.papermc.codebook.report.type.MissingMethodLvtSuggestion;
5755
import java.io.IOException;
5856
import java.util.List;
@@ -92,10 +90,10 @@ public final class RootLvtSuggester extends AbstractModule implements LvtSuggest
9290
private final List<? extends LvtSuggester> suggesters;
9391

9492
public RootLvtSuggester(
95-
final HypoContext hypoContext, final LvtTypeSuggester lvtTypeSuggester, final Reports reports) {
93+
final HypoContext hypoContext, final LvtTypeSuggester lvtTypeSuggester, final Injector reports) {
9694
this.hypoContext = hypoContext;
9795
this.lvtTypeSuggester = lvtTypeSuggester;
98-
this.injector = Guice.createInjector(this, reports);
96+
this.injector = reports.createChildInjector(this);
9997
this.suggesters = SUGGESTERS.stream().map(this.injector::getInstance).toList();
10098
}
10199

codebook-reports/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ dependencies {
77
implementation(platform(libs.hypo.platform))
88

99
api(libs.checker)
10+
api(libs.lorenz)
1011

1112
implementation(libs.bundles.hypo.impl)
1213
implementation(libs.bundles.asm)

codebook-reports/src/main/java/io/papermc/codebook/report/ReportType.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,5 @@
2424

2525
public enum ReportType {
2626
MISSING_METHOD_LVT_SUGGESTION,
27-
;
27+
MISSING_METHOD_PARAM;
2828
}

codebook-reports/src/main/java/io/papermc/codebook/report/Reports.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import com.google.inject.AbstractModule;
2626
import io.papermc.codebook.report.type.MissingMethodLvtSuggestion;
27+
import io.papermc.codebook.report.type.MissingMethodParam;
2728
import io.papermc.codebook.report.type.Report;
2829
import java.io.IOException;
2930
import java.nio.file.Files;
@@ -55,7 +56,9 @@ protected void configure() {
5556
public Reports(final Path reportsDir, final Set<ReportType> typesToGenerate) {
5657
this.reportsDir = reportsDir;
5758
this.typesToGenerate = typesToGenerate;
58-
this.reports = Map.of(ReportType.MISSING_METHOD_LVT_SUGGESTION, new MissingMethodLvtSuggestion());
59+
this.reports = Map.of(
60+
ReportType.MISSING_METHOD_LVT_SUGGESTION, new MissingMethodLvtSuggestion(),
61+
ReportType.MISSING_METHOD_PARAM, new MissingMethodParam());
5962
}
6063

6164
public void generateReports() throws IOException {
@@ -69,6 +72,10 @@ public void generateReports() throws IOException {
6972
}
7073
}
7174

75+
public boolean shouldGenerate(final ReportType reportType) {
76+
return this.typesToGenerate.contains(reportType);
77+
}
78+
7279
@Override
7380
protected void configure() {
7481
this.reports.values().forEach(this::bindReport);
Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
/*
2+
* codebook is a remapper utility for the PaperMC project.
3+
*
4+
* Copyright (c) 2023 Kyle Wood (DenWav)
5+
* Contributors
6+
*
7+
* This library is free software; you can redistribute it and/or
8+
* modify it under the terms of the GNU Lesser General Public
9+
* License as published by the Free Software Foundation;
10+
* version 3 only, no later versions.
11+
*
12+
* This library is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
15+
* Lesser General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU Lesser General Public
18+
* License along with this library; if not, write to the Free Software
19+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
20+
* USA
21+
*/
22+
23+
package io.papermc.codebook.report.type;
24+
25+
import dev.denwav.hypo.hydrate.generic.HypoHydration;
26+
import dev.denwav.hypo.hydrate.generic.LambdaClosure;
27+
import dev.denwav.hypo.hydrate.generic.LocalClassClosure;
28+
import dev.denwav.hypo.model.data.ClassData;
29+
import dev.denwav.hypo.model.data.ClassKind;
30+
import dev.denwav.hypo.model.data.MethodData;
31+
import java.io.IOException;
32+
import java.util.ArrayList;
33+
import java.util.Comparator;
34+
import java.util.List;
35+
import java.util.Map;
36+
import java.util.Optional;
37+
import java.util.concurrent.ConcurrentHashMap;
38+
import java.util.function.IntUnaryOperator;
39+
import java.util.regex.Pattern;
40+
import org.cadixdev.lorenz.model.Mapping;
41+
import org.cadixdev.lorenz.model.MethodMapping;
42+
import org.checkerframework.checker.nullness.qual.Nullable;
43+
44+
public class MissingMethodParam implements Report {
45+
46+
private final Map<ClassData, List<String>> data = new ConcurrentHashMap<>();
47+
48+
private void checkMappings(
49+
final MethodData method,
50+
final @Nullable MethodMapping methodMapping,
51+
final int descriptorParamOffset,
52+
final IntUnaryOperator descriptorToMappingOffset) {
53+
this.checkMappings(method, methodMapping, descriptorParamOffset, descriptorToMappingOffset, null);
54+
}
55+
56+
private void checkMappings(
57+
final MethodData method,
58+
final @Nullable MethodMapping methodMapping,
59+
final int descriptorParamOffset,
60+
final IntUnaryOperator descriptorToMappingOffset,
61+
final @Nullable LambdaClosure lambdaClosure) {
62+
if (method.params().size() == descriptorParamOffset) {
63+
return;
64+
}
65+
if (methodMapping == null
66+
|| (method.params().size() - descriptorParamOffset
67+
> methodMapping.getParameterMappings().size())) {
68+
// != should have been sufficient here, but hypo's CopyMappingsDown for constructors incorrectly applies
69+
// mappings to implicit constructor params
70+
this.reportMissingParam(
71+
method, methodMapping, descriptorParamOffset, descriptorToMappingOffset, lambdaClosure);
72+
}
73+
}
74+
75+
private static final Pattern ANONYMOUS_CLASS = Pattern.compile(".+\\$\\d+$");
76+
77+
private static boolean shouldSkipMapping(
78+
final MethodData method,
79+
final ClassData parentClass,
80+
final @Nullable ClassData superClass,
81+
final @Nullable List<LambdaClosure> lambdaCalls) {
82+
final String name = method.name();
83+
if (name.startsWith("access$") && method.isSynthetic()) {
84+
// never in source
85+
return true;
86+
} else if (name.startsWith("lambda$")
87+
&& method.isSynthetic()
88+
&& (lambdaCalls == null || lambdaCalls.isEmpty())) {
89+
// lambdas that had their use stripped by mojang
90+
return true;
91+
} else {
92+
final String descriptorText = method.descriptorText();
93+
if (superClass != null
94+
&& superClass.name().equals("java/lang/Enum")
95+
&& name.equals("valueOf")
96+
&& descriptorText.startsWith("(Ljava/lang/String;)")) {
97+
// created by the compiler
98+
return true;
99+
} else if (parentClass.is(ClassKind.RECORD)
100+
&& name.equals("equals")
101+
&& descriptorText.equals("(Ljava/lang/Object;)Z")) {
102+
// created by the compiler
103+
return true;
104+
} else if (method.isSynthetic() && method.get(HypoHydration.SYNTHETIC_TARGET) != null) {
105+
// don't trust isBridge, apparently it's not always accurate
106+
return true;
107+
} else {
108+
return false;
109+
}
110+
}
111+
}
112+
113+
private void handleConstructorMappings(
114+
final MethodData method,
115+
final ClassData parentClass,
116+
final @Nullable MethodMapping methodMapping,
117+
final @Nullable LocalClassClosure localClassClosure)
118+
throws IOException {
119+
if (parentClass.is(ClassKind.ENUM)) {
120+
// enum constructors include name and ordinal
121+
this.checkMappings(method, methodMapping, 2, i -> i + 1);
122+
} else {
123+
if (!ANONYMOUS_CLASS.matcher(parentClass.name()).matches()) {
124+
// anonymous classes cannot have constructors in source
125+
if (parentClass.outerClass() != null) {
126+
final int descriptorParamOffset = parentClass.isStaticInnerClass() ? 0 : 1;
127+
if (localClassClosure == null) {
128+
this.checkMappings(method, methodMapping, descriptorParamOffset, i -> i + 1);
129+
} else {
130+
this.checkMappings(
131+
method,
132+
methodMapping,
133+
descriptorParamOffset + localClassClosure.getParamLvtIndices().length,
134+
i -> i + 1);
135+
}
136+
} else {
137+
this.checkMappings(method, methodMapping, 0, i -> i + 1);
138+
}
139+
}
140+
}
141+
}
142+
143+
public void handleCheckingMappings(
144+
final MethodData method,
145+
final ClassData parentClass,
146+
final @Nullable ClassData superClass,
147+
final @Nullable List<LambdaClosure> lambdaCalls,
148+
final @Nullable MethodMapping methodMapping,
149+
final int @Nullable [] outerMethodParamLvtIndices,
150+
final @Nullable LambdaClosure lambdaClosure,
151+
final @Nullable LocalClassClosure localClassClosure)
152+
throws IOException {
153+
if (shouldSkipMapping(method, parentClass, superClass, lambdaCalls)) {
154+
return;
155+
}
156+
if (method.isConstructor()) {
157+
this.handleConstructorMappings(method, parentClass, methodMapping, localClassClosure);
158+
} else {
159+
if (outerMethodParamLvtIndices == null) {
160+
this.checkMappings(method, methodMapping, 0, i -> i + (method.isStatic() ? 0 : 1));
161+
} else {
162+
final int descriptorOffset;
163+
if (!method.isStatic() && outerMethodParamLvtIndices.length > 0 && outerMethodParamLvtIndices[0] == 0) {
164+
descriptorOffset = outerMethodParamLvtIndices.length - 1;
165+
} else {
166+
descriptorOffset = outerMethodParamLvtIndices.length;
167+
}
168+
this.checkMappings(
169+
method, methodMapping, descriptorOffset, i -> i + (method.isStatic() ? 0 : 1), lambdaClosure);
170+
}
171+
}
172+
}
173+
174+
private void reportMissingParam(
175+
final MethodData method,
176+
final @Nullable MethodMapping methodMapping,
177+
final int descriptorParamOffset,
178+
final IntUnaryOperator descriptorToMappingOffset,
179+
final @Nullable LambdaClosure lambdaClosure) {
180+
final ClassData parentClass = method.parentClass();
181+
final StringBuilder msg = new StringBuilder("\t#%s %s".formatted(method.name(), method.descriptorText()));
182+
if (lambdaClosure != null) {
183+
final MethodData containingMethod = lambdaClosure.getContainingMethod();
184+
msg.append("%n\t\tLambda Source: %s#%s %s"
185+
.formatted(
186+
containingMethod.parentClass().equals(parentClass)
187+
? ""
188+
: containingMethod.parentClass().name(),
189+
containingMethod.name(),
190+
containingMethod.descriptorText()));
191+
}
192+
for (int i = descriptorParamOffset; i < method.params().size(); i++) {
193+
final int paramIdx = i;
194+
final int lastIdxOfDot = method.param(i).toString().lastIndexOf('.');
195+
msg.append("%n\t\t%s\t%-50s\t%s"
196+
.formatted(
197+
i,
198+
method.param(i).toString().substring(lastIdxOfDot + 1),
199+
Optional.ofNullable(methodMapping)
200+
.flatMap(m -> m.getParameterMapping(descriptorToMappingOffset.applyAsInt(paramIdx)))
201+
.map(Mapping::getDeobfuscatedName)
202+
.orElse("<<MISSING>>")));
203+
}
204+
this.data.computeIfAbsent(parentClass, ignored -> new ArrayList<>()).add(msg.toString());
205+
}
206+
207+
@Override
208+
public String generate() {
209+
final StringBuilder output = new StringBuilder();
210+
this.data.entrySet().stream()
211+
.sorted(Comparator.comparing(entry -> entry.getKey().name()))
212+
.forEach(entry -> {
213+
output.append("Missing param mappings in %s, Method Count: %s, Param Count: TODO\n"
214+
.formatted(entry.getKey().name(), entry.getValue().size()));
215+
entry.getValue().forEach(msg -> output.append(msg).append("\n"));
216+
});
217+
return output.toString();
218+
}
219+
}

src/test/java/io/papermc/codebook/lvt/LvtAssignmentSuggesterTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static org.mockito.Mockito.when;
3030
import static org.mockito.Mockito.withSettings;
3131

32+
import com.google.inject.Guice;
3233
import dev.denwav.hypo.asm.AsmClassData;
3334
import dev.denwav.hypo.asm.AsmMethodData;
3435
import dev.denwav.hypo.core.HypoContext;
@@ -100,7 +101,8 @@ void setup() throws Exception {
100101

101102
when(this.randomSourceClass.name()).thenReturn(RANDOM_SOURCE_TYPE.asInternalName());
102103

103-
this.suggester = new RootLvtSuggester(context, new LvtTypeSuggester(context), this.reports);
104+
this.suggester =
105+
new RootLvtSuggester(context, new LvtTypeSuggester(context), Guice.createInjector(this.reports));
104106
}
105107

106108
@ParameterizedTest

0 commit comments

Comments
 (0)