Skip to content

Commit b45da02

Browse files
FrottyFrotty
authored
add warning and implicit fix for uninitialized jass local usage (#1164)
1 parent 4cf4a89 commit b45da02

File tree

5 files changed

+220
-0
lines changed

5 files changed

+220
-0
lines changed

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jass/AntlrJassParseTreeTransformer.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@
1515
import org.eclipse.jdt.annotation.Nullable;
1616

1717
import java.util.List;
18+
import java.util.Set;
1819

1920

2021
public class AntlrJassParseTreeTransformer {
22+
private static final Set<String> JASS_PRIMITIVE_TYPES = Set.of("integer", "real", "boolean");
2123
private final String file;
2224
private final ErrorHandler cuErrorHandler;
2325
private final LineOffsets lineOffsets;
@@ -123,12 +125,23 @@ private WStatements transformJassLocals(List<JassParser.JassLocalContext> jassLo
123125
OptTypeExpr optTyp = transformOptionalType(l.jassTypeExpr());
124126
Identifier name = Ast.Identifier(source(l.name), l.name.getText());
125127
OptExpr initialExpr = transformOptionalExpr(l.initial);
128+
if (l.initial == null && shouldDefaultLocalToNull(optTyp)) {
129+
initialExpr = Ast.ExprNull(source(l));
130+
}
126131
result.add(Ast.LocalVarDef(source(l), modifiers, optTyp, name,
127132
initialExpr));
128133
}
129134
return result;
130135
}
131136

137+
private boolean shouldDefaultLocalToNull(OptTypeExpr optTyp) {
138+
if (!(optTyp instanceof TypeExprSimple)) {
139+
return false;
140+
}
141+
String typeName = ((TypeExprSimple) optTyp).getTypeName();
142+
return !JASS_PRIMITIVE_TYPES.contains(typeName);
143+
}
144+
132145
private WStatements transformJassStatements(JassParser.JassStatementsContext stmts) {
133146
WStatements result = Ast.WStatements();
134147
for (JassParser.JassStatementContext s : stmts.jassStatement()) {

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/jurst/AntlrJurstParseTreeTransformer.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
import org.eclipse.jdt.annotation.Nullable;
1818

1919
import java.util.List;
20+
import java.util.Set;
2021

2122
public class AntlrJurstParseTreeTransformer {
23+
private static final Set<String> JASS_PRIMITIVE_TYPES = Set.of("integer", "real", "boolean");
2224

2325
private final String file;
2426
private final ErrorHandler cuErrorHandler;
@@ -134,12 +136,23 @@ private WStatements transformJassLocals(List<JassLocalContext> jassLocals) {
134136
OptTypeExpr optTyp = transformOptionalType(l.typeExpr());
135137
Identifier name = text(l.name);
136138
OptExpr initialExpr = transformOptionalExpr(l.initial);
139+
if (l.initial == null && shouldDefaultJassLocalToNull(optTyp)) {
140+
initialExpr = Ast.ExprNull(source(l));
141+
}
137142
result.add(Ast.LocalVarDef(source(l), modifiers, optTyp, name,
138143
initialExpr));
139144
}
140145
return result;
141146
}
142147

148+
private boolean shouldDefaultJassLocalToNull(OptTypeExpr optTyp) {
149+
if (!(optTyp instanceof TypeExprSimple)) {
150+
return false;
151+
}
152+
String typeName = ((TypeExprSimple) optTyp).getTypeName();
153+
return !JASS_PRIMITIVE_TYPES.contains(typeName);
154+
}
155+
143156
private WStatements transformJassStatements(JassStatementsContext stmts) {
144157
WStatements result = Ast.WStatements();
145158
for (JassStatementContext s : stmts.jassStatement()) {

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/parser/antlr/AntlrWurstParseTreeTransformer.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
import java.util.Comparator;
2828
import java.util.Deque;
2929
import java.util.List;
30+
import java.util.Set;
3031
import java.util.stream.Collectors;
3132

3233
@SuppressWarnings("Duplicates")
3334
public class AntlrWurstParseTreeTransformer {
35+
private static final Set<String> JASS_PRIMITIVE_TYPES = Set.of("integer", "real", "boolean");
3436

3537
private final String file;
3638
private final ErrorHandler cuErrorHandler;
@@ -225,12 +227,23 @@ private WStatements transformJassLocals(List<JassLocalContext> jassLocals) {
225227
OptTypeExpr optTyp = transformOptionalType(l.typeExpr());
226228
Identifier name = text(l.name);
227229
OptExpr initialExpr = transformOptionalExpr(l.initial);
230+
if (l.initial == null && shouldDefaultJassLocalToNull(optTyp)) {
231+
initialExpr = Ast.ExprNull(source(l));
232+
}
228233
result.add(Ast.LocalVarDef(source(l), modifiers, optTyp, name,
229234
initialExpr));
230235
}
231236
return result;
232237
}
233238

239+
private boolean shouldDefaultJassLocalToNull(OptTypeExpr optTyp) {
240+
if (!(optTyp instanceof TypeExprSimple)) {
241+
return false;
242+
}
243+
String typeName = ((TypeExprSimple) optTyp).getTypeName();
244+
return !JASS_PRIMITIVE_TYPES.contains(typeName);
245+
}
246+
234247
private WStatements transformJassStatements(JassStatementsContext stmts) {
235248
WStatements result = Ast.WStatements();
236249
if (stmts != null && stmts.jassStatement() != null) {

‎de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1648,6 +1648,125 @@ private void checkUninitializedVars(FunctionLike f) {
16481648
&& !f.getSource().getFile().endsWith("war3map.j")) {
16491649
new DataflowAnomalyAnalysis(Utils.isJassCode(f)).execute(f);
16501650
}
1651+
checkJassImplicitNullLocalsReadWithoutExplicitWrite(f);
1652+
}
1653+
1654+
/**
1655+
* JASS compatibility shim: we currently synthesize "= null" for uninitialized non-primitive
1656+
* locals to avoid invalid emitted JASS. Still report likely user bugs early when such a local
1657+
* is read before its first explicit assignment in the input.
1658+
*/
1659+
private void checkJassImplicitNullLocalsReadWithoutExplicitWrite(FunctionLike f) {
1660+
if (!Utils.isJassCode(f)) {
1661+
return;
1662+
}
1663+
1664+
List<LocalVarDef> implicitNullLocals = new ArrayList<>();
1665+
f.accept(new Element.DefaultVisitor() {
1666+
@Override
1667+
public void visit(LocalVarDef localVarDef) {
1668+
super.visit(localVarDef);
1669+
if (isImplicitNullInit(localVarDef)) {
1670+
implicitNullLocals.add(localVarDef);
1671+
}
1672+
}
1673+
});
1674+
if (implicitNullLocals.isEmpty()) {
1675+
return;
1676+
}
1677+
1678+
Set<LocalVarDef> implicitNullLocalSet = new HashSet<>(implicitNullLocals);
1679+
Map<LocalVarDef, StmtSet> firstExplicitWrite = new HashMap<>();
1680+
f.accept(new Element.DefaultVisitor() {
1681+
@Override
1682+
public void visit(StmtSet stmtSet) {
1683+
super.visit(stmtSet);
1684+
NameLink link = stmtSet.getUpdatedExpr().attrNameLink();
1685+
if (link != null && link.getDef() instanceof LocalVarDef) {
1686+
LocalVarDef local = (LocalVarDef) link.getDef();
1687+
if (!implicitNullLocalSet.contains(local)) {
1688+
return;
1689+
}
1690+
StmtSet previous = firstExplicitWrite.get(local);
1691+
if (previous == null
1692+
|| stmtSet.attrSource().getLeftPos() < previous.attrSource().getLeftPos()) {
1693+
firstExplicitWrite.put(local, stmtSet);
1694+
}
1695+
}
1696+
}
1697+
});
1698+
1699+
Set<LocalVarDef> readBeforeExplicitWrite = new HashSet<>();
1700+
f.accept(new Element.DefaultVisitor() {
1701+
@Override
1702+
public void visit(ExprVarAccess varAccess) {
1703+
super.visit(varAccess);
1704+
NameLink link = varAccess.attrNameLink();
1705+
if (link == null || !(link.getDef() instanceof LocalVarDef)) {
1706+
return;
1707+
}
1708+
LocalVarDef local = (LocalVarDef) link.getDef();
1709+
if (!implicitNullLocalSet.contains(local)) {
1710+
return;
1711+
}
1712+
if (isWriteTarget(varAccess)) {
1713+
return;
1714+
}
1715+
StmtSet firstWrite = firstExplicitWrite.get(local);
1716+
if (firstWrite == null) {
1717+
readBeforeExplicitWrite.add(local);
1718+
return;
1719+
}
1720+
StmtSet enclosingSet = nearestEnclosingStmtSet(varAccess);
1721+
if (enclosingSet == firstWrite) {
1722+
readBeforeExplicitWrite.add(local);
1723+
return;
1724+
}
1725+
if (varAccess.attrSource().getLeftPos() < firstWrite.attrSource().getLeftPos()) {
1726+
readBeforeExplicitWrite.add(local);
1727+
}
1728+
}
1729+
});
1730+
1731+
for (LocalVarDef local : implicitNullLocals) {
1732+
if (readBeforeExplicitWrite.contains(local)) {
1733+
local.addWarning("Variable " + local.getName()
1734+
+ " is read before explicit initialization in input JASS; defaulting to null.");
1735+
}
1736+
}
1737+
}
1738+
1739+
private boolean isWriteTarget(ExprVarAccess varAccess) {
1740+
if (!(varAccess.getParent() instanceof StmtSet)) {
1741+
return false;
1742+
}
1743+
StmtSet set = (StmtSet) varAccess.getParent();
1744+
return set.getUpdatedExpr() == varAccess;
1745+
}
1746+
1747+
private @Nullable StmtSet nearestEnclosingStmtSet(Element e) {
1748+
Element current = e.getParent();
1749+
while (current != null) {
1750+
if (current instanceof StmtSet) {
1751+
return (StmtSet) current;
1752+
}
1753+
current = current.getParent();
1754+
}
1755+
return null;
1756+
}
1757+
1758+
private boolean isImplicitNullInit(LocalVarDef localVarDef) {
1759+
if (!(localVarDef.getInitialExpr() instanceof ExprNull)) {
1760+
return false;
1761+
}
1762+
// Synthetic null-inits created by the JASS parser use the exact local declaration source span.
1763+
return sameSourceSpan(localVarDef.getInitialExpr().attrSource(), localVarDef.attrSource());
1764+
}
1765+
1766+
private boolean sameSourceSpan(de.peeeq.wurstscript.parser.WPos a, de.peeeq.wurstscript.parser.WPos b) {
1767+
return a.getFile().equals(b.getFile())
1768+
&& a.getLeftPos() == b.getLeftPos()
1769+
&& a.getRightPos() == b.getRightPos();
16511770
}
16521771

16531772

‎de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/CompilationUnitTests.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,66 @@ public void jass() {
3838
));
3939
}
4040

41+
@Test
42+
public void jassLocalHandleDefaultsToNull() {
43+
testAssertOkLinesWithStdLib(false,
44+
"function tiw takes nothing returns nothing",
45+
"local location uiw",
46+
"set uiw = null",
47+
"endfunction",
48+
"package B",
49+
" init",
50+
" tiw()",
51+
"endpackage"
52+
);
53+
}
54+
55+
@Test
56+
public void jassLocalHandleReadWithoutExplicitInitReportsEarly() {
57+
testAssertWarningsLinesWithStdLib(false, "read before explicit initialization in input JASS",
58+
"function tiw takes nothing returns nothing",
59+
"local location uiw",
60+
"call RemoveLocation(uiw)",
61+
"endfunction",
62+
"package B",
63+
" init",
64+
" tiw()",
65+
"endpackage"
66+
);
67+
}
68+
69+
@Test
70+
public void war3mapJassLocalHandleReadWithoutExplicitInitReportsEarly() {
71+
test()
72+
.withStdLib()
73+
.setStopOnFirstError(false)
74+
.executeProg(false)
75+
.expectWarning("read before explicit initialization in input JASS")
76+
.compilationUnits(
77+
compilationUnit("war3map.j",
78+
"function showUnitTextAlliesWithZ takes nothing returns nothing",
79+
"local location p2",
80+
"call RemoveLocation(p2)",
81+
"endfunction"
82+
)
83+
);
84+
}
85+
86+
@Test
87+
public void jassLocalHandleReadBeforeLaterWriteStillWarns() {
88+
testAssertWarningsLinesWithStdLib(false, "read before explicit initialization in input JASS",
89+
"function tiw takes nothing returns nothing",
90+
"local location uiw",
91+
"call RemoveLocation(uiw)",
92+
"set uiw = GetRectCenter(GetPlayableMapRect())",
93+
"call RemoveLocation(uiw)",
94+
"set uiw = null",
95+
"endfunction",
96+
"package B",
97+
" init",
98+
" tiw()",
99+
"endpackage"
100+
);
101+
}
102+
41103
}

0 commit comments

Comments
 (0)