Properly report errors on duplicated constructor names (#8438)

engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/analyse/AliasAnalysis.scala

@@ -26,6 +26,7 @@ import org.enso.compiler.core.ir.expression.{
   Operator,
   Section
 }
+import org.enso.compiler.core.ir.expression.errors.Redefined
 import org.enso.compiler.core.ir.`type`
 import org.enso.compiler.pass.IRPass
 import org.enso.compiler.pass.analyse.AliasAnalysis.Graph.{Occurrence, Scope}
@@ -573,12 +574,16 @@ case object AliasAnalysis extends IRPass {
               new MetadataPair(this, Info.Occurrence(graph, occurrenceId))
             )
         } else {
-          throw new CompilerError(
-            s"""
-                Arguments should never be redefined. This is a bug.
-                ${}
-                """
-          )
+          val f = scope.occurrences.collectFirst {
+            case x if x.symbol == name.name => x
+          }
+          arg
+            .copy(
+              ascribedType = Some(new Redefined.Arg(name, arg.location))
+            )
+            .updateMetadata(
+              new MetadataPair(this, Info.Occurrence(graph, f.get.id))
+            )
         }
     }
   }

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

@@ -581,6 +581,107 @@ object Redefined {
       s"(Redefined (Atom $typeName))"
   }
 
+  /** An error representing the redefinition of an atom in a given module.
+    *
+    * @param name    the name of the atom being redefined
+    * @param location    the location in the source to which this error
+    *                    corresponds
+    * @param passData    the pass metadata for the error
+    * @param diagnostics any diagnostics associated with this error.
+    */
+  sealed case class Arg(
+    name: Name,
+    override val location: Option[IdentifiedLocation],
+    override val passData: MetadataStorage      = new MetadataStorage(),
+    override val diagnostics: DiagnosticStorage = DiagnosticStorage()
+  ) extends Redefined
+      with Diagnostic.Kind.Interactive
+      with module.scope.Definition
+      with IRKind.Primitive {
+    var id: UUID @Identifier = randomId
+
+    /** Creates a copy of `this`.
+      *
+      * @param name    the name of the atom the method was being redefined
+      *                    on
+      * @param location    the location in the source to which this error
+      *                    corresponds
+      * @param passData    the pass metadata for the error
+      * @param diagnostics any diagnostics associated with this error.
+      * @param id          the identifier for the node
+      * @return a copy of `this`, updated with the specified values
+      */
+    def copy(
+      name: Name                           = name,
+      location: Option[IdentifiedLocation] = location,
+      passData: MetadataStorage            = passData,
+      diagnostics: DiagnosticStorage       = diagnostics,
+      id: UUID                             = id
+    ): Arg = {
+      val res =
+        Arg(name, location, passData, diagnostics)
+      res.id = id
+      res
+    }
+
+    /** @inheritdoc */
+    override def duplicate(
+      keepLocations: Boolean   = true,
+      keepMetadata: Boolean    = true,
+      keepDiagnostics: Boolean = true,
+      keepIdentifiers: Boolean = false
+    ): Arg =
+      copy(
+        name = name.duplicate(
+          keepLocations,
+          keepMetadata,
+          keepDiagnostics,
+          keepIdentifiers
+        ),
+        location = if (keepLocations) location else None,
+        passData =
+          if (keepMetadata) passData.duplicate else new MetadataStorage(),
+        diagnostics =
+          if (keepDiagnostics) diagnostics.copy else DiagnosticStorage(),
+        id = if (keepIdentifiers) id else randomId
+      )
+
+    /** @inheritdoc */
+    override def setLocation(location: Option[IdentifiedLocation]): Arg =
+      copy(location = location)
+
+    /** @inheritdoc */
+    override def message(source: (IdentifiedLocation => String)): String =
+      s"Redefining arguments is not supported: ${name.name} is " +
+      s"defined multiple times."
+
+    override def diagnosticKeys(): Array[Any] = Array(name.name)
+
+    /** @inheritdoc */
+    override def mapExpressions(
+      fn: java.util.function.Function[Expression, Expression]
+    ): Arg = this
+
+    /** @inheritdoc */
+    override def toString: String =
+      s"""
+         |Error.Redefined.Arg(
+         |name = $name,
+         |location = $location,
+         |passData = ${this.showPassData},
+         |diagnostics = $diagnostics,
+         |id = $id
+         |)
+         |""".stripMargin
+
+    /** @inheritdoc */
+    override def children: List[IR] = List(name)
+
+    /** @inheritdoc */
+    override def showCode(indent: Int): String =
+      s"(Redefined (Argument $name))"
+  }
+
   /** An error representing the redefinition of a binding in a given scope.
     *
     * While bindings in child scopes are allowed to _shadow_ bindings in

engine/runtime/src/main/scala/org/enso/interpreter/runtime/IrToTruffle.scala

@@ -1842,6 +1842,10 @@ class IrToTruffle(
           context.getBuiltins
             .error()
             .makeCompileError(Text.create(err.message(fileLocationFromSection)))
+        case err: errors.Redefined.Arg =>
+          context.getBuiltins
+            .error()
+            .makeCompileError(Text.create(err.message(fileLocationFromSection)))
         case err: errors.Unexpected.TypeSignature =>
           context.getBuiltins
             .error()

engine/runtime/src/test/java/org/enso/compiler/ExecCompilerTest.java

@@ -1,6 +1,5 @@
 package org.enso.compiler;
 
-import java.io.OutputStream;
 import java.nio.file.Paths;
 import java.util.logging.Level;
 
@@ -10,6 +9,7 @@
 import org.graalvm.polyglot.io.IOAccess;
 import org.junit.AfterClass;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -77,6 +77,25 @@ public void testHalfAssignment() throws Exception {
     }
   }
 
+  @Test
+  public void redefinedArgument() throws Exception {
+    var module = ctx.eval("enso", """
+    type My_Type
+        Value a b c a
+    """);
+    var run = module.invokeMember("eval_expression", "My_Type.Value");
+    var atom = run.newInstance(1, 2, 3, 4);
+    assertFalse("In spite of error we get an instance back: " + atom, atom.isException());
+    assertEquals("Just three keys", 3, atom.getMemberKeys().size());
+    assertTrue("Check a: " + atom.getMemberKeys(), atom.getMemberKeys().contains("a"));
+    assertTrue("Check b: " + atom.getMemberKeys(), atom.getMemberKeys().contains("b"));
+    assertTrue("Check c: " + atom.getMemberKeys(), atom.getMemberKeys().contains("c"));
+
+    assertEquals("c", 3, atom.getMember("c").asInt());
+    assertEquals("b", 2, atom.getMember("b").asInt());
+    assertEquals("First value of a is taken", 1, atom.getMember("a").asInt());
+  }
+
   @Test
   public void testSelfAssignment() throws Exception {
     var module = ctx.eval("enso", """

engine/runtime/src/test/java/org/enso/compiler/ExecStrictCompilerTest.java

@@ -0,0 +1,77 @@
+package org.enso.compiler;
+
+import java.io.ByteArrayOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Paths;
+import java.util.logging.Level;
+
+import org.enso.polyglot.RuntimeOptions;
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.PolyglotException;
+import org.graalvm.polyglot.io.IOAccess;
+import org.junit.AfterClass;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class ExecStrictCompilerTest {
+  private static Context ctx;
+  private static final ByteArrayOutputStream MESSAGES = new ByteArrayOutputStream();
+
+  @BeforeClass
+  public static void initEnsoContext() {
+    ctx = Context.newBuilder()
+        .allowExperimentalOptions(true)
+        .allowIO(IOAccess.ALL)
+        .option(
+            RuntimeOptions.LANGUAGE_HOME_OVERRIDE,
+            Paths.get("../../distribution/component").toFile().getAbsolutePath()
+        )
+        .option(RuntimeOptions.STRICT_ERRORS, "true")
+        .option(
+                RuntimeOptions.LOG_LEVEL,
+                Level.WARNING.getName()
+        )
+        .logHandler(System.err)
+        .out(MESSAGES)
+        .err(MESSAGES)
+        .allowAllAccess(true)
+        .build();
+    assertNotNull("Enso language is supported", ctx.getEngine().getLanguages().get("enso"));
+  }
+
+  @Before
+  public void cleanMessages() {
+    MESSAGES.reset();
+  }
+
+  @AfterClass
+  public static void closeEnsoContext() throws Exception {
+    ctx.close();
+  }
+
+  @Test
+  public void redefinedArgument() throws Exception {
+    var module = ctx.eval("enso", """
+    type My_Type
+        Value a b c a
+    """);
+    try {
+      var run = module.invokeMember("eval_expression", "My_Type.Value");
+      fail("Expecting no returned value: " + run);
+    } catch (PolyglotException ex) {
+      assertEquals("Compilation aborted due to errors.", ex.getMessage());
+      assertTrue("Syntax error", ex.isSyntaxError());
+      assertTrue("Guest exception", ex.isGuestException());
+
+      var errors = new String(MESSAGES.toByteArray(), StandardCharsets.UTF_8);
+      assertNotEquals("Errors reported in " + errors, -1, errors.indexOf("Redefining arguments is not supported"));
+      assertNotEquals("Identifier recognized in " + errors, -1, errors.indexOf("a is defined multiple times"));
+    }
+  }
+}