feat(compiler): Add warning for non-integer array indices (#2077)

Co-authored-by: Oscar Spencer <oscar.spen@gmail.com>

compiler/src/parsing/well_formedness.re

@@ -839,6 +839,46 @@ let mutual_rec_type_improper_rec_keyword = (errs, super) => {
   };
 };
 
+let array_index_non_integer = (errs, super) => {
+  let enter_expression = ({pexp_desc: desc, pexp_loc: loc} as e) => {
+    switch (desc) {
+    | PExpArrayGet(_, {pexp_desc: PExpConstant(PConstNumber(number_type))})
+    | PExpArraySet(
+        _,
+        {pexp_desc: PExpConstant(PConstNumber(number_type))},
+        _,
+      ) =>
+      switch (number_type) {
+      | PConstNumberFloat({txt}) =>
+        let warning = Warnings.ArrayIndexNonInteger(txt);
+        if (Warnings.is_active(warning)) {
+          Location.prerr_warning(loc, warning);
+        };
+      | PConstNumberRational({
+          numerator: {txt: numerator},
+          denominator: {txt: denominator},
+        }) =>
+        let warning =
+          Warnings.ArrayIndexNonInteger(numerator ++ "/" ++ denominator);
+        if (Grain_utils.Warnings.is_active(warning)) {
+          Location.prerr_warning(loc, warning);
+        };
+      | _ => ()
+      }
+    | _ => ()
+    };
+    super.enter_expression(e);
+  };
+
+  {
+    errs,
+    iter_hooks: {
+      ...super,
+      enter_expression,
+    },
+  };
+};
+
 let compose_well_formedness = ({errs, iter_hooks}, cur) =>
   cur(errs, iter_hooks);
 
@@ -854,6 +894,7 @@ let well_formedness_checks = [
   no_local_include,
   provided_multiple_times,
   mutual_rec_type_improper_rec_keyword,
+  array_index_non_integer,
 ];
 
 let well_formedness_checker = () =>

compiler/src/utils/warnings.re

@@ -41,9 +41,10 @@ type t =
   | FromNumberLiteral(number_type, string, string)
   | UselessRecordSpread
   | PrintUnsafe(string)
-  | ToStringUnsafe(string);
+  | ToStringUnsafe(string)
+  | ArrayIndexNonInteger(string);
 
-let last_warning_number = 21;
+let last_warning_number = 22;
 
 let number =
   fun
@@ -67,7 +68,8 @@ let number =
   | FromNumberLiteral(_, _, _) => 18
   | UselessRecordSpread => 19
   | PrintUnsafe(_) => 20
-  | ToStringUnsafe(_) => last_warning_number;
+  | ToStringUnsafe(_) => 21
+  | ArrayIndexNonInteger(_) => last_warning_number;
 
 let message =
   fun
@@ -171,7 +173,9 @@ let message =
   | ToStringUnsafe(typ) =>
     "it looks like you are using `toString` on an unsafe Wasm value here.\nThis is generally unsafe and will cause errors. Use `DebugPrint.toString`"
     ++ typ
-    ++ " from the `runtime/debugPrint` module instead.";
+    ++ " from the `runtime/debugPrint` module instead."
+  | ArrayIndexNonInteger(idx) =>
+    "Array index should be an integer, but found `" ++ idx ++ "`.";
 
 let sub_locs =
   fun
@@ -218,6 +222,7 @@ let defaults = [
   UselessRecordSpread,
   PrintUnsafe(""),
   ToStringUnsafe(""),
+  ArrayIndexNonInteger(""),
 ];
 
 let _ = List.iter(x => current^.active[number(x)] = true, defaults);

compiler/src/utils/warnings.rei

@@ -55,7 +55,8 @@ type t =
   | FromNumberLiteral(number_type, string, string)
   | UselessRecordSpread
   | PrintUnsafe(string)
-  | ToStringUnsafe(string);
+  | ToStringUnsafe(string)
+  | ArrayIndexNonInteger(string);
 
 let is_active: t => bool;
 let is_error: t => bool;

compiler/test/suites/arrays.re

@@ -2,6 +2,10 @@ open Grain_tests.TestFramework;
 open Grain_tests.Runner;
 open Grain_tests.Test_utils;
 open Grain_parsing.Location;
+open Grain_utils;
+
+let {describe} =
+  describeConfig |> withCustomMatchers(customMatchers) |> build;
 
 describe("arrays", ({test, testSkip}) => {
   let test_or_skip =
@@ -12,6 +16,7 @@ describe("arrays", ({test, testSkip}) => {
   let assertRun = makeRunner(test_or_skip);
   let assertRunError = makeErrorRunner(test_or_skip);
   let assertParse = makeParseRunner(test);
+  let assertWarning = makeWarningRunner(test);
 
   assertRun("array1", "print([> 1, 2, 3])", "[> 1, 2, 3]\n");
   assertRun("array2", "print([>])", "[> ]\n");
@@ -45,12 +50,12 @@ describe("arrays", ({test, testSkip}) => {
   );
   assertRunError(
     "array_access_err5",
-    "let x = [> 1, 2, 3]; x[1.5]",
+    "let x = [> 1, 2, 3]; let i = 1.5; x[i]",
     "Index not an integer",
   );
   assertRunError(
     "array_access_err6",
-    "let x = [> 1, 2, 3]; x[1/3]",
+    "let x = [> 1, 2, 3]; let i = 1/3; x[i]",
     "Index not an integer",
   );
   assertRunError(
@@ -90,12 +95,12 @@ describe("arrays", ({test, testSkip}) => {
   );
   assertRunError(
     "array_set_err3",
-    "let x = [> 1, 2, 3]; x[1.5] = 4",
+    "let x = [> 1, 2, 3]; let i = 1.5; x[i] = 4",
     "Index not an integer",
   );
   assertRunError(
     "array_set_err4",
-    "let x = [> 1, 2, 3]; x[1/3] = 4",
+    "let x = [> 1, 2, 3]; let i = 1/3; x[i] = 4",
     "Index not an integer",
   );
   assertRunError(
@@ -113,6 +118,37 @@ describe("arrays", ({test, testSkip}) => {
     "let x = [> true, false, false]; (x[1] = true) + 3",
     "has type Void but",
   );
+  // Ahead of time, float index detection
+  assertWarning(
+    "array_float_get_index0",
+    "let x = [> 1, 2, 3]; x[1.5]",
+    Warnings.ArrayIndexNonInteger("1.5"),
+  );
+  assertWarning(
+    "array_float_get_index1",
+    "let x = [> 1, 2, 3]; x[1.0]",
+    Warnings.ArrayIndexNonInteger("1.0"),
+  );
+  assertWarning(
+    "array_float_get_index2",
+    "let x = [> 1, 2, 3]; x[1/3]",
+    Warnings.ArrayIndexNonInteger("1/3"),
+  );
+  assertWarning(
+    "array_float_set_index0",
+    "let x = [> 1, 2, 3]; x[1.5] = 1",
+    Warnings.ArrayIndexNonInteger("1.5"),
+  );
+  assertWarning(
+    "array_float_set_index1",
+    "let x = [> 1, 2, 3]; x[1.0] = 1",
+    Warnings.ArrayIndexNonInteger("1.0"),
+  );
+  assertWarning(
+    "array_float_set_index2",
+    "let x = [> 1, 2, 3]; x[1/3] = 1",
+    Warnings.ArrayIndexNonInteger("1/3"),
+  );
   // trailing commas
   assertSnapshot("array1_trailing", "[> 1, 2, 3,]");
   assertSnapshot("array1_trailing_space", "[> 1, 2, 3, ]");