Merge pull request #1358 from alixander/keyword-container-name

d2ir: prevent illegal non-tail keywords

ci/release/changelogs/next.md

@@ -15,3 +15,4 @@
 - Fixes grid layout overwriting label placements for nested objects. [#1345](https://github.com/terrastruct/d2/pull/1345)
 - Fixes fonts not rendering correctly on certain platforms. Thanks @mikeday for identifying the solution. [#1356](https://github.com/terrastruct/d2/pull/1356)
 - Fixes folders not rendering in animations (`--animate-interval`) [#1357](https://github.com/terrastruct/d2/pull/1357)
+- Fixes panic using reserved keywords as containers [#1358](https://github.com/terrastruct/d2/pull/1358)

d2compiler/compile_test.go

@@ -1722,6 +1722,13 @@ y -> x.style
 `,
 			expErr: `d2/testdata/d2compiler/TestCompile/edge_to_style.d2:2:8: reserved keywords are prohibited in edges`,
 		},
+		{
+			name: "keyword-container",
+
+			text: `a.near.b
+`,
+			expErr: `d2/testdata/d2compiler/TestCompile/keyword-container.d2:1:3: "near" must be the last part of the key`,
+		},
 		{
 			name: "escaped_id",
 

d2graph/d2graph.go

@@ -1618,9 +1618,6 @@ func Key(k *d2ast.KeyPath) []string {
 // All reserved keywords. See init below.
 var ReservedKeywords map[string]struct{}
 
-// All reserved keywords not including style keywords.
-var ReservedKeywords2 map[string]struct{}
-
 // Non Style/Holder keywords.
 var SimpleReservedKeywords = map[string]struct{}{
 	"label":          {},
@@ -1642,16 +1639,20 @@ var SimpleReservedKeywords = map[string]struct{}{
 	"vertical-gap":   {},
 	"horizontal-gap": {},
 	"class":          {},
-	"classes":        {},
 }
 
-// ReservedKeywordHolders are reserved keywords that are meaningless on its own and exist solely to hold a set of reserved keywords
+// ReservedKeywordHolders are reserved keywords that are meaningless on its own and must hold composites
 var ReservedKeywordHolders = map[string]struct{}{
 	"style":            {},
 	"source-arrowhead": {},
 	"target-arrowhead": {},
 }
 
+// CompositeReservedKeywords are reserved keywords that can hold composites
+var CompositeReservedKeywords = map[string]struct{}{
+	"classes": {},
+}
+
 // StyleKeywords are reserved keywords which cannot exist outside of the "style" keyword
 var StyleKeywords = map[string]struct{}{
 	"opacity":       {},
@@ -1725,23 +1726,15 @@ func init() {
 		ReservedKeywords[k] = v
 	}
 	for k, v := range ReservedKeywordHolders {
-		ReservedKeywords[k] = v
+		CompositeReservedKeywords[k] = v
 	}
 	for k, v := range BoardKeywords {
+		CompositeReservedKeywords[k] = v
+	}
+	for k, v := range CompositeReservedKeywords {
 		ReservedKeywords[k] = v
 	}
 
-	ReservedKeywords2 = make(map[string]struct{})
-	for k, v := range SimpleReservedKeywords {
-		ReservedKeywords2[k] = v
-	}
-	for k, v := range ReservedKeywordHolders {
-		ReservedKeywords2[k] = v
-	}
-	for k, v := range BoardKeywords {
-		ReservedKeywords2[k] = v
-	}
-
 	NearConstants = make(map[string]struct{}, len(NearConstantsArray))
 	for _, k := range NearConstantsArray {
 		NearConstants[k] = struct{}{}

d2ir/d2ir.go

@@ -669,10 +669,9 @@ func (m *Map) ensureField(i int, kp *d2ast.KeyPath, refctx *RefContext) (*Field,
 
 	if _, ok := d2graph.ReservedKeywords[strings.ToLower(head)]; ok {
 		head = strings.ToLower(head)
-	}
-
-	if head == "class" && i < len(kp.Path)-1 {
-		return nil, d2parser.Errorf(kp.Path[i].Unbox(), `"class" must be the last part of the key`)
+		if _, ok := d2graph.CompositeReservedKeywords[head]; !ok && i < len(kp.Path)-1 {
+			return nil, d2parser.Errorf(kp.Path[i].Unbox(), fmt.Sprintf(`"%s" must be the last part of the key`, head))
+		}
 	}
 
 	if head == "_" {

lib/urlenc/testdata/TestChanges.exp.txt

@@ -0,0 +1 @@
+vFdtb9s4Ev6uXzFwWvQuiN5sN0mFwx1yadMUaLHe2rv9UmBBiyOZrUSqJGUn3ea_L4aSbFl2usUusPkQkzPkzMNnXkhZUSIpoBR3qBMYjeB3DwDvqoJJRkwm8K3kHgDAyQmcni72NpyeOo0Pb-QnTC0wbs5gs1K-VX6mikJtzkDJpWKaC5m3a6-VXKM2zjqUitcFdppaG6WFzM8qlovGf6tabJBZ4MjrqhBpXzNHvUYOnFkGhcrzxtE378GboaoKBC5Mqtao7xMYDUXwURrUa5HiyGO8JeGKN6dzVDiiA0qZBEYnaczG2WTkPXje7lzQmkhg9NNO2LO8zzL4_4UhjiNLDu0fWdRi9lJKRyFRR9sY5ppVqy-FB5CqskRpPQBbGFYJ76G3PmjXJfCaBj-__ShvkKOmyoO51cwquHbl0NBB6ZqAKFlOUaPkT2BlbWWSMKyrQjEebMRnUSIXLFA6D2lW0SwkHEqa0K7qchnGYXwRtj5_e6tyFZh1HsbjKKru_KE8qGS-D7s9VJeeJyeUImjD2qAGWofSwuqea5crZ7AWRixFIew9ZKKwSGnmfetbbMhJYPF27l_N3sC_lkjkc6w0pkTHv4_QRkHYD8swSv_x4cCJZzeCWhFklDSLdnKjHWwOTQD3uaWZCSxqzYzVdWqJgdCoVLAijOKJ35r0Y-JrGKmHnkeCfCz-YrZSEmGDy8cXdxzdLhYzuJJcK8G9DS4T-IDLH0bNvtYaww-4fDqO5k1uPx1H19SHw6uq2pO-VCUT0hw_lPemIsxJz_OzzrUxF8H668Y5FCZ0W8Jf0bXuD0JjgcaErKoK9IWz4sdj_3yaL_2q1iR9PoniC7_8JONJUfjsf084Eia_yJ88O4TSkpEcISHlchqQKBPUiB0ialcNO6Epmba-g2B8i-lKqkLlAk34PB6HrLHb6F0ZHLKwwaXLw23UdjloUqVdY-8a90c5d6LRke6WZRyzmLrbSpUImsnPtPeWJu_dxOtZ7pre9kqYP9qmDuAsVlpkFt7Pro-s3nO-Q9m7CROYDsQEP4qiV5OXBP97CFqQzknb8G-3Y-fu5NGmf7z--6b-EtydgSNkDbR_n5zHvLXElEyumLVMwjiBd93Ey8VXuqvTzwm87oZe034cFQnM3aS5dzxLzfi-EpQfbggzgQPXPU8DzdbZQN7zNzxG569tCYN6aKtzIK00cuHei93pxwnMdsI2of_84ku59F0x-5XMg6xglmau0KmGz6fnsft3Gb9obzKHvqsGl39NWX63KreE9QNTYpmydIUJvGtHR1oQj7-cZxHD4i6qlA7SQtU8o-smkGjDShPZ1i9UrkzIMZrGz6OpH3G-9KeT7NxfXr5AP8NoOo0macouud955e15vAxtukrghn56h-gewAlYXeOOR3rhNtuYrTXSRjf48a1EnpB54oiji_qHdzqolAqt8w6Ey7DGmHckN_6p1GghEJpDEDvw2140DlLdvDz6WbVXYPQoPNRuE2c0GuqOVEb_Ttg2on6L2qkOoSVwzSQXnFlsUmTXSsfbF-tjLeuwIOiP5Dvr-t22fQ-X31zcXN68ouUP9GnDdHG_FJon8IqG_xea3o-1ZXkCv1jm7teKUbjn9NO-n2sprECTwPVu4j14aNMEgiBAmx7w63RdWt92L1Gvn23bAO0J27j0Zc7WjsLme9F4fwQAAP__

lib/urlenc/urlenc.go

@@ -20,15 +20,9 @@ var compressionDict = "->" +
 
 func init() {
 	var common []string
-	for k := range d2graph.StyleKeywords {
-		common = append(common, k)
-	}
 	for k := range d2graph.ReservedKeywords {
 		common = append(common, k)
 	}
-	for k := range d2graph.ReservedKeywordHolders {
-		common = append(common, k)
-	}
 	sort.Strings(common)
 	for _, k := range common {
 		compressionDict += k

lib/urlenc/urlenc_test.go

@@ -21,3 +21,168 @@ I just forgot my whole philosophy of life!!!: {
 
 	assert.String(t, script, decoded)
 }
+
+// TestChanges makes it explicit in PRs when encoding changes
+// Something we might want to know for playground compatability
+func TestChanges(t *testing.T) {
+	// Choose something with many keywords and varied text
+	const script = `timeline mixer: "" {
+  explanation: |md
+    ## **Timeline mixer**
+    - Inject ads, who-to-follow, onboarding
+    - Conversation module
+    - Cursoring,pagination
+    - Tweat deduplication
+    - Served data logging
+  |
+}
+People discovery: "People discovery \nservice"
+admixer: Ad mixer {
+  style.fill: "#c1a2f3"
+}
+
+onboarding service: "Onboarding \nservice"
+timeline mixer -> People discovery
+timeline mixer -> onboarding service
+timeline mixer -> admixer
+container0: "" {
+  graphql
+  comment
+  tlsapi
+}
+container0.graphql: GraphQL\nFederated Strato Column {
+  shape: image
+  icon: https://upload.wikimedia.org/wikipedia/commons/thumb/1/17/GraphQL_Logo.svg/1200px-GraphQL_Logo.svg.png
+}
+container0.comment: |md
+  ## Tweet/user content hydration, visibility filtering
+|
+container0.tlsapi: TLS-API (being deprecated)
+container0.graphql -> timeline mixer
+timeline mixer <- container0.tlsapi
+twitter fe: "Twitter Frontend " {
+  icon: https://icons.terrastruct.com/social/013-twitter-1.svg
+  shape: image
+}
+twitter fe -> container0.graphql: iPhone web
+twitter fe -> container0.tlsapi: HTTP Android
+web: Web {
+  icon: https://icons.terrastruct.com/azure/Web%20Service%20Color/App%20Service%20Domains.svg
+  shape: image
+}
+
+Iphone: {
+  icon: 'https://ss7.vzw.com/is/image/VerizonWireless/apple-iphone-12-64gb-purple-53017-mjn13ll-a?$device-lg$'
+  shape: image
+}
+Android: {
+  icon: https://cdn4.iconfinder.com/data/icons/smart-phones-technologies/512/android-phone.png
+  shape: image
+}
+
+web -> twitter fe
+timeline scorer: "Timeline\nScorer" {
+  style.fill: "#ffdef1"
+}
+home ranker: Home Ranker
+
+timeline service: Timeline Service
+timeline mixer -> timeline scorer: Thrift RPC
+timeline mixer -> home ranker: {
+  style.stroke-dash: 4
+  style.stroke: "#000E3D"
+}
+timeline mixer -> timeline service
+home mixer: Home mixer {
+  # style.fill: "#c1a2f3"
+}
+container0.graphql -> home mixer: {
+  style.stroke-dash: 4
+  style.stroke: "#000E3D"
+}
+home mixer -> timeline scorer
+home mixer -> home ranker: {
+  style.stroke-dash: 4
+  style.stroke: "#000E3D"
+}
+home mixer -> timeline service
+manhattan 2: Manhattan
+gizmoduck: Gizmoduck
+socialgraph: Social graph
+tweetypie: Tweety Pie
+home mixer -> manhattan 2
+home mixer -> gizmoduck
+home mixer -> socialgraph
+home mixer -> tweetypie
+Iphone -> twitter fe
+Android -> twitter fe
+prediction service2: Prediction Service {
+  shape: image
+  icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png
+}
+home scorer: Home Scorer {
+  style.fill: "#ffdef1"
+}
+manhattan: Manhattan
+memcache: Memcache {
+  icon: https://d1q6f0aelx0por.cloudfront.net/product-logos/de041504-0ddb-43f6-b89e-fe04403cca8d-memcached.png
+}
+
+fetch: Fetch {
+  style.multiple: true
+  shape: step
+}
+
+feature: Feature {
+  style.multiple: true
+  shape: step
+}
+scoring: Scoring {
+  style.multiple: true
+  shape: step
+}
+fetch -> feature
+feature -> scoring
+
+prediction service: Prediction Service {
+  shape: image
+  icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png
+}
+scoring -> prediction service
+fetch -> container2.crmixer
+
+home scorer -> manhattan: ""
+
+home scorer -> memcache: ""
+home scorer -> prediction service2
+home ranker -> home scorer
+home ranker -> container2.crmixer: Candidate Fetch
+container2: "" {
+  style.stroke: "#000E3D"
+  style.fill: "#ffffff"
+  crmixer: CrMixer {
+    style.fill: "#F7F8FE"
+  }
+  earlybird: EarlyBird
+  utag: Utag
+  space: Space
+  communities: Communities
+}
+etc: ...etc
+
+home scorer -> etc: Feature Hydration
+
+feature -> manhattan
+feature -> memcache
+feature -> etc: Candidate sources
+`
+
+	encoded, err := Encode(script)
+	assert.Success(t, err)
+	assert.Testdata(t, ".txt", []byte(encoded))
+
+	decoded, err := Decode(encoded)
+	assert.Success(t, err)
+
+	assert.String(t, script, decoded)
+}

testdata/d2compiler/TestCompile/keyword-container.exp.json

@@ -0,0 +1,12 @@
+{
+  "graph": null,
+  "err": {
+    "ioerr": null,
+    "errs": [
+      {
+        "range": "d2/testdata/d2compiler/TestCompile/keyword-container.d2,0:2:2-0:6:6",
+        "errmsg": "d2/testdata/d2compiler/TestCompile/keyword-container.d2:1:3: \"near\" must be the last part of the key"
+      }
+    ]
+  }
+}