Fix to use more accurate types with custom isArray() and add lint (#5261)

* Fix to use more accurate types with custom isArray()

* Add lint against Array.isArray()

.eslintrc

@@ -29,6 +29,13 @@
         {
           "name": "isNaN",
           "message": "Use Number.isNaN() instead."
+        },
+      ],
+      "no-restricted-syntax": [
+        "error",
+        {
+          "selector": "CallExpression[callee.object.name='Array'][callee.property.name='isArray']",
+          "message": "Use isArray() in lib/utils.ts instead of Array.isArray()."
         }
       ],
       "semi": [

packages/codemirror-lsp-client/src/lib/utils.ts

@@ -0,0 +1,7 @@
+/**
+ * A safer type guard for arrays since the built-in Array.isArray() asserts `any[]`.
+ */
+export function isArray(val: any): val is unknown[] {
+  // eslint-disable-next-line no-restricted-syntax
+  return Array.isArray(val)
+}

packages/codemirror-lsp-client/src/plugin/util.ts

@@ -2,6 +2,7 @@ import { Text } from '@codemirror/state'
 import { Marked } from '@ts-stack/markdown'
 
 import type * as LSP from 'vscode-languageserver-protocol'
+import { isArray } from '../lib/utils'
 
 // takes a function and executes it after the wait time, if the function is called again before the wait time is up, the timer is reset
 export function deferExecution<T>(func: (args: T) => any, wait: number) {
@@ -45,7 +46,7 @@ export function offsetToPos(doc: Text, offset: number) {
 export function formatMarkdownContents(
   contents: LSP.MarkupContent | LSP.MarkedString | LSP.MarkedString[]
 ): string {
-  if (Array.isArray(contents)) {
+  if (isArray(contents)) {
     return contents.map((c) => formatMarkdownContents(c) + '\n\n').join('')
   } else if (typeof contents === 'string') {
     return Marked.parse(contents)

src/Toolbar.tsx

@@ -22,6 +22,7 @@ import {
 import { isDesktop } from 'lib/isDesktop'
 import { openExternalBrowserIfDesktop } from 'lib/openWindow'
 import { commandBarActor } from 'machines/commandBarMachine'
+import { isArray } from 'lib/utils'
 
 export function Toolbar({
   className = '',
@@ -121,7 +122,7 @@ export function Toolbar({
     return toolbarConfig[currentMode].items.map((maybeIconConfig) => {
       if (maybeIconConfig === 'break') {
         return 'break'
-      } else if (Array.isArray(maybeIconConfig)) {
+      } else if (isArray(maybeIconConfig)) {
         return maybeIconConfig.map(resolveItemConfig)
       } else {
         return resolveItemConfig(maybeIconConfig)
@@ -180,7 +181,7 @@ export function Toolbar({
                 className="h-5 w-[1px] block bg-chalkboard-30 dark:bg-chalkboard-80"
               />
             )
-          } else if (Array.isArray(maybeIconConfig)) {
+          } else if (isArray(maybeIconConfig)) {
             // A button with a dropdown
             return (
               <ActionButtonDropdown

src/components/AstExplorer.tsx

@@ -7,6 +7,7 @@ import { trap } from 'lib/trap'
 import { codeToIdSelections } from 'lib/selections'
 import { codeRefFromRange } from 'lang/std/artifactGraph'
 import { defaultSourceRange, SourceRange, topLevelRange } from 'lang/wasm'
+import { isArray } from 'lib/utils'
 
 export function AstExplorer() {
   const { context } = useModelingContext()
@@ -166,12 +167,12 @@ function DisplayObj({
             {Object.entries(obj).map(([key, value]) => {
               if (filterKeys.includes(key)) {
                 return null
-              } else if (Array.isArray(value)) {
+              } else if (isArray(value)) {
                 return (
                   <li key={key}>
                     {`${key}: [`}
                     <DisplayBody
-                      body={value}
+                      body={value as any}
                       filterKeys={filterKeys}
                       node={node}
                     />

src/components/ModelingSidebar/ModelingPanes/CodeEditor.tsx

@@ -14,6 +14,7 @@ import {
 } from '@codemirror/state'
 import { EditorView } from '@codemirror/view'
 import { oneDark } from '@codemirror/theme-one-dark'
+import { isArray } from 'lib/utils'
 
 //reference: https://github.com/sachinraja/rodemirror/blob/main/src/use-first-render.ts
 const useFirstRender = () => {
@@ -86,6 +87,18 @@ const CodeEditor = forwardRef<CodeEditorRef, CodeEditorProps>((props, ref) => {
   return <div ref={editor}></div>
 })
 
+/**
+ * The extensions type is quite weird.  We need a special helper to preserve the
+ * readonly array type.
+ *
+ * @see https://github.com/microsoft/TypeScript/issues/17002
+ */
+function isExtensionArray(
+  extensions: Extension
+): extensions is readonly Extension[] {
+  return isArray(extensions)
+}
+
 export function useCodeMirror(props: UseCodeMirror) {
   const {
     onCreateEditor,
@@ -103,7 +116,7 @@ export function useCodeMirror(props: UseCodeMirror) {
   const isFirstRender = useFirstRender()
 
   const targetExtensions = useMemo(() => {
-    let exts = Array.isArray(extensions) ? extensions : []
+    let exts = isExtensionArray(extensions) ? extensions : []
     if (theme === 'dark') {
       exts = [...exts, oneDark]
     } else if (theme === 'light') {

src/editor/plugins/lsp/kcl/colors.ts

@@ -9,6 +9,7 @@ import {
 import { Range, Extension, Text } from '@codemirror/state'
 import { NodeProp, Tree } from '@lezer/common'
 import { language, syntaxTree } from '@codemirror/language'
+import { isArray } from 'lib/utils'
 
 interface PickerState {
   from: number
@@ -79,7 +80,7 @@ function discoverColorsInKCL(
           )
 
           if (maybeWidgetOptions) {
-            if (Array.isArray(maybeWidgetOptions)) {
+            if (isArray(maybeWidgetOptions)) {
               console.error('Unexpected nested overlays')
               ret.push(...maybeWidgetOptions)
             } else {
@@ -150,7 +151,7 @@ function colorPickersDecorations(
           return
         }
 
-        if (!Array.isArray(maybeWidgetOptions)) {
+        if (!isArray(maybeWidgetOptions)) {
           widgets.push(
             Decoration.widget({
               widget: new ColorPickerWidget(maybeWidgetOptions),

src/lang/modifyAst/addEdgeTreatment.ts

@@ -36,6 +36,7 @@ import {
 import { err, trap } from 'lib/trap'
 import { Selection, Selections } from 'lib/selections'
 import { KclCommandValue } from 'lib/commandTypes'
+import { isArray } from 'lib/utils'
 import { Artifact, getSweepFromSuspectedPath } from 'lang/std/artifactGraph'
 import { Node } from 'wasm-lib/kcl/bindings/Node'
 import { findKwArg } from 'lang/util'
@@ -866,10 +867,7 @@ export async function deleteEdgeTreatment(
   if (!inPipe) {
     const varDecPathStep = varDec.shallowPath[1]
 
-    if (
+    if (!isArray(varDecPathStep) || typeof varDecPathStep[0] !== 'number') {
-      !Array.isArray(varDecPathStep) ||
-      typeof varDecPathStep[0] !== 'number'
-    ) {
       return new Error(
         'Invalid shallowPath structure: expected a number at shallowPath[1][0]'
       )
@@ -935,7 +933,7 @@ export async function deleteEdgeTreatment(
     if (err(pipeExpressionNode)) return pipeExpressionNode
 
     // Ensure that the PipeExpression.body is an array
-    if (!Array.isArray(pipeExpressionNode.node.body)) {
+    if (!isArray(pipeExpressionNode.node.body)) {
       return new Error('PipeExpression body is not an array')
     }
 
@@ -945,10 +943,7 @@ export async function deleteEdgeTreatment(
     // Remove VariableDeclarator if PipeExpression.body is empty
     if (pipeExpressionNode.node.body.length === 0) {
       const varDecPathStep = varDec.shallowPath[1]
-      if (
+      if (!isArray(varDecPathStep) || typeof varDecPathStep[0] !== 'number') {
-        !Array.isArray(varDecPathStep) ||
-        typeof varDecPathStep[0] !== 'number'
-      ) {
         return new Error(
           'Invalid shallowPath structure: expected a number at shallowPath[1][0]'
         )

src/lang/queryAst.ts

@@ -27,7 +27,7 @@ import {
 import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils'
 import { createIdentifier, splitPathAtLastIndex } from './modifyAst'
 import { getSketchSegmentFromSourceRange } from './std/sketchConstraints'
-import { getAngle } from '../lib/utils'
+import { getAngle, isArray } from '../lib/utils'
 import { ARG_TAG, getArgForEnd, getFirstArg } from './std/sketch'
 import {
   getConstraintLevelFromSourceRange,
@@ -112,7 +112,7 @@ export function getNodeFromPath<T>(
     }
     if (
       typeof stopAt !== 'undefined' &&
-      (Array.isArray(stopAt)
+      (isArray(stopAt)
         ? stopAt.includes(currentNode.type)
         : currentNode.type === stopAt)
     ) {
@@ -167,6 +167,7 @@ export function getNodeFromPathCurry(
 type KCLNode = Node<
   | Expr
   | ExpressionStatement
+  | ImportStatement
   | VariableDeclaration
   | VariableDeclarator
   | ReturnStatement
@@ -263,10 +264,14 @@ export function traverse(
     // hmm this smell
     _traverse(_node.object, [...pathToNode, ['object', 'MemberExpression']])
     _traverse(_node.property, [...pathToNode, ['property', 'MemberExpression']])
-  } else if ('body' in _node && Array.isArray(_node.body)) {
+  } else if (_node.type === 'ImportStatement') {
-    _node.body.forEach((expression, index) =>
+    // Do nothing.
+  } else if ('body' in _node && isArray(_node.body)) {
+    // TODO: Program should have a type field, but it currently doesn't.
+    const program = node as Node<Program>
+    program.body.forEach((expression, index) => {
       _traverse(expression, [...pathToNode, ['body', ''], [index, 'index']])
-    )
+    })
   }
   option?.leave?.(_node)
 }

src/lang/std/sketch.ts

@@ -60,7 +60,7 @@ import {
   mutateObjExpProp,
   findUniqueName,
 } from 'lang/modifyAst'
-import { roundOff, getLength, getAngle } from 'lib/utils'
+import { roundOff, getLength, getAngle, isArray } from 'lib/utils'
 import { err } from 'lib/trap'
 import { perpendicularDistance } from 'sketch-helpers'
 import { TagDeclarator } from 'wasm-lib/kcl/bindings/TagDeclarator'
@@ -96,7 +96,7 @@ export function createFirstArg(
   sketchFn: ToolTip,
   val: Expr | [Expr, Expr] | [Expr, Expr, Expr]
 ): Expr | Error {
-  if (Array.isArray(val)) {
+  if (isArray(val)) {
     if (
       [
         'angledLine',

src/lang/std/sketchcombos.ts

@@ -57,7 +57,7 @@ import {
   getSketchSegmentFromPathToNode,
   getSketchSegmentFromSourceRange,
 } from './sketchConstraints'
-import { getAngle, roundOff, normaliseAngle } from '../../lib/utils'
+import { getAngle, roundOff, normaliseAngle, isArray } from '../../lib/utils'
 import { Node } from 'wasm-lib/kcl/bindings/Node'
 import { findKwArg, findKwArgAny } from 'lang/util'
 
@@ -122,7 +122,7 @@ function createCallWrapper(
   tag?: Expr,
   valueUsedInTransform?: number
 ): CreatedSketchExprResult {
-  if (Array.isArray(val)) {
+  if (isArray(val)) {
     if (tooltip === 'line') {
       const labeledArgs = [createLabeledArg('end', createArrayExpression(val))]
       if (tag) {
@@ -1330,12 +1330,12 @@ export function getRemoveConstraintsTransform(
 
   // check if the function has no constraints
   const isTwoValFree =
-    Array.isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
+    isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
   if (isTwoValFree) {
     return false
   }
   const isOneValFree =
-    !Array.isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
+    !isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
   if (isOneValFree) {
     return transformInfo
   }
@@ -1649,7 +1649,7 @@ export function getConstraintType(
   // and for one val sketch functions that the arg is NOT locked down
   // these conditions should have been checked previously.
   // completely locked down or not locked down at all does not depend on the fnName so we can check that first
-  const isArr = Array.isArray(val)
+  const isArr = isArray(val)
   if (!isArr) {
     if (fnName === 'xLine') return 'yRelative'
     if (fnName === 'yLine') return 'xRelative'
@@ -2113,9 +2113,9 @@ export function getConstraintLevelFromSourceRange(
 
   // check if the function has no constraints
   const isTwoValFree =
-    Array.isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
+    isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
   const isOneValFree =
-    !Array.isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
+    !isArray(firstArg.val) && isLiteralArrayOrStatic(firstArg.val)
 
   if (isTwoValFree) return { level: 'free', range: range }
   if (isOneValFree) return { level: 'partial', range: range }
@@ -2128,7 +2128,7 @@ export function isLiteralArrayOrStatic(
 ): boolean {
   if (!val) return false
 
-  if (Array.isArray(val)) {
+  if (isArray(val)) {
     const a = val[0]
     const b = val[1]
     return isLiteralArrayOrStatic(a) && isLiteralArrayOrStatic(b)
@@ -2142,7 +2142,7 @@ export function isLiteralArrayOrStatic(
 export function isNotLiteralArrayOrStatic(
   val: Expr | [Expr, Expr] | [Expr, Expr, Expr]
 ): boolean {
-  if (Array.isArray(val)) {
+  if (isArray(val)) {
     const a = val[0]
     const b = val[1]
     return isNotLiteralArrayOrStatic(a) && isNotLiteralArrayOrStatic(b)

src/lang/util.ts

@@ -12,7 +12,7 @@ import {
   NumericSuffix,
 } from './wasm'
 import { filterArtifacts } from 'lang/std/artifactGraph'
-import { isOverlap } from 'lib/utils'
+import { isArray, isOverlap } from 'lib/utils'
 
 export function updatePathToNodeFromMap(
   oldPath: PathToNode,
@@ -40,8 +40,8 @@ export function isCursorInSketchCommandRange(
       predicate: (artifact) => {
         return selectionRanges.graphSelections.some(
           (selection) =>
-            Array.isArray(selection?.codeRef?.range) &&
+            isArray(selection?.codeRef?.range) &&
-            Array.isArray(artifact?.codeRef?.range) &&
+            isArray(artifact?.codeRef?.range) &&
             isOverlap(selection?.codeRef?.range, artifact.codeRef.range)
         )
       },

src/lib/settings/initialSettings.tsx

@@ -16,7 +16,7 @@ import { isDesktop } from 'lib/isDesktop'
 import { useRef } from 'react'
 import { CustomIcon } from 'components/CustomIcon'
 import Tooltip from 'components/Tooltip'
-import { toSync } from 'lib/utils'
+import { isArray, toSync } from 'lib/utils'
 import { reportRejection } from 'lib/trap'
 import { CameraProjectionType } from 'wasm-lib/kcl/bindings/CameraProjectionType'
 import { OnboardingStatus } from 'wasm-lib/kcl/bindings/OnboardingStatus'
@@ -240,7 +240,7 @@ export function createSettings() {
                   if (
                     inputRef.current &&
                     inputRefVal &&
-                    !Array.isArray(inputRefVal)
+                    !isArray(inputRefVal)
                   ) {
                     updateValue(inputRefVal)
                   } else {

src/lib/utils.ts

@@ -11,6 +11,7 @@ export const uuidv4 = v4
  * A safer type guard for arrays since the built-in Array.isArray() asserts `any[]`.
  */
 export function isArray(val: any): val is unknown[] {
+  // eslint-disable-next-line no-restricted-syntax
   return Array.isArray(val)
 }