From 745a8440ffdb82bdca3c87b9df0535312c9087ed Mon Sep 17 00:00:00 2001 From: Frotty Date: Thu, 23 Oct 2025 20:38:27 +0200 Subject: [PATCH] Restrict override candidate tracking to class hierarchies --- .../attributes/AttrFunctionSignature.java | 19 +++++ .../attributes/names/NameLinks.java | 55 ++++++++++++-- .../imtranslation/ImTranslator.java | 71 +++++++++++++++---- .../imtranslation/OverrideUtils.java | 3 + .../wurstscript/types/FunctionSignature.java | 7 ++ .../validation/WurstValidator.java | 67 ++++++++++++++++- .../wurstscript/tests/InterfaceTests.java | 20 ++++++ 7 files changed, 223 insertions(+), 19 deletions(-) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFunctionSignature.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFunctionSignature.java index e100bc425..d56756058 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFunctionSignature.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFunctionSignature.java @@ -69,6 +69,11 @@ private static FunctionSignature filterSigs( return candidates.get(0); } + candidates = filterPreferNonAbstract(candidates); + if (candidates.size() == 1) { + return candidates.get(0); + } + boolean b = true; for (WurstType t : argTypes) { @@ -99,6 +104,20 @@ private static List filterByIfNotDefinedAnnotation(List filterPreferNonAbstract(List candidates) { + List nonAbstract = new ArrayList<>(); + for (FunctionSignature sig : candidates) { + FunctionDefinition def = sig.getDef(); + if (def != null && !def.attrIsAbstract()) { + nonAbstract.add(sig); + } + } + if (!nonAbstract.isEmpty()) { + return nonAbstract; + } + return candidates; + } + @NotNull private static List filterByArgumentTypes(Collection sigs, List argTypes, StmtCall location) { List candidates = new ArrayList<>(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/NameLinks.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/NameLinks.java index fb5500699..ac8782e8e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/NameLinks.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/names/NameLinks.java @@ -28,6 +28,9 @@ static private class OverrideCheckResult { // (for interfaces override modifier is optional) boolean requiresOverrideMod = false; + // inherited functions used for override checks only, skip reporting + boolean skipErrorReporting = false; + // errors for functions with same name that it does not override io.vavr.collection.List overrideErrors = io.vavr.collection.List.empty(); @@ -71,6 +74,9 @@ private static void reportOverrideErrors(Map e : map.entrySet()) { FunctionDefinition f = e.getKey().getDef(); OverrideCheckResult check = e.getValue(); + if (check.skipErrorReporting) { + continue; + } if (f.attrIsOverride() && !check.doesOverride) { StringBuilder msg = new StringBuilder("Function " + f.getName() + " does not override anything."); for (String overrideError : check.overrideErrors) { @@ -101,30 +107,36 @@ public static ImmutableMultimap calculate(InterfaceDef i) { private static void addNamesFromExtendedInterfaces(Multimap result, WurstTypeInterface iType, Map> overrideCheckResults) { for (WurstTypeInterface superI : iType.extendedInterfaces()) { - addNewNameLinks(result, overrideCheckResults, superI.nameLinks(), false); + addNewNameLinks(result, overrideCheckResults, superI.nameLinks(), false, null); } } private static void addNamesFromImplementedInterfaces(Multimap result, WurstTypeClass classDef, Map> overrideCheckResults) { for (WurstTypeInterface interfaceType : classDef.implementedInterfaces()) { - addNewNameLinks(result, overrideCheckResults, interfaceType.nameLinks(), false); + addNewNameLinks(result, overrideCheckResults, interfaceType.nameLinks(), false, classDef.getClassDef()); } } private static void addNamesFromSuperClass(Multimap result, WurstTypeClass c, Map> overrideCheckResults) { @Nullable WurstTypeClass superClass = c.extendedClass(); if (superClass != null) { - addNewNameLinks(result, overrideCheckResults, superClass.nameLinks(), false); + addNewNameLinks(result, overrideCheckResults, superClass.nameLinks(), false, c.getClassDef()); } } - private static void addNewNameLinks(Multimap result, Map> overrideCheckResults, ImmutableMultimap newNameLinks, boolean allowStaticOverride) { + private static void addNewNameLinks(Multimap result, Map> overrideCheckResults, ImmutableMultimap newNameLinks, boolean allowStaticOverride, @Nullable ClassDef currentClass) { for (Entry e : newNameLinks.entries()) { DefLink def = e.getValue(); if (!def.getVisibility().isInherited()) { continue; } + if (currentClass != null && def instanceof FuncLink) { + DefLink adapted = ((FuncLink) def).adaptToReceiverType(currentClass.attrTyp()); + if (adapted != null) { + def = adapted; + } + } String name = e.getKey(); boolean isOverridden = false; if (def instanceof FuncLink) { @@ -153,8 +165,41 @@ private static void addNewNameLinks(Multimap result, Map> overrideCheckResults, + String name, FuncLink funcLink, @Nullable ClassDef currentClass) { + if (currentClass == null) { + return; + } + FunctionDefinition def = funcLink.getDef(); + if (!(def instanceof FuncDef)) { + return; + } + if (def.attrIsAbstract()) { + return; + } + if (def.attrNearestClassDef() == null) { + return; + } + if (currentClass != null) { + DefLink adapted = funcLink.adaptToReceiverType(currentClass.attrTyp()); + if (adapted instanceof FuncLink) { + funcLink = (FuncLink) adapted; } } + Map map = overrideCheckResults.computeIfAbsent(name, s -> new HashMap<>()); + OverrideCheckResult check = map.get(funcLink); + if (check == null) { + check = new OverrideCheckResult(); + check.skipErrorReporting = true; + map.put(funcLink, check); + } } @@ -319,7 +364,7 @@ private static void addNameDefDefLink(Multimap result, NameDef private static void addNamesFromUsedModuleInstantiations(ClassOrModuleOrModuleInstanciation c, Multimap result, Map> overrideCheckResults) { for (ModuleInstanciation m : c.getModuleInstanciations()) { - addNewNameLinks(result, overrideCheckResults, m.attrNameLinks(), true); + addNewNameLinks(result, overrideCheckResults, m.attrNameLinks(), true, null); } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index 4ff708502..04e03306e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -1129,23 +1129,70 @@ public Map getClassesWithImplementation(Collection + " in " + Utils.printElementWithSource(Optional.of(c))); } - for (NameLink nameLink : c.attrNameLinks().get(func.getName())) { - NameDef nameDef = nameLink.getDef(); - if (nameLink.getDefinedIn() == c) { - if (nameLink instanceof FuncLink && nameLink.getDef() instanceof FuncDef) { - FuncLink funcLink = (FuncLink) nameLink; - FuncDef f = (FuncDef) funcLink.getDef(); - // check if function f overrides func - if (WurstValidator.canOverride(funcLink, funcNameLink, false)) { - result.put(c, f); - } - } - } + FuncLink implementationLink = findImplementationLink(c, funcNameLink); + if (implementationLink != null && implementationLink.getDef() instanceof FuncDef) { + result.put(c, (FuncDef) implementationLink.getDef()); } } return result; } + private @Nullable FuncLink findImplementationLink(ClassDef c, FuncLink target) { + FuncLink best = null; + int bestDistance = Integer.MAX_VALUE; + for (NameLink candidateLink : c.attrNameLinks().get(target.getName())) { + if (!(candidateLink instanceof FuncLink)) { + continue; + } + FuncLink candidate = (FuncLink) candidateLink; + if (!WurstValidator.canOverride(candidate, target, false)) { + continue; + } + FunctionDefinition candidateDef = candidate.getDef(); + if (!(candidateDef instanceof FuncDef)) { + continue; + } + if (candidateDef.attrIsAbstract()) { + continue; + } + ClassDef owner = candidateDef.attrNearestClassDef(); + if (owner == null) { + continue; + } + int distance = distanceToOwner(c, owner); + if (distance == Integer.MAX_VALUE) { + continue; + } + if (best == null || distance < bestDistance) { + best = candidate; + bestDistance = distance; + } + } + return best; + } + + private int distanceToOwner(ClassDef start, ClassDef owner) { + int distance = 0; + ClassDef current = start; + Set visited = new HashSet<>(); + while (current != null && visited.add(current)) { + if (current == owner) { + return distance; + } + WurstTypeClass type = current.attrTypC(); + if (type == null) { + break; + } + WurstTypeClass superType = type.extendedClass(); + if (superType == null) { + break; + } + current = superType.getClassDef(); + distance++; + } + return Integer.MAX_VALUE; + } + private final Map>> classDynamicInitMap = Maps.newLinkedHashMap(); private final Map> classInitStatements = Maps.newLinkedHashMap(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java index bc060b0dc..7d4b7e04a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/OverrideUtils.java @@ -78,6 +78,9 @@ public static void addOverride( } if (!needConversion) { + if (superMethodIm == subMethod) { + return; + } superMethodIm.getSubMethods().add(subMethod); return; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/FunctionSignature.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/FunctionSignature.java index a9d5a308c..aebe243cb 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/FunctionSignature.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/FunctionSignature.java @@ -61,6 +61,13 @@ public WurstType getReturnType() { return receiverType; } + public @Nullable FunctionDefinition getDef() { + if (trace instanceof FunctionDefinition) { + return (FunctionDefinition) trace; + } + return null; + } + @CheckReturnValue public FunctionSignature setTypeArgs(Element context, VariableBinding newMapping) { if (newMapping.isEmpty()) { diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java index 2958c2281..bf935301a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java @@ -525,8 +525,11 @@ private void checkAbstractMethods(ClassDef c) { } loc.addError("Non-abstract class " + c.getName() + " cannot have abstract functions like " + f.getName()); } else if (link instanceof FuncLink) { - toImplement.append("\n "); - toImplement.append(((FuncLink) link).printFunctionTemplate()); + FuncLink abstractLink = (FuncLink) link; + if (!hasImplementationInHierarchy(c, abstractLink)) { + toImplement.append("\n "); + toImplement.append(abstractLink.printFunctionTemplate()); + } } } } @@ -536,6 +539,66 @@ private void checkAbstractMethods(ClassDef c) { } } + private boolean hasImplementationInHierarchy(ClassDef c, FuncLink abstractFunc) { + return findImplementationLink(c, abstractFunc) != null; + } + + private @Nullable FuncLink findImplementationLink(ClassDef c, FuncLink abstractFunc) { + FuncLink best = null; + int bestDistance = Integer.MAX_VALUE; + for (NameLink nameLink : c.attrNameLinks().get(abstractFunc.getName())) { + if (!(nameLink instanceof FuncLink)) { + continue; + } + FuncLink candidate = (FuncLink) nameLink; + if (!WurstValidator.canOverride(candidate, abstractFunc, false)) { + continue; + } + FunctionDefinition candidateDef = candidate.getDef(); + if (!(candidateDef instanceof FuncDef)) { + continue; + } + if (candidateDef.attrIsAbstract()) { + continue; + } + ClassDef owner = candidateDef.attrNearestClassDef(); + if (owner == null) { + continue; + } + int distance = distanceToOwner(c, owner); + if (distance == Integer.MAX_VALUE) { + continue; + } + if (best == null || distance < bestDistance) { + best = candidate; + bestDistance = distance; + } + } + return best; + } + + private int distanceToOwner(ClassDef start, ClassDef owner) { + int distance = 0; + ClassDef current = start; + Set visited = new HashSet<>(); + while (current != null && visited.add(current)) { + if (current == owner) { + return distance; + } + WurstTypeClass currentType = current.attrTypC(); + if (currentType == null) { + break; + } + WurstTypeClass superType = currentType.extendedClass(); + if (superType == null) { + break; + } + current = superType.getClassDef(); + distance++; + } + return Integer.MAX_VALUE; + } + private void visit(StmtExitwhen exitwhen) { Element parent = exitwhen.getParent(); while (!(parent instanceof FunctionDefinition)) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java index 76d70c47c..474a625f6 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/InterfaceTests.java @@ -537,6 +537,26 @@ public void implGap() { // #676 ); } + @Test + public void interfaceImplementationFromSuperClass() { + testAssertOkLines(true, + "package test", + " native testSuccess()", + " interface Foo", + " function doSomething()", + " interface Bar", + " function doSomething()", + " class BaseFoo implements Foo", + " override function doSomething()", + " testSuccess()", + " class FooBar extends BaseFoo implements Bar", + " init", + " FooBar fb = new FooBar()", + " fb.doSomething()", + "endpackage" + ); + } + @Test public void testEmptyImplements() { CompilationResult res = test().executeProg(false)