Skip to content

Commit e6f6335

Browse files
committed
prevent import shadow over inner class reference for shortener
1 parent 0dec337 commit e6f6335

File tree

12 files changed

+89
-93
lines changed

12 files changed

+89
-93
lines changed

src/main/java/io/papermc/typewriter/ClassNamed.java

+3-6
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,11 @@ public boolean isRoot() {
9494
return this.root() == this;
9595
}
9696

97-
public ClassNamed enclosing() {
97+
public @Nullable ClassNamed enclosing() {
9898
if (this.knownClass != null) {
9999
Class<?> parentClass = this.knownClass.getEnclosingClass();
100100
if (parentClass == null) {
101-
return this;
101+
return null;
102102
}
103103
return new ClassNamed(parentClass);
104104
}
@@ -116,7 +116,7 @@ public ClassNamed enclosing() {
116116
}
117117
return new ClassNamed(this.packageName, simpleName, name, null);
118118
}
119-
return this;
119+
return null;
120120
}
121121

122122
public String canonicalName() {
@@ -133,9 +133,6 @@ public String canonicalName() {
133133

134134
@Override
135135
public int hashCode() {
136-
if (this.knownClass != null) {
137-
return this.knownClass.hashCode();
138-
}
139136
return Objects.hash(this.packageName, this.dottedNestedName);
140137
}
141138

src/main/java/io/papermc/typewriter/context/ImportTypeCollector.java

+35-25
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,26 @@ public String getStaticMemberShortName(String fullName) {
7676

7777
private Optional<String> getShortName0(ClassNamed type, Set<String> imports, Set<String> globalImports, boolean unusualStaticImport) {
7878
ClassNamed foundClass = type;
79-
while (!imports.contains(foundClass.canonicalName()) &&
80-
!globalImports.contains(foundClass.enclosing().canonicalName())) {
81-
if (foundClass.isRoot() ||
82-
(unusualStaticImport && !Modifier.isStatic(foundClass.knownClass().getModifiers())) // static imports are allowed for regular class too but only when the inner classes are all static
83-
) {
84-
foundClass = null;
79+
ClassNamed upperClass = type.enclosing();
80+
while (true) {
81+
if (foundClass == null) {
8582
break;
8683
}
87-
foundClass = foundClass.enclosing();
84+
85+
if (unusualStaticImport && !Modifier.isStatic(foundClass.knownClass().getModifiers())) {
86+
// static imports are allowed for regular class too but only when the inner classes are all static
87+
return Optional.empty();
88+
}
89+
90+
if (imports.contains(foundClass.canonicalName()) ||
91+
(upperClass != null && globalImports.contains(upperClass.canonicalName()))) {
92+
break;
93+
}
94+
95+
foundClass = upperClass;
96+
if (upperClass != null) {
97+
upperClass = upperClass.enclosing();
98+
}
8899
}
89100

90101
if (foundClass != null) {
@@ -107,18 +118,21 @@ public String getShortName(ClassNamed type) {
107118
shortName = getShortName0(key, this.staticImports.keySet(), this.globalStaticImports, true);
108119
}
109120

110-
return shortName.orElseGet(() -> {
111-
// import have priority over those implicit things
112-
if (key.packageName().equals(JAVA_LANG_PACKAGE)) { // auto-import
113-
return key.dottedNestedName();
114-
}
115-
116-
// self classes (with inner classes)
117-
if (this.mainClass.equals(key.root())) {
118-
return getInnerShortName(this.accessSource, key);
121+
// self classes (with inner classes)
122+
if (this.mainClass.equals(key.root())) {
123+
int importedSize = shortName.map(String::length).orElse(0);
124+
String innerName = getInnerShortName(this.accessSource, key);
125+
if (importedSize == 0 || innerName.length() < importedSize) {
126+
// inner name might be shorter than self import
127+
return innerName;
119128
}
129+
}
120130

121-
if (key.packageName().equals(this.mainClass.packageName())) { // same package don't need fqn too
131+
return shortName.orElseGet(() -> {
132+
// import have priority over those implicit things
133+
if (key.packageName().equals(JAVA_LANG_PACKAGE) || // auto-import
134+
key.packageName().equals(this.mainClass.packageName()) // same package don't need fqn too
135+
) {
122136
return key.dottedNestedName();
123137
}
124138
return key.canonicalName();
@@ -135,27 +149,23 @@ private String getInnerShortName(ClassNamed fromType, ClassNamed targetType) {
135149
ClassNamed intersectType = null;
136150
ClassNamed type = targetType;
137151

138-
while (!type.isRoot()) {
152+
while (type != null) {
139153
if (type.dottedNestedName().equals(fromType.dottedNestedName())) {
140154
intersectType = type;
141155
break;
142156
}
143-
type = type.enclosing();
144157
visitedTypes.add(type);
145-
}
146-
147-
if (intersectType == null && visitedTypes.contains(fromType)) {
148-
intersectType = fromType;
158+
type = type.enclosing();
149159
}
150160

151161
if (intersectType == null) {
152162
type = fromType;
153-
while (!type.isRoot()) {
154-
type = type.enclosing();
163+
while (type != null) {
155164
if (visitedTypes.contains(type)) {
156165
intersectType = type;
157166
break;
158167
}
168+
type = type.enclosing();
159169
}
160170
}
161171

src/main/java/io/papermc/typewriter/parser/LineParser.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ public boolean consumeImports(StringReader line, ImportCollector collector) {
196196

197197
// not commented
198198
char c = line.peek();
199-
if (AnnotationSkipSteps.canStart(c)) { // handle annotation with param to avoid open curly bracket that occur in array argument
199+
if (AnnotationSkipSteps.canStart(line)) { // handle annotation with param to avoid open curly bracket that occur in array argument
200200
this.stepManager.enqueue(new AnnotationSkipSteps());
201201
continue;
202202
} else if (c == '{') {
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
package io.papermc.typewriter.parser.closure;
22

3-
import java.util.Comparator;
43
import java.util.EnumSet;
5-
import java.util.List;
64
import java.util.Set;
75

86
public enum ClosureType {
97
PARENTHESIS("(", ")"),
108
CURLY_BRACKET("{", "}"),
119
BRACKET("[", "]"),
1210
COMMENT("/*", "*/"),
13-
PARAGRAPH("\"\"\"", "\"\"\""),
11+
PARAGRAPH("\"\"\"", "\"\"\""), // order matter here (paragraph > string)
1412
STRING("\"", "\""),
1513
CHAR("'", "'");
1614

15+
private static final Set<ClosureType> ALLOW_ESCAPE = EnumSet.of(STRING, CHAR, PARAGRAPH);
16+
public static final Set<ClosureType> LEAFS = EnumSet.of(COMMENT, STRING, CHAR, PARAGRAPH);
17+
1718
public final String start;
1819
public final String end;
1920

@@ -25,18 +26,4 @@ public enum ClosureType {
2526
public boolean escapableByPreviousChar() {
2627
return ALLOW_ESCAPE.contains(this) && this.end.length() == 1 && this.start.equals(this.end);
2728
}
28-
29-
private static final Set<ClosureType> ALLOW_ESCAPE = EnumSet.of(STRING, CHAR, PARAGRAPH);
30-
public static final Set<ClosureType> LEAFS = EnumSet.of(COMMENT, STRING, CHAR, PARAGRAPH);
31-
32-
private static final List<ClosureType> CHECK_FIRST = List.of(PARAGRAPH);
33-
34-
public static List<ClosureType> prioritySort(Set<ClosureType> types) {
35-
return types.stream()
36-
.sorted(Comparator.comparingInt(o -> {
37-
int index = CHECK_FIRST.indexOf(o);
38-
return index == -1 ? CHECK_FIRST.size() : index;
39-
}))
40-
.toList();
41-
}
4229
}

src/main/java/io/papermc/typewriter/parser/step/StepManager.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void enqueue(StepHolder holder) {
2424
}
2525
}
2626

27-
public void addPriority(IterativeStep step) {
27+
public void executeNext(IterativeStep step) {
2828
if (!this.steps.offerFirst(step)) {
2929
throw new IllegalStateException("Cannot add a priority step into the queue!");
3030
}

src/main/java/io/papermc/typewriter/parser/step/sequence/AnnotationSkipSteps.java

+12-13
Original file line numberDiff line numberDiff line change
@@ -12,27 +12,27 @@
1212
import io.papermc.typewriter.utils.Formatting;
1313
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
1414

15-
import java.util.HashSet;
15+
import java.util.ArrayList;
16+
import java.util.Collections;
1617
import java.util.List;
17-
import java.util.Set;
1818

1919
// start once "@" is detected unless commented
20-
// order is: skipAtSign -> skipPartName -> checkAnnotationName -> checkOpenParenthesis (-> skipParentheses)
20+
// order is: skipPartName -> checkAnnotationName -> checkOpenParenthesis (-> skipParentheses)
2121
public final class AnnotationSkipSteps implements StepHolder {
2222

23-
public static boolean canStart(char currentChar) {
24-
return currentChar == '@';
23+
public static boolean canStart(StringReader line) {
24+
if (line.peek() == '@') {
25+
line.skip();
26+
return true;
27+
}
28+
return false;
2529
}
2630

2731
private final IterativeStep skipParenthesesStep = this.repeatStep(this::skipParentheses);
2832

2933
private @MonotonicNonNull ProtoTypeName name;
3034
private Closure parenthesisClosure;
3135

32-
public void skipAtSign(StringReader line, LineParser parser) {
33-
line.skip(); // skip @
34-
}
35-
3636
public boolean skipPartName(StringReader line, LineParser parser) {
3737
boolean checkStartId = this.name == null || this.name.shouldCheckStartIdentifier();
3838

@@ -58,12 +58,12 @@ public boolean skipPartName(StringReader line, LineParser parser) {
5858

5959
private static final List<ClosureType> IGNORE_NESTED_CLOSURES;
6060
static {
61-
Set<ClosureType> types = new HashSet<>(ClosureType.LEAFS.size());
61+
List<ClosureType> types = new ArrayList<>(ClosureType.LEAFS.size());
6262
types.addAll(ClosureType.LEAFS);
6363
types.remove(ClosureType.COMMENT); // comment will be skipped separately to simplify the iteration
6464
types.add(ClosureType.PARENTHESIS); // skip nested annotation too
6565

66-
IGNORE_NESTED_CLOSURES = ClosureType.prioritySort(types);
66+
IGNORE_NESTED_CLOSURES = Collections.unmodifiableList(types);
6767
}
6868

6969
public boolean skipParentheses(StringReader line, LineParser parser) {
@@ -121,15 +121,14 @@ public boolean checkOpenParenthesis(StringReader line, LineParser parser) {
121121

122122
if (parser.tryAdvanceStartClosure(ClosureType.PARENTHESIS, line)) { // open parenthesis?
123123
this.parenthesisClosure = parser.getNearestClosure();
124-
parser.getSteps().addPriority(this.skipParenthesesStep);
124+
parser.getSteps().executeNext(this.skipParenthesesStep);
125125
}
126126
return false;
127127
}
128128

129129
@Override
130130
public IterativeStep[] initialSteps() {
131131
return new IterativeStep[] {
132-
this.onceStep(this::skipAtSign),
133132
this.repeatStep(this::skipPartName),
134133
this.onceStep(this::checkAnnotationName),
135134
this.repeatStep(this::checkOpenParenthesis),

src/main/java/io/papermc/typewriter/parser/step/sequence/ImportStatementSteps.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public boolean checkStatic(StringReader line, LineParser parser) {
5959
}
6060

6161
if (line.trySkipString("static")) {
62-
parser.getSteps().addPriority(this.enforceSpaceStep);
62+
parser.getSteps().executeNext(this.enforceSpaceStep);
6363
this.isStatic = true;
6464
}
6565
return false;
@@ -119,7 +119,7 @@ public boolean getPartName(StringReader line, LineParser parser) {
119119
}
120120

121121
line.skip();
122-
parser.getSteps().addPriority(this.skipUntilSemicolonAfterStarStep);
122+
parser.getSteps().executeNext(this.skipUntilSemicolonAfterStarStep);
123123
return false;
124124
} else if (parser.peekSingleLineComment(line)) {
125125
// ignore single line comment at the end of the name
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,34 @@
11
package io.papermc.typewriter.yaml;
22

3+
import java.util.Collections;
4+
import java.util.Set;
5+
36
public class ImportMapping {
47

5-
public ImportSetBean imports;
6-
public ImportSetBean staticImports;
8+
public ImportSet imports;
9+
public ImportSet staticImports;
710

8-
public ImportSetBean getImports() {
11+
public ImportSet getImports() {
912
return this.imports;
1013
}
1114

12-
public ImportSetBean getStaticImports() {
15+
public ImportSet getStaticImports() {
1316
return this.staticImports;
1417
}
18+
19+
public static class ImportSet implements io.papermc.typewriter.context.ImportSet {
20+
21+
public Set<String> single;
22+
public Set<String> global;
23+
24+
@Override
25+
public Set<String> single() {
26+
return this.single == null ? Collections.emptySet() : this.single;
27+
}
28+
29+
@Override
30+
public Set<String> global() {
31+
return this.global == null ? Collections.emptySet() : this.global;
32+
}
33+
}
1534
}

src/test/java/io/papermc/typewriter/yaml/ImportSetBean.java

-22
This file was deleted.

src/testData/java/name/SelfInnerClass.java

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package name;
22

3+
import name.SelfInnerClass.D;
4+
35
public class SelfInnerClass {
46
public class A {
57
public class B {
@@ -30,6 +32,7 @@ public class D {
3032
var b = A.B.class;
3133
var c = A.B.C.class;
3234
var d = D.class;
35+
var f = F.class;
3336
}
3437

3538
public class F {
@@ -42,5 +45,6 @@ public class F {
4245
var b = A.B.class;
4346
var c = A.B.C.class;
4447
var d = D.class;
48+
var f = D.F.class;
4549
}
4650
}

src/testData/resources/expected/name/SelfInnerClass$D.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@ shortNames: {
22
"name.SelfInnerClass$A": "A",
33
"name.SelfInnerClass$A$B": "A.B",
44
"name.SelfInnerClass$A$B$C": "A.B.C",
5-
"name.SelfInnerClass$D": "D"
5+
"name.SelfInnerClass$D": "D",
6+
"name.SelfInnerClass$D$F": "F"
67
}

src/testData/resources/expected/name/SelfInnerClass.yaml

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,6 @@ shortNames: {
22
"name.SelfInnerClass$A": "A",
33
"name.SelfInnerClass$A$B": "A.B",
44
"name.SelfInnerClass$A$B$C": "A.B.C",
5-
"name.SelfInnerClass$D": "D"
5+
"name.SelfInnerClass$D": "D",
6+
"name.SelfInnerClass$D$F": "D.F"
67
}

0 commit comments

Comments
 (0)