Improve error messages in the macros (#4182)

Author: odow

docs/src/manual/constraints.md

@@ -178,25 +178,20 @@ julia> @variable(model, y[1:2, 1:2], Symmetric);
 
 julia> @constraint(model, x >= y)
 ERROR: At none:1: `@constraint(model, x >= y)`:
-The syntax `x >= y` is ambiguous for matrices because we cannot tell if
-you intend a positive semidefinite constraint or an elementwise
-inequality.
 
-To create a positive semidefinite constraint, pass `PSDCone()` or
-`HermitianPSDCone()`:
+The syntax `x >= y` is ambiguous for matrices because JuMP cannot determine if you intend a positive semidefinite constraint or an elementwise inequality.
 
+To create a positive semidefinite constraint, pass `PSDCone()` or `HermitianPSDCone()`:
 ```julia
 @constraint(model, x >= y, PSDCone())
 ```
-
-To create an element-wise inequality, pass `Nonnegatives()`, or use
-broadcasting:
-
+To create an elementwise inequality, pass `Nonnegatives()` or use broadcasting:
 ```julia
 @constraint(model, x >= y, Nonnegatives())
 # or
 @constraint(model, x .>= y)
 ```
+
 Stacktrace:
 [...]
 ````

docs/src/manual/variables.md

@@ -1148,9 +1148,14 @@ julia> type = :PSD
 :PSD
 
 julia> @variable(model, x[1:2, 1:2], type)
-ERROR: At none:1: `@variable(model, x[1:2, 1:2], type)`: Unrecognized positional arguments: (:PSD,). (You may have passed it as a positional argument, or as a keyword value to `variable_type`.)
+ERROR: At none:1: `@variable(model, x[1:2, 1:2], type)`:
+
+Unrecognized positional arguments: (:PSD,).
+
+You may have passed an unrecognized type or a keyword value to `variable_type`.
 
 If you're trying to create a JuMP extension, you need to implement `build_variable`. Read the docstring for more details.
+
 Stacktrace:
 [...]
 ```
@@ -1212,9 +1217,14 @@ julia> type = :Symmetric
 :Symmetric
 
 julia> @variable(model, x[1:2, 1:2], type)
-ERROR: At none:1: `@variable(model, x[1:2, 1:2], type)`: Unrecognized positional arguments: (:Symmetric,). (You may have passed it as a positional argument, or as a keyword value to `variable_type`.)
+ERROR: At none:1: `@variable(model, x[1:2, 1:2], type)`:
+
+Unrecognized positional arguments: (:Symmetric,).
+
+You may have passed an unrecognized type or a keyword value to `variable_type`.
 
 If you're trying to create a JuMP extension, you need to implement `build_variable`. Read the docstring for more details.
+
 Stacktrace:
 [...]
 ```

src/Containers/macro.jl

@@ -49,15 +49,27 @@ function parse_macro_arguments(
         if Meta.isexpr(arg, :(=), 2)
             if haskey(kwargs, arg.args[1])
                 error_fn(
-                    "the keyword argument `$(arg.args[1])` was given " *
-                    "multiple times.",
+                    """
+                    The keyword argument `$(arg.args[1])` was given multiple times.
+                    """,
                 )
             elseif valid_kwargs !== nothing && !(arg.args[1] in valid_kwargs)
-                error_fn("unsupported keyword argument `$(arg.args[1])`.")
+                error_fn(
+                    """
+                    Unsupported keyword argument `$(arg.args[1])`.
+
+                    If you are trying to construct an equality constraint, use `==` instead of `=`.
+                    """,
+                )
             elseif !(arg.args[1] isa Symbol)
                 error_fn(
-                    "Invalid keyword argument detected. If you are trying to " *
-                    "construct an equality constraint, use `==` instead of `=`.",
+                    """
+                    Invalid keyword argument detected.
+
+                    The left-hand side of the `=` operator was `$(arg.args[1])`.
+
+                    If you are trying to construct an equality constraint, use `==` instead of `=`.
+                    """,
                 )
             end
             kwargs[arg.args[1]] = arg.args[2]
@@ -69,14 +81,18 @@ function parse_macro_arguments(
         n = length(pos_args)
         if n != num_positional_args
             error_fn(
-                "expected $num_positional_args positional arguments, got $n.",
+                """
+                Expected $num_positional_args positional arguments, got $n.
+                """,
             )
         end
     elseif num_positional_args isa UnitRange{Int}
         if !(length(pos_args) in num_positional_args)
             a, b = num_positional_args.start, num_positional_args.stop
             error_fn(
-                "expected $a to $b positional arguments, got $(length(pos_args)).",
+                """
+                Expected $a to $b positional arguments, got $(length(pos_args)).
+                """,
             )
         end
     end
@@ -96,8 +112,9 @@ function _explicit_oneto(error_fn, index_set)
             $index_set
         catch
             $error_fn(
-                "unexpected error parsing reference set: ",
+                "Unexpected error parsing reference set: ",
                 $(Meta.quot(_drop_esc(index_set))),
+                "\n\nSee the \"caused by\" stacktrace below for the underlying error.\n",
             )
         end
     end
@@ -178,8 +195,11 @@ function _parse_ref_sets(error_fn::Function, expr::Expr)
         name = popfirst!(c.args)
         if !(name isa Symbol)
             error_fn(
-                "Unsupported syntax: the expression `$name` cannot be used " *
-                "as a name.",
+                """
+                The expression `$name` cannot be used as a name.
+
+                Names must be symbols, not expressions, and they cannot contain spaces.
+                """,
             )
         end
     end
@@ -188,9 +208,12 @@ function _parse_ref_sets(error_fn::Function, expr::Expr)
         # the second argument.
         if length(c.args) > 2
             error_fn(
-                "Unsupported syntax $c: There can be at most one filtering " *
-                "condition, which is separated from the indices by a single " *
-                "`;`.",
+                """
+                Unsupported syntax `$c`.
+
+                There can be at most one filtering condition, which is separated \
+                from the indices by a single `;`.
+                """,
             )
         elseif length(c.args) == 2
             condition = pop!(c.args)
@@ -247,7 +270,13 @@ function _has_dependent_sets(index_vars::Vector{Any}, index_sets::Vector{Any})
 end
 
 function _container_name(error_fn::Function, x)
-    return error_fn("Expression `$x::$(typeof(x))` cannot be used as a name.")
+    return error_fn(
+        """
+        Expression `$x::$(typeof(x))` cannot be used as a name.
+
+        Names must be symbols, not expressions, and they cannot contain spaces.
+        """,
+    )
 end
 
 _container_name(::Function, expr::Union{Symbol,Nothing}) = expr
@@ -258,7 +287,13 @@ function _container_name(error_fn::Function, expr::Expr)
     elseif Meta.isexpr(expr, (:ref, :typed_vcat))
         return _container_name(error_fn, expr.args[1])
     end
-    return error_fn("Expression $expr cannot be used as a name.")
+    return error_fn(
+        """
+        Expression `$expr` cannot be used as a name.
+
+        Names must be symbols, not expressions, and they cannot contain spaces.
+        """,
+    )
 end
 
 """
@@ -305,8 +340,11 @@ function parse_ref_sets(
     for name in invalid_index_variables
         if name in index_vars
             error_fn(
-                "the index name `$name` conflicts with another variable in " *
-                "this scope. Use a different name for the index.",
+                """
+                The index name `$name` conflicts with another variable in this scope.
+
+                Use a different name for the index.
+                """,
             )
         end
     end
@@ -342,7 +380,9 @@ additionally prints the macro from which it was called.
 """
 function build_error_fn(macro_name, args, source)
     msg = build_macro_expression_string(macro_name, args)
-    error_fn(str...) = error("At $(source.file):$(source.line): $msg: ", str...)
+    function error_fn(str...)
+        return error("At $(source.file):$(source.line): $msg:\n\n", str...)
+    end
     return error_fn
 end
 
@@ -357,8 +397,11 @@ function build_ref_sets(error_fn::Function, expr)
     index_vars, index_sets, condition = _parse_ref_sets(error_fn, expr)
     if any(_expr_is_splat, index_sets)
         error_fn(
-            "cannot use splatting operator `...` in the definition of an " *
-            "index set.",
+            """
+            Cannot use the splatting operator `...` in the definition of an index set.
+
+            Each index must be explicitly separated by a comma.
+            """,
         )
     end
     if !_has_dependent_sets(index_vars, index_sets) && condition == :()
@@ -377,8 +420,9 @@ function build_ref_sets(error_fn::Function, expr)
                     $(index_sets[i])
                 catch
                     $error_fn(
-                        "unexpected error parsing reference set: ",
+                        "Unexpected error parsing reference set: ",
                         $(Meta.quot(_drop_esc(index_sets[i]))),
+                        "\n\nSee the \"caused by\" stacktrace below for the underlying error.\n",
                     )
                 end
             end,
@@ -390,8 +434,9 @@ function build_ref_sets(error_fn::Function, expr)
                 $(esc(condition))
             catch
                 $error_fn(
-                    "unexpected error parsing condition: ",
+                    "Unexpected error parsing condition: ",
                     $(Meta.quot(condition)),
+                    "\n\nSee the \"caused by\" stacktrace below for the underlying error.\n",
                 )
             end
         end

src/complement.jl

@@ -10,21 +10,25 @@
 # @constraint(model, 2x - 1 ⟂ x)
 
 function _build_complements_constraint(
-    errorf::Function,
+    error_fn::Function,
     F::AbstractArray{<:Union{Real,AbstractJuMPScalar}},
     x::AbstractArray{<:AbstractVariableRef},
 )
     if size(F) != size(x)
-        errorf(
-            "size of mapping does not match size of variables: " *
-            "$(size(F)) != $(size(x)).",
+        error_fn(
+            """
+            The number of elements in the left-hand side $(size(F)) does not match the right-hand side $(size(x)).
+
+            There must be a one-to-one mapping between the left- and right-hand \
+            sides of a complementarity constraint.
+            """,
         )
     end
     return VectorConstraint([F; x], MOI.Complements(2 * length(F)))
 end
 
 function _build_complements_constraint(
-    errorf::Function,
+    error_fn::Function,
     F::Containers.SparseAxisArray{<:Union{Real,AbstractJuMPScalar}},
     x::Containers.SparseAxisArray{<:AbstractVariableRef},
 )
@@ -33,18 +37,31 @@ function _build_complements_constraint(
         if haskey(x, i)
             push!(elements, x[i])
         else
-            errorf("keys of the SparseAxisArray's do not match.")
+            error_fn(
+                """
+                Keys of the SparseAxisArray's do not match.
+
+                The left-hand side has key `$i`, but the right-hand side does not.
+                """,
+            )
         end
     end
     return VectorConstraint(elements, MOI.Complements(length(elements)))
 end
 
 function _build_complements_constraint(
-    errorf::Function,
+    error_fn::Function,
     ::AbstractArray{<:Union{Real,AbstractJuMPScalar}},
-    ::AbstractArray{<:AbstractJuMPScalar},
+    x::AbstractArray{<:AbstractJuMPScalar},
 )
-    return errorf("second term must be an array of variables.")
+    return error_fn(
+        """
+        The right-hand side term in a complementarity constraint must be a \
+        variable or an array of variables.
+
+        Currently, it is a `$(typeof(x))`.
+        """,
+    )
 end
 
 function _build_complements_constraint(
@@ -56,20 +73,26 @@ function _build_complements_constraint(
 end
 
 function _build_complements_constraint(
-    errorf::Function,
+    error_fn::Function,
     ::Union{Real,AbstractJuMPScalar},
-    ::AbstractJuMPScalar,
+    x::AbstractJuMPScalar,
 )
-    return errorf("second term must be a variable.")
+    return error_fn(
+        """
+        The right-hand side term in a complementarity constraint must be a variable.
+
+        Currently, it is a `$(typeof(x))`.
+        """,
+    )
 end
 
 function parse_constraint_call(
-    errorf::Function,
+    error_fn::Function,
     ::Bool,
     ::Union{Val{:complements},Val{:⟂}},
     F,
     x,
 )
-    f, parse_code = _rewrite_expression(F)
-    return parse_code, :(_build_complements_constraint($errorf, $f, $(esc(x))))
+    f, parse = _rewrite_expression(F)
+    return parse, :(_build_complements_constraint($error_fn, $f, $(esc(x))))
 end