Skip to content

Smarter jass validation #1109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Smarter jass validation #1109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e640e98
fix buildscript
Frotty Oct 29, 2025
f00c190
Smarter jass validation
Frotty Oct 30, 2025
06f4931
Merge branch 'master' into smarter-jass-validation
Frotty Oct 30, 2025
415426b
fix buildscript
Frotty Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
296 changes: 290 additions & 6 deletions de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.google.common.base.Charsets;
import com.google.common.collect.*;
import com.google.common.io.BaseEncoding;
import com.google.common.io.Files;
import de.peeeq.wurstio.ModelChangedException;
import de.peeeq.wurstio.WurstCompilerJassImpl;
Expand All @@ -10,6 +11,8 @@
import de.peeeq.wurstscript.WLogger;
import de.peeeq.wurstscript.ast.*;
import de.peeeq.wurstscript.attributes.CompileError;
import de.peeeq.wurstscript.attributes.prettyPrint.DefaultSpacer;
import de.peeeq.wurstscript.attributes.prettyPrint.PrettyPrinter;
import de.peeeq.wurstscript.gui.WurstGui;
import de.peeeq.wurstscript.gui.WurstGuiLogger;
import de.peeeq.wurstscript.utils.Utils;
Expand All @@ -18,6 +21,8 @@
import org.eclipse.lsp4j.PublishDiagnosticsParams;

import java.io.*;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.*;
Expand Down Expand Up @@ -47,6 +52,12 @@ public class ModelManagerImpl implements ModelManager {
// hashcode for each compilation unit content as string
private final Map<WFile, Integer> fileHashcodes = new HashMap<>();

// hash for each function inside a Jass compilation unit
private final Map<WFile, Map<String, String>> jassFunctionSnapshots = new HashMap<>();

// functions that changed in recently modified Jass compilation units
private final Map<WFile, Set<String>> pendingJassFunctionChanges = new HashMap<>();

// file for each compilation unit
private final WeakHashMap<CompilationUnit, WFile> compilationunitFile = new WeakHashMap<>();

Expand Down Expand Up @@ -88,6 +99,10 @@ private List<CompilationUnit> getJassdocCUs(Path jassdoc, WurstGui gui) {
@Override
public Changes removeCompilationUnit(WFile resource) {
parseErrors.remove(resource);
if (isJassFile(resource)) {
jassFunctionSnapshots.remove(resource);
pendingJassFunctionChanges.remove(resource);
}
WurstModel model2 = model;
if (model2 == null) {
return Changes.empty();
Expand All @@ -114,6 +129,8 @@ public void clean() {
parseErrors.clear();
model = null;
dependencies.clear();
jassFunctionSnapshots.clear();
pendingJassFunctionChanges.clear();
WLogger.info("Clean done.");
}

Expand Down Expand Up @@ -539,6 +556,17 @@ private CompilationUnit replaceCompilationUnit(WFile filename, String contents,
WurstCompilerJassImpl c = getCompiler(gui);
CompilationUnit cu = c.parse(filename.toString(), new StringReader(contents));
cu.getCuInfo().setFile(filename.toString());

if (isJassFile(filename)) {
Map<String, String> newFunctions = collectJassFunctions(cu);
Map<String, String> oldFunctions = jassFunctionSnapshots.getOrDefault(filename, Collections.emptyMap());
Set<String> changedFunctions = determineChangedJassFunctions(oldFunctions, newFunctions);
pendingJassFunctionChanges.put(filename, changedFunctions);
jassFunctionSnapshots.put(filename, newFunctions);
} else {
Comment on lines +560 to +566
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Jass fan‑out ignores non‑function declarations

The change detection for Jass files only fingerprints FunctionDefinitions (collectJassFunctions / determineChangedJassFunctions) and calculateCUsToUpdate only propagates updates when one of those functions changes. If a Jass file modifies or removes other top‑level declarations—e.g. globals, native declarations, or struct typedefs—pendingJassFunctionChanges remains empty, so no dependent Wurst compilation units are rechecked. Previously any edit in a .j file forced a full rebuild, so these dependencies were safe. After this commit, packages that rely on a changed global can continue using stale attribute caches and miss the resulting errors until a manual rebuild, which is a regression in correctness. Consider hashing all relevant Jass declarations or falling back to the previous whole‑project invalidation when non‑function changes are detected.

Useful? React with 👍 / 👎.

pendingJassFunctionChanges.remove(filename);
}

updateModel(cu, gui);
fileHashcodes.put(filename, contents.hashCode());
if (reportErrors) {
Expand Down Expand Up @@ -625,6 +653,7 @@ private void doTypeCheckPartial(WurstGui gui, List<WFile> toCheckFilenames, Set<
Collection<CompilationUnit> toCheckRec = calculateCUsToUpdate(toCheck, oldPackages, model2);

partialTypecheck(model2, toCheckRec, gui, comp);
clearPendingJassChanges(toCheckFilenames);
}

@Override
Expand All @@ -644,6 +673,7 @@ public void reconcile(Changes changes) {
WurstGui gui = new WurstGuiLogger();
WurstCompilerJassImpl comp = getCompiler(gui);
partialTypecheck(model2, toCheckRec, gui, comp);
clearPendingJassChanges(changes.getAffectedFiles().toJavaSet());
}

private void partialTypecheck(WurstModel model2, Collection<CompilationUnit> toCheckRec, WurstGui gui, WurstCompilerJassImpl comp) {
Expand Down Expand Up @@ -676,18 +706,26 @@ private Set<CompilationUnit> calculateCUsToUpdate(Collection<CompilationUnit> ch
Set<CompilationUnit> result = new TreeSet<>(Comparator.comparing(cu -> cu.getCuInfo().getFile()));
result.addAll(changed);

boolean b = false;
Map<WFile, Set<String>> changedJassFunctions = new HashMap<>();
boolean missingJassInfo = false;
for (CompilationUnit compilationUnit : changed) {
if (compilationUnit.getCuInfo().getFile().endsWith(".j")) {
b = true;
break;
if (isJassFile(compilationUnit)) {
WFile file = wFile(compilationUnit);
Set<String> changedFunctions = pendingJassFunctionChanges.get(file);
if (changedFunctions == null) {
missingJassInfo = true;
} else {
changedJassFunctions.put(file, changedFunctions);
}
}
}
if (b) {
// when plain Jass files are changed, everything must be checked again:
if (missingJassInfo) {
result.addAll(model);
return result;
}
if (!changedJassFunctions.isEmpty()) {
addAffectedByJass(changedJassFunctions, model, result);
}

// get packages provided by the changed CUs
Stream<String> providedPackages = changed.stream()
Expand All @@ -703,6 +741,236 @@ private Set<CompilationUnit> calculateCUsToUpdate(Collection<CompilationUnit> ch
return result;
}

private Map<String, String> collectJassFunctions(CompilationUnit cu) {
Map<String, String> result = new LinkedHashMap<>();
for (JassToplevelDeclaration decl : cu.getJassDecls()) {
if (decl instanceof FunctionDefinition) {
FunctionDefinition function = (FunctionDefinition) decl;
result.put(function.getName(), fingerprintJassFunction(function));
}
}
return result;
}

private String fingerprintJassFunction(FunctionDefinition function) {
DefaultSpacer spacer = new DefaultSpacer();
String rendered = function.match(new FunctionDefinition.Matcher<String>() {
@Override
public String case_NativeFunc(NativeFunc nativeFunc) {
return prettyPrint(nativeFunc, spacer);
}

@Override
public String case_TupleDef(TupleDef tupleDef) {
return prettyPrint(tupleDef, spacer);
}

@Override
public String case_ExtensionFuncDef(ExtensionFuncDef extensionFuncDef) {
return prettyPrint(extensionFuncDef, spacer);
}

@Override
public String case_FuncDef(FuncDef funcDef) {
return prettyPrint(funcDef, spacer);
}
});
return sha256(rendered);
}

private String prettyPrint(NativeFunc nativeFunc, DefaultSpacer spacer) {
StringBuilder sb = new StringBuilder();
seedBuilder(sb);
PrettyPrinter.jassPrettyPrint(nativeFunc, spacer, sb, 0);
trimBuilderPrefix(sb);
return sb.toString();
}

private String prettyPrint(FuncDef funcDef, DefaultSpacer spacer) {
StringBuilder sb = new StringBuilder();
seedBuilder(sb);
PrettyPrinter.jassPrettyPrint(funcDef, spacer, sb, 0);
trimBuilderPrefix(sb);
return sb.toString();
}

private String prettyPrint(FunctionDefinition function, DefaultSpacer spacer) {
StringBuilder sb = new StringBuilder();
seedBuilder(sb);
function.prettyPrint(spacer, sb, 0);
trimBuilderPrefix(sb);
return sb.toString();
}

private void seedBuilder(StringBuilder sb) {
sb.append('\n').append('\n');
}

private void trimBuilderPrefix(StringBuilder sb) {
while (sb.length() > 0 && sb.charAt(0) == '\n') {
sb.deleteCharAt(0);
}
}

private String sha256(String content) {
try {
MessageDigest digest = MessageDigest.getInstance("SHA-256");
byte[] hash = digest.digest(content.getBytes(UTF_8));
return BaseEncoding.base16().lowerCase().encode(hash);
} catch (NoSuchAlgorithmException e) {
throw new IllegalStateException("SHA-256 not available", e);
}
}

private Set<String> determineChangedJassFunctions(Map<String, String> oldFunctions, Map<String, String> newFunctions) {
Set<String> changed = new HashSet<>();
for (Map.Entry<String, String> entry : newFunctions.entrySet()) {
String oldFingerprint = oldFunctions.get(entry.getKey());
String newFingerprint = entry.getValue();
if (oldFingerprint == null || !newFingerprint.equals(oldFingerprint)) {
changed.add(entry.getKey());
}
}
for (String oldName : oldFunctions.keySet()) {
if (!newFunctions.containsKey(oldName)) {
changed.add(oldName);
}
}
return Collections.unmodifiableSet(changed);
}

private void addAffectedByJass(Map<WFile, Set<String>> changedJassFunctions, WurstModel model, Set<CompilationUnit> result) {
boolean hasRealChanges = changedJassFunctions.values().stream().anyMatch(s -> !s.isEmpty());
if (!hasRealChanges) {
return;
}
for (CompilationUnit cu : model) {
if (result.contains(cu)) {
continue;
}
if (usesChangedJassFunctions(cu, changedJassFunctions)) {
result.add(cu);
}
}
}

private boolean usesChangedJassFunctions(CompilationUnit cu, Map<WFile, Set<String>> changedJassFunctions) {
JassFunctionUsageCollector collector = new JassFunctionUsageCollector(changedJassFunctions);
cu.accept(collector);
return collector.isFound();
}

private final class JassFunctionUsageCollector extends Element.DefaultVisitor {
private final Map<WFile, Set<String>> changedJassFunctions;
private boolean found = false;

private JassFunctionUsageCollector(Map<WFile, Set<String>> changedJassFunctions) {
this.changedJassFunctions = changedJassFunctions;
}

private boolean isFound() {
return found;
}

private void checkFuncRef(FuncRef funcRef) {
if (found) {
return;
}
FunctionDefinition funcDef = null;
try {
funcDef = funcRef.attrFuncDef();
} catch (RuntimeException ignored) {
// fall back to name-based matching below
}
if (funcDef != null) {
CompilationUnit defCu = funcDef.attrCompilationUnit();
if (defCu == null) {
return;
}
WFile defFile = wFile(defCu);
Set<String> changed = changedJassFunctions.get(defFile);
if (changed == null || changed.isEmpty()) {
return;
}
if (changed.contains(funcDef.getName())) {
found = true;
}
return;
}
String funcName = funcRef.getFuncName();
if (functionNameChanged(funcName)) {
found = true;
}
}

private boolean functionNameChanged(String funcName) {
if (funcName == null) {
return false;
}
for (Set<String> changed : changedJassFunctions.values()) {
if (changed.contains(funcName)) {
return true;
}
}
return false;
}

@Override
public void visit(ExprFunctionCall e) {
if (found) {
return;
}
checkFuncRef(e);
if (!found) {
super.visit(e);
}
}

@Override
public void visit(ExprMemberMethodDot e) {
if (found) {
return;
}
checkFuncRef(e);
if (!found) {
super.visit(e);
}
}

@Override
public void visit(ExprMemberMethodDotDot e) {
if (found) {
return;
}
checkFuncRef(e);
if (!found) {
super.visit(e);
}
}

@Override
public void visit(ExprFuncRef e) {
if (found) {
return;
}
checkFuncRef(e);
if (!found) {
super.visit(e);
}
}

@Override
public void visit(Annotation annotation) {
if (found) {
return;
}
checkFuncRef(annotation);
if (!found) {
super.visit(annotation);
}
}
}


/**
* Add all packages that directly or indirectly depend on the providedPackages
Expand Down Expand Up @@ -792,6 +1060,22 @@ private void addDependencyWurstFiles(Set<File> result, File file) {
}
}

private boolean isJassFile(CompilationUnit cu) {
return cu.getCuInfo().getFile().endsWith(".j");
}

private boolean isJassFile(WFile file) {
return file.toString().endsWith(".j");
}

private void clearPendingJassChanges(Collection<WFile> files) {
for (WFile file : files) {
if (isJassFile(file)) {
pendingJassFunctionChanges.remove(file);
}
}
}

private WFile wFile(CompilationUnit cu) {
return compilationunitFile.computeIfAbsent(cu, c -> WFile.create(cu.getCuInfo().getFile()));
}
Expand Down
Loading
Loading