Support Branch Coverage for if (#540)

Author: EagleoutIce

src/dataflow/common/environments/environment.ts

@@ -60,23 +60,26 @@ export function diffIdentifierReferences(a: IdentifierReference, b: IdentifierRe
 	}
 }
 
+
+function makeReferenceMaybe(ref: IdentifierReference, graph: DataflowGraph, environments: REnvironmentInformation): IdentifierReference {
+	const node = graph.get(ref.nodeId, true)
+	const definitions = resolveByName(ref.name, LocalScope, environments)
+	for(const definition of definitions ?? []) {
+		if(definition.kind !== 'built-in-function') {
+			definition.used = 'maybe'
+		}
+	}
+	if(node) {
+		node[0].when = 'maybe'
+	}
+	return { ...ref, used: 'maybe'}
+}
+
 export function makeAllMaybe(references: IdentifierReference[] | undefined, graph: DataflowGraph, environments: REnvironmentInformation): IdentifierReference[] {
 	if(references === undefined) {
 		return []
 	}
-	return references.map(ref => {
-		const node = graph.get(ref.nodeId, true)
-		const definitions = resolveByName(ref.name, LocalScope, environments)
-		for(const definition of definitions ?? []) {
-			if(definition.kind !== 'built-in-function') {
-				definition.used = 'maybe'
-			}
-		}
-		if(node) {
-			node[0].when = 'maybe'
-		}
-		return { ...ref, used: 'maybe'}
-	})
+	return references.map(ref => makeReferenceMaybe(ref, graph, environments))
 }
 
 
@@ -180,26 +183,30 @@ export function diffEnvironment(a: IEnvironment | undefined, b: IEnvironment | u
 			continue
 		}
 
+		// we sort both value arrays by their id so that we have no problems with differently ordered arrays (which have no impact)
+		const sorted = [...value].sort((a, b) => a.nodeId.localeCompare(b.nodeId))
+		const sorted2 = [...value2].sort((a, b) => a.nodeId.localeCompare(b.nodeId))
+
 		for(let i = 0; i < value.length; ++i) {
-			const aVal = value[i]
-			const bVal = value2[i]
+			const aVal = sorted[i]
+			const bVal = sorted2[i]
 			if(aVal.name !== bVal.name) {
 				info.report.addComment(`${info.position}Different names for ${key}. ${info.leftname}: ${aVal.name} vs. ${info.rightname}: ${bVal.name}`)
 			}
 			if(aVal.nodeId !== bVal.nodeId) {
 				info.report.addComment(`${info.position}Different ids for ${key}. ${info.leftname}: ${aVal.nodeId} vs. ${info.rightname}: ${bVal.nodeId}`)
 			}
 			if(aVal.scope !== bVal.scope) {
-				info.report.addComment(`${info.position}Different scopes for ${key}. ${info.leftname}: ${aVal.scope} vs. ${info.rightname}: ${bVal.scope}`)
+				info.report.addComment(`${info.position}Different scopes for ${key} (${aVal.nodeId}). ${info.leftname}: ${aVal.scope} vs. ${info.rightname}: ${bVal.scope}`)
 			}
 			if(aVal.used !== bVal.used) {
-				info.report.addComment(`${info.position}Different used for ${key}. ${info.leftname}: ${aVal.used} vs. ${info.rightname}: ${bVal.used}`)
+				info.report.addComment(`${info.position}Different used for ${key} (${aVal.nodeId}). ${info.leftname}: ${aVal.used} vs. ${info.rightname}: ${bVal.used}`)
 			}
 			if(aVal.definedAt !== bVal.definedAt) {
-				info.report.addComment(`${info.position}Different definition ids (definedAt) for ${key}. ${info.leftname}: ${aVal.definedAt} vs. ${info.rightname}: ${bVal.definedAt}`)
+				info.report.addComment(`${info.position}Different definition ids (definedAt) for ${key} (${aVal.nodeId}). ${info.leftname}: ${aVal.definedAt} vs. ${info.rightname}: ${bVal.definedAt}`)
 			}
 			if(aVal.kind !== bVal.kind) {
-				info.report.addComment(`${info.position}Different kinds for ${key}. ${info.leftname}: ${aVal.kind} vs. ${info.rightname}: ${bVal.kind}`)
+				info.report.addComment(`${info.position}Different kinds for ${key} (${aVal.nodeId}). ${info.leftname}: ${aVal.kind} vs. ${info.rightname}: ${bVal.kind}`)
 			}
 		}
 	}

src/dataflow/common/environments/register.ts

@@ -2,6 +2,16 @@ import { guard } from '../../../util/assert'
 import { cloneEnvironments, IdentifierDefinition, IEnvironment, REnvironmentInformation } from './environment'
 import { DataflowScopeName, GlobalScope, LocalScope } from './scopes'
 
+function defInEnv(newEnvironments: IEnvironment, definition: IdentifierDefinition) {
+	const existing = newEnvironments.memory.get(definition.name)
+	// check if it is maybe or not
+	if(existing === undefined || definition.used === 'always') {
+		newEnvironments.memory.set(definition.name, [definition])
+	} else {
+		existing.push(definition)
+	}
+}
+
 /**
  * Insert the given `definition` --- defined within the given scope --- into the passed along `environments` will take care of propagation.
  * Does not modify the passed along `environments` in-place! It returns the new reference.
@@ -10,7 +20,7 @@ export function define(definition: IdentifierDefinition, withinScope: DataflowSc
 	let newEnvironments = environments
 	if(withinScope === LocalScope) {
 		newEnvironments = cloneEnvironments(environments, false)
-		newEnvironments.current.memory.set(definition.name, [definition])
+		defInEnv(newEnvironments.current, definition)
 	} else if(withinScope === GlobalScope) {
 		newEnvironments = cloneEnvironments(environments, true)
 		let current: IEnvironment | undefined = newEnvironments.current

src/dataflow/v1/internal/process/access.ts

@@ -1,15 +1,15 @@
 import { ParentInformation, RAccess } from '../../../../r-bridge'
 import { DataflowInformation } from '../info'
 import { DataflowProcessorInformation, processDataflowFor } from '../../processor'
-import { makeAllMaybe, overwriteEnvironments } from '../../../common/environments'
+import { makeAllMaybe } from '../../../common/environments'
 import { EdgeType } from '../../graph'
 
 export function processAccess<OtherInfo>(node: RAccess<OtherInfo & ParentInformation>, data: DataflowProcessorInformation<OtherInfo & ParentInformation>): DataflowInformation {
 	const processedAccessed = processDataflowFor(node.accessed, data)
 	const nextGraph = processedAccessed.graph
 	const outgoing = processedAccessed.out
 	const ingoing = processedAccessed.in
-	const environments = processedAccessed.environments
+	let environments = processedAccessed.environments
 
 	const accessedNodes = processedAccessed.unknownReferences
 
@@ -18,6 +18,7 @@ export function processAccess<OtherInfo>(node: RAccess<OtherInfo & ParentInforma
 			if(access === null || access.value === undefined) {
 				continue
 			}
+			data = { ...data, environments }
 			const processedAccess = processDataflowFor(access, data)
 
 			nextGraph.mergeWith(processedAccess.graph)
@@ -29,7 +30,7 @@ export function processAccess<OtherInfo>(node: RAccess<OtherInfo & ParentInforma
 				}
 			}
 			ingoing.push(...processedAccess.in, ...processedAccess.unknownReferences)
-			overwriteEnvironments(environments, processedAccess.environments)
+			environments = processedAccess.environments
 		}
 	}
 

src/dataflow/v1/internal/process/expression-list.ts

@@ -96,7 +96,7 @@ function updateSideEffectsForCalledFunctions(calledEnvs: {
 				}
 				current = current.parent
 			}
-			// we update all definitions to be linked with teh corresponding function call
+			// we update all definitions to be linked with the corresponding function call
 			environments = overwriteEnvironments(environments, environment)
 		}
 	}
@@ -129,6 +129,7 @@ export function processExpressionList<OtherInfo>(exprList: RExpressionList<Other
 		// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- seems to be a bug in eslint
 		if(!foundNextOrBreak) {
 			visitAst(expression, n => {
+				// we should track returns more consistently
 				if(n.type === RType.Next || n.type === RType.Break) {
 					foundNextOrBreak = true
 				}
@@ -155,7 +156,11 @@ export function processExpressionList<OtherInfo>(exprList: RExpressionList<Other
 
 		const calledEnvs = linkFunctionCalls(nextGraph, data.completeAst.idMap, functionCallIds, processed.graph)
 
-		environments = overwriteEnvironments(environments, processed.environments)
+		if(foundNextOrBreak) {
+			environments = overwriteEnvironments(environments, processed.environments)
+		} else {
+			environments = processed.environments
+		}
 
 		// if the called function has global redefinitions, we have to keep them within our environment
 		environments = updateSideEffectsForCalledFunctions(calledEnvs, environments, nextGraph)

src/dataflow/v1/internal/process/if-then-else.ts

@@ -29,8 +29,9 @@ export function processIfThenElse<OtherInfo>(ifThen: RIfThenElse<OtherInfo & Par
 
 	const nextGraph = cond.graph.mergeWith(then?.graph).mergeWith(otherwise?.graph)
 
-	const thenEnvironment = appendEnvironments(cond.environments, then?.environments)
-	const finalEnvironment = otherwise ? appendEnvironments(thenEnvironment, otherwise.environments) : thenEnvironment
+	const thenEnvironment = then?.environments ?? cond.environments
+	// if there is no "else" case we have to recover whatever we had before as it may be not executed
+	const finalEnvironment = appendEnvironments(thenEnvironment, otherwise ? otherwise.environments : cond.environments)
 
 	// again within an if-then-else we consider all actives to be read
 	const ingoing: IdentifierReference[] = [

src/dataflow/v1/internal/process/operators/assignment.ts

@@ -37,6 +37,7 @@ export function processAssignment<OtherInfo>(op: RAssignmentOp<OtherInfo & Paren
 			}
 		}
 	}
+
 	return {
 		unknownReferences: [],
 		in:                readTargets,
@@ -124,7 +125,6 @@ function processReadAndWriteForAssignmentBasedOnOp<OtherInfo>(
 	for(const write of writeNodes) {
 		environments = define(write, global ? GlobalScope: LocalScope, environments)
 	}
-
 	return {
 		readTargets:   [...source.unknownReferences, ...read, ...readFromSourceWritten],
 		writeTargets:  [...writeNodes, ...target.out, ...readFromSourceWritten],

src/util/quads.ts

@@ -277,7 +277,6 @@ function serializeObject(obj: DataForQuad | undefined | null, quads: Quad[], con
 	} else if(obj instanceof Set) {
 		let i = 0
 		for(const value of obj.values()) {
-			console.log('set', value)
 			processObjectEntry('idx-'+String(i++), value, obj, quads, config)
 		}
 	} else {

test/functionality/dataflow/processing-of-elements/expression-lists/if-then-tests.ts

@@ -83,4 +83,91 @@ describe('Lists with if-then constructs', withShell(shell => {
 			})
 		})
 	}
+	describe('Branch Coverage', () => {
+		//All test related to branch coverage (testing the interaction between then end else block)
+		const envWithX = () => define({ nodeId: '0', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '2', used: 'always' }, LocalScope, initializeCleanEnvironments())
+		const envThenBranch = () => define({nodeId: '4', scope: LocalScope, name: 'x', used: 'maybe', kind: 'variable',definedAt: '6'}, LocalScope, initializeCleanEnvironments())
+		assertDataflow('assignment both branches in if',
+			shell,
+			'x <- 1\nif(r) { x <- 2 } else { x <- 3}\n y <- x',
+			new DataflowGraph()
+				.addVertex( { tag: 'variable-definition', id: '0', name: 'x', scope: LocalScope})
+				.addVertex( { tag: 'use', id: '3', name: 'r', scope: LocalScope,environment: envWithX()} )
+				.addVertex( { tag: 'variable-definition', id: '4', name: 'x', scope: LocalScope, environment: envWithX(), when: 'maybe' } )
+				.addVertex( { tag: 'variable-definition', id: '8', name: 'x', scope: LocalScope, environment: envWithX(), when: 'maybe' } )
+				.addVertex( { tag: 'use', id: '14', name: 'x', scope: LocalScope, environment: define({ nodeId: '8',scope: LocalScope, name: 'x', used: 'maybe',kind: 'variable',definedAt: '10'}, LocalScope, envThenBranch())})
+				.addVertex( { tag: 'variable-definition', id: '13', name: 'y', scope: LocalScope, environment: define({ nodeId: '8',scope: LocalScope, name: 'x', used: 'maybe',kind: 'variable',definedAt: '10'}, LocalScope, envThenBranch())})
+				.addEdge('0', '8', EdgeType.SameDefDef, 'maybe')
+				.addEdge('0', '4', EdgeType.SameDefDef, 'maybe')
+				.addEdge('14', '4', EdgeType.Reads, 'maybe')
+				.addEdge('14', '8', EdgeType.Reads, 'maybe')
+				.addEdge('13', '14', EdgeType.DefinedBy, 'always')
+		)
+		const envWithXMaybe = () => define({ nodeId: '0', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '2', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		const envWithSecondXMaybe = () => define({ nodeId: '4', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '6', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		assertDataflow('assignment if one branch',
+			shell,
+			'x <- 1\nif(r) { x <- 2 } \n y <- x',
+			new DataflowGraph()
+				.addVertex( { tag: 'variable-definition', id: '0', name: 'x', scope: LocalScope})
+				.addVertex( { tag: 'use', id: '3', name: 'r', scope: LocalScope, environment: envWithX()} )
+				.addVertex( { tag: 'variable-definition', id: '4', name: 'x', scope: LocalScope, environment: envWithX(), when: 'maybe' } )
+				.addVertex( { tag: 'use', id: '10', name: 'x', scope: LocalScope, environment: appendEnvironments(envWithSecondXMaybe(), envWithXMaybe()) })
+				.addVertex( { tag: 'variable-definition', id: '9', name: 'y', scope: LocalScope, environment: appendEnvironments(envWithSecondXMaybe(), envWithXMaybe()) })
+				.addEdge('0', '4', EdgeType.SameDefDef, 'maybe')
+				.addEdge('10', '0', EdgeType.Reads, 'maybe')
+				.addEdge('10', '4', EdgeType.Reads, 'maybe')
+				.addEdge('9', '10', EdgeType.DefinedBy, 'always')
+		)
+		const envWithY = () => define({ nodeId: '3', name: 'y', scope: LocalScope, kind: 'variable', definedAt: '5', used: 'always' }, LocalScope, initializeCleanEnvironments())
+		const envWithXThen = () => define({ nodeId: '7', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '9', used: 'always' }, LocalScope, initializeCleanEnvironments())
+		const envWithXThenMaybe = () => define({ nodeId: '7', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '9', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		const envWithXElseMaybe = () => define({ nodeId: '14', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '16', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		const envWithYMaybeBeforeIf = () => define({ nodeId: '3', name: 'y', scope: LocalScope, kind: 'variable', definedAt: '5', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		const envWithYMaybeInThen = () => define({ nodeId: '10', name: 'y', scope: LocalScope, kind: 'variable', definedAt: '12', used: 'maybe'}, LocalScope, initializeCleanEnvironments())
+		const envForYAfterIf = () => appendEnvironments(envWithYMaybeBeforeIf(), envWithYMaybeInThen())
+		const envForXAfterIf = () => appendEnvironments(envWithXThenMaybe(), envWithXElseMaybe())
+		const envDirectlyAfterIf = () => appendEnvironments(envForYAfterIf(), envForXAfterIf())
+		const envWithW = () => define({ nodeId: '19', name: 'w', scope: LocalScope, kind: 'variable', definedAt: '21', used: 'always' }, LocalScope, initializeCleanEnvironments())
+		//const envFirstYReassignment = () => appendEnvironments(envWithXThen(), envWithY())
+		assertDataflow('assignment if multiple variables with else',
+			shell,
+			'x <- 1 \n y <- 2 \n if(r){ x <- 3 \n y <- 4} else {x <- 5} \n w <- x \n z <- y',
+			new DataflowGraph()
+				.addVertex({ tag: 'variable-definition', id: '0', name: 'x', scope: LocalScope})
+				.addVertex({ tag: 'variable-definition', id: '3', name: 'y', scope: LocalScope, environment: envWithX()})
+				.addVertex({ tag: 'use', id: '6', name: 'r', scope: LocalScope, environment: appendEnvironments(envWithX(), envWithY())})
+				.addVertex({ tag: 'variable-definition', id: '7', name: 'x', scope: LocalScope, environment: appendEnvironments(envWithX(),envWithY()), when: 'maybe' })
+				.addVertex({ tag: 'variable-definition', id: '10', name: 'y', scope: LocalScope, environment: appendEnvironments(envWithXThen(), envWithY()), when: 'maybe'})
+				.addVertex({ tag: 'variable-definition', id: '14', name: 'x', scope: LocalScope, environment: appendEnvironments(envWithX(),envWithY()), when: 'maybe' })
+				.addVertex({ tag: 'use', id: '20', name: 'x', scope: LocalScope, environment: envDirectlyAfterIf()})
+				.addVertex({ tag: 'use', id: '23', name: 'y', scope: LocalScope, environment: appendEnvironments(envDirectlyAfterIf(), envWithW())})
+				.addVertex({ tag: 'variable-definition', id: '19', name: 'w', scope: LocalScope, environment: envDirectlyAfterIf()})
+				.addVertex({ tag: 'variable-definition', id: '22', name: 'z', scope: LocalScope, environment: appendEnvironments(envDirectlyAfterIf(), envWithW())})
+				.addEdge('20', '7', EdgeType.Reads, 'maybe')
+				.addEdge('20', '14', EdgeType.Reads, 'maybe')
+				.addEdge('23', '3', EdgeType.Reads, 'maybe')
+				.addEdge('23', '10', EdgeType.Reads, 'maybe')
+				.addEdge('19', '20', EdgeType.DefinedBy, 'always')
+				.addEdge('22', '23', EdgeType.DefinedBy, 'always')
+				.addEdge('7', '0', EdgeType.SameDefDef, 'maybe')
+				.addEdge('14', '0', EdgeType.SameDefDef, 'maybe')
+				.addEdge('10', '3', EdgeType.SameDefDef, 'maybe')
+		)
+		const envWithElseXMaybe = () => define({ nodeId: '5', name: 'x', scope: LocalScope, kind: 'variable', definedAt: '7', used: 'maybe' }, LocalScope, initializeCleanEnvironments())
+		assertDataflow('assignment in else block',
+			shell,
+			'x <- 1 \n if(r){} else{x <- 2} \n y <- x',
+			new DataflowGraph()
+				.addVertex( { tag: 'variable-definition', id: '0', name: 'x', scope: LocalScope})
+				.addVertex( { tag: 'use', id: '3', name: 'r', scope: LocalScope, environment: envWithX()} )
+				.addVertex( { tag: 'variable-definition', id: '5', name: 'x', scope: LocalScope, environment: envWithX(), when: 'maybe' } )
+				.addVertex( { tag: 'use', id: '11', name: 'x', scope: LocalScope, environment: appendEnvironments(envWithElseXMaybe(), envWithXMaybe()) })
+				.addVertex( { tag: 'variable-definition', id: '10', name: 'y', scope: LocalScope, environment: appendEnvironments(envWithElseXMaybe(), envWithXMaybe()) })
+				.addEdge('0', '5', EdgeType.SameDefDef, 'maybe')
+				.addEdge('11', '0', EdgeType.Reads, 'maybe')
+				.addEdge('11', '5', EdgeType.Reads, 'maybe')
+				.addEdge('10', '11', EdgeType.DefinedBy, 'always')
+		)
+	})
 }))