[JENKINS-53825] Choose from ambiguous class names via a simple heuristic

plugin/src/main/java/org/jenkinsci/plugins/structs/describable/DescribableModel.java

@@ -50,6 +50,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
 import static org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable.*;
 
@@ -399,7 +400,7 @@ private Object coerce(String context, Type type, Object o) throws Exception {
                 m.put((String) entry.getKey(), entry.getValue());
             }
 
-            Class<?> clazz = resolveClass(erased, (String) m.remove(CLAZZ), null);
+            Class<?> clazz = resolveClass(erased, (String) m.remove(CLAZZ), null, getType());
             return new DescribableModel(clazz).instantiate(m);
         } else if (o instanceof String && erased.isEnum()) {
             return Enum.valueOf(erased.asSubclass(Enum.class), (String) o);
@@ -456,7 +457,8 @@ private Object coerceStringToNumber(@Nonnull String context, @Nonnull Class numb
      * @param base
      *      Signature of the type that the resolved class should be assignable to.
      */
-    /*package*/ static Class<?> resolveClass(Class<?> base, @Nullable String name, @Nullable String symbol) throws ClassNotFoundException {
+    /*package*/ static Class<?> resolveClass(Class<?> base, @Nullable String name, @Nullable String symbol,
+                                             @Nullable Class<?> contextClass) throws ClassNotFoundException {
         // TODO: if both name & symbol are present, should we verify its consistency?
 
         if (name != null) {
@@ -465,19 +467,43 @@ private Object coerceStringToNumber(@Nonnull String context, @Nonnull Class numb
                 ClassLoader loader = j != null ? j.getPluginManager().uberClassLoader : Thread.currentThread().getContextClassLoader();
                 return Class.forName(name, true, loader);
             } else {
-                Class<?> clazz = null;
+                List<Class<?>> possibleClazzes = new ArrayList<>();
                 for (Class<?> c : findSubtypes(base)) {
                     if (c.getSimpleName().equals(name)) {
-                        if (clazz != null) {
-                            throw new UnsupportedOperationException(name + " as a " + base + " could mean either " + clazz.getName() + " or " + c.getName());
-                        }
-                        clazz = c;
+                        possibleClazzes.add(c);
                     }
                 }
-                if (clazz == null) {
+                if (possibleClazzes.isEmpty()) {
                     throw new UnsupportedOperationException("no known implementation of " + base + " is named " + name);
                 }
-                return clazz;
+                if (possibleClazzes.size() != 1) {
+                    // Try to heuristically determine the correct class.
+                    List<Class<?>> narrowedClazzes = new ArrayList<>();
+                    for (Class<?> possible : possibleClazzes) {
+                        if (contextClass != null) {
+                            if (contextClass.equals(possible.getEnclosingClass())
+                                    || contextClass.getPackage().equals(possible.getPackage())
+                                    || possible.getPackage().getName().startsWith(contextClass.getPackage().getName())) {
+                                narrowedClazzes.add(possible);
+                            }
+                        }
+                    }
+
+                    // We found just one that was eligible, return that.
+                    if (narrowedClazzes.size() == 1) {
+                        return narrowedClazzes.get(0);
+                    }
+
+                    // Couldn't heuristically determine the correct class, error out.
+                    String errorString;
+                    if (possibleClazzes.size() == 2) {
+                        errorString = possibleClazzes.stream().map(Class::getName).collect(Collectors.joining(" or "));
+                    } else {
+                        errorString = possibleClazzes.stream().map(Class::getName).collect(Collectors.joining(", "));
+                    }
+                    throw new UnsupportedOperationException(name + " as a " + base + " could mean any of " + errorString);
+                }
+                return possibleClazzes.get(0);
             }
         }
 

plugin/src/main/java/org/jenkinsci/plugins/structs/describable/UninstantiatedDescribable.java

@@ -164,7 +164,7 @@ public Object instantiate() throws Exception {
      *      depends on this parameter.
      */
     public <T> T instantiate(Class<T> base) throws Exception {
-        Class<?> c = DescribableModel.resolveClass(base, klass, symbol);
+        Class<?> c = DescribableModel.resolveClass(base, klass, symbol, null);
         return base.cast(new DescribableModel(c).instantiate(arguments));
     }
 

plugin/src/test/java/org/jenkinsci/plugins/structs/describable/AbstractSecondSharedName.java

@@ -0,0 +1,30 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2018, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.jenkinsci.plugins.structs.describable;
+
+import hudson.model.AbstractDescribableImpl;
+
+public abstract class AbstractSecondSharedName extends AbstractDescribableImpl<AbstractSecondSharedName> {
+}

plugin/src/test/java/org/jenkinsci/plugins/structs/describable/AbstractThirdSharedName.java

@@ -0,0 +1,30 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2018, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.jenkinsci.plugins.structs.describable;
+
+import hudson.model.AbstractDescribableImpl;
+
+public abstract class AbstractThirdSharedName extends AbstractDescribableImpl<AbstractThirdSharedName> {
+}

plugin/src/test/java/org/jenkinsci/plugins/structs/describable/DescribableModelTest.java

@@ -42,9 +42,15 @@
 import org.jenkinsci.plugins.structs.FishingNet;
 import org.jenkinsci.plugins.structs.Internet;
 import org.jenkinsci.plugins.structs.Tech;
+import org.jenkinsci.plugins.structs.describable.first.NarrowAmbiguousArrayContainer;
+import org.jenkinsci.plugins.structs.describable.first.NarrowAmbiguousContainer;
+import org.jenkinsci.plugins.structs.describable.first.NarrowAmbiguousListContainer;
 import org.jenkinsci.plugins.structs.describable.first.SharedName;
+import org.jenkinsci.plugins.structs.describable.first.second.SecondSharedName;
 import org.junit.ClassRule;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -61,6 +67,7 @@
 import java.util.logging.Level;
 
 import static org.apache.commons.lang3.SerializationUtils.roundtrip;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.jenkinsci.plugins.structs.describable.DescribableModel.*;
 import static org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable.ANONYMOUS_KEY;
@@ -74,6 +81,9 @@ public class DescribableModelTest {
     @ClassRule
     public static LoggerRule logging = new LoggerRule().record(DescribableModel.class, Level.ALL);
 
+    @Rule
+    public ExpectedException thrown = ExpectedException.none();
+
     @Test
     public void instantiate() throws Exception {
         Map<String,Object> args = map("text", "hello", "flag", true, "ignored", "!");
@@ -669,17 +679,17 @@ public void setFoo(Recursion r) {}
      */
     @Test
     public void resolveClass() throws Exception {
-        assertEquals(FishingNet.class, DescribableModel.resolveClass(Fishing.class, null, "net"));
-        assertEquals(FishingNet.class, DescribableModel.resolveClass(Fishing.class, "FishingNet", null));
-        assertEquals(Internet.class, DescribableModel.resolveClass(Tech.class, null, "net"));
-        assertEquals(Internet.class, DescribableModel.resolveClass(Tech.class, "Internet", null));
+        assertEquals(FishingNet.class, DescribableModel.resolveClass(Fishing.class, null, "net", null));
+        assertEquals(FishingNet.class, DescribableModel.resolveClass(Fishing.class, "FishingNet", null, null));
+        assertEquals(Internet.class, DescribableModel.resolveClass(Tech.class, null, "net", null));
+        assertEquals(Internet.class, DescribableModel.resolveClass(Tech.class, "Internet", null, null));
     }
 
     @Issue("JENKINS-46122")
     @Test
     public void resolveSymbolOnWrongBaseClass() throws Exception {
         try {
-            DescribableModel.resolveClass(Tech.class, null, "rod");
+            DescribableModel.resolveClass(Tech.class, null, "rod", null);
             fail("No symbol for Tech should exist.");
         } catch (UnsupportedOperationException e) {
             assertEquals("no known implementation of " + Tech.class + " is using symbol ‘rod’", e.getMessage());
@@ -870,6 +880,87 @@ public void ambiguousTopLevelSimpleNameInArray() throws Exception {
         assertThat(roundtrip.getArray()[1], instanceOf(UnambiguousClassName.class));
     }
 
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameSamePackage() throws Exception {
+        NarrowAmbiguousContainer container = new NarrowAmbiguousContainer(new SharedName("first"),
+                new UnambiguousClassName("second"));
+
+        NarrowAmbiguousContainer fromInstantiate = instantiate(NarrowAmbiguousContainer.class,
+                map("ambiguous", map("$class", "SharedName", "one", "first"),
+                        "unambiguous", map("$class", "UnambiguousClassName", "one", "second")));
+
+        assertEquals(container.toString(), fromInstantiate.toString());
+    }
+
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameInnerClass() throws Exception {
+        NarrowAmbiguousContainer container = new NarrowAmbiguousContainer(new NarrowAmbiguousContainer.ThirdSharedName("first"),
+                new UnambiguousClassName("second"));
+
+        NarrowAmbiguousContainer fromInstantiate = instantiate(NarrowAmbiguousContainer.class,
+                map("ambiguous", map("$class", "ThirdSharedName", "one", "first"),
+                        "unambiguous", map("$class", "UnambiguousClassName", "one", "second")));
+
+        assertEquals(container.toString(), fromInstantiate.toString());
+    }
+
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameChildPackage() throws Exception {
+        NarrowAmbiguousContainer container = new NarrowAmbiguousContainer(new SecondSharedName("first"),
+                new UnambiguousClassName("second"));
+
+        NarrowAmbiguousContainer fromInstantiate = instantiate(NarrowAmbiguousContainer.class,
+                map("ambiguous", map("$class", "SecondSharedName", "one", "first"),
+                        "unambiguous", map("$class", "UnambiguousClassName", "one", "second")));
+
+        assertEquals(container.toString(), fromInstantiate.toString());
+    }
+
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameFailToDistinguish() throws Exception {
+        thrown.expect(IllegalArgumentException.class);
+        thrown.expectMessage(containsString("SharedName as a interface hudson.model.Describable could mean any of org.jenkinsci.plugins.structs.describable.first.SharedName or org.jenkinsci.plugins.structs.describable.second.SharedName"));
+        instantiate(AmbiguousContainer.class,
+                map("ambiguous", map("$class", "SharedName", "one", "first"),
+                        "unambiguous", map("$class", "UnambiguousClassName", "one", "second")));
+    }
+
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameList() throws Exception {
+        SharedName first = new SharedName("first");
+        first.setTwo("something");
+        NarrowAmbiguousListContainer container = new NarrowAmbiguousListContainer(Arrays.<Describable<?>>asList(first,
+                new UnambiguousClassName("second")));
+
+        NarrowAmbiguousListContainer fromInstantiate = instantiate(NarrowAmbiguousListContainer.class,
+                map("list",
+                        Arrays.asList(map("$class", "SharedName", "one", "first", "two", "something"),
+                                map("$class", "UnambiguousClassName", "one", "second"))));
+
+        assertEquals(container.toString(), fromInstantiate.toString());
+    }
+
+    @Issue("JENKINS-53825")
+    @Test
+    public void heuristicSharedNameInArray() throws Exception {
+        SharedName first = new SharedName("first");
+        first.setTwo("something");
+        NarrowAmbiguousArrayContainer container = new NarrowAmbiguousArrayContainer(first,
+                new UnambiguousClassName("second"));
+
+        NarrowAmbiguousArrayContainer fromInstantiate = instantiate(NarrowAmbiguousArrayContainer.class,
+                map("array",
+                        Arrays.asList(map("$class", "SharedName", "one", "first", "two", "something"),
+                                map("$class", "UnambiguousClassName", "one", "second"))));
+
+        assertEquals(container.toString(), fromInstantiate.toString());
+    }
+
     private static Map<String,Object> map(Object... keysAndValues) {
         if (keysAndValues.length % 2 != 0) {
             throw new IllegalArgumentException();

plugin/src/test/java/org/jenkinsci/plugins/structs/describable/UnambiguousClassName.java

@@ -37,6 +37,11 @@ public UnambiguousClassName(String one) {
             this.one = one;
         }
 
+    @Override
+    public String toString() {
+        return "UnambiguousClassName[one[" + one + "]]";
+    }
+
     @Extension
     public static class DescriptorImpl extends Descriptor<UnambiguousClassName> {
         @Override

plugin/src/test/java/org/jenkinsci/plugins/structs/describable/first/NarrowAmbiguousArrayContainer.java

@@ -0,0 +1,59 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2018, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.jenkinsci.plugins.structs.describable.first;
+
+import hudson.Extension;
+import hudson.model.AbstractDescribableImpl;
+import hudson.model.Describable;
+import hudson.model.Descriptor;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+import java.util.Arrays;
+
+public class NarrowAmbiguousArrayContainer extends AbstractDescribableImpl<NarrowAmbiguousArrayContainer> {
+    private final Describable<?>[] array;
+
+    @DataBoundConstructor
+    public NarrowAmbiguousArrayContainer(Describable<?>... array) {
+        this.array = array.clone();
+    }
+
+    public Describable<?>[] getArray() {
+        return array.clone();
+    }
+
+    @Override
+    public String toString() {
+        return "NarrowAmbiguousArrayContainer[array[" + Arrays.asList(array).toString() + "]]";
+    }
+
+    @Extension
+    public static class DescriptorImpl extends Descriptor<NarrowAmbiguousArrayContainer> {
+        @Override
+        public String getDisplayName() {
+            return "ambiguous array container";
+        }
+    }
+}