From 725c4620b79a276e8a6526d22101f0229dfdc38a Mon Sep 17 00:00:00 2001 From: zprigge Date: Fri, 1 Jun 2018 14:38:18 +0200 Subject: [PATCH] Idea to solve escString attribute replacement generally, avoiding loops on the \L escape sequence and to avoid NullPtrExceptions. Including test cases. --- .../gef/dot/internal/generator/DotAttribute.java | 30 +++--- .../internal/generator/DotAttributeProcessor.java | 9 +- .../Dot2ZestEdgeAttributesConversionTests.xtend | 33 +++++++ .../Dot2ZestNodeAttributesConversionTests.xtend | 58 ++++++++++++ .../internal/ui/Dot2ZestAttributesConverter.java | 7 +- .../eclipse/gef/dot/internal/DotAttributes.xtend | 20 ++-- .../gef/dot/internal/language/terminals/ID.java | 103 +++++++++++++++++++++ 7 files changed, 232 insertions(+), 28 deletions(-) diff --git a/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttribute.java b/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttribute.java index 6ce6a15..5ce0c38 100644 --- a/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttribute.java +++ b/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttribute.java @@ -13,7 +13,7 @@ package org.eclipse.gef.dot.internal.generator; import java.lang.annotation.ElementType; import java.lang.annotation.Target; -import org.eclipse.gef.dot.internal.generator.DotAttributeProcessor; + import org.eclipse.xtend.lib.macro.Active; /** @@ -22,14 +22,22 @@ import org.eclipse.xtend.lib.macro.Active; @Target(ElementType.FIELD) @Active(DotAttributeProcessor.class) public @interface DotAttribute { - /** - * A string matching an ID.Type to use for this attribute - */ - public String[] rawType() default { "" }; - /** - * Type of the attribute. - * - * @return The type of the attribute. - */ - public Class[] parsedType(); + /** + * A string matching an ID.Type to use for this attribute + */ + public String[] rawType() default { "" }; + + /** + * Type of the attribute. + * + * @return The type of the attribute. + */ + public Class[] parsedType(); + + /** + * Whether the toValue function should be executed context sensitively + * + * @return true, if so. + */ + public boolean valueIsContextSensitive() default false; } diff --git a/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttributeProcessor.java b/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttributeProcessor.java index 4a9913f..7f7cbef 100644 --- a/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttributeProcessor.java +++ b/org.eclipse.gef.dot.generator/src/org/eclipse/gef/dot/internal/generator/DotAttributeProcessor.java @@ -61,6 +61,8 @@ public class DotAttributeProcessor extends AbstractFieldProcessor { "rawType"); TypeReference[] attributeParsedTypes = (TypeReference[]) annotationValue( field, context, "parsedType"); + Boolean valueIsContextSensitive = (Boolean) annotationValue(field, + context, "valueIsContextSensitive"); // field comment field.setDocComment("The '" + attributeName @@ -200,12 +202,17 @@ public class DotAttributeProcessor extends AbstractFieldProcessor { method.setReturnType( context.newTypeReference(String.class)); + String toValueParameter = valueIsContextSensitive + ? paramName(c) + : ""; + StringBuilder body = new StringBuilder(); body.append("ID " + attributeName + "Raw = " + rawGetterName(field) + "(" + paramName(c) + ");\n"); body.append("return " + attributeName + "Raw != null ? " - + attributeName + "Raw.toValue() : null;"); + + attributeName + "Raw.toValue(" + + toValueParameter + ") : null;"); method.setBody((ctx) -> body.toString()); context.setPrimarySourceElement(method, field); context.setPrimarySourceElement(method, field); diff --git a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestEdgeAttributesConversionTests.xtend b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestEdgeAttributesConversionTests.xtend index 60776dd..2c888ab 100644 --- a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestEdgeAttributesConversionTests.xtend +++ b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestEdgeAttributesConversionTests.xtend @@ -196,6 +196,14 @@ class Dot2ZestEdgeAttributesConversionTests { '''.assertEdgeExternalLabel("foo\nbar\nbaz") } + @Test def edge_externalLabel004() { + ''' + digraph testedGraphName { + 1->2[xlabel="g: \G e:\E h:\H t:\T"] + } + '''.assertEdgeExternalLabel("g: testedGraphName e:1->2 h:2 t:1") + } + @Test def edge_sourceLabel001() { ''' digraph { @@ -219,6 +227,14 @@ class Dot2ZestEdgeAttributesConversionTests { } '''.assertEdgeSourceLabel("foo\nbar\nbaz") } + + @Test def edge_sourceLabel004() { + ''' + digraph testedGraphName { + 1->2[taillabel="g: \G e:\E h:\H t:\T"] + } + '''.assertEdgeSourceLabel("g: testedGraphName e:1->2 h:2 t:1") + } @Test def edge_targetLabel001() { ''' @@ -244,6 +260,23 @@ class Dot2ZestEdgeAttributesConversionTests { '''.assertEdgeTargetLabel("foo\nbar\nbaz") } + @Test def edge_targetLabel004() { + ''' + digraph testedGraphName { + 1->2[headlabel="g: \G e:\E h:\H t:\T"] + } + '''.assertEdgeTargetLabel("g: testedGraphName e:1->2 h:2 t:1") + } + + @Test def edge_targetLabel005() { + //test against null pointer exception + ''' + digraph { + 1->2[headlabel="\G\L"] + } + '''.assertEdgeTargetLabel("") + } + private def assertEdgeStyle(CharSequence dotText, String expected) { val actual = dotText.firstEdge.convert.curveCssStyle.split expected.assertEquals(actual) diff --git a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestNodeAttributesConversionTests.xtend b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestNodeAttributesConversionTests.xtend index 227eb7d..77b702a 100644 --- a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestNodeAttributesConversionTests.xtend +++ b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/Dot2ZestNodeAttributesConversionTests.xtend @@ -139,6 +139,48 @@ class Dot2ZestNodeAttributesConversionTests { '''.assertNodeLabel("label") } + @Test def node_label006() { + ''' + digraph { + 1[label="label \N"] + } + '''.assertNodeLabel("label 1") + } + + @Test def node_label007() { + ''' + digraph { + sample [label="\N"] + } + '''.assertNodeLabel("sample") + } + + @Test def node_label008() { + ''' + graph mygraph { + a[label="graph: \G, node no. \N"] + } + '''.assertNodeLabel("graph: mygraph, node no. a") + } + + @Test def node_label009() { + //test to ascertain no loop is reached + ''' + graph { + a[label="\L"] + } + '''.assertNodeLabel("\\L") + } + + @Test def node_label010() { + //test to ascertain no NPE is reached + ''' + graph { + a[label="\G"] + } + '''.assertNodeLabel("") + } + @Test def node_id001() { ''' digraph { @@ -179,6 +221,14 @@ class Dot2ZestNodeAttributesConversionTests { '''.assertNodeXLabel("fantastic label") } + @Test def node_xlabel003() { + ''' + digraph testedGraphName { + 1[xlabel="node:\L graph:\G"] + } + '''.assertNodeXLabel("node:1 graph:testedGraphName") + } + @Test def node_polygonbasedshape001() { ''' digraph { @@ -471,6 +521,14 @@ class Dot2ZestNodeAttributesConversionTests { } '''.assertNodeTooltip("testing\nis\nfun") } + + @Test def node_tooltip003() { + ''' + digraph testing{ + nodename[label="label of \N", tooltip="l:\L n:\N g:\G"] + } + '''.assertNodeTooltip("l:label of nodename n:nodename g:testing") + } private def assertNodeWidth(CharSequence dotText, double expected) { val actual = dotText.firstNode.convert.size.width diff --git a/org.eclipse.gef.dot.ui/src/org/eclipse/gef/dot/internal/ui/Dot2ZestAttributesConverter.java b/org.eclipse.gef.dot.ui/src/org/eclipse/gef/dot/internal/ui/Dot2ZestAttributesConverter.java index 0e407ec..9265ea6 100644 --- a/org.eclipse.gef.dot.ui/src/org/eclipse/gef/dot/internal/ui/Dot2ZestAttributesConverter.java +++ b/org.eclipse.gef.dot.ui/src/org/eclipse/gef/dot/internal/ui/Dot2ZestAttributesConverter.java @@ -130,11 +130,6 @@ public class Dot2ZestAttributesConverter implements IAttributeCopier { } String dotLabel = DotAttributes.getLabel(dot); - if ("\\E".equals(dotLabel)) { //$NON-NLS-1$ - // The escString '\E' indicates that the edge's name becomes its - // label. - dotLabel = DotAttributes._getName(dot); - } if (dotLabel != null) { dotLabel = decode(dotLabel); ZestProperties.setLabel(zest, dotLabel); @@ -504,7 +499,7 @@ public class Dot2ZestAttributesConverter implements IAttributeCopier { // label String dotLabel = DotAttributes.getLabel(dot); - if (dotLabel == null || dotLabel.equals("\\N")) { //$NON-NLS-1$ + if (dotLabel == null) { // The escString '\N' (the node's default label) indicates that the // node's name becomes its label. dotLabel = DotAttributes._getName(dot); diff --git a/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/DotAttributes.xtend b/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/DotAttributes.xtend index 0a5f5c5..92cb9c9 100644 --- a/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/DotAttributes.xtend +++ b/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/DotAttributes.xtend @@ -1444,7 +1444,7 @@ class DotAttributes { @DotAttribute(rawType="NUMERAL", parsedType=Double) public static val String DISTORTION__N = "distortion" - @DotAttribute(parsedType=EscString) + @DotAttribute(parsedType=EscString, valueIsContextSensitive=true) public static val String EDGETOOLTIP__E = "edgetooltip" /** @@ -1467,28 +1467,28 @@ class DotAttributes { @DotAttribute(rawType="QUOTED_STRING", parsedType=Point) public static val String HEAD_LP__E = "head_lp" - @DotAttribute(parsedType=String) + @DotAttribute(parsedType=String, valueIsContextSensitive=true) public static val String HEADLABEL__E = "headlabel" @DotAttribute(parsedType=PortPos) public static val String HEADPORT__E = "headport" - @DotAttribute(parsedType=EscString) + @DotAttribute(parsedType=EscString, valueIsContextSensitive=true) public static val String HEADTOOLTIP__E = "headtooltip" @DotAttribute(rawType="NUMERAL", parsedType=Double) public static val String HEIGHT__N = "height" - @DotAttribute(parsedType=String) + @DotAttribute(parsedType=String, valueIsContextSensitive=true) public static val String ID__GCNE = "id" - @DotAttribute(parsedType=String) + @DotAttribute(parsedType=String, valueIsContextSensitive=true) public static val String LABEL__GCNE = "label" @DotAttribute(parsedType=Color) public static val String LABELFONTCOLOR__E = "labelfontcolor" - @DotAttribute(parsedType=EscString) + @DotAttribute(parsedType=EscString, valueIsContextSensitive=true) public static val String LABELTOOLTIP__E = "labeltooltip" @DotAttribute(rawType="STRING", parsedType=Layout) @@ -1538,22 +1538,22 @@ class DotAttributes { @DotAttribute(rawType="QUOTED_STRING", parsedType=Point) public static val String TAIL_LP__E = "tail_lp" - @DotAttribute(parsedType=String) + @DotAttribute(parsedType=String, valueIsContextSensitive=true) public static val String TAILLABEL__E = "taillabel" @DotAttribute(parsedType=PortPos) public static val String TAILPORT__E = "tailport" - @DotAttribute(parsedType=EscString) + @DotAttribute(parsedType=EscString, valueIsContextSensitive=true) public static val String TAILTOOLTIP__E = "tailtooltip" - @DotAttribute(parsedType=EscString) + @DotAttribute(parsedType=EscString, valueIsContextSensitive=true) public static val String TOOLTIP__CNE = "tooltip" @DotAttribute(rawType="NUMERAL", parsedType=Double) public static val String WIDTH__N = "width" - @DotAttribute(parsedType=String) + @DotAttribute(parsedType=String, valueIsContextSensitive=true) public static val String XLABEL__NE = "xlabel" @DotAttribute(rawType="QUOTED_STRING", parsedType=Point) diff --git a/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/terminals/ID.java b/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/terminals/ID.java index 888ac2a..95b339c 100644 --- a/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/terminals/ID.java +++ b/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/terminals/ID.java @@ -14,6 +14,11 @@ package org.eclipse.gef.dot.internal.language.terminals; import java.util.regex.Pattern; +import org.eclipse.gef.dot.internal.DotAttributes; +import org.eclipse.gef.graph.Edge; +import org.eclipse.gef.graph.Graph; +import org.eclipse.gef.graph.Node; + /** * A representation of a Dot ID according to the Graphviz DOT grammar. * @@ -270,6 +275,104 @@ public class ID { } /** + * @param node + * node related to ID + * @return decoded value + */ + public String toValue(Node node) { + Graph graph = node.getGraph(); + + ID rawLabel = DotAttributes.getLabelRaw(node); + + String nodeName = DotAttributes._getName(node); + String graphName = graph != null ? DotAttributes._getName(graph) : null; + + /* + * \L is replaced first using the raw Label, such that we can avoid a + * loop if a label contains \L. As such, we need to double all + * backslashes as single backslashes are consumed by replace all. + * + * For a node, the label defaults to \N. + * + * Graphviz behaviour differs slightly for unset names and error + * handling, however we cannot reproduce this (i.e. an internally used + * variable is produced and for escape sequences invalid in this + * context, e.g. \E, graphviz removes the backslash.) + */ + return toValue() + .replaceAll("\\\\L", + (rawLabel != null ? rawLabel.toValue() : "\\N") + .replaceAll("\\\\", "\\\\\\\\")) + .replaceAll("\\\\N", nodeName != null ? nodeName : "") + .replaceAll("\\\\G", graphName != null ? graphName : ""); + } + + /** + * @param edge + * edge related to ID + * @return decoded value + */ + public String toValue(Edge edge) { + Node tail = edge.getSource(); + Node head = edge.getTarget(); + Graph graph = edge.getGraph(); + + ID rawLabel = DotAttributes.getLabelRaw(edge); + + String edgeName = DotAttributes._getName(edge); + String tailName = tail != null ? DotAttributes._getName(tail) : null; + String headName = head != null ? DotAttributes._getName(head) : null; + String graphName = graph != null ? DotAttributes._getName(graph) : null; + + /* + * \L is replaced first using the raw Label, such that we can avoid a + * loop if a label contains \L. As such, we need to double all + * backslashes as single backslashes are consumed by replace all. + * + * Graphviz behaviour differs slightly for unset names and error + * handling, however we cannot reproduce this (i.e. an internally used + * variable is produced and for escape sequences invalid in this + * context, e.g. \N, graphviz removes the backslash.) + */ + return toValue() + .replaceAll("\\\\L", + (rawLabel != null ? rawLabel.toValue() : "") + .replaceAll("\\\\", "\\\\\\\\")) + .replaceAll("\\\\E", edgeName != null ? edgeName : "") + .replaceAll("\\\\T", tailName != null ? tailName : "") + .replaceAll("\\\\H", headName != null ? headName : "") + .replaceAll("\\\\G", graphName != null ? graphName : ""); + } + + /** + * @param graph + * graph related to ID + * @return decoded value + */ + public String toValue(Graph graph) { + + ID rawLabel = DotAttributes.getLabelRaw(graph); + + String graphName = DotAttributes._getName(graph); + + /* + * \L is replaced first using the raw Label, such that we can avoid a + * loop if a label contains \L. As such, we need to double all + * backslashes as single backslashes are consumed by replace all. + * + * Graphviz behaviour differs slightly for unset names and error + * handling, however we cannot reproduce this (i.e. an internally used + * variable is produced and for escape sequences invalid in this + * context, e.g. \N, graphviz removes the backslash.) + */ + return toValue() + .replaceAll("\\\\L", + (rawLabel != null ? rawLabel.toValue() : "") + .replaceAll("\\\\", "\\\\\\\\")) + .replaceAll("\\\\G", graphName != null ? graphName : ""); + } + + /** * Returns the (decoded) value. * * @return The (decoded) value. -- 2.7.4