[520919] Ensure that Dot Files with color="" are properly rendered.

- Modify the DotColor.xtext grammar to accept an empty color value. - Add er.dot example file to demonstrate the corresponding use case. - Implement corresponding DotParserTests test case. - Adapt the DotAttributesTests and DotContentAssistantTests test cases.
eclipse · Aug 19, 2017 · 20e8ee5 · 20e8ee5
1 parent 43b89ad
commit 20e8ee5
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 26 deletions.
diff --git a/org.eclipse.gef.dot.tests/resources/er.dot b/org.eclipse.gef.dot.tests/resources/er.dot
@@ -0,0 +1,129 @@
+/*******************************************************************************
+ * Copyright (c) 2017 itemis AG and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *       Tamas Miklossy  (itemis AG) - Initial text (bug #520919=
+ *******************************************************************************/
+
+// Sample graph from 
+// http://www.graphviz.org/content/ER
+// http://www.graphviz.org/Gallery/undirected/ER.gv.txt
+graph ER {
+	graph [bb="-154.44,170.56,189.1,-257.81",
+		fontsize=20,
+		label="\n\nEntity Relation Diagram\ndrawn by NEATO",
+		layout=neato,
+		lheight=1.28,
+		lp="17.33,120.56",
+		lwidth=2.76
+	];
+	node [color=lightgrey,
+		label="\N",
+		shape=diamond,
+		style=filled
+	];
+	{
+		node [label=name];
+		name0		 [color="",
+			height=0.5,
+			label=name,
+			pos="144.95,-173.45",
+			shape=ellipse,
+			style="",
+			width=0.83048];
+		name1		 [color="",
+			height=0.5,
+			label=name,
+			pos="-92.878,-239.81",
+			shape=ellipse,
+			style="",
+			width=0.83048];
+		name2		 [color="",
+			height=0.5,
+			label=name,
+			pos="-124.54,-10.021",
+			shape=ellipse,
+			style="",
+			width=0.83048];
+	}
+	course	 [color="",
+		height=0.5,
+		pos="86.543,-117.7",
+		shape=box,
+		style="",
+		width=0.75];
+	"C-I"	 [height=0.5,
+		pos="33.722,-183.7",
+		width=0.89363];
+	course -- "C-I"	 [label=n,
+		len=1.00,
+		lp="54.531,-145.83",
+		pos="72.109,-135.74 63.19,-146.88 51.958,-160.92 43.953,-170.92"];
+	institute	 [color="",
+		height=0.5,
+		pos="-48.25,-174.96",
+		shape=box,
+		style="",
+		width=0.81944];
+	institute -- name1	 [pos="-60.681,-193.03 -67.181,-202.47 -75.082,-213.95 -81.43,-223.18"];
+	"S-I"	 [height=0.5,
+		pos="-77.181,-98.113",
+		width=0.86809];
+	institute -- "S-I"	 [label=1,
+		len=1.00,
+		lp="-66.823,-142.42",
+		pos="-55.106,-156.75 -60.119,-143.43 -66.836,-125.59 -71.539,-113.1"];
+	student	 [color="",
+		height=0.5,
+		pos="-41.682,-21.084",
+		shape=box,
+		style="",
+		width=0.79167];
+	student -- name2	 [pos="-70.197,-17.277 -78.249,-16.202 -87.024,-15.03 -95.128,-13.948"];
+	grade	 [color="",
+		height=0.5,
+		pos="-1.7507,52.559",
+		shape=ellipse,
+		style="",
+		width=0.84854];
+	student -- grade	 [pos="-31.811,-2.8798 -25.529,8.7056 -17.438,23.628 -11.233,35.072"];
+	number	 [color="",
+		height=0.5,
+		pos="-77.705,51.111",
+		shape=ellipse,
+		style="",
+		width=1.0652];
+	student -- number	 [pos="-50.772,-2.866 -56.274,8.1616 -63.263,22.168 -68.742,33.149"];
+	"S-C"	 [height=0.5,
+		pos="38.268,-49.244",
+		width=0.99575];
+	student -- "S-C"	 [label=m,
+		len=1.00,
+		lp="-3.4218,-43.997",
+		pos="-12.818,-31.25 -2.9778,-34.716 7.838,-38.526 16.974,-41.744"];
+	name0 -- course	 [pos="128.99,-158.22 121.83,-151.38 113.32,-143.26 105.81,-136.09"];
+	code	 [color="",
+		height=0.5,
+		pos="160.5,-82.63",
+		shape=ellipse,
+		style="",
+		width=0.79437];
+	code -- course	 [pos="137.51,-93.534 129.96,-97.113 121.5,-101.12 113.67,-104.83"];
+	"C-I" -- institute	 [label=1,
+		len=1.00,
+		lp="-9.5553,-186.96",
+		pos="6.4263,-180.79 -1.5843,-179.94 -10.383,-179 -18.537,-178.13"];
+	"S-C" -- course	 [label=n,
+		len=1.00,
+		lp="57.27,-88.652",
+		pos="47.805,-62.769 55.225,-73.289 65.591,-87.99 73.734,-99.536"];
+	"S-I" -- student	 [label=n,
+		len=1.00,
+		lp="-63.793,-68.969",
+		pos="-70.504,-83.624 -64.719,-71.073 -56.323,-52.854 -50.083,-39.314"];
+}
diff --git a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotAttributesTests.java b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotAttributesTests.java
@@ -775,11 +775,11 @@ public void edge_labelfontcolor() {
 
 		// set invalid string values
 		try {
-			DotAttributes.setLabelfontcolor(edge, "");
+			DotAttributes.setLabelfontcolor(edge, "_");
 			fail("IllegalArgumentException expected.");
 		} catch (IllegalArgumentException e) {
 			assertEquals(
-					"Cannot set edge attribute 'labelfontcolor' to ''. The value '' is not a syntactically correct color: No viable alternative at input '<EOF>'.",
+					"Cannot set edge attribute 'labelfontcolor' to '_'. The value '_' is not a syntactically correct color: No viable alternative at character '_'.",
 					e.getMessage());
 		}
 	}

diff --git a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotContentAssistTests.java b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotContentAssistTests.java
@@ -370,21 +370,21 @@ public void edge_color() throws Exception {
 		// test global attribute values
 		newBuilder().append("digraph {edge[ color= ]}")
 				.assertTextAtCursorPosition(21,
-						combine(expectedX11ColorNames, "#", "/"))
+						combine(expectedX11ColorNames, "#", "/", ":", ";"))
 				.applyProposal(21, "#")
 				.expectContent("digraph {edge[ color=# ]}");
 
 		// test local attribute values
 		newBuilder().append("digraph {1->2[ color= ]}")
 				.assertTextAtCursorPosition(21,
-						combine(expectedX11ColorNames, "#", "/"))
+						combine(expectedX11ColorNames, "#", "/", ":", ";"))
 				.applyProposal(21, "/")
 				.expectContent("digraph {1->2[ color=/ ]}");
 
 		// test local attribute values with quotes
 		newBuilder().append("digraph {1->2[ color=\"\" ]}")
 				.assertTextAtCursorPosition(22,
-						combine(expectedX11ColorNames, "#", "/"))
+						combine(expectedX11ColorNames, "#", "/", ":", ";"))
 				.applyProposal(22, "#")
 				.expectContent("digraph {1->2[ color=\"#\" ]}");
 
@@ -404,15 +404,15 @@ public void edge_color() throws Exception {
 
 		// test local attribute value with local color scheme value
 		newBuilder().append("graph{1--2[color=; colorscheme=brbg10]}")
-				.assertTextAtCursorPosition(17, "#", "/", "1", "2", "3", "4",
-						"5", "6", "7", "8", "9", "10")
+				.assertTextAtCursorPosition(17, "#", "/", ":", ";", "1", "2",
+						"3", "4", "5", "6", "7", "8", "9", "10")
 				.applyProposal(17, "10")
 				.expectContent("graph{1--2[color=10; colorscheme=brbg10]}");
 
 		// test local attribute value with global color scheme value
 		newBuilder().append("graph{edge[colorscheme=brbg10] 1--2[color=]}")
-				.assertTextAtCursorPosition(42, "#", "/", "1", "2", "3", "4",
-						"5", "6", "7", "8", "9", "10")
+				.assertTextAtCursorPosition(42, "#", "/", ":", ";", "1", "2",
+						"3", "4", "5", "6", "7", "8", "9", "10")
 				.applyProposal(42, "10").expectContent(
 						"graph{edge[colorscheme=brbg10] 1--2[color=10]}");
 	}
@@ -878,21 +878,21 @@ public void graph_bgcolor() throws Exception {
 		// test global attribute values
 		newBuilder().append("graph {graph[ colorscheme=svg bgcolor= ]}")
 				.assertTextAtCursorPosition(38,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(38, "aliceblue").expectContent(
 						"graph {graph[ colorscheme=svg bgcolor=aliceblue ]}");
 
 		// test local attribute values
 		newBuilder().append("graph { colorscheme=svg bgcolor= }")
 				.assertTextAtCursorPosition(32,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(32, "aqua")
 				.expectContent("graph { colorscheme=svg bgcolor=aqua }");
 
 		// test local attribute values with quotes
 		newBuilder().append("graph { colorscheme=svg bgcolor=\"\" }")
 				.assertTextAtCursorPosition(33,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(33, "aquamarine").expectContent(
 						"graph { colorscheme=svg bgcolor=\"aquamarine\" }");
 
@@ -917,15 +917,15 @@ public void graph_bgcolor() throws Exception {
 
 		// test local attribute value with local color scheme value
 		newBuilder().append("graph{colorscheme=brbg10 bgcolor= 1}")
-				.assertTextAtCursorPosition(33, "#", "/", "1", "2", "3", "4",
-						"5", "6", "7", "8", "9", "10")
+				.assertTextAtCursorPosition(33, "#", "/", ":", ";", "1", "2",
+						"3", "4", "5", "6", "7", "8", "9", "10")
 				.applyProposal(33, "10")
 				.expectContent("graph{colorscheme=brbg10 bgcolor=10 1}");
 
 		// test local attribute value with global color scheme value
 		newBuilder().append("graph{graph[colorscheme=brbg10] bgcolor= 1}")
-				.assertTextAtCursorPosition(40, "#", "/", "1", "2", "3", "4",
-						"5", "6", "7", "8", "9", "10")
+				.assertTextAtCursorPosition(40, "#", "/", ":", ";", "1", "2",
+						"3", "4", "5", "6", "7", "8", "9", "10")
 				.applyProposal(40, "10")
 				.expectContent("graph{graph[colorscheme=brbg10] bgcolor=10 1}");
 	}
@@ -1424,21 +1424,21 @@ public void node_fillcolor() throws Exception {
 		// test global attribute values
 		newBuilder().append("graph {node[ colorscheme=svg fillcolor= ]}")
 				.assertTextAtCursorPosition(39,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(39, "#")
 				.expectContent("graph {node[ colorscheme=svg fillcolor=# ]}");
 
 		// test local attribute values
 		newBuilder().append("graph {1[ colorscheme=svg fillcolor= ]}")
 				.assertTextAtCursorPosition(36,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(36, "#")
 				.expectContent("graph {1[ colorscheme=svg fillcolor=# ]}");
 
 		// test local attribute values with quotes
 		newBuilder().append("graph {1[ colorscheme=svg fillcolor=\"\" ]}")
 				.assertTextAtCursorPosition(37,
-						combine(expectedSvgColorNames, "#", "/"))
+						combine(expectedSvgColorNames, "#", "/", ":", ";"))
 				.applyProposal(37, "#")
 				.expectContent("graph {1[ colorscheme=svg fillcolor=\"#\" ]}");
 

diff --git a/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotParserTests.java b/org.eclipse.gef.dot.tests/src/org/eclipse/gef/dot/tests/DotParserTests.java
@@ -339,6 +339,11 @@ public void testColor() {
 		testFile("color.dot");
 	}
 
+	@Test
+	public void testER() {
+		testFile("er.dot");
+	}
+
 	@Test
 	public void testGrdangles() {
 		testFile("grdangles.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
@@ -414,12 +414,15 @@ class DotAttributes {
 			case CLUSTERRANK__G: validateAttributeRawValue(CLUSTERMODE_PARSER, null, attributeContext, attributeName, attributeValue)
 			case COLORSCHEME__GCNE: validateAttributeRawValue(null, COLORSCHEME_VALIDATOR,	attributeContext, attributeName, attributeValue)
 			case COLOR__CNE:
-				// TODO: remove "attributeContext == Context.GRAPH", since color is not a valid graph attribute
-				if(attributeContext == Context.GRAPH || attributeContext == Context.CLUSTER || attributeContext == Context.NODE)
-					validateAttributeRawValue(COLOR_PARSER, COLOR_VALIDATOR, attributeContext, attributeName, attributeValue)
-				else if (attributeContext == Context.EDGE)
-					validateAttributeRawValue(COLORLIST_PARSER, COLORLIST_VALIDATOR, attributeContext, attributeName, attributeValue)
-				else
+				if(attributeValue!==null && !attributeValue.toValue.isEmpty){
+					// TODO: remove "attributeContext == Context.GRAPH", since color is not a valid graph attribute
+					if(attributeContext == Context.GRAPH || attributeContext == Context.CLUSTER || attributeContext == Context.NODE)
+						validateAttributeRawValue(COLOR_PARSER, COLOR_VALIDATOR, attributeContext, attributeName, attributeValue)
+					else if (attributeContext == Context.EDGE)
+						validateAttributeRawValue(COLORLIST_PARSER, COLORLIST_VALIDATOR, attributeContext, attributeName, attributeValue)
+					else
+						Collections.emptyList
+				}else
 					Collections.emptyList
 			case DIR__E: validateAttributeRawValue(DIRTYPE_PARSER, null, attributeContext, attributeName, attributeValue)
 			case DISTORTION__N: validateAttributeRawValue(DOUBLE_PARSER, DISTORTION_VALIDATOR, attributeContext, attributeName, attributeValue)

diff --git a/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/DotColor.xtext b/org.eclipse.gef.dot/src/org/eclipse/gef/dot/internal/language/DotColor.xtext
@@ -21,8 +21,9 @@ import "http://www.eclipse.org/emf/2002/Ecore" as ecore
  * to be able to parse the hex values (such as '#000000' and '#ffffff') properly.
  */
 
+// XXX: While not documented explicitly, an empty color seems to be valid as well (see bug #520919)
 Color:
-	RGBColor | HSVColor | StringColor
+	(RGBColor | HSVColor | StringColor)?
 ;
 
 RGBColor: