Infer toString in string concatenation

de.peeeq.wurstscript/build.gradle

@@ -109,7 +109,7 @@ dependencies {
     implementation 'com.github.albfernandez:juniversalchardet:2.4.0'
     implementation 'org.xerial:sqlite-jdbc:3.46.1.3'
     implementation 'com.github.inwc3:jmpq3:e28f6999c0'
-    implementation 'com.github.inwc3:wc3libs:ac41f780a5'
+    implementation 'com.github.inwc3:wc3libs:ac41f780a5e2dfc35310be4ed3267f23ab3fea44'
     implementation 'com.github.wurstscript:wurst-project-config:348fcd4ef5'
     implementation 'org.slf4j:slf4j-api:2.0.17'
     implementation 'ch.qos.logback:logback-classic:1.5.20'

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java

@@ -336,7 +336,9 @@ private CompilationUnit loadLibPackage(Function<File, CompilationUnit> addCompil
             gui.sendError(new CompileError(new WPos("", null, 0, 0), "Could not find lib-package " + imp + ". Is your dependency present in _build/dependencies?"));
             return Ast.CompilationUnit(new CompilationUnitInfo(errorHandler), Ast.JassToplevelDeclarations(), Ast.WPackages());
         } else {
-            return addCompilationUnit.apply(file);
+            CompilationUnit lib = addCompilationUnit.apply(file);
+            lib.getCuInfo().setLibrary(true);
+            return lib;
         }
     }
 

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java

@@ -644,6 +644,9 @@ 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 (isUnderDependenciesFolder(filename)) {
+            cu.getCuInfo().setLibrary(true);
+        }
         updateModel(cu, gui);
         fileHashcodes.put(filename, newHash);
         if (reportErrors) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/CodeActionRequest.java

@@ -1,10 +1,12 @@
 package de.peeeq.wurstio.languageserver.requests;
 
 import de.peeeq.wurstio.languageserver.BufferManager;
+import de.peeeq.wurstio.languageserver.Convert;
 import de.peeeq.wurstio.languageserver.ModelManager;
 import de.peeeq.wurstio.languageserver.WFile;
 import de.peeeq.wurstscript.WLogger;
 import de.peeeq.wurstscript.ast.*;
+import de.peeeq.wurstscript.attributes.AttrFuncDef;
 import de.peeeq.wurstscript.attributes.CompilationUnitInfo;
 import de.peeeq.wurstscript.attributes.names.DefLink;
 import de.peeeq.wurstscript.attributes.names.FuncLink;
@@ -155,6 +157,21 @@ private List<Either<Command, CodeAction>> makeWarningQuickFixes(Optional<Element
             });
         }
 
+        if (e.isPresent() && hasDiagnosticMessage(AttrFuncDef.REDUNDANT_TO_STRING_WARNING::equals)) {
+            findNearest(e.get(), ExprMemberMethodDot.class)
+                .filter(call -> call.getFuncName().equals("toString") && call.getArgs().isEmpty())
+                .ifPresent(call -> {
+                    Range receiverRange = Convert.range(call.getLeft());
+                    Range callRange = Convert.range(call);
+                    TextEdit edit = new TextEdit(
+                        new Range(receiverRange.getEnd(), callRange.getEnd()), "");
+                    result.add(Either.forRight(makeQuickFix(
+                        "Remove redundant .toString()",
+                        workspaceEdit(filename.getUriString(), edit)
+                    )));
+                });
+        }
+
         if (hasDiagnosticMessage(msg -> msg.startsWith("The import ") && msg.endsWith(UNUSED_IMPORT_WARNING_SUFFIX))) {
             int line0 = e.flatMap(elem -> findNearest(elem, WImport.class))
                 .map(imp -> imp.attrSource().getLine() - 1)

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrExprType.java

@@ -285,6 +285,21 @@ public static WurstType calculate(final ExprBinary term) {
                 if (leftType instanceof WurstTypeString && rightType instanceof WurstTypeString) {
                     return WurstTypeString.instance();
                 }
+                if (term.attrFuncLink() != null) {
+                    return handleOperatorOverloading(term);
+                }
+                if (AttrFuncDef.implicitToStringForConcatOperand(term, term.getLeft()) != null
+                        || AttrFuncDef.implicitToStringForConcatOperand(term, term.getRight()) != null) {
+                    return WurstTypeString.instance();
+                }
+                String conversionError = AttrFuncDef.implicitToStringErrorForConcatOperand(term, term.getLeft());
+                if (conversionError == null) {
+                    conversionError = AttrFuncDef.implicitToStringErrorForConcatOperand(term, term.getRight());
+                }
+                if (conversionError != null) {
+                    term.addError(conversionError);
+                    return WurstTypeUnknown.instance();
+                }
                 if (bothTypesRealOrInt(term)) {
                     return caseMathOperation(term);
                 } else {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/AttrFuncDef.java

@@ -27,6 +27,8 @@
  * this attribute find the variable definition for every variable reference
  */
 public class AttrFuncDef {
+    public static final String REDUNDANT_TO_STRING_WARNING =
+        "Explicit .toString() is redundant in this string concatenation.";
 
     // TODO just use the attr function signature to get the def
 
@@ -67,7 +69,126 @@ public static FuncLink calculate(final ExprFuncRef node) {
 
 
     public static @Nullable FuncLink calculate(ExprBinary node) {
-        return getExtensionFunction(node.getLeft(), node.getRight(), node.getOp());
+        FuncLink overloadedOperator = getExtensionFunction(node.getLeft(), node.getRight(), node.getOp());
+        if (overloadedOperator != null && matchesArguments(node, overloadedOperator,
+                Collections.singletonList(node.getRight().attrTyp()))) {
+            return overloadedOperator;
+        }
+        if (implicitToStringForConcatOperand(node, node.getLeft()) != null
+                || implicitToStringForConcatOperand(node, node.getRight()) != null) {
+            return null;
+        }
+        return overloadedOperator;
+    }
+
+    /** Returns the implicit conversion for a non-string operand next to a string in a + expression. */
+    public static @Nullable FuncLink implicitToStringForConcatOperand(ExprBinary concat, Expr operand) {
+        if (concat.getOp() != WurstOperator.PLUS || operand.attrTyp() instanceof WurstTypeString) {
+            return null;
+        }
+        Expr other = concat.getLeft() == operand ? concat.getRight() : concat.getLeft();
+        if (!(other.attrTyp() instanceof WurstTypeString)) {
+            return null;
+        }
+
+        return findToStringConversion(operand);
+    }
+
+    /** Resolves the same zero-argument string conversion that an explicit operand.toString() call would use. */
+    public static @Nullable FuncLink findToStringConversion(Expr operand) {
+        return resolveToStringConversion(operand).conversion;
+    }
+
+    /** Whether replacing the right operand with the given type could expose a left-hand plus overload. */
+    public static boolean hasApplicablePlusOverload(Expr leftOperand, WurstType rightType) {
+        List<WurstType> argumentTypes = Collections.singletonList(rightType);
+        for (FuncLink candidate : leftOperand.lookupMemberFuncs(leftOperand.attrTyp(), overloadingPlus)) {
+            if (matchesArguments(leftOperand, candidate, argumentTypes)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /** Returns why an otherwise applicable implicit conversion cannot be selected. */
+    public static @Nullable String implicitToStringErrorForConcatOperand(ExprBinary concat, Expr operand) {
+        if (concat.getOp() != WurstOperator.PLUS || operand.attrTyp() instanceof WurstTypeString) {
+            return null;
+        }
+        Expr other = concat.getLeft() == operand ? concat.getRight() : concat.getLeft();
+        if (!(other.attrTyp() instanceof WurstTypeString)) {
+            return null;
+        }
+        return resolveToStringConversion(operand).error;
+    }
+
+    private static ToStringConversionResolution resolveToStringConversion(Expr operand) {
+        Collection<FuncLink> raw = NameResolution.lookupMemberFuncs(
+            operand, operand.attrTyp(), "toString", false);
+        List<FuncLink> methods = new ArrayList<>();
+        List<FuncLink> extensions = new ArrayList<>();
+        for (FuncLink candidate : raw) {
+            if (!isVisible(candidate)
+                    || (candidate.getDef() instanceof FuncDef && ((FuncDef) candidate.getDef()).attrIsStatic())) {
+                continue;
+            }
+            FunctionSignature matched = FunctionSignature.fromNameLink(candidate)
+                .matchAgainstArgs(Collections.emptyList(), operand);
+            if (matched == null) {
+                continue;
+            }
+            if (isExtension(candidate)) {
+                if (!extensions.contains(candidate)) {
+                    extensions.add(candidate);
+                }
+            } else {
+                if (!methods.contains(candidate)) {
+                    methods.add(candidate);
+                }
+            }
+        }
+
+        if (!methods.isEmpty()) {
+            return selectToStringConversion(
+                keepMostSpecificReceivers(methods, FuncLink::getReceiverType, operand), operand);
+        }
+        if (!extensions.isEmpty()) {
+            return selectToStringConversion(
+                keepMostSpecificReceivers(extensions, FuncLink::getReceiverType, operand), operand);
+        }
+        return new ToStringConversionResolution(null, null);
+    }
+
+    private static ToStringConversionResolution selectToStringConversion(List<FuncLink> candidates, Expr operand) {
+        if (candidates.size() != 1) {
+            return new ToStringConversionResolution(null,
+                "Call to function toString is ambiguous. Alternatives are:\n" + Utils.printAlternatives(candidates));
+        }
+        FuncLink candidate = candidates.get(0);
+        FunctionSignature matched = FunctionSignature.fromNameLink(candidate)
+            .matchAgainstArgs(Collections.emptyList(), operand);
+        if (matched == null) {
+            return new ToStringConversionResolution(null, null);
+        }
+        if (matched.getMapping().hasUnboundTypeVars()) {
+            return new ToStringConversionResolution(null,
+                "Cannot infer type for type parameter " + matched.getMapping().printUnboundTypeVars());
+        }
+        if (!matched.getReturnType().isSubtypeOf(WurstTypeString.instance(), operand)) {
+            return new ToStringConversionResolution(null, null);
+        }
+        return new ToStringConversionResolution(
+            candidate.withTypeArgBinding(operand, matched.getMapping()), null);
+    }
+
+    private static class ToStringConversionResolution {
+        private final @Nullable FuncLink conversion;
+        private final @Nullable String error;
+
+        private ToStringConversionResolution(@Nullable FuncLink conversion, @Nullable String error) {
+            this.conversion = conversion;
+            this.error = error;
+        }
     }
 
     public static @Nullable FuncLink calculate(final ExprMemberMethod node) {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/attributes/CompilationUnitInfo.java

@@ -11,6 +11,7 @@ public class CompilationUnitInfo {
     private de.peeeq.wurstscript.attributes.ErrorHandler cuErrorHandler;
     private IndentationMode indentationMode = IndentationMode.spaces(4);
     private TriviaIndex triviaIndex = TriviaIndex.empty();
+    private boolean library;
 
     public CompilationUnitInfo(ErrorHandler cuErrorHandler) {
         this.cuErrorHandler = cuErrorHandler;
@@ -48,6 +49,14 @@ public void setTriviaIndex(TriviaIndex triviaIndex) {
         this.triviaIndex = triviaIndex == null ? TriviaIndex.empty() : triviaIndex;
     }
 
+    public boolean isLibrary() {
+        return library;
+    }
+
+    public void setLibrary(boolean library) {
+        this.library = library;
+    }
+
     public interface IndentationMode {
 
         static IndentationMode tabs() {

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ExprTranslation.java

@@ -172,7 +172,12 @@ public static ImExpr translateIntern(ExprBinary e, ImTranslator t, ImFunction f)
         ImExpr left = e.getLeft().imTranslateExpr(t, f);
         ImExpr right = e.getRight().imTranslateExpr(t, f);
         WurstOperator op = e.getOp();
-        if (e.attrFuncLink() != null) {
+        FuncLink overloadedOperator = e.attrFuncLink();
+        if (op == WurstOperator.PLUS && overloadedOperator == null) {
+            left = wrapImplicitToString(e, e.getLeft(), left, t);
+            right = wrapImplicitToString(e, e.getRight(), right, t);
+        }
+        if (overloadedOperator != null) {
             // overloaded operator
             ImFunction calledFunc = t.getFuncFor(e.attrFuncDef());
             return ImFunctionCall(e, calledFunc, ImTypeArguments(), ImExprs(left, right), false, CallType.NORMAL);
@@ -199,6 +204,30 @@ public static ImExpr translateIntern(ExprBinary e, ImTranslator t, ImFunction f)
         return ImOperatorCall(op, ImExprs(left, right));
     }
 
+    private static ImExpr wrapImplicitToString(ExprBinary concat, Expr operand, ImExpr translated,
+                                               ImTranslator t) {
+        FuncLink toString = AttrFuncDef.implicitToStringForConcatOperand(concat, operand);
+        if (toString == null) {
+            return translated;
+        }
+
+        FunctionDefinition calledFunc = toString.getDef().attrRealFuncDef();
+        FunctionSignature signature = FunctionSignature.fromNameLink(toString);
+        if (calledFunc instanceof FuncDef
+                && !((FuncDef) calledFunc).attrIsStatic()
+                && operand.attrTyp().allowsDynamicDispatch()) {
+            ImMethod method = t.getMethodFor((FuncDef) calledFunc);
+            ImTypeArguments typeArguments = getFunctionCallTypeArguments(
+                t, signature, operand, method.getImplementation().getTypeVariables());
+            return ImMethodCall(operand, method, typeArguments, translated, ImExprs(), false);
+        }
+
+        ImFunction calledImFunc = t.getFuncFor(calledFunc);
+        ImTypeArguments typeArguments = getFunctionCallTypeArguments(
+            t, signature, operand, calledImFunc.getTypeVariables());
+        return ImFunctionCall(operand, calledImFunc, typeArguments, ImExprs(translated), false, CallType.NORMAL);
+    }
+
     public static ImExpr translateIntern(ExprUnary e, ImTranslator t, ImFunction f) {
         return ImOperatorCall(e.getOpU(), ImExprs(e.getRight().imTranslateExpr(t, f)));
     }

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

@@ -2,12 +2,13 @@
 
 import com.google.common.collect.*;
 import de.peeeq.wurstscript.WLogger;
+import de.peeeq.wurstscript.WurstOperator;
 import de.peeeq.wurstscript.ast.*;
+import de.peeeq.wurstscript.attributes.AttrFuncDef;
 import de.peeeq.wurstscript.attributes.CofigOverridePackages;
 import de.peeeq.wurstscript.attributes.CompileError;
 import de.peeeq.wurstscript.attributes.ImplicitFuncs;
 import de.peeeq.wurstscript.attributes.OverloadingResolver;
-import de.peeeq.wurstscript.attributes.AttrFuncDef;
 import de.peeeq.wurstscript.attributes.names.DefLink;
 import de.peeeq.wurstscript.attributes.names.FuncLink;
 import de.peeeq.wurstscript.attributes.names.NameLink;
@@ -333,6 +334,16 @@ private void collectUsedPackages(Set<PackageOrGlobal> used, Element root) {
                 if (def != null) {
                     used.add(def.getDef().attrNearestPackage());
                 }
+                if (def == null) {
+                    FuncLink leftConversion = AttrFuncDef.implicitToStringForConcatOperand(binop, binop.getLeft());
+                    if (leftConversion != null) {
+                        used.add(leftConversion.getDef().attrNearestPackage());
+                    }
+                    FuncLink rightConversion = AttrFuncDef.implicitToStringForConcatOperand(binop, binop.getRight());
+                    if (rightConversion != null) {
+                        used.add(rightConversion.getDef().attrNearestPackage());
+                    }
+                }
             }
 
             if (e instanceof Expr) {
@@ -2080,12 +2091,58 @@ private void visit(ExprBinary expr) {
             FunctionSignature sig = FunctionSignature.fromNameLink(def);
             CallSignature callSig = new CallSignature(expr.getLeft(), Collections.singletonList(expr.getRight()));
             callSig.checkSignatureCompatibility(sig, "" + expr.getOp(), expr);
+        } else {
+            checkNameRefDeprecated(expr, AttrFuncDef.implicitToStringForConcatOperand(expr, expr.getLeft()));
+            checkNameRefDeprecated(expr, AttrFuncDef.implicitToStringForConcatOperand(expr, expr.getRight()));
         }
     }
 
     private void visit(ExprMemberMethod stmtCall) {
         // calculating the exprType should reveal all errors:
         stmtCall.attrTyp();
+        if (stmtCall.attrCompilationUnit().getCuInfo().isLibrary()) {
+            return;
+        }
+        if (!(stmtCall instanceof ExprMemberMethodDot)
+                || !stmtCall.getFuncName().equals("toString")
+                || !stmtCall.getArgs().isEmpty()
+                || !(stmtCall.getParent() instanceof ExprBinary)) {
+            return;
+        }
+        ExprBinary concat = (ExprBinary) stmtCall.getParent();
+        if (concat.getOp() != WurstOperator.PLUS) {
+            return;
+        }
+        Expr other = concat.getLeft() == stmtCall ? concat.getRight() : concat.getLeft();
+        if (!(other.attrTyp() instanceof WurstTypeString)) {
+            return;
+        }
+        if (stmtCall.getLeft().attrTyp() instanceof WurstTypeString) {
+            // Removing the explicit call would leave an ordinary string operand, so the
+            // implicit conversion path would not invoke this potentially non-identity method.
+            return;
+        }
+        Expr replacement = stmtCall.getLeft();
+        if (concat.getLeft() == stmtCall
+                && AttrFuncDef.hasApplicablePlusOverload(replacement, concat.getRight().attrTyp())) {
+            return;
+        }
+        if (concat.getRight() == stmtCall
+                && AttrFuncDef.hasApplicablePlusOverload(concat.getLeft(), replacement.attrTyp())) {
+            return;
+        }
+        FuncLink explicit = stmtCall.attrFuncLink();
+        if (explicit != null
+                && stmtCall.getLeft() instanceof ExprThis
+                && explicit.getDef() == stmtCall.attrNearestFuncDef()) {
+            // Explicit recursive calls on this are lowered statically. Removing the call would
+            // make the implicit conversion dispatch virtually and could select an override.
+            return;
+        }
+        FuncLink inferred = AttrFuncDef.findToStringConversion(stmtCall.getLeft());
+        if (explicit != null && explicit.equals(inferred)) {
+            stmtCall.addWarning(AttrFuncDef.REDUNDANT_TO_STRING_WARNING);
+        }
     }
 
     private void visit(ExprNewObject stmtCall) {