Fix rendering of redefined Conversion error

engine/runtime-parser/src/main/java/org/enso/compiler/core/TreeToIr.java

@@ -251,7 +251,7 @@ private List<Definition> translateModuleSymbolImpl(Tree inputAst, List<Definitio
           methodRef,
           args,
           body,
-          getIdentifiedLocation(inputAst, 0, 1, null),
+          getIdentifiedLocation(inputAst, 0, 0, null),
           meta(), diag()
         );
         yield join(binding, appendTo);

engine/runtime-parser/src/main/scala/org/enso/compiler/core/ir/expression/errors/Redefined.scala

@@ -109,7 +109,7 @@ object Redefined {
   }
 
   /** An error representing the redefinition of a conversion in a given
-    * module. This is also known as a method overload.
+    * module
     *
     * @param targetType  the name of the atom the conversion was being
     *                    redefined on
@@ -195,7 +195,7 @@ object Redefined {
 
     /** @inheritdoc */
     override def message(source: Source): String =
-      s"Method overloads are not supported: ${targetType.map(_.name + ".").getOrElse("")}from " +
+      s"Ambiguous conversion: ${targetType.map(_.name + ".").getOrElse("")}from " +
       s"${sourceType.showCode()} is defined multiple times in this module."
 
     override def diagnosticKeys(): Array[Any] = targetType
@@ -230,7 +230,7 @@ object Redefined {
 
     /** @inheritdoc */
     override def showCode(indent: Int): String =
-      s"(Redefined (Conversion ${targetType.map(_.showCode() + ".").getOrElse("")}from $sourceType))"
+      s"(Redefined (Conversion ${targetType.map(_.showCode() + ".").getOrElse("")}from ${sourceType.showCode()}))"
   }
 
   /** An error representing the redefinition of a method in a given module.

engine/runtime-with-instruments/src/test/scala/org/enso/interpreter/test/instrument/ReplTest.scala

@@ -102,7 +102,7 @@ class ReplTest
           result.toString shouldEqual "Error in method `to_text` of [Bar 1]: Expected Text but got 42"
         }
         inside(executor.evaluate("C.Baz 1")) { case Right(result) =>
-          result.toString shouldEqual "Error in method `to_text` of [Baz 1]: Expected Text but got C.to_text[Test:18:1-29]"
+          result.toString shouldEqual "Error in method `to_text` of [Baz 1]: Expected Text but got C.to_text[Test:18:1-28]"
         }
         inside(executor.evaluate("Pattern.compile 'foo'")) {
           case Right(result) =>

engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/ConversionMethodTests.java

@@ -1,26 +1,44 @@
 package org.enso.interpreter.test;
 
-import static org.junit.Assert.assertEquals;
-
 import org.graalvm.polyglot.Context;
 import org.graalvm.polyglot.Value;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
 import org.junit.AfterClass;
+import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
 public class ConversionMethodTests extends TestBase {
   private static Context ctx;
 
+  private static final ByteArrayOutputStream out = new ByteArrayOutputStream();
+
   @BeforeClass
   public static void initCtx() {
-    ctx = createDefaultContext();
+    ctx = createDefaultContext(out);
   }
 
   @AfterClass
   public static void disposeCtx() {
     ctx.close();
   }
 
+  @Before
+  public void resetOutput() {
+    out.reset();
+  }
+
+  private String getStdOut() {
+    return out.toString(StandardCharsets.UTF_8);
+  }
+
   @Test
   public void testSimpleConversion() {
     String src = """
@@ -101,4 +119,26 @@ public void testDispatchOnJSDateTime() {
     Value res = evalModule(ctx, src);
     assertEquals(7, res.asInt());
   }
+
+  @Test
+  public void testAmbiguousConversionStrictUnused() {
+    String src = """      
+       type Foo
+          Mk_Foo data
+       type Bar
+          Mk_Bar x
+
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+100
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+1000
+
+       main = 42
+       """;
+    try {
+      Value res = evalModule(ctx, src);
+      fail("Expected an exception, but got " + res);
+    } catch (Exception e) {
+      assertEquals("Compilation aborted due to errors.", e.getMessage());
+      MatcherAssert.assertThat(getStdOut(), Matchers.containsString("Unnamed:7:1: error: Ambiguous conversion: Foo.from Bar is defined multiple times in this module."));
+    }
+  }
 }

engine/runtime-with-polyglot/src/test/java/org/enso/interpreter/test/NonStrictConversionMethodTests.java

@@ -0,0 +1,74 @@
+package org.enso.interpreter.test;
+
+import org.enso.polyglot.RuntimeOptions;
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.Language;
+import org.graalvm.polyglot.Value;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class NonStrictConversionMethodTests extends TestBase {
+  private static Context nonStrictCtx;
+
+  @BeforeClass
+  public static void initCtx() {
+    nonStrictCtx = createNonStrictContext();
+  }
+
+  protected static Context createNonStrictContext() {
+    var context =
+        defaultContextBuilder().option(RuntimeOptions.STRICT_ERRORS, "false").build();
+    final Map<String, Language> langs = context.getEngine().getLanguages();
+    assertNotNull("Enso found: " + langs, langs.get("enso"));
+    return context;
+  }
+
+  @AfterClass
+  public static void disposeCtx() {
+    nonStrictCtx.close();
+  }
+
+  @Test
+  public void testAmbiguousConversion() {
+    String src = """      
+       type Foo
+          Mk_Foo data
+       type Bar
+          Mk_Bar x
+
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+100
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+1000
+
+       main = 42
+       """;
+    Value res = evalModule(nonStrictCtx, src);
+    assertEquals(42, res.asInt());
+  }
+
+  @Test
+  public void testAmbiguousConversionUsage() {
+    // In non-strict mode, the conversion declarations will have errors attached to the IR, but the overall operation
+    // will simply not see the second conversion and succeed with the first one.
+    String src = """      
+       type Foo
+          Mk_Foo data
+       type Bar
+          Mk_Bar x
+
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+100
+       Foo.from (that:Bar) = Foo.Mk_Foo that.x+1000
+
+       main = (Foo.from (Bar.Mk_Bar 42)) . data
+       """;
+
+    Value res = evalModule(nonStrictCtx, src);
+    assertEquals(142, res.asInt());
+  }
+
+}

engine/runtime/src/test/scala/org/enso/compiler/test/pass/resolve/OverloadsResolutionTest.scala

@@ -96,16 +96,22 @@ class OverloadsResolutionTest extends CompilerTest {
     }
 
     "raise an error if there are multiple definitions with the same source type" in {
-      val ir =
+      val sourceCode =
         """Unit.from (that : Integer) = undefined
           |Unit.from (that : Boolean) = undefined
           |Unit.from (that : Boolean) = undefined
-          |""".stripMargin.preprocessModule.resolve
+          |""".stripMargin
+      val ir = sourceCode.preprocessModule.resolve
 
       ir.bindings.length shouldEqual 3
       ir.bindings.head shouldBe a[definition.Method.Conversion]
       ir.bindings(1) shouldBe a[definition.Method.Conversion]
       ir.bindings(2) shouldBe an[errors.Redefined.Conversion]
+
+      val errLoc = ir.bindings(2).location().get
+      val code   = sourceCode.substring(errLoc.start, errLoc.end)
+      // The code should be the whole line, but without ending newlines.
+      code shouldEqual "Unit.from (that : Boolean) = undefined"
     }
   }
 

engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/OverloadsResolutionErrorTest.scala

@@ -22,7 +22,7 @@ class OverloadsResolutionErrorTest extends InterpreterTest {
     interpreterContext: InterpreterContext
   ): Unit = {
 
-    "result in an error at runtime for method overloads" in {
+    "result in an error at runtime for member method overloads" in {
       val code =
         """import Standard.Base.Nothing
           |
@@ -41,6 +41,23 @@ class OverloadsResolutionErrorTest extends InterpreterTest {
       )
     }
 
+    "result in an error at runtime for module method overloads" in {
+      val code =
+        """bar x y = x + y
+          |bar x = x + 10
+          |""".stripMargin.linesIterator.mkString("\n")
+
+      the[InterpreterException] thrownBy eval(code) should have message
+      "Compilation aborted due to errors."
+
+      val diagnostics = consumeOut
+      diagnostics
+        .filterNot(isDiagnosticLine)
+        .toSet shouldEqual Set(
+        "Test:2:1: error: Method overloads are not supported: bar is defined multiple times in this module."
+      )
+    }
+
     "result in an error at runtime for atom overloads" in {
       val code =
         """
@@ -59,5 +76,56 @@ class OverloadsResolutionErrorTest extends InterpreterTest {
       )
     }
 
+    "result in a compiler error for clashing from conversions" in {
+      val code =
+        """type Foo
+          |    Mk_Foo data
+          |type Bar
+          |    Mk_Bar x
+          |
+          |Foo.from (that : Bar) = Foo.Mk_Foo that.x+100
+          |Foo.from (that : Bar) = Foo.Mk_Foo that.x+200
+          |""".stripMargin.linesIterator.mkString("\n")
+
+      the[InterpreterException] thrownBy eval(code) should have message
+      "Compilation aborted due to errors."
+
+      val diagnostics = consumeOut
+      diagnostics should have length 3
+      val line0 =
+        "Test:7:1: error: Ambiguous conversion: Foo.from Bar is defined multiple times in this module."
+      val line1 = "    7 | Foo.from (that : Bar) = Foo.Mk_Foo that.x+200"
+      val line2 = "      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"
+      diagnostics shouldEqual List(line0, line1, line2)
+    }
+
+    "render the compiler error correctly for clashing multiline from conversions" in {
+      val code =
+        """type Foo
+          |    Mk_Foo data
+          |type Bar
+          |    Mk_Bar x
+          |
+          |Foo.from (that : Bar) =
+          |    y = that.x+100
+          |    Foo.Mk_Foo y
+          |Foo.from (that : Bar) =
+          |    y = that.x+200
+          |    Foo.Mk_Foo y
+          |""".stripMargin.linesIterator.mkString("\n")
+
+      the[InterpreterException] thrownBy eval(code) should have message
+      "Compilation aborted due to errors."
+
+      val diagnostics = consumeOut
+      diagnostics should have length 4
+      val line0 =
+        "Test:[9:1-11:16]: error: Ambiguous conversion: Foo.from Bar is defined multiple times in this module."
+      val line1 = "    9 | Foo.from (that : Bar) ="
+      val line2 = "   10 |     y = that.x+200"
+      val line3 = "   11 |     Foo.Mk_Foo y"
+      diagnostics shouldEqual List(line0, line1, line2, line3)
+    }
+
   }
 }