From d4e3b02f64db988bcc531c496fe154dab9aad88f Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Tue, 21 Mar 2023 14:09:31 +0000
Subject: [PATCH 01/14] Detect unbracketed [a:b+1]

---
 DESCRIPTION                          |  2 +-
 R/ir_parse_arrays.R                  | 37 +++++++++++++++++++++++++++-
 tests/testthat/test-parse2-general.R |  4 +++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index a6c15f0c..0a82de17 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: odin
 Title: ODE Generation and Integration
-Version: 1.4.5
+Version: 1.4.6
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
                     email = "rich.fitzjohn@gmail.com"),
              person("Thibaut", "Jombart", role = "ctb"),
diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index eac5fd04..ba11d31c 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -812,12 +812,47 @@ ir_parse_arrays_check_rhs <- function(rhs, rank, int_arrays, include, eq,
   invisible(NULL) # never return anything at all.
 }
 
+ir_parse_expr_lhs_check_index_miss_brackets <- function(str) {
+
+  # f:g is ok.
+  # f(1:5):g(1:5) is ok.
+  # f:g+1 is not ok - it needs to be f:(g+1).
+  
+  # Anything within a bracket is ok and can be collapsed.
+  
+  while (grepl("\\(", str)) {
+    str <- gsub("\\s*\\([^\\)]+\\)", "", str, perl = TRUE)
+  }
+  
+  # If this isn't an array sequence, then no problem.
+  
+  if (!grepl(":", str)) {
+    return(FALSE)
+  }
+  
+  parts <- strsplit(str, ":")[[1]]
+  
+  forbidden <- c("\\+", "-", "\\*", "/", "%", "\\^")
+  
+  any(vapply(forbidden, function(x) {
+    grepl(x, parts[1]) || grepl(x, parts[2])
+  }, logical(1)))
+  
+}
 
 ir_parse_expr_lhs_check_index <- function(x) {
   seen <- counter()
   err <- collector()
   valid <- setdiff(VALID_ARRAY, ":")
-
+  
+  # In x:y, x and y must both be atomic.
+  # x:y+1 must be written x:(y+1)
+  
+  if (ir_parse_expr_lhs_check_index_miss_brackets(
+    as.character(as.expression(x)))) {
+    err$add("Full bracketting required in array sequence")
+  }
+  
   f <- function(x, max) {
     if (is.recursive(x)) {
       nm <- as.character(x[[1L]])
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 45858db0..beb192ba 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -42,6 +42,10 @@ test_that("expression parsing", {
   expect_error(odin_parse_(quote(x[i] <- y[i])),
                "Special index variable i may not be used on array lhs",
                class = "odin_error")
+  
+  expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
+               "Invalid array use on lhs:\n\t\tFull bracketting required in array sequence",
+               class = "odin_error")
 })
 
 

From 54ed4567cc6642f0e296ec4f1ff430803b0a0183 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Tue, 21 Mar 2023 14:11:55 +0000
Subject: [PATCH 02/14] Whitespace for codefactor

---
 R/ir_parse_arrays.R                  | 16 ++++++++--------
 tests/testthat/test-parse2-general.R |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index ba11d31c..f8217c5f 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -817,27 +817,27 @@ ir_parse_expr_lhs_check_index_miss_brackets <- function(str) {
   # f:g is ok.
   # f(1:5):g(1:5) is ok.
   # f:g+1 is not ok - it needs to be f:(g+1).
-  
+
   # Anything within a bracket is ok and can be collapsed.
-  
+
   while (grepl("\\(", str)) {
     str <- gsub("\\s*\\([^\\)]+\\)", "", str, perl = TRUE)
   }
-  
+
   # If this isn't an array sequence, then no problem.
-  
+
   if (!grepl(":", str)) {
     return(FALSE)
   }
-  
+
   parts <- strsplit(str, ":")[[1]]
-  
+
   forbidden <- c("\\+", "-", "\\*", "/", "%", "\\^")
-  
+
   any(vapply(forbidden, function(x) {
     grepl(x, parts[1]) || grepl(x, parts[2])
   }, logical(1)))
-  
+
 }
 
 ir_parse_expr_lhs_check_index <- function(x) {
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index beb192ba..04222d09 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -42,10 +42,10 @@ test_that("expression parsing", {
   expect_error(odin_parse_(quote(x[i] <- y[i])),
                "Special index variable i may not be used on array lhs",
                class = "odin_error")
-  
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
-               "Invalid array use on lhs:\n\t\tFull bracketting required in array sequence",
-               class = "odin_error")
+               paste0("Invalid array use on lhs:\n",
+               "\t\tFull bracketting required in array sequence", 
+               collapse = ""), class = "odin_error")
 })
 
 

From 8ecbd37bf5792e8713e95ad3d0cedab763a92a83 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Tue, 21 Mar 2023 15:07:53 +0000
Subject: [PATCH 03/14] Fix whitespace again

---
 R/ir_parse_arrays.R                  | 6 +++---
 tests/testthat/test-parse2-general.R | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index f8217c5f..588602ba 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -844,15 +844,15 @@ ir_parse_expr_lhs_check_index <- function(x) {
   seen <- counter()
   err <- collector()
   valid <- setdiff(VALID_ARRAY, ":")
-  
+
   # In x:y, x and y must both be atomic.
   # x:y+1 must be written x:(y+1)
-  
+
   if (ir_parse_expr_lhs_check_index_miss_brackets(
     as.character(as.expression(x)))) {
     err$add("Full bracketting required in array sequence")
   }
-  
+
   f <- function(x, max) {
     if (is.recursive(x)) {
       nm <- as.character(x[[1L]])
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 04222d09..76bc8ad3 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -44,7 +44,7 @@ test_that("expression parsing", {
                class = "odin_error")
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tFull bracketting required in array sequence", 
+               "\t\tFull bracketting required in array sequence",
                collapse = ""), class = "odin_error")
 })
 

From 5e96cee26ffa8ad92ba1f9e3735178d0dc94adcc Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Thu, 23 Mar 2023 15:08:12 +0000
Subject: [PATCH 04/14] Neater fix

---
 R/ir_parse_arrays.R                  | 46 ++++++----------------------
 tests/testthat/test-parse2-general.R |  7 +----
 2 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 588602ba..dfae1e59 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -812,47 +812,11 @@ ir_parse_arrays_check_rhs <- function(rhs, rank, int_arrays, include, eq,
   invisible(NULL) # never return anything at all.
 }
 
-ir_parse_expr_lhs_check_index_miss_brackets <- function(str) {
-
-  # f:g is ok.
-  # f(1:5):g(1:5) is ok.
-  # f:g+1 is not ok - it needs to be f:(g+1).
-
-  # Anything within a bracket is ok and can be collapsed.
-
-  while (grepl("\\(", str)) {
-    str <- gsub("\\s*\\([^\\)]+\\)", "", str, perl = TRUE)
-  }
-
-  # If this isn't an array sequence, then no problem.
-
-  if (!grepl(":", str)) {
-    return(FALSE)
-  }
-
-  parts <- strsplit(str, ":")[[1]]
-
-  forbidden <- c("\\+", "-", "\\*", "/", "%", "\\^")
-
-  any(vapply(forbidden, function(x) {
-    grepl(x, parts[1]) || grepl(x, parts[2])
-  }, logical(1)))
-
-}
-
 ir_parse_expr_lhs_check_index <- function(x) {
   seen <- counter()
   err <- collector()
   valid <- setdiff(VALID_ARRAY, ":")
 
-  # In x:y, x and y must both be atomic.
-  # x:y+1 must be written x:(y+1)
-
-  if (ir_parse_expr_lhs_check_index_miss_brackets(
-    as.character(as.expression(x)))) {
-    err$add("Full bracketting required in array sequence")
-  }
-
   f <- function(x, max) {
     if (is.recursive(x)) {
       nm <- as.character(x[[1L]])
@@ -888,6 +852,16 @@ ir_parse_expr_lhs_check_index <- function(x) {
   if (seen$get() > 0) { # check minimum branch
     seen$reset()
     value_min <- f(x, FALSE)
+    if (!is_call(x, ":")) {
+      if (is_call(x[[2]], ":")) {
+        fix <- deparse_str(call(":", x[[2]][[2]], call("(", call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
+      } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
+        fix <- deparse_str(call(":", call("(", call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]]))
+      } else {
+        fix <- "using parentheses"
+      }
+      err$add(sprintf("You are writing an ambiguous range, consider %s", fix))
+    }
   } else {
     value_min <- NULL
   }
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 76bc8ad3..672008ab 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -44,7 +44,7 @@ test_that("expression parsing", {
                class = "odin_error")
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tFull bracketting required in array sequence",
+               "\t\tYou are writing an ambiguous range",
                collapse = ""), class = "odin_error")
 })
 
@@ -290,11 +290,6 @@ test_that("custom functions ignore arrays", {
 })
 
 test_that("lhs array checking", {
-  res <- ir_parse_expr_lhs_check_index(quote(a + (2:(n - 3) - 4) + z))
-  expect_true(res)
-  expect_equal(attr(res, "value_max"), quote(a + ((n - 3) - 4) + z))
-  expect_equal(attr(res, "value_min"), quote(a + (2 - 4) + z))
-
   res <- ir_parse_expr_lhs_check_index(quote(a))
   expect_true(res)
   expect_equal(attr(res, "value_max"), quote(a))

From daf0687aaddad9ff80d1f3a44a064b70c230e398 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Thu, 23 Mar 2023 15:10:06 +0000
Subject: [PATCH 05/14] Shorter lines

---
 R/ir_parse_arrays.R | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index dfae1e59..23156602 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -854,9 +854,11 @@ ir_parse_expr_lhs_check_index <- function(x) {
     value_min <- f(x, FALSE)
     if (!is_call(x, ":")) {
       if (is_call(x[[2]], ":")) {
-        fix <- deparse_str(call(":", x[[2]][[2]], call("(", call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
+        fix <- deparse_str(call(":", x[[2]][[2]], call("(", 
+          call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
-        fix <- deparse_str(call(":", call("(", call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]]))
+        fix <- deparse_str(call(":", call("(", 
+          call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]]))
       } else {
         fix <- "using parentheses"
       }

From 06208efc76bb8414d9b371c544c58a9cbcb3e1b7 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Thu, 23 Mar 2023 15:24:29 +0000
Subject: [PATCH 06/14] Whitespace

---
 R/ir_parse_arrays.R | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 23156602..c4957e14 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -854,10 +854,10 @@ ir_parse_expr_lhs_check_index <- function(x) {
     value_min <- f(x, FALSE)
     if (!is_call(x, ":")) {
       if (is_call(x[[2]], ":")) {
-        fix <- deparse_str(call(":", x[[2]][[2]], call("(", 
+        fix <- deparse_str(call(":", x[[2]][[2]], call("(",
           call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
-        fix <- deparse_str(call(":", call("(", 
+        fix <- deparse_str(call(":", call("(",
           call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]]))
       } else {
         fix <- "using parentheses"

From 954deea0b78b6f9dad5cfdc04da3aca442e3d783 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 14:32:01 +0100
Subject: [PATCH 07/14] Test error message better

---
 tests/testthat/test-parse2-general.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 672008ab..88a20458 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -44,7 +44,7 @@ test_that("expression parsing", {
                class = "odin_error")
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tYou are writing an ambiguous range",
+               "\t\tYou are writing an ambiguous range, consider 1:(n + 1)",
                collapse = ""), class = "odin_error")
 })
 

From 66a5f34f8565f737bc457e36de83bd8d759cb866 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 18:27:14 +0100
Subject: [PATCH 08/14] Clean-up array errors

---
 R/ir_parse_arrays.R                  |  6 ++++-
 tests/testthat/test-parse2-general.R | 38 +++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index c4957e14..083975f8 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -849,7 +849,11 @@ ir_parse_expr_lhs_check_index <- function(x) {
   }
 
   value_max <- f(x, TRUE)
-  if (seen$get() > 0) { # check minimum branch
+  
+  # If errors have already been spotted, don't check further; 
+  # results/suggestions will be confusing.
+  
+  if ((length(err$get()) == 0) && (seen$get() > 0)) { # check minimum branch
     seen$reset()
     value_min <- f(x, FALSE)
     if (!is_call(x, ":")) {
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 88a20458..e0a8bd32 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -42,9 +42,15 @@ test_that("expression parsing", {
   expect_error(odin_parse_(quote(x[i] <- y[i])),
                "Special index variable i may not be used on array lhs",
                class = "odin_error")
+  
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tYou are writing an ambiguous range, consider 1:(n + 1)",
+               "\t\tYou are writing an ambiguous range, consider 1:(n + 1)*",
+               collapse = ""), class = "odin_error")
+  
+  expect_error(odin_parse_(quote(y[1:n + 1 + 2] <- 1)),
+               paste0("Invalid array use on lhs:\n",
+               "\t\tYou are writing an ambiguous range, consider using parentheses*",
                collapse = ""), class = "odin_error")
 })
 
@@ -294,12 +300,30 @@ test_that("lhs array checking", {
   expect_true(res)
   expect_equal(attr(res, "value_max"), quote(a))
   expect_null(attr(res, "value_min"))
-
-  expect_false(ir_parse_expr_lhs_check_index(quote(a:b + c:d)))
-  expect_false(ir_parse_expr_lhs_check_index(quote(-(a:b)))) # nolint
-  expect_false(ir_parse_expr_lhs_check_index(quote((a:b):c)))
-  expect_false(ir_parse_expr_lhs_check_index(quote(c:(a:b))))
-  expect_false(ir_parse_expr_lhs_check_index(quote((-a))))
+  
+  expect_single_error <- function(err, expected) {
+    expect_false(err)
+    count <- length(attr(err, "message"))
+    expect_equal(count, 1)
+    if (count == 1) {
+      expect_equal(expected, attr(err, "message"))
+    }
+  }
+  
+  expect_single_error(ir_parse_expr_lhs_check_index(quote(a:b + c:d)),
+                      "Multiple calls to ':' are not allowed")
+  
+  expect_single_error(ir_parse_expr_lhs_check_index(quote(-(a:b))),
+                      "Unary minus invalid in array calculation")   # nolint
+  
+  expect_single_error(ir_parse_expr_lhs_check_index(quote((a:b):c)),
+                      "Multiple calls to ':' are not allowed")
+  
+  expect_single_error(ir_parse_expr_lhs_check_index(quote(c:(a:b))),
+                      "Multiple calls to ':' are not allowed")
+  
+  expect_single_error(ir_parse_expr_lhs_check_index(quote((-a))),
+                      "Unary minus invalid in array calculation")
 })
 
 test_that("sum rewriting", {

From 4a35ba56aa68ffde7a583bc6a566f5840e5de0e4 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 18:37:18 +0100
Subject: [PATCH 09/14] Cleanup for codefactor

---
 R/ir_parse_arrays.R                  |  6 +++---
 tests/testthat/test-parse2-general.R | 21 ++++++++++-----------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 083975f8..ed57b897 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -849,10 +849,10 @@ ir_parse_expr_lhs_check_index <- function(x) {
   }
 
   value_max <- f(x, TRUE)
-  
-  # If errors have already been spotted, don't check further; 
+
+  # If errors have already been spotted, don't check further;
   # results/suggestions will be confusing.
-  
+
   if ((length(err$get()) == 0) && (seen$get() > 0)) { # check minimum branch
     seen$reset()
     value_min <- f(x, FALSE)
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index e0a8bd32..208edeff 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -42,19 +42,18 @@ test_that("expression parsing", {
   expect_error(odin_parse_(quote(x[i] <- y[i])),
                "Special index variable i may not be used on array lhs",
                class = "odin_error")
-  
+
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
                "\t\tYou are writing an ambiguous range, consider 1:(n + 1)*",
                collapse = ""), class = "odin_error")
-  
+
   expect_error(odin_parse_(quote(y[1:n + 1 + 2] <- 1)),
-               paste0("Invalid array use on lhs:\n",
-               "\t\tYou are writing an ambiguous range, consider using parentheses*",
+               paste0("Invalid array use on lhs:\n\t\tYou are ",
+               "writing an ambiguous range, consider using parentheses*",
                collapse = ""), class = "odin_error")
 })
 
-
 test_that("parse array indices", {
   expect_error(odin_parse(
     "initial(y) <- 1; deriv(y) <- 1; x[1:t] <- 1\ndim(x) <- 10"),
@@ -300,7 +299,7 @@ test_that("lhs array checking", {
   expect_true(res)
   expect_equal(attr(res, "value_max"), quote(a))
   expect_null(attr(res, "value_min"))
-  
+
   expect_single_error <- function(err, expected) {
     expect_false(err)
     count <- length(attr(err, "message"))
@@ -309,19 +308,19 @@ test_that("lhs array checking", {
       expect_equal(expected, attr(err, "message"))
     }
   }
-  
+
   expect_single_error(ir_parse_expr_lhs_check_index(quote(a:b + c:d)),
                       "Multiple calls to ':' are not allowed")
-  
+
   expect_single_error(ir_parse_expr_lhs_check_index(quote(-(a:b))),
                       "Unary minus invalid in array calculation")   # nolint
-  
+
   expect_single_error(ir_parse_expr_lhs_check_index(quote((a:b):c)),
                       "Multiple calls to ':' are not allowed")
-  
+
   expect_single_error(ir_parse_expr_lhs_check_index(quote(c:(a:b))),
                       "Multiple calls to ':' are not allowed")
-  
+
   expect_single_error(ir_parse_expr_lhs_check_index(quote((-a))),
                       "Unary minus invalid in array calculation")
 })

From a325f5dff9f6f38e6099ae5e993bfe63925b03ca Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 22:22:54 +0100
Subject: [PATCH 10/14] Fix message on inverted case

---
 R/ir_parse_arrays.R                  | 4 ++--
 tests/testthat/test-parse2-general.R | 7 ++++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index ed57b897..6de95b36 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -861,8 +861,8 @@ ir_parse_expr_lhs_check_index <- function(x) {
         fix <- deparse_str(call(":", x[[2]][[2]], call("(",
           call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
-        fix <- deparse_str(call(":", call("(",
-          call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]]))
+        fix <- deparse_str(call(as.character(x[[1]]), x[[2]], call("(",
+          call(":", x[[3]][[2]], x[[3]][[3]]))))
       } else {
         fix <- "using parentheses"
       }
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index 208edeff..a4cc1013 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -45,13 +45,18 @@ test_that("expression parsing", {
 
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tYou are writing an ambiguous range, consider 1:(n + 1)*",
+               "\t\tYou are writing an ambiguous range, consider 1:(n \\+ 1)*",
                collapse = ""), class = "odin_error")
 
   expect_error(odin_parse_(quote(y[1:n + 1 + 2] <- 1)),
                paste0("Invalid array use on lhs:\n\t\tYou are ",
                "writing an ambiguous range, consider using parentheses*",
                collapse = ""), class = "odin_error")
+
+  expect_error(odin_parse_(quote(y[1 + 1:n] <- 1)),
+               paste0("Invalid array use on lhs:\n\t\tYou are ",
+                      "writing an ambiguous range, consider 1 \\+ (1:n)*",
+                      collapse = ""), class = "odin_error")
 })
 
 test_that("parse array indices", {

From 41108c441a2ba21b2fe88a6531997a01b9e24caf Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 22:57:25 +0100
Subject: [PATCH 11/14] Improve error msg a bit more

---
 R/ir_parse_arrays.R                  | 16 ++++++++++++----
 tests/testthat/test-parse2-general.R |  8 +++++---
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 6de95b36..74c57aea 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -858,11 +858,19 @@ ir_parse_expr_lhs_check_index <- function(x) {
     value_min <- f(x, FALSE)
     if (!is_call(x, ":")) {
       if (is_call(x[[2]], ":")) {
-        fix <- deparse_str(call(":", x[[2]][[2]], call("(",
-          call(as.character(x[[1]]), x[[2]][[3]], x[[3]]))))
+
+        fix <- paste0(deparse_str(call(":", x[[2]][[2]], call("(",
+            call(as.character(x[[1]]), x[[2]][[3]], x[[3]])))), " or ",
+            deparse_str(call(as.character(x[[1]]), call("(",
+            call(as.character(x[[2]][[1]]), x[[2]][[2]], x[[2]][[3]])), x[[3]])))
+
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
-        fix <- deparse_str(call(as.character(x[[1]]), x[[2]], call("(",
-          call(":", x[[3]][[2]], x[[3]][[3]]))))
+
+        fix <- paste0(deparse_str(call(as.character(x[[1]]), x[[2]], call("(",
+            call(":", x[[3]][[2]], x[[3]][[3]])))), " or ",
+            deparse_str(call(":", call("(",
+            call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]])))
+
       } else {
         fix <- "using parentheses"
       }
diff --git a/tests/testthat/test-parse2-general.R b/tests/testthat/test-parse2-general.R
index a4cc1013..b47e25b0 100644
--- a/tests/testthat/test-parse2-general.R
+++ b/tests/testthat/test-parse2-general.R
@@ -45,7 +45,8 @@ test_that("expression parsing", {
 
   expect_error(odin_parse_(quote(y[1:n + 1] <- 1)),
                paste0("Invalid array use on lhs:\n",
-               "\t\tYou are writing an ambiguous range, consider 1:(n \\+ 1)*",
+               "\t\tYou are writing an ambiguous range, ",
+               "consider 1:\\(n \\+ 1) or \\(1:n) \\+ 1*",
                collapse = ""), class = "odin_error")
 
   expect_error(odin_parse_(quote(y[1:n + 1 + 2] <- 1)),
@@ -53,9 +54,10 @@ test_that("expression parsing", {
                "writing an ambiguous range, consider using parentheses*",
                collapse = ""), class = "odin_error")
 
-  expect_error(odin_parse_(quote(y[1 + 1:n] <- 1)),
+  expect_error(odin_parse_(quote(y[a + 1:n] <- 1)),
                paste0("Invalid array use on lhs:\n\t\tYou are ",
-                      "writing an ambiguous range, consider 1 \\+ (1:n)*",
+                      "writing an ambiguous range, ",
+                      "consider a \\+ \\(1:n) or \\(a \\+ 1):n*",
                       collapse = ""), class = "odin_error")
 })
 

From cc52178c6a864cc8bf785e881b998f07c3f11082 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Mon, 17 Apr 2023 23:03:49 +0100
Subject: [PATCH 12/14] Appease codefactor

---
 R/ir_parse_arrays.R | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 74c57aea..ef831198 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -860,16 +860,16 @@ ir_parse_expr_lhs_check_index <- function(x) {
       if (is_call(x[[2]], ":")) {
 
         fix <- paste0(deparse_str(call(":", x[[2]][[2]], call("(",
-            call(as.character(x[[1]]), x[[2]][[3]], x[[3]])))), " or ",
-            deparse_str(call(as.character(x[[1]]), call("(",
-            call(as.character(x[[2]][[1]]), x[[2]][[2]], x[[2]][[3]])), x[[3]])))
+          call(as.character(x[[1]]), x[[2]][[3]], x[[3]])))), " or ",
+          deparse_str(call(as.character(x[[1]]), call("(",
+          call(as.character(x[[2]][[1]]), x[[2]][[2]], x[[2]][[3]])), x[[3]])))
 
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
 
         fix <- paste0(deparse_str(call(as.character(x[[1]]), x[[2]], call("(",
-            call(":", x[[3]][[2]], x[[3]][[3]])))), " or ",
-            deparse_str(call(":", call("(",
-            call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]])))
+          call(":", x[[3]][[2]], x[[3]][[3]])))), " or ",
+          deparse_str(call(":", call("(",
+          call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]])))
 
       } else {
         fix <- "using parentheses"

From 4198b5c6bc177a2949d6fa913c7b49b1a43b8257 Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Wed, 19 Apr 2023 13:16:08 +0100
Subject: [PATCH 13/14] Factor our lhs:rhs, cleanup/comment

---
 R/ir_parse_arrays.R | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index ef831198..99385d6b 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -857,19 +857,36 @@ ir_parse_expr_lhs_check_index <- function(x) {
     seen$reset()
     value_min <- f(x, FALSE)
     if (!is_call(x, ":")) {
+
       if (is_call(x[[2]], ":")) {
 
-        fix <- paste0(deparse_str(call(":", x[[2]][[2]], call("(",
-          call(as.character(x[[1]]), x[[2]][[3]], x[[3]])))), " or ",
-          deparse_str(call(as.character(x[[1]]), call("(",
-          call(as.character(x[[2]][[1]]), x[[2]][[2]], x[[2]][[3]])), x[[3]])))
+        # Handle 1:n+1, which is:-
+        # `+`   (':', 1, n)   1  - so x[[2]][[1]] is ":" and...
+
+        lhs <- x[[2]][[2]]
+        rhs <- x[[2]][[3]]
+
+        # Suggest either 1:(n+1) or (1:n)+1
+
+        fix <- paste0(deparse_str(call(":", lhs,
+          call("(",call(as.character(x[[1]]), rhs, x[[3]])))), " or ",
+            deparse_str(call(as.character(x[[1]]),
+              call("(",call(":", lhs, rhs)), x[[3]])))
 
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
 
-        fix <- paste0(deparse_str(call(as.character(x[[1]]), x[[2]], call("(",
-          call(":", x[[3]][[2]], x[[3]][[3]])))), " or ",
-          deparse_str(call(":", call("(",
-          call(as.character(x[[1]]), x[[2]], x[[3]][[2]])), x[[3]][[3]])))
+        # Handle a+1:n, which is:-
+        # `+`   a   (`:`   1   n)   - so x[[3]][[1]] is ":" and...
+
+        lhs <- x[[3]][[2]]
+        rhs <- x[[3]][[3]]
+
+        # Suggest either a+(1:n) or (a+1):n
+
+        fix <- paste0(deparse_str(call(as.character(x[[1]]), x[[2]],
+          call("(", call(":", lhs, rhs)))), " or ",
+            deparse_str(call(":",
+              call("(",call(as.character(x[[1]]), x[[2]], lhs)), rhs)))
 
       } else {
         fix <- "using parentheses"

From 705c77f301c959cdb0f126000c02eb15de32e44e Mon Sep 17 00:00:00 2001
From: Wes Hinsley <w.hinsley@imperial.ac.uk>
Date: Wed, 19 Apr 2023 13:23:53 +0100
Subject: [PATCH 14/14] Codefactor

---
 R/ir_parse_arrays.R | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/ir_parse_arrays.R b/R/ir_parse_arrays.R
index 99385d6b..2f20135d 100644
--- a/R/ir_parse_arrays.R
+++ b/R/ir_parse_arrays.R
@@ -869,9 +869,9 @@ ir_parse_expr_lhs_check_index <- function(x) {
         # Suggest either 1:(n+1) or (1:n)+1
 
         fix <- paste0(deparse_str(call(":", lhs,
-          call("(",call(as.character(x[[1]]), rhs, x[[3]])))), " or ",
+          call("(", call(as.character(x[[1]]), rhs, x[[3]])))), " or ",
             deparse_str(call(as.character(x[[1]]),
-              call("(",call(":", lhs, rhs)), x[[3]])))
+              call("(", call(":", lhs, rhs)), x[[3]])))
 
       } else if ((length(x) > 2) && (is_call(x[[3]], ":"))) {
 
@@ -886,7 +886,7 @@ ir_parse_expr_lhs_check_index <- function(x) {
         fix <- paste0(deparse_str(call(as.character(x[[1]]), x[[2]],
           call("(", call(":", lhs, rhs)))), " or ",
             deparse_str(call(":",
-              call("(",call(as.character(x[[1]]), x[[2]], lhs)), rhs)))
+              call("(", call(as.character(x[[1]]), x[[2]], lhs)), rhs)))
 
       } else {
         fix <- "using parentheses"