diff --git a/ci/release/changelogs/next.md b/ci/release/changelogs/next.md index 8451bba92..ff0a475fa 100644 --- a/ci/release/changelogs/next.md +++ b/ci/release/changelogs/next.md @@ -6,6 +6,7 @@ - ELK self loops get distributed around the object instead of stacking [#1232](https://github.com/terrastruct/d2/pull/1232) - ELK preserves order of objects in cycles [#1235](https://github.com/terrastruct/d2/pull/1235) +- Improper usages of `class` and `style` get error messages [#1254](https://github.com/terrastruct/d2/pull/1254) #### Bugfixes ⛑️ diff --git a/d2compiler/compile.go b/d2compiler/compile.go index 7cec4208a..c6bc4ec34 100644 --- a/d2compiler/compile.go +++ b/d2compiler/compile.go @@ -197,6 +197,7 @@ func (c *compiler) compileField(obj *d2graph.Object, f *d2ir.Field) { return } else if f.Name == "style" { if f.Map() == nil { + c.errorf(f.LastRef().AST(), `"style" expected to be set to a map, or contain an additional keyword like "style.opacity: 0.4"`) return } c.compileStyle(&obj.Attributes, f.Map()) @@ -481,6 +482,10 @@ func (c *compiler) compileStyle(attrs *d2graph.Attributes, m *d2ir.Map) { } func (c *compiler) compileStyleField(attrs *d2graph.Attributes, f *d2ir.Field) { + if _, ok := d2graph.StyleKeywords[strings.ToLower(f.Name)]; !ok { + c.errorf(f.LastRef().AST(), `invalid style keyword: "%s"`, f.Name) + return + } if f.Primary() == nil { return } diff --git a/d2compiler/compile_test.go b/d2compiler/compile_test.go index 628e03e80..3662ee2b2 100644 --- a/d2compiler/compile_test.go +++ b/d2compiler/compile_test.go @@ -1678,6 +1678,24 @@ x.a.b`, }`, expErr: `d2/testdata/d2compiler/TestCompile/no-nested-columns-class.d2:3:5: class fields cannot have children`, }, + { + name: "improper-class-ref", + + text: `myobj.class.style.stroke-dash: 3`, + expErr: `d2/testdata/d2compiler/TestCompile/improper-class-ref.d2:1:7: "class" must be the last part of the key`, + }, + { + name: "tail-style", + + text: `myobj.style: 3`, + expErr: `d2/testdata/d2compiler/TestCompile/tail-style.d2:1:7: "style" expected to be set to a map, or contain an additional keyword like "style.opacity: 0.4"`, + }, + { + name: "bad-style-nesting", + + text: `myobj.style.style.stroke-dash: 3`, + expErr: `d2/testdata/d2compiler/TestCompile/bad-style-nesting.d2:1:13: invalid style keyword: "style"`, + }, { name: "edge_to_style", diff --git a/d2ir/d2ir.go b/d2ir/d2ir.go index 26f60c57d..50fb8c104 100644 --- a/d2ir/d2ir.go +++ b/d2ir/d2ir.go @@ -671,6 +671,10 @@ func (m *Map) ensureField(i int, kp *d2ast.KeyPath, refctx *RefContext) (*Field, 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 head == "_" { return nil, d2parser.Errorf(kp.Path[i].Unbox(), `parent "_" can only be used in the beginning of paths, e.g. "_.x"`) } diff --git a/d2renderers/d2sketch/sketch_test.go b/d2renderers/d2sketch/sketch_test.go index 177ed47dc..c1aa0cb06 100644 --- a/d2renderers/d2sketch/sketch_test.go +++ b/d2renderers/d2sketch/sketch_test.go @@ -244,7 +244,7 @@ Android: { web -> twitter fe timeline scorer: "Timeline\nScorer" { - style.fill "#ffdef1" + style.fill: "#ffdef1" } home ranker: Home Ranker @@ -283,7 +283,7 @@ prediction service2: Prediction Service { icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png } home scorer: Home Scorer { - style.fill "#ffdef1" + style.fill: "#ffdef1" } manhattan: Manhattan memcache: Memcache { @@ -766,7 +766,7 @@ Android: { web -> twitter fe timeline scorer: "Timeline\nScorer" { - style.fill "#ffdef1" + style.fill: "#ffdef1" } home ranker: Home Ranker @@ -805,7 +805,7 @@ prediction service2: Prediction Service { icon: https://cdn-icons-png.flaticon.com/512/6461/6461819.png } home scorer: Home Scorer { - style.fill "#ffdef1" + style.fill: "#ffdef1" } manhattan: Manhattan memcache: Memcache { diff --git a/d2renderers/d2sketch/testdata/twitter_dark/sketch.exp.svg b/d2renderers/d2sketch/testdata/twitter_dark/sketch.exp.svg index 88e22dde2..cfe5de158 100644 --- a/d2renderers/d2sketch/testdata/twitter_dark/sketch.exp.svg +++ b/d2renderers/d2sketch/testdata/twitter_dark/sketch.exp.svg @@ -1,27 +1,27 @@ - @@ -859,7 +859,7 @@ -People discovery serviceAd mixerOnboarding serviceTwitter Frontend WebIphoneAndroidTimelineScorerHome RankerTimeline ServiceHome mixerManhattanGizmoduckSocial graphTweety PiePrediction ServiceHome ScorerManhattanMemcacheFetchFeatureScoringPrediction Service...etc

Timeline mixer

+People discovery serviceAd mixerOnboarding serviceTwitter Frontend WebIphoneAndroidTimelineScorerHome RankerTimeline ServiceHome mixerManhattanGizmoduckSocial graphTweety PiePrediction ServiceHome ScorerManhattanMemcacheFetchFeatureScoringPrediction Service...etc

Timeline mixer

  • Inject ads, who-to-follow, onboarding
  • Conversation module
    • @@ -868,7 +868,7 @@
  • Served data logging
GraphQLFederated Strato Column

Tweet/user content hydration, visibility filtering

-
TLS-API (being deprecated)CrMixerEarlyBirdUtagSpaceCommunities iPhone web HTTP Android Thrift RPC Candidate Fetch Feature Hydration Candidate sources +
TLS-API (being deprecated)CrMixerEarlyBirdUtagSpaceCommunities iPhone web HTTP Android Thrift RPC Candidate Fetch Feature Hydration Candidate sources diff --git a/testdata/d2compiler/TestCompile/bad-style-nesting.exp.json b/testdata/d2compiler/TestCompile/bad-style-nesting.exp.json new file mode 100644 index 000000000..4017433f8 --- /dev/null +++ b/testdata/d2compiler/TestCompile/bad-style-nesting.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/bad-style-nesting.d2,0:12:12-0:17:17", + "errmsg": "d2/testdata/d2compiler/TestCompile/bad-style-nesting.d2:1:13: invalid style keyword: \"style\"" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/improper-class-ref.exp.json b/testdata/d2compiler/TestCompile/improper-class-ref.exp.json new file mode 100644 index 000000000..02fd3f333 --- /dev/null +++ b/testdata/d2compiler/TestCompile/improper-class-ref.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/improper-class-ref.d2,0:6:6-0:11:11", + "errmsg": "d2/testdata/d2compiler/TestCompile/improper-class-ref.d2:1:7: \"class\" must be the last part of the key" + } + ] + } +} diff --git a/testdata/d2compiler/TestCompile/tail-style.exp.json b/testdata/d2compiler/TestCompile/tail-style.exp.json new file mode 100644 index 000000000..de962fbdf --- /dev/null +++ b/testdata/d2compiler/TestCompile/tail-style.exp.json @@ -0,0 +1,12 @@ +{ + "graph": null, + "err": { + "ioerr": null, + "errs": [ + { + "range": "d2/testdata/d2compiler/TestCompile/tail-style.d2,0:6:6-0:11:11", + "errmsg": "d2/testdata/d2compiler/TestCompile/tail-style.d2:1:7: \"style\" expected to be set to a map, or contain an additional keyword like \"style.opacity: 0.4\"" + } + ] + } +}