Should not put Boolean into List<String> property

[Kerry Sainsbury]

I have a bean that I do NOT want to have a matching getter (yes, I know that technically it's not a bean in that case, but still), but when I do so SnakeYaml's guess at the data type is wrong.

In fact, it generates a List<String> that contains a Boolean!

    public void testListOfStrings() {
        Constructor constructor = new Constructor();
        constructor.addTypeDescription(new TypeDescription(Foo.class, "!foo"));
        Yaml yaml = new Yaml(constructor);

        Foo foo = (Foo) yaml.load("!foo\ncountryCodes: [NZ,AU,NO]");

        assertEquals(3, foo.countryCodes.size());
        assertEquals("NZ", foo.countryCodes.get(0));
        assertEquals("AU", foo.countryCodes.get(1));
        assertEquals("NO", foo.countryCodes.get(2));
    }

    public static class Foo {
        private List<String> countryCodes = new ArrayList<String>();

        public void setCountryCodes(List<String> countryCodes) {
            for (Object countryCode : countryCodes) {
                System.out.println(countryCode.getClass().getName());
            }
            this.countryCodes = countryCodes;
        }
    }

The output of this is:

java.lang.String
java.lang.String
java.lang.Boolean

java.lang.ClassCastException: java.lang.Boolean cannot be cast to java.lang.String
    at examples.CustomHalfBeanResolverTest.testListOfStrings(CustomHalfBeanResolverTest.java:65)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:86)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:78)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:212)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:68)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at com.intellij.rt.execution.application.AppMain.main(AppMain.java:140)

This is because somewhere there is code that think 'NO' means 'False', rather than 'Norway' :-)

I know I have work-arounds:

• Add a Getter (even if it just throws an exception!)
• Put quotes around the country codes

.... but I think SnakeYaml can be a little bit more clever by looking at 'setters' to find a data type if there is no 'getter'.

Thanks for listening. This was a great bug to find -- a List<String> that contains a Boolean!

PS: Version 1.18 (test run against trunk actually, using Java 1.8.0_121

[Andrey Somov]

It is not a "guess". It is a requirement: http://yaml.org/type/bool.html

If you want to tune it check Resolver.

[Andrey Somov]

Thanks for listening. This was a great bug to find -- a List<String> that contains a Boolean!

Exactly the same happens if you do this: ["foo", "bar", false]

[Andrey Somov]

Feel free to point out where SnakeYAML does not work according to the spec.

(Otherwise the issue will be closed.)

[Kerry Sainsbury]

Hmmm. I would think that List<String> is a pretty strong hint that it's a list of Strings, and therefore all associated values should be treated as Strings, not Booleans.

That said, if you are adamant that there is no bug here, then I feel obliged to report another bug, because according to your logic the following test should fail, and it doesn't. The presence of the 'getCountryCodes' method changes the behaviour.

   public void testThatShouldFailApparently() {
        Constructor constructor = new Constructor();
        constructor.addTypeDescription(new TypeDescription(Foo.class, "!foo"));
        Yaml yaml = new Yaml(constructor);

        Foo foo = (Foo) yaml.load("!foo\ncountryCodes: [NZ,AU,NO]");

        assertEquals(3, foo.countryCodes.size());
        assertEquals("NZ", foo.countryCodes.get(0));
        assertEquals("AU", foo.countryCodes.get(1));
        assertEquals("NO", foo.countryCodes.get(2));
    }


    public static class Foo {
        private List<String> countryCodes = new ArrayList<String>();

        // If this method is not here then "NO" is a Boolean,
        // but when this method is here, then "NO" is a String.
        // *I* think it should always be considered a String, given the 'countryCodes' datatype :-)
        public List<String> getCountryCodes() {
            throw new RuntimeException("I serve no purpose other than to allow 'NO' to be treated as a String");
        }

        public void setCountryCodes(List<String> countryCodes) {
            for (Object countryCode : countryCodes) {
                System.out.println(countryCode.getClass().getName());
            }
            this.countryCodes = countryCodes;
        }
    }

The "guessing" I was talking about seems to be happening somewhere in org.yaml.snakeyaml.introspector.PropertyUtils, I think.

[Andrey Somov]

I added a test to show how it works.

Feel free to ask if it is still unclear.

[Alexander Maslov]

maybe #322 is related

[Kerry Sainsbury]

Sadly, I do find it unclear :-(

testStaticFooWithoutGetter seems to be saying that NO in a List<String> should be interpreted as Boolean.FALSE, but YES in a String should be interpreted as the string literal 'YES'. That doesn't make a lot of sense to me.

If the properties were declared as Object rather than String then this data-type selection of NO --> Boolean would be fine.

I'm also not sure what PublicFooWithoutGetter is proving. Yes, there is no getter, but because the value is public there's no need for a getter. What happens if you make 'countryCodes' private instead -- that's the more realistic example (for me).

Fundamentally, can you explain why anybody would expect different behaviour, depending on whether there is a getter or not -- especially when the difference is so subtle, and catastrophic.

[Alexander Maslov]

As I mentioned in previous comment, I believe it is related to #322 and to the fact that we are not so "smart" in detecting Property Type when readMethod is missing.

[Alexander Maslov]

Duplicate of #322.

[Kerry Sainsbury]

I agree. It's exactly that problem. I'm sorry I didn't explain myself clearer. You'll notice that your #322 comment was posted at the same time as my last one -- so I didn't see it.

[Alexander Maslov]

fixes #322 and #382 Now in case of missing readMethod we are trying to "guess" MethodProperty type from writeMethod's first (should be the only) parameter.

→ <<cset 55b33e57b1ab>>

[Alexander Maslov]

@ksainsbury , hope after this changeset we are a bit "smarter" ;) Feel free to add more failing test cases.

[Kerry Sainsbury]

Thanks for the speedy fix!

[Andrey Somov]

It will be delivered in version 1.19: https://bitbucket.org/asomov/snakeyaml/wiki/Changes