From 16b71e8e07c95cb0086c8238a6a4784db5d143a4 Mon Sep 17 00:00:00 2001
From: Oscar Dowson <odow@users.noreply.github.com>
Date: Tue, 8 Oct 2024 19:53:32 +1300
Subject: [PATCH] Convert AbstractString to String in set_attribute (#3840)

---
 .github/workflows/documentation.yml |  2 +-
 src/optimizer_interface.jl          | 30 +++++++++++++++++++++++++----
 test/test_model.jl                  | 25 ++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/documentation.yml b/.github/workflows/documentation.yml
index e8e1145d4b1..d6599e4ccdf 100644
--- a/.github/workflows/documentation.yml
+++ b/.github/workflows/documentation.yml
@@ -14,7 +14,7 @@ jobs:
       - uses: actions/checkout@v4
       - uses: julia-actions/setup-julia@latest
         with:
-          version: '1'
+          version: '1.10'
       - name: Install dependencies
         shell: julia --color=yes --project=docs/ {0}
         run: |
diff --git a/src/optimizer_interface.jl b/src/optimizer_interface.jl
index 04f7840df0f..93717e65d4c 100644
--- a/src/optimizer_interface.jl
+++ b/src/optimizer_interface.jl
@@ -1251,6 +1251,26 @@ function get_attribute(
     return get_attribute(model, String(name))
 end
 
+# Some MOI attributes have a strict value type, like ::String or ::Float64, but
+# users may pass other generic subtypes, like SubString instead of String.
+# Rather than throw obtuse errors, we can catch and fix some common cases. We
+# shouldn't fix every case (for example, AbstractString -> String) because
+# users might intentionally want the other subtype.
+#
+# The default case is to do nothing:
+_to_concrete_value_type_if_needed(::MOI.AnyAttribute, value) = value
+
+# A common case is passing an AbstractString instead of a String.
+function _to_concrete_value_type_if_needed(
+    attr::MOI.AnyAttribute,
+    value::AbstractString,
+)
+    if !(value isa String) && MOI.attribute_value_type(attr) === String
+        return String(value)
+    end
+    return value
+end
+
 """
     set_attribute(model::GenericModel, attr::MOI.AbstractModelAttribute, value)
     set_attribute(x::GenericVariableRef, attr::MOI.AbstractVariableAttribute, value)
@@ -1285,7 +1305,7 @@ function set_attribute(
     attr::MOI.AbstractModelAttribute,
     value,
 )
-    MOI.set(model, attr, value)
+    MOI.set(model, attr, _to_concrete_value_type_if_needed(attr, value))
     return
 end
 
@@ -1294,7 +1314,8 @@ function set_attribute(
     attr::MOI.AbstractVariableAttribute,
     value,
 )
-    MOI.set(owner_model(x), attr, x, value)
+    model = owner_model(x)
+    MOI.set(model, attr, x, _to_concrete_value_type_if_needed(attr, value))
     return
 end
 
@@ -1303,7 +1324,8 @@ function set_attribute(
     attr::MOI.AbstractConstraintAttribute,
     value,
 )
-    MOI.set(owner_model(cr), attr, cr, value)
+    model = owner_model(cr)
+    MOI.set(model, attr, cr, _to_concrete_value_type_if_needed(attr, value))
     return
 end
 
@@ -1344,7 +1366,7 @@ function set_attribute(
     attr::MOI.AbstractOptimizerAttribute,
     value,
 )
-    MOI.set(model, attr, value)
+    MOI.set(model, attr, _to_concrete_value_type_if_needed(attr, value))
     return
 end
 
diff --git a/test/test_model.jl b/test/test_model.jl
index 3f568250743..c0786f9e13c 100644
--- a/test/test_model.jl
+++ b/test/test_model.jl
@@ -1313,4 +1313,29 @@ function test_is_solved_and_feasible()
     return
 end
 
+function test_set_abstract_string()
+    abstract_string = split("foo.bar", ".")[1]
+    model = Model() do
+        return MOI.Utilities.MockOptimizer(
+            MOI.Utilities.UniversalFallback(MOI.Utilities.Model{Float64}()),
+        )
+    end
+    set_attribute(model, MOI.Name(), abstract_string)
+    @test get_attribute(model, MOI.Name()) == "foo"
+    @variable(model, x)
+    set_attribute(x, MOI.VariableName(), abstract_string)
+    ret = get_attribute(x, MOI.VariableName())
+    @test ret isa String && ret == "foo"
+    c = @constraint(model, 2x >= 1)
+    set_attribute(c, MOI.ConstraintName(), abstract_string)
+    ret = get_attribute(c, MOI.ConstraintName())
+    @test ret isa String && ret == "foo"
+    set_attribute(model, abstract_string, abstract_string)
+    ret = get_attribute(model, abstract_string)
+    # This `ret` is NOT a `::String` because we don't know the value type of the
+    # attribute.
+    @test ret isa typeof(abstract_string) && ret == "foo"
+    return
+end
+
 end  # module TestModels