From 6517384bbb5688d4e2cc565c826b7c37421a00e9 Mon Sep 17 00:00:00 2001
From: Adam Obuchowicz <adam.obuchowicz@enso.org>
Date: Tue, 6 Feb 2024 11:45:16 +0100
Subject: [PATCH] Interpret documented nodes properly (#8978)

When a node has documentation, it becomes a special "Documented" AST node.
---
 app/gui2/e2e/graphRenderNodes.spec.ts         | 10 +++-
 app/gui2/mock/engine.ts                       |  1 +
 .../src/components/GraphEditor/collapsing.ts  |  4 +-
 app/gui2/src/stores/graph/graphDatabase.ts    |  1 +
 app/gui2/src/util/ast/node.ts                 | 52 +++++++++++++------
 5 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/app/gui2/e2e/graphRenderNodes.spec.ts b/app/gui2/e2e/graphRenderNodes.spec.ts
index 1ccd24dd48..0e68041a8e 100644
--- a/app/gui2/e2e/graphRenderNodes.spec.ts
+++ b/app/gui2/e2e/graphRenderNodes.spec.ts
@@ -1,4 +1,4 @@
-import { test } from '@playwright/test'
+import { expect, test } from '@playwright/test'
 import * as actions from './actions'
 import * as customExpect from './customExpect'
 import * as locate from './locate'
@@ -7,4 +7,12 @@ test('graph can open and render nodes', async ({ page }) => {
   await actions.goToGraph(page)
   await customExpect.toExist(locate.graphEditor(page))
   await customExpect.toExist(locate.graphNode(page))
+
+  // check simple node's content (without input widgets)
+  const sumNode = locate.graphNodeByBinding(page, 'sum')
+  await expect(sumNode.locator('.WidgetToken')).toHaveText(['five', '+', 'ten'])
+
+  // check documented node's content
+  const finalNode = locate.graphNodeByBinding(page, 'final')
+  await expect(finalNode.locator('.WidgetToken')).toHaveText(['Main', '.', 'func1', 'prod'])
 })
diff --git a/app/gui2/mock/engine.ts b/app/gui2/mock/engine.ts
index 56a4ec06d6..20ecd04bbf 100644
--- a/app/gui2/mock/engine.ts
+++ b/app/gui2/mock/engine.ts
@@ -56,6 +56,7 @@ main =
     ten = 10
     sum = five + ten
     prod = sum * 3
+    ## This node can be entered
     final = Main.func1 prod
     list = []
     text = 'test'
diff --git a/app/gui2/src/components/GraphEditor/collapsing.ts b/app/gui2/src/components/GraphEditor/collapsing.ts
index 35cd980f24..dac4b19742 100644
--- a/app/gui2/src/components/GraphEditor/collapsing.ts
+++ b/app/gui2/src/components/GraphEditor/collapsing.ts
@@ -195,7 +195,9 @@ export function performCollapse(
   })
 
   // Insert a new function.
-  const collapsedNodeIds = collapsed.map((ast) => asNodeId(nodeFromAst(ast).rootSpan.id)).reverse()
+  const collapsedNodeIds = collapsed
+    .map((ast) => asNodeId(nodeFromAst(ast)?.rootSpan.id ?? ast.id))
+    .reverse()
   let outputNodeId: NodeId | undefined
   const outputIdentifier = info.extracted.output?.identifier
   if (outputIdentifier != null) {
diff --git a/app/gui2/src/stores/graph/graphDatabase.ts b/app/gui2/src/stores/graph/graphDatabase.ts
index f241e0dc53..5306045334 100644
--- a/app/gui2/src/stores/graph/graphDatabase.ts
+++ b/app/gui2/src/stores/graph/graphDatabase.ts
@@ -339,6 +339,7 @@ export class GraphDb {
     const currentNodeIds = new Set<NodeId>()
     for (const nodeAst of functionAst_.bodyExpressions()) {
       const newNode = nodeFromAst(nodeAst)
+      if (!newNode) continue
       const nodeId = asNodeId(newNode.rootSpan.id)
       const node = this.nodeIdToNode.get(nodeId)
       const nodeMeta = (node ?? newNode).rootSpan.nodeMetadata
diff --git a/app/gui2/src/util/ast/node.ts b/app/gui2/src/util/ast/node.ts
index 0f28e500f6..5164f4deb7 100644
--- a/app/gui2/src/util/ast/node.ts
+++ b/app/gui2/src/util/ast/node.ts
@@ -2,22 +2,40 @@ import type { Node } from '@/stores/graph'
 import { Ast } from '@/util/ast'
 import { Vec2 } from '@/util/data/vec2'
 
-export function nodeFromAst(ast: Ast.Ast): Node {
-  if (ast instanceof Ast.Assignment) {
-    return {
-      outerExprId: ast.id,
-      pattern: ast.pattern ?? undefined,
-      rootSpan: ast.expression ?? ast,
-      position: Vec2.Zero,
-      vis: undefined,
-    }
-  } else {
-    return {
-      outerExprId: ast.id,
-      pattern: undefined,
-      rootSpan: ast,
-      position: Vec2.Zero,
-      vis: undefined,
-    }
+export function nodeFromAst(ast: Ast.Ast): Node | undefined {
+  const nodeCode = ast instanceof Ast.Documented ? ast.expression : ast
+  if (!nodeCode) return
+  return {
+    outerExprId: ast.id,
+    pattern: nodeCode instanceof Ast.Assignment ? nodeCode.pattern : undefined,
+    rootSpan: nodeCode instanceof Ast.Assignment ? nodeCode.expression : nodeCode,
+    position: Vec2.Zero,
+    vis: undefined,
   }
 }
+
+if (import.meta.vitest) {
+  const { test, expect } = await import('vitest')
+  const { initializeFFI } = await import('shared/ast/ffi')
+  await initializeFFI()
+
+  test.each`
+    line                               | pattern      | rootSpan
+    ${'2 + 2'}                         | ${undefined} | ${'2 + 2'}
+    ${'foo = bar'}                     | ${'foo'}     | ${'bar'}
+    ${'## Documentation\n2 + 2'}       | ${undefined} | ${'2 + 2'}
+    ${'## Documentation\nfoo = 2 + 2'} | ${'foo'}     | ${'2 + 2'}
+  `('Node information from AST $line line', ({ line, pattern, rootSpan }) => {
+    const ast = Ast.Ast.parse(line)
+    const node = nodeFromAst(ast)
+    expect(node?.outerExprId).toBe(ast.id)
+    expect(node?.pattern?.code()).toBe(pattern)
+    expect(node?.rootSpan.code()).toBe(rootSpan)
+  })
+
+  test.each(['## Documentation only'])("'%s' should not be a node", (line) => {
+    const ast = Ast.Ast.parse(line)
+    const node = nodeFromAst(ast)
+    expect(node).toBeUndefined()
+  })
+}