fix: detect a wider range of scheduled functions (#1105)

* fix: detect a wider range of scheduled functions

scheduled functions which are reassigned or not directly exported are now detected

* Update src/runtimes/node/parser/bindings.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* Update src/runtimes/node/parser/exports.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* Update src/runtimes/node/parser/exports.ts

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* Apply suggestions from code review

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

* refactor: clean up code

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>

src/runtimes/node/in_source_config/index.ts

@@ -1,6 +1,7 @@
 import { ArgumentPlaceholder, Expression, SpreadElement, JSXNamespacedName } from '@babel/types'
 
 import { nonNullable } from '../../../utils/non_nullable.js'
+import { createBindingsMethod } from '../parser/bindings.js'
 import { getMainExport } from '../parser/exports.js'
 import { getImports } from '../parser/imports.js'
 import { safelyParseFile } from '../parser/index.js'
@@ -22,7 +23,8 @@ export const findISCDeclarationsInPath = async (sourcePath: string): Promise<ISC
   }
 
   const imports = ast.body.flatMap((node) => getImports(node, IN_SOURCE_CONFIG_MODULE))
-  const mainExports = getMainExport(ast.body)
+  const getAllBindings = createBindingsMethod(ast.body)
+  const mainExports = getMainExport(ast.body, getAllBindings)
   const iscExports = mainExports
     .map(({ args, local: exportName }) => {
       const matchingImport = imports.find(({ local: importName }) => importName === exportName)
@@ -33,7 +35,7 @@ export const findISCDeclarationsInPath = async (sourcePath: string): Promise<ISC
 
       switch (matchingImport.imported) {
         case 'schedule':
-          return parseSchedule({ args })
+          return parseSchedule({ args }, getAllBindings)
 
         default:
         // no-op

src/runtimes/node/in_source_config/properties/schedule.ts

@@ -1,7 +1,16 @@
+import type { BindingMethod } from '../../parser/bindings.js'
 import type { ISCHandlerArg } from '../index.js'
 
-export const parse = ({ args }: { args: ISCHandlerArg[] }) => {
-  const [expression] = args
+export const parse = ({ args }: { args: ISCHandlerArg[] }, getAllBindings: BindingMethod) => {
+  let [expression] = args
+
+  if (expression.type === 'Identifier') {
+    const binding = getAllBindings().get(expression.name)
+    if (binding) {
+      expression = binding
+    }
+  }
+
   const schedule = expression.type === 'StringLiteral' ? expression.value : undefined
 
   return {

src/runtimes/node/parser/bindings.ts

@@ -0,0 +1,56 @@
+import type { Expression, Statement, VariableDeclaration } from '@babel/types'
+
+type Bindings = Map<string, Expression>
+
+const getBindingFromVariableDeclaration = function (node: VariableDeclaration, bindings: Bindings): void {
+  node.declarations.forEach((declaration) => {
+    if (declaration.id.type === 'Identifier' && declaration.init) {
+      bindings.set(declaration.id.name, declaration.init)
+    }
+  })
+}
+
+// eslint-disable-next-line complexity
+const getBindingsFromNode = function (node: Statement, bindings: Bindings): void {
+  if (node.type === 'VariableDeclaration') {
+    // A variable was created, so create it and store the potential value
+    getBindingFromVariableDeclaration(node, bindings)
+  } else if (
+    node.type === 'ExpressionStatement' &&
+    node.expression.type === 'AssignmentExpression' &&
+    node.expression.left.type === 'Identifier'
+  ) {
+    // The variable was reassigned, so let's store the new value
+    bindings.set(node.expression.left.name, node.expression.right)
+  } else if (node.type === 'ExportNamedDeclaration' && node.declaration?.type === 'VariableDeclaration') {
+    // A `export const|let ...` creates a binding that can later be referenced again
+    getBindingFromVariableDeclaration(node.declaration, bindings)
+  }
+}
+
+/**
+ * Goes through all relevant nodes and creates a map from binding name to assigned value/expression
+ */
+const getAllBindings = function (nodes: Statement[]): Bindings {
+  const bindings: Bindings = new Map()
+
+  nodes.forEach((node) => {
+    getBindingsFromNode(node, bindings)
+  })
+
+  return bindings
+}
+
+export type BindingMethod = () => Bindings
+
+export const createBindingsMethod = function (nodes: Statement[]): BindingMethod {
+  // memoize the result for these nodes
+  let result: Bindings
+  return () => {
+    if (!result) {
+      result = getAllBindings(nodes)
+    }
+
+    return result
+  }
+}

src/runtimes/node/parser/exports.ts

@@ -1,15 +1,16 @@
-import { CallExpression, Statement } from '@babel/types'
+import type { ExportNamedDeclaration, ExportSpecifier, Expression, Statement } from '@babel/types'
 
 import type { ISCExport } from '../in_source_config/index.js'
 
+import type { BindingMethod } from './bindings.js'
 import { isModuleExports } from './helpers.js'
 
 // Finds the main handler export in an AST.
-export const getMainExport = (nodes: Statement[]) => {
+export const getMainExport = (nodes: Statement[], getAllBindings: BindingMethod) => {
   let handlerExport: ISCExport[] = []
 
   nodes.find((node) => {
-    const esmExports = getMainExportFromESM(node)
+    const esmExports = getMainExportFromESM(node, getAllBindings)
 
     if (esmExports.length !== 0) {
       handlerExport = esmExports
@@ -39,24 +40,27 @@ const getMainExportFromCJS = (node: Statement) => {
   ]
 
   return handlerPaths.flatMap((handlerPath) => {
-    if (!isModuleExports(node, handlerPath) || node.expression.right.type !== 'CallExpression') {
+    if (!isModuleExports(node, handlerPath)) {
       return []
     }
 
-    return getExportsFromCallExpression(node.expression.right)
+    return getExportsFromExpression(node.expression.right)
   })
 }
 
 // Finds the main handler export in an ESM AST.
-// eslint-disable-next-line complexity
-const getMainExportFromESM = (node: Statement) => {
+const getMainExportFromESM = (node: Statement, getAllBindings: BindingMethod) => {
   if (node.type !== 'ExportNamedDeclaration' || node.exportKind !== 'value') {
     return []
   }
 
-  const { declaration } = node
+  const { declaration, specifiers } = node
 
-  if (!declaration || declaration.type !== 'VariableDeclaration') {
+  if (specifiers?.length > 0) {
+    return getExportsFromBindings(specifiers, getAllBindings)
+  }
+
+  if (declaration?.type !== 'VariableDeclaration') {
     return []
   }
 
@@ -66,16 +70,46 @@ const getMainExportFromESM = (node: Statement) => {
     return type === 'VariableDeclarator' && id.type === 'Identifier' && id.name === 'handler'
   })
 
-  if (handlerDeclaration?.init?.type !== 'CallExpression') {
+  const exports = getExportsFromExpression(handlerDeclaration?.init)
+
+  return exports
+}
+
+// Check if the Node is an ExportSpecifier that has a named export called `handler`
+// either with Identifier `export { handler }`
+// or with StringLiteral `export { x as "handler" }`
+const isHandlerExport = (node: ExportNamedDeclaration['specifiers'][number]): node is ExportSpecifier => {
+  const { type, exported } = node
+  return (
+    type === 'ExportSpecifier' &&
+    ((exported.type === 'Identifier' && exported.name === 'handler') ||
+      (exported.type === 'StringLiteral' && exported.value === 'handler'))
+  )
+}
+
+// Tries to resolve the export from a binding (variable)
+// for example `let handler; handler = () => {}; export { handler }` would
+// resolve correctly to the handler function
+const getExportsFromBindings = (specifiers: ExportNamedDeclaration['specifiers'], getAllBindings: BindingMethod) => {
+  const specifier = specifiers.find(isHandlerExport)
+
+  if (!specifier) {
     return []
   }
 
-  const exports = getExportsFromCallExpression(handlerDeclaration.init)
+  const binding = getAllBindings().get(specifier.local.name)
+  const exports = getExportsFromExpression(binding)
 
   return exports
 }
 
-const getExportsFromCallExpression = (node: CallExpression) => {
+const getExportsFromExpression = (node: Expression | undefined | null) => {
+  // We're only interested in expressions representing function calls, because
+  // the ISC patterns we implement at the moment are all helper functions.
+  if (node?.type !== 'CallExpression') {
+    return []
+  }
+
   const { arguments: args, callee } = node
 
   if (callee.type !== 'Identifier') {

tests/fixtures/in-source-config/functions/cron_esm_no_direct_export.js

@@ -0,0 +1,7 @@
+import { schedule } from '@netlify/functions'
+
+const handler = schedule('@daily', async () => {
+  // function handler
+})
+
+export { handler }

tests/fixtures/in-source-config/functions/cron_esm_no_direct_export_renamed.js

@@ -0,0 +1,6 @@
+import { schedule } from '@netlify/functions'
+
+const _handler = schedule('@daily', async () => {
+  // function handler
+})
+export { _handler as handler }

tests/fixtures/in-source-config/functions/cron_esm_no_direct_export_renamed_reassigned.js

@@ -0,0 +1,8 @@
+import { schedule } from '@netlify/functions'
+
+const handler = async () => {
+  // function handler
+}
+
+const _handler = schedule('@daily', handler)
+export { _handler as handler }

tests/fixtures/in-source-config/functions/cron_esm_schedule_reassigned.js

@@ -0,0 +1,7 @@
+import { schedule } from '@netlify/functions'
+
+const SCHEDULE = '@daily'
+
+export const handler = schedule(SCHEDULE, async () => {
+  // function handler
+})

tests/main.js

@@ -2662,7 +2662,7 @@ testMany(
   'Finds in-source config declarations using the `schedule` helper',
   ['bundler_default', 'bundler_esbuild', 'bundler_nft'],
   async (options, t) => {
-    const FUNCTIONS_COUNT = 7
+    const FUNCTIONS_COUNT = 11
     const { files } = await zipFixture(t, join('in-source-config', 'functions'), {
       opts: options,
       length: FUNCTIONS_COUNT,
@@ -2692,7 +2692,7 @@ test('listFunctions includes in-source config declarations', async (t) => {
   const functions = await listFunctions(join(FIXTURES_DIR, 'in-source-config', 'functions'), {
     parseISC: true,
   })
-  const FUNCTIONS_COUNT = 7
+  const FUNCTIONS_COUNT = 11
   t.is(functions.length, FUNCTIONS_COUNT)
   functions.forEach((func) => {
     t.is(func.schedule, '@daily')