fix: inconsistencies when analyzing unnamed closures (#823)

src/dataflow/graph/vertex.ts

@@ -8,9 +8,9 @@ export type DataflowGraphVertices<Vertex extends DataflowGraphVertexInfo = Dataf
 
 
 export const enum VertexType {
-	Value = 'value',
-	Use = 'use',
-	FunctionCall = 'function-call',
+	Value              = 'value',
+	Use                = 'use',
+	FunctionCall       = 'function-call',
 	VariableDefinition = 'variable-definition',
 	FunctionDefinition = 'function-definition'
 }

src/dataflow/internal/linker.ts

@@ -163,18 +163,18 @@ export function linkFunctionCalls(
 }
 
 
-export function getAllLinkedFunctionDefinitions(functionDefinitionReadIds: Set<NodeId>, dataflowGraph: DataflowGraph): Map<NodeId, DataflowGraphVertexInfo> {
+export function getAllLinkedFunctionDefinitions(functionDefinitionReadIds: ReadonlySet<NodeId>, dataflowGraph: DataflowGraph): Map<NodeId, DataflowGraphVertexInfo> {
 	const potential: NodeId[] = [...functionDefinitionReadIds]
 	const visited = new Set<NodeId>()
 	const result = new Map<NodeId, DataflowGraphVertexInfo>()
 	while(potential.length > 0) {
 		const currentId = potential.pop() as NodeId
 
+		// do not traverse builtins
 		if(currentId === BuiltIn) {
-			// do not traverse builtins
-			slicerLogger.trace('skipping builtin function definition during collection')
 			continue
 		}
+
 		const currentInfo = dataflowGraph.get(currentId, true)
 		if(currentInfo === undefined) {
 			slicerLogger.trace('skipping unknown link')

src/dataflow/internal/process/functions/call/built-in/built-in-assignment.ts

@@ -69,7 +69,7 @@ export function processAssignment<OtherInfo>(
 
 	const effectiveArgs = getEffectiveOrder(config, args as [RFunctionArgument<OtherInfo & ParentInformation>, RFunctionArgument<OtherInfo & ParentInformation>])
 	const { target, source } = extractSourceAndTarget(effectiveArgs, name)
-	const { type, flavor } = target
+	const { type, named } = target
 
 	if(type === RType.Symbol) {
 		const res = processKnownFunctionCall({ name, args, rootId, data, reverseOrder: !config.swapSourceAndTarget })
@@ -83,7 +83,7 @@ export function processAssignment<OtherInfo>(
 			data,
 			information: res.information,
 		})
-	} else if(config.canBeReplacement && type === RType.FunctionCall && flavor === 'named') {
+	} else if(config.canBeReplacement && type === RType.FunctionCall && named) {
 		/* as replacement functions take precedence over the lhs fn-call (i.e., `names(x) <- ...` is independent from the definition of `names`), we do not have to process the call */
 		dataflowLogger.debug(`Assignment ${name.content} has a function call as target => replacement function ${target.lexeme}`)
 		const replacement = toReplacementSymbol(target, target.functionName.content, config.superAssignment ?? false)

src/dataflow/internal/process/functions/call/built-in/built-in-function-definition.ts

@@ -1,6 +1,6 @@
 import type { DataflowProcessorInformation } from '../../../../../processor'
 import { processDataflowFor } from '../../../../../processor'
-import type { DataflowInformation, ExitPoint } from '../../../../../info'
+import type { DataflowInformation } from '../../../../../info'
 import { ExitPointType } from '../../../../../info'
 import { linkInputs } from '../../../../linker'
 import { processKnownFunctionCall } from '../known-call-handling'
@@ -22,6 +22,7 @@ import type { REnvironmentInformation } from '../../../../../environments/enviro
 import { initializeCleanEnvironments } from '../../../../../environments/environment'
 import { resolveByName } from '../../../../../environments/resolve-by-name'
 import { EdgeType } from '../../../../../graph/edge'
+import { expensiveTrace } from '../../../../../../util/log'
 
 export function processFunctionDefinition<OtherInfo>(
 	name: RSymbol<OtherInfo & ParentInformation>,
@@ -79,7 +80,7 @@ export function processFunctionDefinition<OtherInfo>(
 				tag:                 VertexType.Use,
 				id:                  read.nodeId,
 				environment:         undefined,
-				controlDependencies: []
+				controlDependencies: undefined
 			})
 		}
 	}
@@ -93,8 +94,8 @@ export function processFunctionDefinition<OtherInfo>(
 		environment:       outEnvironment
 	}
 
+	updateNestedFunctionClosures(subgraph, outEnvironment, name)
 	const exitPoints = body.exitPoints
-	updateNestedFunctionClosures(exitPoints, subgraph, outEnvironment, name)
 
 	const graph = new DataflowGraph(data.completeAst.idMap).mergeWith(subgraph, false)
 	graph.addVertex({
@@ -120,38 +121,32 @@ export function processFunctionDefinition<OtherInfo>(
 
 
 function updateNestedFunctionClosures<OtherInfo>(
-	exitPoints: readonly ExitPoint[],
 	subgraph: DataflowGraph,
 	outEnvironment: REnvironmentInformation,
 	name: RSymbol<OtherInfo & ParentInformation>
 ) {
-	// track *all* function definitions - including those nested within the current graph
+	// track *all* function definitions - including those nested within the current graph,
 	// try to resolve their 'in' by only using the lowest scope which will be popped after this definition
-	for(const [id, info] of subgraph.vertices(true)) {
-		if(info.tag !== VertexType.FunctionDefinition) {
+	for(const [id, { subflow, tag }] of subgraph.vertices(true)) {
+		if(tag !== VertexType.FunctionDefinition) {
 			continue
 		}
 
-		const ingoingRefs = info.subflow.in
-		const remainingIn: Set<IdentifierReference> = new Set()
+		const ingoingRefs = subflow.in
+		const remainingIn: IdentifierReference[] = []
 		for(const ingoing of ingoingRefs) {
-			for(const { nodeId } of exitPoints) {
-				const node = subgraph.getVertex(nodeId, true)
-				const env = initializeCleanEnvironments()
-				env.current.memory = node === undefined ? outEnvironment.current.memory : (node.environment?.current.memory ?? outEnvironment.current.memory)
-				const resolved = ingoing.name ? resolveByName(ingoing.name, env) : undefined
-				if(resolved === undefined) {
-					remainingIn.add(ingoing)
-					continue
-				}
-				dataflowLogger.trace(`Found ${resolved.length} references to open ref ${id} in closure of function definition ${name.info.id}`)
-				for(const ref of resolved) {
-					subgraph.addEdge(ingoing, ref, { type: EdgeType.Reads })
-				}
+			const resolved = ingoing.name ? resolveByName(ingoing.name, outEnvironment) : undefined
+			if(resolved === undefined) {
+				remainingIn.push(ingoing)
+				continue
+			}
+			expensiveTrace(dataflowLogger, () => `Found ${resolved.length} references to open ref ${id} in closure of function definition ${name.info.id}`)
+			for(const ref of resolved) {
+				subgraph.addEdge(ingoing, ref, { type: EdgeType.Reads })
 			}
 		}
-		dataflowLogger.trace(`Keeping ${remainingIn.size} (unique) references to open ref ${id} in closure of function definition ${name.info.id}`)
-		info.subflow.in = [...remainingIn]
+		expensiveTrace(dataflowLogger, () => `Keeping ${remainingIn.length} references to open ref ${id} in closure of function definition ${name.info.id}`)
+		subflow.in = remainingIn
 	}
 }
 

src/dataflow/internal/process/functions/call/default-call-handling.ts

@@ -7,7 +7,7 @@ import type { RFunctionCall } from '../../../../../r-bridge/lang-4.x/ast/model/n
 
 
 export function processFunctionCall<OtherInfo>(functionCall: RFunctionCall<OtherInfo & ParentInformation>, data: DataflowProcessorInformation<OtherInfo & ParentInformation>): DataflowInformation {
-	if(functionCall.flavor === 'named') {
+	if(functionCall.named) {
 		return processNamedCall(functionCall.functionName, functionCall.arguments, functionCall.info.id, data)
 	} else {
 		return processUnnamedFunctionCall(functionCall, data)

src/dataflow/internal/process/functions/call/unnamed-call-handling.ts

@@ -66,7 +66,7 @@ export function processUnnamedFunctionCall<OtherInfo>(functionCall: RUnnamedFunc
 		out:               calledFunction.out,
 		graph:             finalGraph,
 		environment:       finalEnv,
-		entryPoint:        calledRootId,
+		entryPoint:        functionCall.info.id,
 		exitPoints:        calledFunction.exitPoints
 	}
 }

src/r-bridge/lang-4.x/ast/model/nodes/r-function-call.ts

@@ -14,7 +14,7 @@ export type RFunctionArgument<Info = NoInfo> = RArgument<Info> | typeof EmptyArg
  */
 export interface RNamedFunctionCall<Info = NoInfo> extends Base<Info>, Location {
 	readonly type:      RType.FunctionCall;
-	readonly flavor:    'named';
+	readonly named:     true;
 	functionName:       RSymbol<Info>;
 	/** arguments can be empty, for example when calling as `a(1, ,3)` */
 	readonly arguments: readonly RFunctionArgument<Info>[];
@@ -28,7 +28,7 @@ export interface RNamedFunctionCall<Info = NoInfo> extends Base<Info>, Location
  */
 export interface RUnnamedFunctionCall<Info = NoInfo> extends Base<Info>, Location {
 	readonly type:      RType.FunctionCall;
-	readonly flavor:    'unnamed';
+	readonly named:     false | undefined;
 	calledFunction:     RNode<Info>; /* can be either a function definition or another call that returns a function etc. */
 	/** marks function calls like `3 %xx% 4` which have been written in special infix notation; deprecated in v2 */
 	infixSpecial?:      boolean;

src/r-bridge/lang-4.x/ast/model/processing/decorate.ts

@@ -344,7 +344,7 @@ function createFoldForFunctionCall<OtherInfo>(info: FoldInfo<OtherInfo>) {
 	return (data: RFunctionCall<OtherInfo>, functionName: RNodeWithParent<OtherInfo>, args: readonly (RNodeWithParent<OtherInfo> | typeof EmptyArgument)[], depth: number): RNodeWithParent<OtherInfo> => {
 		const id = info.getId(data)
 		let decorated: RFunctionCall<OtherInfo & ParentInformation>
-		if(data.flavor === 'named') {
+		if(data.named) {
 			decorated = { ...data, info: { ...data.info, id, parent: undefined, depth }, functionName, arguments: args } as RNamedFunctionCall<OtherInfo & ParentInformation>
 		} else {
 			decorated = { ...data, info: { ...data.info, id, parent: undefined, depth }, calledFunction: functionName, arguments: args } as RUnnamedFunctionCall<OtherInfo & ParentInformation>

src/r-bridge/lang-4.x/ast/model/processing/stateful-fold.ts

@@ -107,7 +107,7 @@ export function foldAstStateful<Info, Down, Up>(ast: RNode<Info>, down: Down, fo
 		case RType.RepeatLoop:
 			return folds.loop.foldRepeat(ast, foldAstStateful(ast.body, down, folds), down)
 		case RType.FunctionCall:
-			return folds.functions.foldFunctionCall(ast, foldAstStateful(ast.flavor === 'named' ? ast.functionName : ast.calledFunction, down, folds), ast.arguments.map(param => param === EmptyArgument ? param : foldAstStateful(param, down, folds)), down)
+			return folds.functions.foldFunctionCall(ast, foldAstStateful(ast.named ? ast.functionName : ast.calledFunction, down, folds), ast.arguments.map(param => param === EmptyArgument ? param : foldAstStateful(param, down, folds)), down)
 		case RType.FunctionDefinition:
 			return folds.functions.foldFunctionDefinition(ast, ast.parameters.map(param => foldAstStateful(param, down, folds)), foldAstStateful(ast.body, down, folds), down)
 		case RType.Parameter:

src/r-bridge/lang-4.x/ast/model/processing/visitor.ts

@@ -24,11 +24,11 @@ class NodeVisitor<OtherInfo = NoInfo> {
 			return
 		}
 
-		/* let the type system know, that the type does not change */
+		/* let the type system know that the type does not change */
 		const type = node.type
 		switch(type) {
 			case RType.FunctionCall:
-				this.visitSingle(node.flavor === 'named' ? node.functionName : node.calledFunction)
+				this.visitSingle(node.named ? node.functionName : node.calledFunction)
 				this.visit(node.arguments)
 				break
 			case RType.FunctionDefinition:

src/r-bridge/lang-4.x/ast/parser/xml/internal/functions/normalize-call.ts

@@ -110,7 +110,7 @@ function tryParseUnnamedFunctionCall(data: NormalizerData, mappedWithName: Named
 
 	return {
 		type:           RType.FunctionCall,
-		flavor:         'unnamed',
+		named:          undefined,
 		location,
 		lexeme:         content,
 		calledFunction: calledFunction,
@@ -146,7 +146,7 @@ function parseNamedFunctionCall(data: NormalizerData, symbolContent: NamedXmlBas
 
 	return {
 		type:      RType.FunctionCall,
-		flavor:    'named',
+		named:     true,
 		location,
 		lexeme:    content,
 		functionName,

src/r-bridge/lang-4.x/ast/parser/xml/internal/operators/normalize-binary.ts

@@ -50,7 +50,7 @@ function parseBinaryOp(data: NormalizerData, lhs: NamedXmlBasedJson, operator: N
 		// parse as infix function call!
 		return {
 			type:         RType.FunctionCall,
-			flavor:       'named',
+			named:        true,
 			infixSpecial: true,
 			lexeme:       data.currentLexeme ?? content,
 			location,

src/reconstruct/reconstruct.ts

@@ -313,7 +313,7 @@ function reconstructFunctionDefinition(definition: RFunctionDefinition<ParentInf
 		const empty = body === undefined || body.length === 0
 		const selected = isSelected(config, definition)
 		if(empty && selected) { // give function stub
-			return plain(`function(${reconstructParameters(definition.parameters).join(', ')}) { }`)
+			return plain(`${definition.lexeme}(${reconstructParameters(definition.parameters).join(', ')}) { }`)
 		} else if(!selected) { // do not require function
 			return body
 		}
@@ -323,11 +323,11 @@ function reconstructFunctionDefinition(definition: RFunctionDefinition<ParentInf
 		// 'inline'
 		const bodyStr = body.length === 0 ? '{ }' : `${body[0].line}`
 		// we keep the braces in every case because I do not like no-brace functions
-		return [{ line: `function(${parameters}) ${bodyStr}`, indent: 0 }]
+		return [{ line: `${definition.lexeme}(${parameters}) ${bodyStr}`, indent: 0 }]
 	} else {
 		// 'block'
 		return [
-			{ line: `function(${parameters}) ${body[0].line}`, indent: 0 },
+			{ line: `${definition.lexeme}(${parameters}) ${body[0].line}`, indent: 0 },
 			...body.slice(1),
 		]
 	}
@@ -336,14 +336,14 @@ function reconstructFunctionDefinition(definition: RFunctionDefinition<ParentInf
 
 function reconstructSpecialInfixFunctionCall(args: (Code | typeof EmptyArgument)[], call: RFunctionCall<ParentInformation>): Code {
 	guard(args.length === 2, () => `infix special call must have exactly two arguments, got: ${args.length} (${JSON.stringify(args)})`)
-	guard(call.flavor === 'named', `infix special call must be named, got: ${call.flavor}`)
+	guard(call.named, `infix special call must be named, got: ${call.named}`)
 	const [lhs, rhs] = args
 
 	if((lhs === undefined || lhs.length === 0) && (rhs === undefined || rhs.length === 0)) {
 		return []
 	}
 	// else if (rhs === undefined || rhs.length === 0) {
-	// if rhs is undefined we still  have to keep both now, but reconstruct manually :/
+	// if rhs is undefined we still have to keep both now, but reconstruct manually :/
 	if(lhs !== EmptyArgument && lhs.length > 0) {
 		const lhsText = lhs.map(l => `${getIndentString(l.indent)}${l.line}`).join('\n')
 		if(rhs !== EmptyArgument && rhs.length > 0) {
@@ -370,7 +370,7 @@ function reconstructFunctionCall(call: RFunctionCall<ParentInformation>, functio
 	if(call.infixSpecial === true) {
 		return reconstructSpecialInfixFunctionCall(args, call)
 	}
-	if(call.flavor === 'named' && selected) {
+	if(call.named && selected) {
 		return plain(getLexeme(call))
 	}
 	const filteredArgs = args.filter(a => a !== undefined && a.length > 0)
@@ -381,15 +381,13 @@ function reconstructFunctionCall(call: RFunctionCall<ParentInformation>, functio
 	if(args.length === 0) {
 		guard(functionName.length > 0, `without args, we need the function name to be present! got: ${JSON.stringify(functionName)}`)
 		const last = functionName[functionName.length - 1]
-		if(call.flavor === 'unnamed' && !last.line.endsWith(')')) {
+		if(!call.named && !last.line.endsWith(')')) {
 			functionName[0].line = `(${functionName[0].line}`
 			last.line += ')'
 		}
 
-		if(!last.line.endsWith('()')) {
-			// add empty call braces if not present
-			last.line += '()'
-		}
+		// add empty call braces if not present
+		last.line += '()'
 		return functionName
 	} else {
 		return plain(getLexeme(call))
@@ -413,7 +411,7 @@ export function doNotAutoSelect(_node: RNode<ParentInformation>): boolean {
 const libraryFunctionCall = /^(library|require|((require|load|attach)Namespace))$/
 
 export function autoSelectLibrary(node: RNode<ParentInformation>): boolean {
-	if(node.type !== RType.FunctionCall || node.flavor !== 'named') {
+	if(node.type !== RType.FunctionCall || !node.named) {
 		return false
 	}
 	return libraryFunctionCall.test(node.functionName.content)

src/slicing/criterion/filters/all-variables.ts

@@ -43,7 +43,7 @@ const defaultAllVariablesCollectorFolds: FoldFunctions<ParentInformation, NodeId
 		foldFunctionDefinition: (_: unknown, a: NodeId[][], b: NodeId[]) => [...a.flat(),...b],
 		foldFunctionCall:       (c: RFunctionCall, a: NodeId[], b: (NodeId[] | typeof EmptyArgument)[]) => {
 			const args = b.flatMap(b => b !== EmptyArgument ? b.flat() : [])
-			if(c.flavor === 'named') {
+			if(c.named) {
 				return c.functionName.content === 'library' ? args.slice(1) : args
 			} else {
 				return [...a.filter(x => x !== EmptyArgument), ...args]

src/statistics/features/common-syntax-probability.ts

@@ -108,7 +108,7 @@ export function updateCommonSyntaxTypeCounts(current: CommonSyntaxTypeCounts, ..
 			}
 			break
 		case RType.FunctionCall:
-			if(node.flavor === 'unnamed') {
+			if(!node.named) {
 				current.unnamedCall++
 			} else {
 				incrementEntry(current.call, node.functionName.content)

src/statistics/features/supported/control-flow/control-flow.ts

@@ -29,7 +29,7 @@ function visitIfThenElse(info: ControlflowInfo, input: FeatureProcessorInput): v
 	visitAst(input.normalizedRAst.ast,
 		node => {
 			if(node.type !== RType.IfThenElse) {
-				if(node.type === RType.FunctionCall && node.flavor === 'named' && node.functionName.content === 'switch') {
+				if(node.type === RType.FunctionCall && node.named && node.functionName.content === 'switch') {
 					const initialArg = unpackArgument(node.arguments[0])
 					if(initialArg) {
 						info.switchCase = updateCommonSyntaxTypeCounts(info.switchCase, initialArg)

src/statistics/features/supported/defined-functions/defined-functions.ts

@@ -136,7 +136,7 @@ function visitDefinitions(info: FunctionDefinitionInfo, input: FeatureProcessorI
 			// track all calls with the same name that do not already have a bound calls edge, superfluous if recursive tracking is explicit
 			const recursiveCalls: RNodeWithParent[] = []
 			visitAst(node.body, n => {
-				if(n.type === RType.FunctionCall && n.flavor === 'named' && assigned.has(n.functionName.lexeme)) {
+				if(n.type === RType.FunctionCall && n.named && assigned.has(n.functionName.lexeme)) {
 					recursiveCalls.push(n)
 				}
 			})

src/statistics/features/supported/loops/loops.ts

@@ -39,7 +39,7 @@ function visitLoops(info: LoopInfo, input: FeatureProcessorInput): void {
 				case RType.Next:         info.nextStatements++; return
 				case RType.Break:        info.breakStatements++; return
 				case RType.FunctionCall:
-					if(node.flavor === 'named' && isImplicitLoop.test(node.functionName.lexeme)) {
+					if(node.named && isImplicitLoop.test(node.functionName.lexeme)) {
 						info.implicitLoops++
 						appendStatisticsFile(loops.name, 'implicit-loop', [node.functionName.info.fullLexeme ?? node.functionName.lexeme], input.filepath)
 					}

src/statistics/features/supported/used-functions/used-functions.ts

@@ -97,7 +97,7 @@ function visitCalls(info: FunctionUsageInfo, input: FeatureProcessorInput): void
 				hasCallsEdge = [...dataflowNode[1].values()].some(e => edgeIncludesType(e.types, EdgeType.Calls))
 			}
 
-			if(node.flavor === 'unnamed') {
+			if(!node.named) {
 				info.unnamedCalls++
 				appendStatisticsFile(usedFunctions.name, 'unnamed-calls', [node.lexeme], input.filepath)
 				allCalls.push([

src/util/assert.ts

@@ -31,7 +31,7 @@ export type GuardMessage = string | (() => string)
  * @param message - if a string, will be used as error message, if a function, will be called to produce the error message (can be used to avoid costly message generations)
  * @throws GuardError - if the assertion fails
  */
-export function guard(assertion: boolean, message: GuardMessage = 'Assertion failed'): asserts assertion {
+export function guard(assertion: boolean | undefined, message: GuardMessage = 'Assertion failed'): asserts assertion {
 	if(!assertion) {
 		throw new GuardError( typeof message === 'string' ? message : message())
 	}

src/util/mermaid/ast.ts

@@ -2,6 +2,7 @@ import { escapeMarkdown, mermaidCodeToUrl } from './mermaid'
 import { RoleInParent } from '../../r-bridge/lang-4.x/ast/model/processing/role'
 import type { RNodeWithParent } from '../../r-bridge/lang-4.x/ast/model/processing/decorate'
 import { visitAst } from '../../r-bridge/lang-4.x/ast/model/processing/visitor'
+import { RType } from '../../r-bridge/lang-4.x/ast/model/type'
 
 export function normalizedAstToMermaid(ast: RNodeWithParent, prefix = ''): string {
 	let output = prefix + 'flowchart TD\n'
@@ -13,6 +14,10 @@ export function normalizedAstToMermaid(ast: RNodeWithParent, prefix = ''): strin
 			const roleSuffix = context.role === RoleInParent.ExpressionListChild || context.role === RoleInParent.FunctionCallArgument || context.role === RoleInParent.FunctionDefinitionParameter ? `-${context.index}` : ''
 			output += `    n${n.info.parent} -->|"${context.role}${roleSuffix}"| n${n.info.id}\n`
 		}
+		if(n.type === RType.ExpressionList && n.grouping !== undefined) {
+			output += `    n${n.info.id} -.-|"group-open"| n${n.grouping[0].info.id}\n`
+			output += `    n${n.info.id} -.-|"group-close"| n${n.grouping[1].info.id}\n`
+		}
 		return false
 	})
 	return output