Add Support for Fillet with Extrude in the Sketch Pipe (#4168)

* update code mod

* A snapshot a day keeps the bugs away! 📷🐛 (OS: windows-latest)

* fmt

* lint

* make yarn-tsc happy

* fmt

* typo

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

src/lang/modifyAst/addFillet.test.ts

@@ -77,22 +77,30 @@ const runGetPathToExtrudeForSegmentSelectionTest = async (
       code.indexOf(expectedExtrudeSnippet),
       code.indexOf(expectedExtrudeSnippet) + expectedExtrudeSnippet.length,
     ]
-    const expedtedExtrudePath = getNodePathFromSourceRange(ast, extrudeRange)
+    const expectedExtrudePath = getNodePathFromSourceRange(ast, extrudeRange)
-    const expedtedExtrudeNodeResult = getNodeFromPath<VariableDeclarator>(
+    const expectedExtrudeNodeResult = getNodeFromPath<
-      ast,
+      VariableDeclarator | CallExpression
-      expedtedExtrudePath
+    >(ast, expectedExtrudePath)
-    )
+    if (err(expectedExtrudeNodeResult)) {
-    if (err(expedtedExtrudeNodeResult)) {
+      return expectedExtrudeNodeResult
-      return expedtedExtrudeNodeResult
     }
-    const expectedExtrudeNode = expedtedExtrudeNodeResult.node
+    const expectedExtrudeNode = expectedExtrudeNodeResult.node
-    const init = expectedExtrudeNode.init
+
-    if (init.type !== 'CallExpression' && init.type !== 'PipeExpression') {
+    // check whether extrude is in the sketch pipe
-      return new Error(
+    const extrudeInSketchPipe = expectedExtrudeNode.type === 'CallExpression'
-        'Expected extrude expression is not a CallExpression or PipeExpression'
+    if (extrudeInSketchPipe) {
-      )
+      return expectedExtrudeNode
     }
-    return init
+    if (!extrudeInSketchPipe) {
+      const init = expectedExtrudeNode.init
+      if (init.type !== 'CallExpression' && init.type !== 'PipeExpression') {
+        return new Error(
+          'Expected extrude expression is not a CallExpression or PipeExpression'
+        )
+      }
+      return init
+    }
+    return new Error('Expected extrude expression not found')
   }
 
   // ast
@@ -160,6 +168,23 @@ extrude001 = extrude(-15, sketch001)`
       expectedExtrudeSnippet
     )
   }, 5_000)
+  it('should return the correct paths when extrusion occurs within the sketch pipe', async () => {
+    const code = `sketch001 = startSketchOn('XY')
+  |> startProfileAt([-10, 10], %)
+  |> line([20, 0], %)
+  |> line([0, -20], %)
+  |> line([-20, 0], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+  |> extrude(15, %)`
+    const selectedSegmentSnippet = `line([20, 0], %)`
+    const expectedExtrudeSnippet = `extrude(15, %)`
+    await runGetPathToExtrudeForSegmentSelectionTest(
+      code,
+      selectedSegmentSnippet,
+      expectedExtrudeSnippet
+    )
+  }, 5_000)
   it('should return the correct paths for a valid selection and extrusion in case of several extrusions and sketches', async () => {
     const code = `sketch001 = startSketchOn('XY')
   |> startProfileAt([-30, 30], %)
@@ -296,6 +321,34 @@ extrude001 = extrude(-15, sketch001)`
   |> lineTo([profileStartX(%), profileStartY(%)], %)
   |> close(%)
 extrude001 = extrude(-15, sketch001)
+  |> fillet({ radius: 3, tags: [seg01] }, %)`
+
+    await runModifyAstCloneWithFilletAndTag(
+      code,
+      segmentSnippets,
+      radiusValue,
+      expectedCode
+    )
+  })
+  it('should add a fillet to the sketch pipe', async () => {
+    const code = `sketch001 = startSketchOn('XY')
+  |> startProfileAt([-10, 10], %)
+  |> line([20, 0], %)
+  |> line([0, -20], %)
+  |> line([-20, 0], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+  |> extrude(-15, %)`
+    const segmentSnippets = ['line([0, -20], %)']
+    const radiusValue = 3
+    const expectedCode = `sketch001 = startSketchOn('XY')
+  |> startProfileAt([-10, 10], %)
+  |> line([20, 0], %)
+  |> line([0, -20], %, $seg01)
+  |> line([-20, 0], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+  |> extrude(-15, %)
   |> fillet({ radius: 3, tags: [seg01] }, %)`
 
     await runModifyAstCloneWithFilletAndTag(

src/lang/modifyAst/addFillet.ts

@@ -146,7 +146,7 @@ export function modifyAstCloneWithFilletAndTag(
 
     // Modify the extrude expression to include this fillet expression
     // CallExpression - no fillet
-    // PipeExpression - fillet exists
+    // PipeExpression - fillet exists or extrude in sketch pipe
 
     let pathToFilletNode: PathToNode = []
 
@@ -167,15 +167,7 @@ export function modifyAstCloneWithFilletAndTag(
       )
       pathToFilletNodes.push(pathToFilletNode)
     } else if (extrudeDeclarator.init.type === 'PipeExpression') {
-      // 2. case when fillet exists
+      // 2. case when fillet exists or extrude in sketch pipe
-
-      const existingFilletCall = extrudeDeclarator.init.body.find((node) => {
-        return node.type === 'CallExpression' && node.callee.name === 'fillet'
-      })
-
-      if (!existingFilletCall || existingFilletCall.type !== 'CallExpression') {
-        return new Error('Fillet CallExpression not found.')
-      }
 
       // mutate the extrude node with the new fillet call
       extrudeDeclarator.init.body.push(filletCall)
@@ -317,14 +309,14 @@ function locateExtrudeDeclarator(
   node: Program,
   pathToExtrudeNode: PathToNode
 ): { extrudeDeclarator: VariableDeclarator } | Error {
-  const extrudeChunk = getNodeFromPath<VariableDeclaration>(
+  const nodeOfExtrudeCall = getNodeFromPath<VariableDeclaration>(
     node,
     pathToExtrudeNode,
     'VariableDeclaration'
   )
-  if (err(extrudeChunk)) return extrudeChunk
+  if (err(nodeOfExtrudeCall)) return nodeOfExtrudeCall
 
-  const { node: extrudeVarDecl } = extrudeChunk
+  const { node: extrudeVarDecl } = nodeOfExtrudeCall
   const extrudeDeclarator = extrudeVarDecl.declarations[0]
   if (!extrudeDeclarator) {
     return new Error('Extrude Declarator not found.')

src/lang/queryAst.test.ts

@@ -530,14 +530,25 @@ describe('Testing hasSketchPipeBeenExtruded', () => {
   |> line([-17.67, 0.85], %)
   |> close(%)
 extrude001 = extrude(10, sketch001)
-sketch002 = startSketchOn(extrude001, $seg01)
+sketch002 = startSketchOn(extrude001, seg01)
   |> startProfileAt([-12.94, 6.6], %)
   |> line([2.45, -0.2], %)
   |> line([-2, -1.25], %)
   |> lineTo([profileStartX(%), profileStartY(%)], %)
   |> close(%)
+sketch003 = startSketchOn(extrude001, 'END')
+  |> startProfileAt([8.14, 2.8], %)
+  |> line([-1.24, 4.39], %)
+  |> line([3.79, 1.91], %)
+  |> line([1.77, -2.95], %)
+  |> line([3.12, 1.74], %)
+  |> line([1.91, -4.09], %)
+  |> line([-5.6, -2.75], %)
+  |> lineTo([profileStartX(%), profileStartY(%)], %)
+  |> close(%)
+  |> extrude(3.14, %)
 `
-  it('finds sketch001 pipe to be extruded', async () => {
+  it('identifies sketch001 pipe as extruded (extrusion after pipe)', async () => {
     const ast = parse(exampleCode)
     if (err(ast)) throw ast
     const lineOfInterest = `line([4.99, -0.46], %, $seg01)`
@@ -552,7 +563,7 @@ sketch002 = startSketchOn(extrude001, $seg01)
     )
     expect(extruded).toBeTruthy()
   })
-  it('find sketch002 NOT pipe to be extruded', async () => {
+  it('identifies sketch002 pipe as not extruded', async () => {
     const ast = parse(exampleCode)
     if (err(ast)) throw ast
     const lineOfInterest = `line([2.45, -0.2], %)`
@@ -567,6 +578,21 @@ sketch002 = startSketchOn(extrude001, $seg01)
     )
     expect(extruded).toBeFalsy()
   })
+  it('identifies sketch003 pipe as extruded (extrusion within pipe)', async () => {
+    const ast = parse(exampleCode)
+    if (err(ast)) throw ast
+    const lineOfInterest = `|> line([3.12, 1.74], %)`
+    const characterIndex =
+      exampleCode.indexOf(lineOfInterest) + lineOfInterest.length
+    const extruded = hasSketchPipeBeenExtruded(
+      {
+        range: [characterIndex, characterIndex],
+        type: 'default',
+      },
+      ast
+    )
+    expect(extruded).toBeTruthy()
+  })
 })
 
 describe('Testing doesSceneHaveSweepableSketch', () => {

src/lang/queryAst.ts

@@ -927,7 +927,11 @@ export function findUsesOfTagInPipe(
 
 export function hasSketchPipeBeenExtruded(selection: Selection, ast: Program) {
   const path = getNodePathFromSourceRange(ast, selection.range)
-  const _node = getNodeFromPath<PipeExpression>(ast, path, 'PipeExpression')
+  const _node = getNodeFromPath<Node<PipeExpression>>(
+    ast,
+    path,
+    'PipeExpression'
+  )
   if (err(_node)) return false
   const { node: pipeExpression } = _node
   if (pipeExpression.type !== 'PipeExpression') return false
@@ -940,19 +944,33 @@ export function hasSketchPipeBeenExtruded(selection: Selection, ast: Program) {
   const varDec = _varDec.node
   if (varDec.type !== 'VariableDeclarator') return false
   let extruded = false
-  traverse(ast as any, {
+  // option 1: extrude or revolve is called in the sketch pipe
+  traverse(pipeExpression, {
     enter(node) {
       if (
         node.type === 'CallExpression' &&
-        node.callee.type === 'Identifier' &&
+        (node.callee.name === 'extrude' || node.callee.name === 'revolve')
-        (node.callee.name === 'extrude' || node.callee.name === 'revolve') &&
-        node.arguments?.[1]?.type === 'Identifier' &&
-        node.arguments[1].name === varDec.id.name
       ) {
         extruded = true
       }
     },
   })
+  // option 2: extrude or revolve is called in the separate pipe
+  if (!extruded) {
+    traverse(ast as any, {
+      enter(node) {
+        if (
+          node.type === 'CallExpression' &&
+          node.callee.type === 'Identifier' &&
+          (node.callee.name === 'extrude' || node.callee.name === 'revolve') &&
+          node.arguments?.[1]?.type === 'Identifier' &&
+          node.arguments[1].name === varDec.id.name
+        ) {
+          extruded = true
+        }
+      },
+    })
+  }
   return extruded
 }