diff --git a/beast-base/src/main/java/beast/base/core/Input.java b/beast-base/src/main/java/beast/base/core/Input.java index 5739284..4650ad9 100644 --- a/beast-base/src/main/java/beast/base/core/Input.java +++ b/beast-base/src/main/java/beast/base/core/Input.java @@ -509,7 +509,7 @@ public void setValue(final Object value, final BEASTInterface beastObject) { } } - throw new RuntimeException("Input 102b: type mismatch for input " + getName()); + throw new RuntimeException(typeMismatchMessage("102b", value, beastObject)); } else { if (theClass.isAssignableFrom(value.getClass())) { if (value instanceof BEASTInterface) { @@ -705,6 +705,54 @@ public boolean isTensorClass(Class rawType) { || Vector.class.isAssignableFrom(rawType); } + /** + * Build a type-mismatch message that says which input failed, on which object, + * what it expected and what it was actually given. + * + *

The bare "type mismatch for input x" is very hard to act on: it names neither + * the expected nor the supplied type, and the surrounding XML parser error only + * points at the enclosing element, not at the offending child. Where the supplied + * value is a legacy BEAST 2 class and the input wants a BEAST 3 spec one -- by far + * the most common cause while packages are being migrated -- say so explicitly, + * because the two are usually indistinguishable by name. + * + * @param code the error code to report, e.g. "102b" + * @param value the value that could not be assigned + * @param beastObject the object whose input was being set, may be null + */ + private String typeMismatchMessage(String code, Object value, BEASTInterface beastObject) { + final Class expected = tensorClass != null ? tensorClass : theClass; + final String expectedName = expected == null ? "" : expected.getName(); + final String actualName = value == null ? "null" : value.getClass().getName(); + + final StringBuilder b = new StringBuilder(); + b.append("Input ").append(code).append(": type mismatch for input '").append(getName()).append("'"); + if (beastObject != null) { + b.append(" of ").append(beastObject.getClass().getName()); + if (beastObject.getID() != null) { + b.append(" id='").append(beastObject.getID()).append("'"); + } + } + b.append("\n expected: ").append(expectedName); + if (tensorClass != null && theClass != null) { + // for a tensor input the domain is carried separately, e.g. RealScalar + b.append(" with domain ").append(theClass.getName()); + } + b.append("\n but got: ").append(actualName); + + if (expectedName.startsWith("beast.base.spec.") + && actualName.startsWith("beast.base.") + && !actualName.startsWith("beast.base.spec.")) { + final String simpleName = value.getClass().getSimpleName(); + b.append("\n\n").append(actualName).append(" is a legacy BEAST 2 class, but this input expects") + .append(" a BEAST 3 spec class.\nIn XML this usually means a bare element name was used, as in") + .append(" <").append(simpleName).append(" name='").append(getName()).append("'>, which resolves") + .append(" to the legacy\nclass. Name the input and give it an explicit spec instead, as in") + .append(" <").append(getName()).append(" spec='beast.base.spec...'>."); + } + return b.toString(); + } + /** * Try to parse value of string into Integer, Double or Boolean, * or it this types differs, just assign as string. diff --git a/beast-base/src/test/java/test/beast/core/InputTypeMismatchMessageTest.java b/beast-base/src/test/java/test/beast/core/InputTypeMismatchMessageTest.java new file mode 100644 index 0000000..9c98925 --- /dev/null +++ b/beast-base/src/test/java/test/beast/core/InputTypeMismatchMessageTest.java @@ -0,0 +1,83 @@ +package test.beast.core; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +import beast.base.inference.parameter.RealParameter; +import beast.base.spec.domain.PositiveReal; +import beast.base.spec.inference.distribution.Exponential; +import beast.base.spec.inference.parameter.RealScalarParam; +import beast.base.spec.inference.parameter.RealVectorParam; + +/** + * A type mismatch has to say enough to be acted on. + * + *

The message used to read only "type mismatch for input mean", naming neither the + * expected nor the supplied type, and the XML parser error that wraps it points at the + * enclosing element rather than the offending child. Working out that a legacy BEAST 2 + * class had been supplied where a BEAST 3 spec one was wanted meant reading the source + * of the class that declared the input. + */ +public class InputTypeMismatchMessageTest { + + private static String mismatchMessageFor(Object badValue) { + Exponential exponential = new Exponential(); + RuntimeException e = assertThrows(RuntimeException.class, + () -> exponential.meanInput.setValue(badValue, exponential)); + return e.getMessage(); + } + + /** Supplying a legacy RealParameter where a spec RealScalar is wanted. */ + @Test + public void testMessageNamesInputExpectedAndActual() { + String msg = mismatchMessageFor(new RealParameter("1.0")); + + assertTrue(msg.contains("'mean'"), "should name the offending input, was: " + msg); + assertTrue(msg.contains(Exponential.class.getName()), + "should name the object whose input failed, was: " + msg); + assertTrue(msg.contains("beast.base.spec.type.RealScalar"), + "should name the expected type, was: " + msg); + assertTrue(msg.contains(RealParameter.class.getName()), + "should name the supplied type, was: " + msg); + } + + /** + * Legacy and spec classes often share a simple name, so the message has to be + * explicit that the supplied class is the BEAST 2 one. + */ + @Test + public void testLegacyClassGetsMigrationHint() { + String msg = mismatchMessageFor(new RealParameter("1.0")); + + assertTrue(msg.contains("legacy BEAST 2 class"), + "should flag the legacy/spec confusion, was: " + msg); + assertTrue(msg.contains("spec="), + "should show how to declare the input explicitly, was: " + msg); + } + + /** + * A mismatch between two spec types -- here a vector where a scalar is wanted -- + * has nothing to do with the migration and must not get the misleading hint. + */ + @Test + public void testNonLegacyMismatchHasNoMigrationHint() { + String msg = mismatchMessageFor( + new RealVectorParam<>(new double[]{1.0, 2.0}, PositiveReal.INSTANCE)); + + assertTrue(msg.contains("'mean'"), "should still name the input, was: " + msg); + assertTrue(msg.contains(RealVectorParam.class.getName()), + "should name the supplied type, was: " + msg); + assertTrue(!msg.contains("legacy BEAST 2 class"), + "should not claim a legacy class was supplied, was: " + msg); + } + + /** A correctly typed spec parameter is still accepted. */ + @Test + public void testSpecParameterIsAccepted() { + Exponential exponential = new Exponential(); + exponential.meanInput.setValue(new RealScalarParam<>(1.0, PositiveReal.INSTANCE), exponential); + } + +}