Discover Converter capabilities and validate shared binding drafts

designer-target-tools/src/main/java/de/neitzel/fxdesigner/target/BindingDiscovery.java

@@ -15,9 +15,14 @@ public final class BindingDiscovery {
   private static final String FX = "http://javafx.com/fxml/1";
   private final ClassLoader loader;
   private final List<String> diagnostics = new ArrayList<>();
+  private boolean incompleteModel;
   private final Map<String, Class<?>> paths = new TreeMap<>();
   private final List<Attribute> attributes = new ArrayList<>();
 
+  private record TargetType(Class<?> type, boolean writable) {}
+
+  private final Map<String, TargetType> targetTypes = new HashMap<>();
+
   private BindingDiscovery(ClassLoader loader) {
     this.loader = loader;
   }
@@ -26,7 +31,20 @@ public final class BindingDiscovery {
       String fxml, String path, String direction, ClassLoader loader) throws Exception {
     if (!Set.of("view", "model", "both").contains(direction))
       throw new IllegalArgumentException("Invalid direction");
-    return new BindingDiscovery(loader).inspect(fxml, path, direction);
+    return inspect(fxml, path, direction, "", "", loader);
+  }
+
+  public static BindingMetadata inspect(
+      String fxml,
+      String path,
+      String direction,
+      String control,
+      String property,
+      ClassLoader loader)
+      throws Exception {
+    if (!Set.of("view", "model", "both").contains(direction))
+      throw new IllegalArgumentException("Invalid direction");
+    return new BindingDiscovery(loader).inspect(fxml, path, direction, control, property);
   }
 
   private static Document parse(String xml) throws Exception {
@@ -63,6 +81,7 @@ public final class BindingDiscovery {
     for (Class<?> c = type; c != null && c != Object.class; c = c.getSuperclass()) {
       try {
         load(c.getName() + "BeanInfo");
+        incompleteModel = true;
         diagnostics.add("Custom BeanInfo requires generated metadata: " + c.getName());
         return List.of();
       } catch (ClassNotFoundException absent) {
@@ -104,6 +123,7 @@ public final class BindingDiscovery {
   private void attributes(Class<?> type, String prefix, Set<Class<?>> ancestors, int depth)
       throws Exception {
     if (depth > 4 || ancestors.contains(type)) {
+      incompleteModel = true;
       diagnostics.add("Nested path expansion stopped at " + prefix);
       return;
     }
@@ -137,7 +157,13 @@ public final class BindingDiscovery {
     }
   }
 
-  private BindingMetadata inspect(String xml, String selected, String direction) throws Exception {
+  private BindingMetadata inspect(
+      String xml,
+      String selected,
+      String direction,
+      String selectedControl,
+      String selectedProperty)
+      throws Exception {
     Document doc = parse(xml);
     String model = model(doc);
     if (model.isEmpty())
@@ -149,7 +175,11 @@ public final class BindingDiscovery {
           List.of(),
           List.of(
               "Declare this View's Model with <?neitzel-model package.Model?> before discovering"
-                  + " bindings."));
+                  + " bindings."),
+          List.of(),
+          List.of(
+              new Problem(
+                  "", "", "WARNING", "No Model declaration; semantic validation unavailable")));
     attributes(load(model), "", Set.of(), 0);
     if (selected.isEmpty())
       selected =
@@ -196,6 +226,9 @@ public final class BindingDiscovery {
             value = load(componentModel);
             constrained = true;
           }
+          targetTypes.put(
+              id + "." + property,
+              new TargetType(value, Property.class.isAssignableFrom(m.getReturnType())));
           String status, reason;
           if (!Property.class.isAssignableFrom(m.getReturnType())) {
             status = "INCOMPATIBLE";
@@ -245,7 +278,129 @@ public final class BindingDiscovery {
                         .indexOf(t.status()))
             .thenComparing(Target::control)
             .thenComparing(Target::property));
-    return new BindingMetadata(model, selected, direction, attributes, targets, diagnostics);
+    var converters = new ConverterDiscovery(loader);
+    var chosen = targetTypes.get(selectedControl + "." + selectedProperty);
+    var candidates =
+        bypassesConverter(selected)
+            ? List.<BindingMetadata.Converter>of()
+            : converters.candidates(source, chosen == null ? null : chosen.type(), direction);
+    return new BindingMetadata(
+        model,
+        selected,
+        direction,
+        attributes,
+        targets,
+        diagnostics,
+        candidates,
+        validate(doc, imports, ids, converters));
+  }
+
+  private boolean bypassesConverter(String path) {
+    return attributes.stream()
+        .anyMatch(a -> a.path().equals(path) && Set.of("COMPLEX", "COLLECTION").contains(a.kind()));
+  }
+
+  private List<Problem> validate(
+      Document doc, List<String> imports, Set<String> ids, ConverterDiscovery converters)
+      throws Exception {
+    var problems = new ArrayList<Problem>();
+    var occupied = new HashSet<String>();
+    var nodes = doc.getElementsByTagName("*");
+    for (int i = 0; i < nodes.getLength(); i++) {
+      var e = (Element) nodes.item(i);
+      if (!library(e, "DirectBinding", imports) && !library(e, "ComputedBinding", imports))
+        continue;
+      Node parent = e.getParentNode();
+      if (parent instanceof Element p && p.getTagName().equals("elements"))
+        parent = p.getParentNode();
+      if (!(parent instanceof Element list)
+          || !library(list, "BindingList", imports)
+          || !(list.getParentNode() instanceof Element define)
+          || !FX.equals(define.getNamespaceURI())
+          || !"define".equals(define.getLocalName())
+          || define.getParentNode() != doc.getDocumentElement()) continue;
+      String control = e.getAttribute("control"),
+          property = e.getAttribute("property"),
+          key = control + "." + property;
+      if (!occupied.add(key))
+        problems.add(new Problem(control, property, "ERROR", "Duplicate binding target"));
+      if (library(e, "ComputedBinding", imports)) continue;
+      boolean nested = false;
+      for (Node n = e.getFirstChild(); n != null; n = n.getNextSibling())
+        if (n instanceof Element) nested = true;
+      if (nested) {
+        problems.add(
+            new Problem(control, property, "WARNING", "Nested declaration requires Source review"));
+        continue;
+      }
+      String path = e.getAttribute("path"),
+          direction = e.getAttribute("direction"),
+          converter = e.getAttribute("converter");
+      if (!converter.isEmpty() && bypassesConverter(path)) {
+        problems.add(
+            new Problem(
+                control,
+                property,
+                "WARNING",
+                "Runtime bypasses Converters for complex and collection Attributes"));
+        converter = "";
+      }
+      String severity = "", message = "";
+      Class<?> source = paths.get(path);
+      TargetType target = targetTypes.get(key);
+      if (!Set.of("view", "model", "both").contains(direction)) {
+        severity = "ERROR";
+        message = "Invalid direction";
+      } else if (!ids.contains(control)) {
+        severity = "ERROR";
+        message = "Unknown control: " + control;
+      } else if (target == null) {
+        severity =
+            diagnostics.stream().anyMatch(d -> d.startsWith(control + ":")) ? "WARNING" : "ERROR";
+        message = "No discoverable JavaFX property: " + key;
+      } else if (!target.writable()) {
+        severity = "ERROR";
+        message = "Runtime requires a writable JavaFX Property";
+      } else if (source == null) {
+        severity = incompleteModel ? "WARNING" : "ERROR";
+        message = "Unknown or unresolved Model path: " + path;
+      } else if (!converter.isEmpty()) {
+        var c = converters.check(converter, source, target.type(), direction);
+        if (!c.status().equals("COMPATIBLE")) {
+          severity = c.status().equals("UNVERIFIED") ? "WARNING" : "ERROR";
+          message = converter + ": " + c.reason();
+        }
+      } else if (target.type() == null
+          || source == Object.class
+          || target.type() == Object.class
+          || Collection.class.isAssignableFrom(source)
+          || Collection.class.isAssignableFrom(target.type())
+          || Map.class.isAssignableFrom(source)
+          || Map.class.isAssignableFrom(target.type())) {
+        severity = "WARNING";
+        message = "Value types require additional metadata";
+      } else {
+        Class<?> a = box(source), b = box(target.type());
+        if ((!direction.equals("model") && !b.isAssignableFrom(a))
+            || (!direction.equals("view") && !a.isAssignableFrom(b))) {
+          severity = "ERROR";
+          message = "Incompatible value types; select a compatible Converter";
+        }
+      }
+      if (!severity.isEmpty()) problems.add(new Problem(control, property, severity, message));
+    }
+    return problems;
+  }
+
+  private boolean library(Element e, String simple, List<String> imports) {
+    String name = "de.neitzel.fx.view." + simple;
+    if (e.getTagName().equals(name)) return true;
+    if (!e.getTagName().equals(simple)) return false;
+    try {
+      return resolve(e, imports).getName().equals(name);
+    } catch (Exception | LinkageError ignored) {
+      return false;
+    }
   }
 
   private static boolean defined(Element e) {
@@ -310,7 +465,7 @@ public final class BindingDiscovery {
     return null;
   }
 
-  private static Class<?> box(Class<?> c) {
+  static Class<?> box(Class<?> c) {
     if (!c.isPrimitive()) return c;
     return Map.<Class<?>, Class<?>>of(
             int.class,

designer-target-tools/src/main/java/de/neitzel/fxdesigner/target/ConverterDiscovery.java

@@ -0,0 +1,138 @@
+package de.neitzel.fxdesigner.target;
+
+import de.neitzel.fx.view.Converter;
+import de.neitzel.fxdesigner.preview.ConverterCapabilities;
+import de.neitzel.fxdesigner.protocol.BindingMetadata;
+import java.io.*;
+import java.lang.reflect.Modifier;
+import java.nio.file.*;
+import java.util.*;
+import java.util.jar.JarFile;
+
+/** Reads class headers before loading only Converter descendants, always without initialization. */
+final class ConverterDiscovery {
+  private final ClassLoader loader;
+  private int indexed;
+  private final SortedSet<String> names = new TreeSet<>();
+  private final Map<String, List<String>> hierarchy = new HashMap<>();
+
+  ConverterDiscovery(ClassLoader loader) throws IOException {
+    this.loader = loader;
+    var entries = new LinkedHashSet<String>();
+    for (String key : List.of("java.class.path", "jdk.module.path"))
+      entries.addAll(Arrays.asList(System.getProperty(key, "").split(File.pathSeparator)));
+    for (String entry : entries) {
+      if (entry.isEmpty()) continue;
+      Path path = Path.of(entry);
+      if (Files.isDirectory(path)) {
+        try (var files = Files.walk(path)) {
+          var it = files.filter(p -> p.toString().endsWith(".class")).iterator();
+          while (it.hasNext())
+            try (var in = Files.newInputStream(it.next())) {
+              header(in);
+            }
+        }
+      } else if (Files.isRegularFile(path) && entry.endsWith(".jar")) {
+        try (var jar = new JarFile(path.toFile(), false, JarFile.OPEN_READ, Runtime.version())) {
+          var it = jar.versionedStream().filter(e -> e.getName().endsWith(".class")).iterator();
+          while (it.hasNext())
+            try (var in = jar.getInputStream(it.next())) {
+              header(in);
+            }
+        }
+      }
+    }
+    for (String name : hierarchy.keySet())
+      if (descendant(name, new HashSet<>()) && !name.equals(Converter.class.getName()))
+        names.add(name);
+    if (names.size() > 256) throw new IOException("Too many Converter candidates");
+  }
+
+  private boolean descendant(String name, Set<String> visiting) {
+    if (name.equals(Converter.class.getName())) return true;
+    if (!visiting.add(name)) return false;
+    return hierarchy.getOrDefault(name, List.of()).stream().anyMatch(p -> descendant(p, visiting));
+  }
+
+  private void header(InputStream stream) throws IOException {
+    if (++indexed > 100000) throw new IOException("Converter class index exceeds limit");
+    var in = new DataInputStream(new BufferedInputStream(stream));
+    if (in.readInt() != 0xcafebabe) throw new IOException("Invalid class header");
+    in.readUnsignedShort();
+    in.readUnsignedShort();
+    Object[] cp = new Object[in.readUnsignedShort()];
+    for (int i = 1; i < cp.length; i++)
+      switch (in.readUnsignedByte()) {
+        case 1 -> cp[i] = in.readUTF();
+        case 7 -> cp[i] = in.readUnsignedShort();
+        case 3, 4, 9, 10, 11, 12, 17, 18 -> in.skipNBytes(4);
+        case 5, 6 -> {
+          in.skipNBytes(8);
+          i++;
+        }
+        case 8, 16, 19, 20 -> in.skipNBytes(2);
+        case 15 -> in.skipNBytes(3);
+        default -> throw new IOException("Unsupported constant pool entry");
+      }
+    in.readUnsignedShort();
+    String name = name(cp, in.readUnsignedShort());
+    int parent = in.readUnsignedShort();
+    var parents = new ArrayList<String>();
+    if (parent != 0) parents.add(name(cp, parent));
+    int n = in.readUnsignedShort();
+    for (int i = 0; i < n; i++) parents.add(name(cp, in.readUnsignedShort()));
+    hierarchy.putIfAbsent(name, parents);
+  }
+
+  private static String name(Object[] cp, int index) {
+    return ((String) cp[(Integer) cp[index]]).replace('/', '.');
+  }
+
+  List<BindingMetadata.Converter> candidates(Class<?> model, Class<?> control, String direction) {
+    return names.stream().map(n -> check(n, model, control, direction)).toList();
+  }
+
+  BindingMetadata.Converter check(String name, Class<?> model, Class<?> control, String direction) {
+    try {
+      Class<?> type = Class.forName(name, false, loader);
+      if (!Converter.class.isAssignableFrom(type)
+          || type.isInterface()
+          || Modifier.isAbstract(type.getModifiers())
+          || !Modifier.isPublic(type.getModifiers()))
+        return result(name, "ERROR", "Converter must be a public concrete implementation");
+      var meta = type.getDeclaredAnnotation(ConverterCapabilities.class);
+      if (meta == null)
+        return result(name, "UNVERIFIED", "No capabilities declared on this concrete class");
+      if (meta.reverse()
+          && type.getMethod("toAttributeValue", Object.class).getDeclaringClass()
+              == Converter.class)
+        return result(
+            name,
+            "ERROR",
+            "Reverse conversion declared but default throwing implementation inherited");
+      if (meta.modelType() == void.class || meta.controlType() == void.class)
+        return result(name, "ERROR", "Converter value types cannot be void");
+      if (!direction.equals("view") && !meta.reverse())
+        return result(name, "INCOMPATIBLE", "This direction requires reverse conversion");
+      if (model == null || control == null || model == Object.class || control == Object.class)
+        return result(name, "UNVERIFIED", "Binding value types are unresolved");
+      Class<?> m = BindingDiscovery.box(model),
+          c = BindingDiscovery.box(control),
+          a = BindingDiscovery.box(meta.modelType()),
+          b = BindingDiscovery.box(meta.controlType());
+      // Forward conversion is also used for initial value in model-direction bindings.
+      boolean ok = a.isAssignableFrom(m) && c.isAssignableFrom(b);
+      if (!direction.equals("view")) ok &= b.isAssignableFrom(c) && m.isAssignableFrom(a);
+      return result(
+          name,
+          ok ? "COMPATIBLE" : "INCOMPATIBLE",
+          ok ? "Declared types and direction match" : "Declared types do not support this binding");
+    } catch (ReflectiveOperationException | LinkageError | RuntimeException e) {
+      return result(name, "ERROR", "Converter metadata unavailable: " + e.getMessage());
+    }
+  }
+
+  private static BindingMetadata.Converter result(String n, String s, String r) {
+    return new BindingMetadata.Converter(n, s, r);
+  }
+}

designer-target-tools/src/main/java/de/neitzel/fxdesigner/target/WorkerMain.java

@@ -92,7 +92,7 @@ public final class WorkerMain {
               new Version(1, 0),
               new Version(1, 0),
               new Version(1, 0),
-              Set.of("discover-v1", "render-rgba-v1", "layout-v1", "interactive-v1", "bindings-v1"),
+              Set.of("discover-v1", "render-rgba-v1", "layout-v1", "interactive-v1", "bindings-v2"),
               Set.of()));
       var welcome = FrameCodec.read(socket.getInputStream());
       if (!(welcome instanceof Welcome w) || !session.equals(w.session()) || w.wire().major() != 1)
@@ -115,11 +115,16 @@ public final class WorkerMain {
           var loader = WorkerMain.class.getClassLoader();
           if (p.operation() == Operation.BINDINGS) {
             var input = PreviewMessages.decode(p.payload());
-            if (!input.keySet().equals(Set.of("fxml", "path", "direction")))
+            if (!input.keySet().equals(Set.of("fxml", "path", "direction", "control", "property")))
               throw new IOException("Invalid binding discovery input");
             var metadata =
                 BindingDiscovery.inspect(
-                    input.get("fxml"), input.get("path"), input.get("direction"), loader);
+                    input.get("fxml"),
+                    input.get("path"),
+                    input.get("direction"),
+                    input.get("control"),
+                    input.get("property"),
+                    loader);
             send(
                 socket,
                 new Packet(

designer-target-tools/src/test/java/de/neitzel/fxdesigner/target/ConverterDiscoveryTest.java

@@ -0,0 +1,148 @@
+package de.neitzel.fxdesigner.target;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import de.neitzel.fx.view.Converter;
+import de.neitzel.fxdesigner.preview.ConverterCapabilities;
+import java.util.*;
+import org.junit.jupiter.api.Test;
+
+class ConverterDiscoveryTest {
+  @ConverterCapabilities(modelType = Integer.class, controlType = String.class, reverse = true)
+  public static class Valid implements Converter {
+    static {
+      System.setProperty("converter.initialized", "yes");
+    }
+
+    public Valid() {
+      throw new AssertionError("constructed");
+    }
+
+    public Object toControlValue(Object value) {
+      throw new AssertionError("invoked");
+    }
+
+    public Object toAttributeValue(Object value) {
+      throw new AssertionError("invoked");
+    }
+  }
+
+  public static class Legacy extends Valid {}
+
+  @ConverterCapabilities(modelType = Integer.class, controlType = String.class, reverse = true)
+  public static class Broken implements Converter {
+    public Object toControlValue(Object v) {
+      return null;
+    }
+  }
+
+  @ConverterCapabilities(modelType = Integer.class, controlType = String.class)
+  public static class Forward implements Converter {
+    public Object toControlValue(Object v) {
+      return null;
+    }
+  }
+
+  private ConverterDiscovery discovery() throws Exception {
+    return new ConverterDiscovery(getClass().getClassLoader());
+  }
+
+  @Test
+  void discoveryNeverInitializesOrConstructsAndDoesNotInheritCapabilities() throws Exception {
+    var d = discovery();
+    assertTrue(
+        d.candidates(int.class, String.class, "both").stream()
+            .anyMatch(
+                c -> c.name().equals(Valid.class.getName()) && c.status().equals("COMPATIBLE")));
+    assertEquals(
+        "UNVERIFIED", d.check(Legacy.class.getName(), int.class, String.class, "both").status());
+    assertNull(System.getProperty("converter.initialized"));
+  }
+
+  @Test
+  void reverseContractAndConcreteTypeAreChecked() throws Exception {
+    var d = discovery();
+    assertEquals(
+        "ERROR", d.check(Broken.class.getName(), int.class, String.class, "view").status());
+    assertEquals(
+        "ERROR", d.check(String.class.getName(), int.class, String.class, "both").status());
+    assertEquals("ERROR", d.check("missing.Converter", int.class, String.class, "both").status());
+    assertEquals(
+        "INCOMPATIBLE",
+        d.check(Forward.class.getName(), int.class, String.class, "model").status());
+    assertEquals(
+        "COMPATIBLE", d.check(Forward.class.getName(), int.class, String.class, "view").status());
+  }
+
+  @Test
+  void forwardInitializationIsRequiredEvenForModelDirection() throws Exception {
+    var d = discovery();
+    assertEquals(
+        "INCOMPATIBLE",
+        d.check(Valid.class.getName(), Number.class, String.class, "model").status());
+    assertEquals(
+        "COMPATIBLE",
+        d.check(Valid.class.getName(), int.class, CharSequence.class, "view").status());
+    assertEquals(
+        "INCOMPATIBLE",
+        d.check(Valid.class.getName(), int.class, CharSequence.class, "both").status());
+  }
+
+  private String xml(String path, String property, String converter) {
+    return "<?neitzel-model "
+        + BindingDiscoveryTest.Model.class.getName()
+        + "?><?import javafx.scene.control.TextField?><TextField"
+        + " xmlns:fx='http://javafx.com/fxml/1'"
+        + " fx:id='input'><fx:define><de.neitzel.fx.view.BindingList><de.neitzel.fx.view.DirectBinding"
+        + " control='input' property='"
+        + property
+        + "' path='"
+        + path
+        + "' direction='both' converter='"
+        + converter
+        + "'/>"
+        + "</de.neitzel.fx.view.BindingList></fx:define></TextField>";
+  }
+
+  private de.neitzel.fxdesigner.protocol.BindingMetadata inspect(
+      String path, String property, String converter) throws Exception {
+    return BindingDiscovery.inspect(
+        xml(path, property, converter),
+        "rank",
+        "both",
+        "input",
+        "text",
+        getClass().getClassLoader());
+  }
+
+  @Test
+  void validatesPathsPropertiesAndConverterContracts() throws Exception {
+    assertTrue(inspect("rank", "text", Valid.class.getName()).problems().isEmpty());
+    assertTrue(
+        BindingDiscovery.inspect(
+                xml("absent", "text", ""),
+                "absent",
+                "both",
+                "input",
+                "text",
+                getClass().getClassLoader())
+            .problems()
+            .stream()
+            .anyMatch(p -> p.severity().equals("ERROR")));
+    for (var input :
+        List.of(
+            new String[] {"absent", "text", ""},
+            new String[] {"rank", "absent", ""},
+            new String[] {"rank", "width", ""},
+            new String[] {"rank", "text", ""},
+            new String[] {"rank", "text", Broken.class.getName()}))
+      assertTrue(
+          inspect(input[0], input[1], input[2]).problems().stream()
+              .anyMatch(p -> p.severity().equals("ERROR")),
+          Arrays.toString(input));
+    assertEquals(
+        "WARNING",
+        inspect("rank", "text", Legacy.class.getName()).problems().getFirst().severity());
+    assertNull(System.getProperty("converter.initialized"));
+  }
+}