Add source highlighting and revision-checked diagnostic fixes in both hosts

This commit is contained in:
2026-11-13 18:19:39 +01:00
parent 8a4f456e38
commit 9bc4569407
18 changed files with 1033 additions and 14 deletions
@@ -25,7 +25,8 @@ public final class DesignerPane extends BorderPane implements AutoCloseable {
private final Path file;
private final KitView kit;
private final Label status = new Label("Opening document");
private final TextArea source = new TextArea(), output = new TextArea();
private final DiagnosticTextArea source = new DiagnosticTextArea();
private final TextArea output = new TextArea();
private final ComboBox<DesignerWorkflow.Example> examples = new ComboBox<>();
private final SplitPane design;
private final ContentPanelController content;
@@ -124,6 +125,7 @@ public final class DesignerPane extends BorderPane implements AutoCloseable {
private DiagnosticsPane diagnosticsPane;
private final String diagnosticsOwner = java.util.UUID.randomUUID().toString();
private long diagnosticsRevision = -1;
private long diagnosticGeneration;
private SplitPane sourceSplit;
private Integer pendingSourceOffset;
private String pendingReferenceValidation;
@@ -727,6 +729,8 @@ public final class DesignerPane extends BorderPane implements AutoCloseable {
host.diagnostics().clear(diagnosticsOwner);
if (diagnosticsPane != null) diagnosticsPane.show(java.util.List.of(), message);
diagnosticsRevision = -1;
diagnosticGeneration++;
source.diagnostics("", java.util.List.of());
validatedSource = null;
validatingSource = null;
validationPreview = false;
@@ -743,8 +747,34 @@ public final class DesignerPane extends BorderPane implements AutoCloseable {
var diagnostics = java.util.List.copyOf(all);
diagnosticsRevision = draft.revision();
String label = context + " · draft revision " + draft.revision();
diagnosticsPane.show(diagnostics, label);
host.diagnostics().replace(diagnosticsOwner, draft, label, diagnostics);
var captured = draft;
long generation = ++diagnosticGeneration;
java.util.function.Consumer<HostServices.QuickFix> apply =
fix -> fx(() -> applyDiagnosticFix(captured, generation, diagnostics, fix));
diagnosticsPane.show(diagnostics, label, apply);
source.diagnostics(captured.text(), diagnostics);
host.diagnostics().replace(diagnosticsOwner, captured, label, diagnostics, apply);
}
private void applyDiagnosticFix(
DocumentSnapshot captured,
long generation,
java.util.List<HostServices.Diagnostic> diagnostics,
HostServices.QuickFix fix) {
if (closed || generation != diagnosticGeneration) return;
withDraft(
() -> {
if (generation != diagnosticGeneration
|| draft.revision() != captured.revision()
|| !draft.text().equals(captured.text())
|| !draft.writable()
|| diagnostics.stream().noneMatch(d -> d.fixes().contains(fix))) {
status.setText("Diagnostic changed; validate again before applying a fix");
return;
}
pendingReferenceValidation = apply(draft.text(), fix.edits());
submit(fix.edits(), fix.label());
});
}
private void validationUnavailable(Throwable error) {
@@ -1378,6 +1408,8 @@ public final class DesignerPane extends BorderPane implements AutoCloseable {
public void close() {
if (closed) return;
closed = true;
diagnosticGeneration++;
source.diagnostics("", java.util.List.of());
host.diagnostics().clear(diagnosticsOwner);
try {
changes.close();
@@ -0,0 +1,115 @@
package de.neitzel.fxdesigner.kit;
import de.neitzel.fxdesigner.contracts.DocumentSnapshot;
import de.neitzel.fxdesigner.contracts.EditBatch.Replacement;
import de.neitzel.fxdesigner.host.HostServices.*;
import java.util.*;
import org.w3c.dom.Element;
/** Conservative, lexical fixes; no inference about application semantics. */
final class DiagnosticActions {
static List<Diagnostic> localBindings(DocumentSnapshot source, BindingDocument xml) {
var out = new ArrayList<Diagnostic>();
var occupied = new HashSet<String>();
for (int i = 0; i < xml.elements.size(); i++) {
Element e = xml.elements.get(i);
if (!binding(xml, e) || children(e)) continue;
if (xml.named(e, "DirectBinding")
&& !Set.of("view", "model", "both").contains(e.getAttribute("direction"))) {
var tag = xml.source.tags.get(i);
var attr = tag.attributes().get("direction");
var range =
attr == null
? new SourceRange(tag.start(), tag.openEnd())
: new SourceRange(
attr.valueStart() == attr.valueEnd() ? attr.start() : attr.valueStart(),
attr.valueStart() == attr.valueEnd() ? attr.end() : attr.valueEnd());
var choices = new ArrayList<QuickFix>();
if (source.writable())
for (String direction : List.of("view", "model", "both"))
choices.add(
new QuickFix(
"Set binding direction to " + direction,
xml.source.attributes(i, Map.of("direction", direction))));
out.add(
new Diagnostic(
"FXML",
Severity.ERROR,
"Invalid or missing direction: "
+ e.getAttribute("control")
+ "."
+ e.getAttribute("property"),
Optional.of(new Location(source.document(), range.start())),
Optional.of(range),
choices));
}
if (!xml.named(e, "Repeat")) {
String target = e.getAttribute("control") + "." + e.getAttribute("property");
if (!occupied.add(target))
out.add(
at(
source,
xml,
i,
"FXML",
Severity.ERROR,
"Duplicate binding target: " + target,
true));
}
}
return out;
}
static Diagnostic at(
DocumentSnapshot source,
BindingDocument xml,
int index,
String operation,
Severity severity,
String message,
boolean allowRemoval) {
var tag = xml.source.tags.get(index);
var e = xml.elements.get(index);
var fixes =
source.writable()
&& allowRemoval
&& severity == Severity.ERROR
&& binding(xml, e)
&& !children(e)
&& !e.hasAttributeNS(BindingDocument.FX, "id")
? List.of(
new QuickFix(
"Remove this binding declaration",
List.of(new Replacement(tag.start(), tag.end(), ""))))
: List.<QuickFix>of();
return new Diagnostic(
operation,
severity,
message,
Optional.of(new Location(source.document(), tag.start())),
Optional.of(new SourceRange(tag.start(), tag.openEnd())),
fixes);
}
private static boolean binding(BindingDocument xml, Element e) {
// Only declarations in the root BindingList are eligible, including its elements wrapper.
if (!(xml.named(e, "DirectBinding")
|| xml.named(e, "ComputedBinding")
|| xml.named(e, "Repeat"))) return false;
var parent = e.getParentNode();
if (parent instanceof Element p && p.getTagName().equals("elements"))
parent = p.getParentNode();
return parent instanceof Element list
&& xml.named(list, "BindingList")
&& list.getParentNode() instanceof Element define
&& BindingDocument.FX.equals(define.getNamespaceURI())
&& "define".equals(define.getLocalName())
&& define.getParentNode() == xml.document.getDocumentElement();
}
private static boolean children(Element e) {
for (var n = e.getFirstChild(); n != null; n = n.getNextSibling())
if (n instanceof Element) return true;
return false;
}
}
@@ -0,0 +1,94 @@
package de.neitzel.fxdesigner.kit;
import de.neitzel.fxdesigner.host.HostServices.Diagnostic;
import java.util.*;
import javafx.scene.Node;
import javafx.scene.control.*;
import javafx.scene.control.skin.TextAreaSkin;
import javafx.scene.paint.Color;
import javafx.scene.shape.Path;
/** Diagnostic underlines in the existing TextArea; input, selection and undo stay unchanged. */
final class DiagnosticTextArea extends TextArea {
private String evidence = "";
private List<Diagnostic> diagnostics = List.of();
DiagnosticTextArea() {
setId("fxml-source");
}
void diagnostics(String text, List<Diagnostic> values) {
evidence = text;
diagnostics = List.copyOf(values);
if (getSkin() instanceof HighlightSkin skin) skin.refresh();
}
@Override
protected Skin<?> createDefaultSkin() {
return new HighlightSkin(this);
}
private static final class HighlightSkin extends TextAreaSkin {
private final DiagnosticTextArea area;
private final List<Node> marks = new ArrayList<>();
private boolean dirty = true;
HighlightSkin(DiagnosticTextArea area) {
super(area);
this.area = area;
registerChangeListener(area.textProperty(), o -> refresh());
registerChangeListener(area.fontProperty(), o -> refresh());
registerChangeListener(area.widthProperty(), o -> refresh());
registerChangeListener(area.heightProperty(), o -> refresh());
registerChangeListener(area.wrapTextProperty(), o -> refresh());
}
void refresh() {
dirty = true;
area.requestLayout();
}
@Override
protected void layoutChildren(double x, double y, double w, double h) {
super.layoutChildren(x, y, w, h);
// TextAreaSkin only resizes its ScrollPane; lay out its content before reading glyphs.
for (var child : getChildren())
if (child instanceof javafx.scene.Parent parent) parent.layout();
if (!dirty) return;
dirty = false;
removeHighlight(marks);
marks.clear();
if (!area.getText().equals(area.evidence)) return;
for (var d : area.diagnostics)
d.range()
.ifPresent(
range -> {
if (range.end() > area.getLength() || range.start() == range.end()) return;
var path = new Path(getUnderlineShape(range.start(), range.end()));
path.setManaged(false);
path.setMouseTransparent(true);
path.setFill(null);
path.setStroke(
switch (d.severity()) {
case ERROR -> Color.FIREBRICK;
case WARNING -> Color.DARKORANGE;
case INFO -> Color.STEELBLUE;
});
path.setStrokeWidth(1.5);
path.getStyleClass().add("diagnostic-highlight");
path.getStyleClass()
.add("diagnostic-" + d.severity().name().toLowerCase(java.util.Locale.ROOT));
path.setUserData(d);
marks.add(path);
addHighlight(List.of(path), range.start());
});
}
@Override
public void dispose() {
removeHighlight(marks);
marks.clear();
super.dispose();
}
}
}
@@ -1,6 +1,7 @@
package de.neitzel.fxdesigner.kit;
import de.neitzel.fxdesigner.host.HostServices.Diagnostic;
import de.neitzel.fxdesigner.host.HostServices.QuickFix;
import java.util.List;
import java.util.function.Consumer;
import javafx.scene.control.*;
@@ -10,6 +11,8 @@ import javafx.scene.layout.VBox;
/** One shared, navigable diagnostic list for both hosts. */
final class DiagnosticsPane extends VBox {
private final ListView<Diagnostic> problems = new ListView<>();
private final ComboBox<QuickFix> fixes = new ComboBox<>();
private Consumer<QuickFix> apply = f -> {};
private final Label state = new Label("Target validation not run");
DiagnosticsPane(Runnable validate, Consumer<Diagnostic> navigate) {
@@ -46,15 +49,40 @@ final class DiagnosticsPane extends VBox {
e.consume();
}
});
fixes.setId("diagnostic-fixes");
fixes.setPromptText("Choose a fix");
fixes.setPrefWidth(280);
problems
.getSelectionModel()
.selectedItemProperty()
.addListener(
(o, a, b) -> {
fixes.getSelectionModel().clearSelection();
fixes.getItems().setAll(b == null ? List.of() : b.fixes());
});
var fix = new Button("Apply diagnostic fix");
fix.disableProperty().bind(fixes.getSelectionModel().selectedItemProperty().isNull());
fix.setOnAction(
e -> {
var selected = fixes.getValue();
if (selected != null) apply.accept(selected);
});
var check = new Button("Validate View");
check.setOnAction(e -> validate.run());
var go = new Button("Go to FXML");
go.setOnAction(e -> open.run());
go.disableProperty().bind(problems.getSelectionModel().selectedItemProperty().isNull());
getChildren().addAll(new javafx.scene.layout.HBox(6, check, go), state, problems);
getChildren().addAll(new javafx.scene.layout.HBox(6, check, go, fixes, fix), state, problems);
}
void show(List<Diagnostic> diagnostics, String context) {
show(diagnostics, context, f -> {});
}
void show(List<Diagnostic> diagnostics, String context, Consumer<QuickFix> action) {
apply = action;
fixes.getSelectionModel().clearSelection();
fixes.getItems().clear();
problems.getItems().setAll(diagnostics);
state.setText(context + " · " + diagnostics.size() + " diagnostics");
}
@@ -12,10 +12,14 @@ final class FxmlDiagnostics {
static List<Diagnostic> local(DocumentSnapshot source) {
var messages = new LinkedHashSet<String>();
int offset = 0;
var located = new ArrayList<Diagnostic>();
try {
var xml = new BindingDocument(source.text());
offset = xml.source.tags.getFirst().start();
messages.addAll(xml.errors());
located.addAll(DiagnosticActions.localBindings(source, xml));
var detailed =
located.stream().map(Diagnostic::message).collect(java.util.stream.Collectors.toSet());
xml.errors().stream().filter(m -> !detailed.contains(m)).forEach(messages::add);
messages.addAll(new TableDocument(source.text()).errors());
messages.addAll(new ReferenceDocument(source.text()).referenceErrors());
} catch (Exception e) {
@@ -24,7 +28,9 @@ final class FxmlDiagnostics {
messages.add(String.valueOf(e.getMessage()));
}
int origin = offset;
return messages.stream().map(m -> diagnostic(source, "FXML", "ERROR", m, origin)).toList();
located.addAll(
messages.stream().map(m -> diagnostic(source, "FXML", "ERROR", m, origin)).toList());
return List.copyOf(located);
}
static List<Diagnostic> target(DocumentSnapshot source, List<Problem> problems) {
@@ -55,8 +61,8 @@ final class FxmlDiagnostics {
if (matches.isEmpty()) matches.add(0);
for (int i : matches)
result.add(
diagnostic(
source, "Bindings", p.severity(), p.message(), xml.source.tags.get(i).start()));
DiagnosticActions.at(
source, xml, i, "Bindings", Severity.valueOf(p.severity()), p.message(), true));
}
} catch (Exception e) {
return local(source);
@@ -78,7 +84,9 @@ final class FxmlDiagnostics {
operation,
Severity.valueOf(severity),
message,
Optional.of(new Location(source.document(), offset)));
Optional.of(new Location(source.document(), offset)),
Optional.of(new SourceRange(offset, Math.min(source.text().length(), offset + 1))),
List.of());
}
static int offset(String text, int line, int column) {
@@ -0,0 +1,112 @@
package de.neitzel.fxdesigner.kit;
import static org.junit.jupiter.api.Assertions.*;
import de.neitzel.fxdesigner.contracts.*;
import de.neitzel.fxdesigner.host.HostServices.*;
import de.neitzel.fxdesigner.protocol.BindingMetadata.Problem;
import java.net.URI;
import java.util.*;
import org.junit.jupiter.api.Test;
class DiagnosticActionsTest {
private static final String XML =
"<?neitzel-model app.Person?>\r\n"
+ "<?import de.neitzel.fx.view.*?><VBox"
+ " xmlns:fx='http://javafx.com/fxml/1'><fx:define><BindingList><!-- before"
+ " --><DirectBinding control='name' property='text' path='name' direction = 'bad'"
+ " extra='keep'/><!-- after --></BindingList></fx:define><Label fx:id='name'"
+ " text='bad'/></VBox>";
private DocumentSnapshot snapshot(String text, boolean writable) {
return new DocumentSnapshot(URI.create("file:/View.fxml"), 7, text, writable, true);
}
private Diagnostic direction(DocumentSnapshot d) {
return FxmlDiagnostics.local(d).stream()
.filter(p -> p.message().startsWith("Invalid or missing direction:"))
.findFirst()
.orElseThrow();
}
private String apply(DocumentSnapshot d, QuickFix fix) {
return new EditBatch(d.revision(), fix.label(), fix.edits()).applyTo(d);
}
@Test
void directionChoicesAreExplicitAndChangeOnlyTheAttributeValue() {
var d = snapshot(XML, true);
var problem = direction(d);
assertEquals(
"bad",
XML.substring(problem.range().orElseThrow().start(), problem.range().orElseThrow().end()));
assertEquals(3, problem.fixes().size());
for (var fix : problem.fixes()) {
String value = fix.label().substring(fix.label().lastIndexOf(' ') + 1);
assertEquals(XML.replace("direction = 'bad'", "direction = '" + value + "'"), apply(d, fix));
}
}
@Test
void missingAndEmptyDirectionKeepSourceAndHighlightAnActualRange() {
for (String text :
List.of(XML.replace(" direction = 'bad'", ""), XML.replace("'bad' extra", "'' extra"))) {
var d = snapshot(text, true);
var p = direction(d);
assertTrue(p.range().orElseThrow().end() > p.range().orElseThrow().start());
String fixed = apply(d, p.fixes().getFirst());
assertTrue(fixed.contains("<!-- before -->"));
assertTrue(fixed.contains("extra='keep'"));
assertFalse(
FxmlDiagnostics.local(snapshot(fixed, true)).stream()
.anyMatch(x -> x.message().startsWith("Invalid or missing direction:")));
}
}
@Test
void duplicateRemovalPreservesFirstDeclarationAndSurroundingComments() {
String declaration = "<ComputedBinding control='name' property='text' method='#name'/>";
String text =
XML.replace("'bad' extra", "'view' extra")
.replace("<!-- after -->", "<!-- after -->" + declaration + "<!-- end -->");
var d = snapshot(text, true);
var p =
FxmlDiagnostics.local(d).stream()
.filter(x -> x.message().startsWith("Duplicate binding target:"))
.findFirst()
.orElseThrow();
assertEquals(text.replace(declaration, ""), apply(d, p.fixes().getFirst()));
}
@Test
void targetFixesRequireKnownSimpleDeclarationsAndNeverRemoveControls() {
var p = new Problem("name", "text", "ERROR", "unknown path");
var d = snapshot(XML, true);
assertEquals(1, FxmlDiagnostics.target(d, List.of(p)).getFirst().fixes().size());
for (String text :
List.of(
XML.replace("extra='keep'", "fx:id='binding'"),
XML.replace(
"extra='keep'/>", "extra='keep'><converter>app.C</converter></DirectBinding>"),
XML.replace("DirectBinding", "app.UnknownBinding")))
assertTrue(
FxmlDiagnostics.target(snapshot(text, true), List.of(p)).getFirst().fixes().isEmpty());
assertTrue(
FxmlDiagnostics.target(d, List.of(new Problem("name", "text", "WARNING", "unverified")))
.getFirst()
.fixes()
.isEmpty());
}
@Test
void readOnlyAndStaleSourceCannotApplyFixes() {
assertTrue(direction(snapshot(XML, false)).fixes().isEmpty());
var d = snapshot(XML, true);
var fix = direction(d).fixes().getFirst();
var edit = new EditBatch(d.revision(), fix.label(), fix.edits());
assertThrows(
ConcurrentModificationException.class,
() -> edit.applyTo(new DocumentSnapshot(d.document(), 8, XML, true, true)));
assertThrows(IllegalStateException.class, () -> edit.applyTo(snapshot(XML, false)));
}
}
@@ -760,6 +760,137 @@ public final class ProjectionHostChecks {
+ " withdrawal, correction, exact undo/redo and malformed FXML recovery");
}
@SuppressWarnings("unchecked")
private static void diagnosticFixes(DesignerPane pane, DocumentSession document, String full)
throws Exception {
String bad = full.replaceFirst("direction=\"both\"", "direction = 'bad'");
set(document, bad);
await(() -> fx(() -> source(pane).getText().equals(bad)));
bindingTab(pane, "Problems");
var issue =
diagnostics(pane).stream()
.filter(d -> d.message().startsWith("Invalid or missing direction:"))
.findFirst()
.orElseThrow();
fx(
() -> {
((ListView<de.neitzel.fxdesigner.host.HostServices.Diagnostic>)
pane.lookup("#diagnostics-list"))
.getSelectionModel()
.select(issue);
return null;
});
click(pane, "Go to FXML");
await(() -> fx(() -> !pane.lookupAll(".diagnostic-highlight").isEmpty()));
fx(
() -> {
var area = source(pane);
int caret = area.getCaretPosition();
area.setWrapText(true);
area.setScrollTop(100);
area.setScrollLeft(20);
pane.applyCss();
pane.layout();
check(area.getCaretPosition() == caret, "Highlighting changed caret");
var combo =
(ComboBox<de.neitzel.fxdesigner.host.HostServices.QuickFix>)
pane.lookup("#diagnostic-fixes");
combo
.getSelectionModel()
.select(
combo.getItems().stream()
.filter(f -> f.label().equals("Set binding direction to both"))
.findFirst()
.orElseThrow());
return null;
});
fx(
() -> {
pane.applyCss();
pane.layout();
var area = source(pane);
var text =
area.lookupAll(".text").stream()
.filter(
n ->
n instanceof javafx.scene.text.Text t
&& t.getText().equals(area.getText()))
.map(n -> (javafx.scene.text.Text) n)
.findFirst()
.orElseThrow();
for (var mark : area.lookupAll(".diagnostic-highlight")) {
var d = (de.neitzel.fxdesigner.host.HostServices.Diagnostic) mark.getUserData();
var range = d.range().orElseThrow();
var shape =
new javafx.scene.shape.Path(text.underlineShape(range.start(), range.end()));
shape.setStrokeWidth(1.5);
var expected = text.localToScene(shape.getBoundsInLocal());
var actual = mark.localToScene(mark.getBoundsInLocal());
check(
Math.abs(expected.getMinX() - actual.getMinX()) < .5
&& Math.abs(expected.getMinY() - actual.getMinY()) < .5
&& Math.abs(expected.getMaxX() - actual.getMaxX()) < .5
&& Math.abs(expected.getMaxY() - actual.getMaxY()) < .5,
"Underline is not aligned with wrapped source glyphs: "
+ expected
+ " / "
+ actual);
}
var image = java.nio.file.Files.createTempFile("designer-source-highlights-", ".png");
javax.imageio.ImageIO.write(
javafx.embed.swing.SwingFXUtils.fromFXImage(pane.snapshot(null, null), null),
"png",
image.toFile());
System.out.println("Diagnostic source image: " + image);
return null;
});
click(pane, "Apply diagnostic fix");
String fixed = bad.replace("direction = 'bad'", "direction = 'both'");
await(() -> documentText(document).equals(fixed));
repeatHistory(pane, document, bad, fixed);
await(
() ->
diagnostics(pane).stream()
.noneMatch(d -> d.message().startsWith("Invalid or missing direction:")));
String duplicate =
"<de.neitzel.fx.view.DirectBinding control=\"rankInput\" property=\"text\" path=\"rank\""
+ " direction=\"view\"/>";
String duplicated =
full.replace("</BindingList>", "<!--keep-->" + duplicate + "</BindingList>");
set(document, duplicated);
await(() -> fx(() -> source(pane).getText().equals(duplicated)));
bindingTab(pane, "Problems");
var conflict =
diagnostics(pane).stream()
.filter(d -> d.message().startsWith("Duplicate binding target:"))
.findFirst()
.orElseThrow();
fx(
() -> {
((ListView<de.neitzel.fxdesigner.host.HostServices.Diagnostic>)
pane.lookup("#diagnostics-list"))
.getSelectionModel()
.select(conflict);
((ComboBox<?>) pane.lookup("#diagnostic-fixes")).getSelectionModel().selectFirst();
return null;
});
click(pane, "Apply diagnostic fix");
String removed = duplicated.replace(duplicate, "");
await(() -> documentText(document).equals(removed));
repeatHistory(pane, document, duplicated, removed);
set(document, full);
await(() -> fx(() -> source(pane).getText().equals(full)));
fx(
() -> {
source(pane).setWrapText(false);
return null;
});
click(pane, "Split");
System.out.println(
"PASS diagnostic fixes: source underlines, wrap/scroll without caret edits, explicit"
+ " direction and duplicate removal, lexical preservation and single-step Undo/Redo");
}
private static void structureEdits(DesignerPane pane, DocumentSession document, String full)
throws Exception {
String base =
@@ -1778,6 +1909,7 @@ public final class ProjectionHostChecks {
tableEdits(pane, document, full);
structureEdits(pane, document, full);
diagnosticsEdits(pane, document, full);
diagnosticFixes(pane, document, full);
document
.save(document.snapshot().toCompletableFuture().get().revision())
.toCompletableFuture()