test-fix: port the reconstruct/slice test to the new stepping slicer

src/core/input.ts

@@ -1,8 +1,8 @@
 import { MergeableRecord } from '../util/objects'
 import { IdGenerator, NoInfo, RParseRequest, RShell, TokenMap, XmlParserHooks } from '../r-bridge'
 import { DeepPartial } from 'ts-essentials'
-import { SlicingCriteria } from '../slicing'
-import { SubStepName } from './steps'
+import { AutoSelectPredicate, SlicingCriteria } from '../slicing'
+import { STEPS_PER_SLICE, SubStepName } from './steps'
 
 /**
  * We split the types, as if you are only interested in what can be done per-file, you do not need a slicing criterion.
@@ -23,10 +23,12 @@ interface BaseSteppingSlicerInput<InterestedIn extends SubStepName | undefined>
 	tokenMap?:       TokenMap
 	/** These hooks only make sense if you at least want to normalize the parsed R AST. They can augment the normalization process */
 	hooks?:          DeepPartial<XmlParserHooks>
-	/** This id generator is only necessary if you want to retrieve a dataflow from the parsed R AST, it determines the id generator to use and if you are unsure, use the {@link deterministicCountingIdGenerator}*/
+	/** This id generator is only necessary if you want to retrieve a dataflow from the parsed R AST, it determines the id generator to use and by default uses the {@link deterministicCountingIdGenerator}*/
 	getId?:          IdGenerator<NoInfo>
 	/** The slicing criterion is only of interest if you actually want to slice the R code */
 	criterion?:      SlicingCriteria
+	/** If you want to auto-select something in the reconstruction add it here, otherwise, it will use the default defined alongside {@link reconstructToCode}*/
+	autoSelectIf?:   AutoSelectPredicate
 }
 
 interface NormalizeSteppingSlicerInput<InterestedIn extends 'dataflow' | 'decorate' | 'normalize ast'> extends BaseSteppingSlicerInput<InterestedIn> {
@@ -45,6 +47,6 @@ interface SliceSteppingSlicerInput<InterestedIn extends 'reconstruct' | 'slice'
  * All arguments are documented alongside {@link BaseSteppingSlicerInput}.
  */
 export type SteppingSlicerInput<InterestedIn extends SubStepName | undefined = undefined> =
-		InterestedIn extends 'reconstruct' | 'slice' | 'decode criteria' | undefined ? SliceSteppingSlicerInput<InterestedIn> :
+		InterestedIn extends keyof typeof STEPS_PER_SLICE | undefined ? SliceSteppingSlicerInput<InterestedIn> :
 			InterestedIn extends 'dataflow' | 'decorate' | 'normalize ast' ? NormalizeSteppingSlicerInput<InterestedIn> :
 				BaseSteppingSlicerInput<InterestedIn>

src/core/slicer.ts

@@ -1,25 +1,25 @@
 import {
 	DecoratedAst, IdGenerator,
-	NodeId, NoInfo, RNode,
+	NodeId, NoInfo, ParentInformation, RNode,
 	RParseRequest,
 	RShell,
 	TokenMap,
 	XmlParserHooks
 } from '../r-bridge'
 import {
-	executeSingleSubStep, LAST_STEP,
+	executeSingleSubStep, LAST_PER_FILE_STEP, LAST_STEP,
 	StepRequired, STEPS,
 	STEPS_PER_FILE,
 	STEPS_PER_SLICE,
 	SubStepName,
 	SubStepProcessor
 } from './steps'
 import { guard } from '../util/assert'
-import { SlicingCriteria } from '../slicing'
-import { DataflowGraph } from '../dataflow'
+import { DecodedCriteria, SliceResult, SlicingCriteria } from '../slicing'
 import { DeepPartial } from 'ts-essentials'
 import { SteppingSlicerInput } from './input'
 import { StepResults } from './output'
+import { DataflowInformation } from '../dataflow/internal/info'
 
 /**
  * This is ultimately the root of flowR's static slicing procedure.
@@ -82,8 +82,8 @@ import { StepResults } from './output'
  * @see SubStepName
  */
 export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
-	public readonly maximumNumberOfStepsPerFile = Object.keys(STEPS_PER_FILE).length
-	public readonly maximumNumberOfStepsPerSlice = Object.keys(STEPS_PER_SLICE).length
+	public static readonly maximumNumberOfStepsPerFile = Object.keys(STEPS_PER_FILE).length
+	public static readonly maximumNumberOfStepsPerSlice = SteppingSlicer.maximumNumberOfStepsPerFile + Object.keys(STEPS_PER_SLICE).length
 
 	private readonly shell:          RShell
 	private readonly tokenMap?:      TokenMap
@@ -128,7 +128,7 @@ export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
 	 * @see getCurrentStage
 	 */
 	public switchToSliceStage(): void {
-		guard(this.stepCounter === this.maximumNumberOfStepsPerFile, 'First need to complete all steps before switching')
+		guard(this.stepCounter === SteppingSlicer.maximumNumberOfStepsPerFile, 'First need to complete all steps before switching')
 		guard(this.stage === 'once-per-file', 'Cannot switch to next stage, already in once-per-slice stage')
 		this.stage = 'once-per-slice'
 	}
@@ -146,8 +146,8 @@ export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
 	 */
 	public hasNextStep(): boolean {
 		return !this.reachedWanted && (this.stage === 'once-per-file' ?
-			this.stepCounter < this.maximumNumberOfStepsPerFile
-			: this.stepCounter < this.maximumNumberOfStepsPerSlice
+			this.stepCounter < SteppingSlicer.maximumNumberOfStepsPerFile
+			: this.stepCounter < SteppingSlicer.maximumNumberOfStepsPerSlice
 		)
 	}
 
@@ -215,11 +215,11 @@ export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
 				break
 			case 5:
 				step = guardStep('slice')
-				result = executeSingleSubStep(step, this.results.dataflow as DataflowGraph, (this.results.decorate as DecoratedAst).idMap, this.results['decode criteria'] as NodeId[])
+				result = executeSingleSubStep(step, (this.results.dataflow as DataflowInformation).graph, (this.results.decorate as DecoratedAst<NoInfo>).idMap, (this.results['decode criteria'] as DecodedCriteria).map(({id}) => id))
 				break
 			case 6:
 				step = guardStep('reconstruct')
-				result = executeSingleSubStep(step, this.results.decorate as DecoratedAst, this.results.slice as Set<NodeId>)
+				result = executeSingleSubStep(step, this.results.decorate as DecoratedAst<NoInfo>, (this.results.slice as SliceResult).result)
 				break
 			default:
 				throw new Error(`Unknown step ${this.stepCounter}, reaching this should not happen!`)
@@ -234,9 +234,9 @@ export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
 	 * @param newCriterion - the new slicing criterion to use for the next slice
 	 */
 	public updateCriterion(newCriterion: SlicingCriteria): void {
-		guard(this.stepCounter >= this.maximumNumberOfStepsPerFile , 'Cannot reset slice prior to once-per-slice stage')
+		guard(this.stepCounter >= SteppingSlicer.maximumNumberOfStepsPerFile , 'Cannot reset slice prior to once-per-slice stage')
 		this.criterion = newCriterion
-		this.stepCounter = this.maximumNumberOfStepsPerFile
+		this.stepCounter = SteppingSlicer.maximumNumberOfStepsPerFile
 		this.results['decode criteria'] = undefined
 		this.results.slice = undefined
 		this.results.reconstruct = undefined
@@ -245,17 +245,25 @@ export class SteppingSlicer<InterestedIn extends SubStepName | undefined> {
 		}
 	}
 
+	public async allRemainingSteps(canSwitchStage: false): Promise<StepResults<InterestedIn extends keyof typeof STEPS_PER_SLICE | undefined ? typeof LAST_PER_FILE_STEP : InterestedIn>>
+	public async allRemainingSteps(canSwitchStage?: true): Promise<StepResults<InterestedIn>>
 	/**
 	 * Execute all remaining steps and automatically call {@link switchToSliceStage} if necessary.
-	 * @param switchStage - if true, automatically switch to the slice stage if necessary
-	 * (i.e., this is what you want if you have never executed {@link nextStep} and you want to execute *all* steps).
-	 * However, passing false allows you to only execute the steps of the 'once-per-file' stage (i.e., the steps that can be cached).
+	 * @param canSwitchStage - if true, automatically switch to the slice stage if necessary
+	 *       (i.e., this is what you want if you have never executed {@link nextStep} and you want to execute *all* steps).
+	 *       However, passing false allows you to only execute the steps of the 'once-per-file' stage (i.e., the steps that can be cached).
+	 *
+	 * **Note:** There is a small type difference if you pass 'false' and already have manually switched to the 'once-per-slice' stage.
+	 *       Because now, the results of these steps are no longer part of the result type (although they are still included).
+	 *       In such a case, you may be better off with simply passing 'true' as the function will detect that the stage is already switched.
+	 *       We could solve this type problem by separating the SteppingSlicer class into two for each stage, but this would break the improved readability and unified handling
+	 *       of the slicer that I wanted to achieve with this class.
 	 */
-	public async allRemainingSteps(switchStage = true): Promise<StepResults<InterestedIn>> {
+	public async allRemainingSteps(canSwitchStage = true): Promise<StepResults<InterestedIn | typeof LAST_PER_FILE_STEP>> {
 		while(this.hasNextStep()) {
 			await this.nextStep()
 		}
-		if(switchStage && !this.reachedWanted && this.stage === 'once-per-file') {
+		if(canSwitchStage && !this.reachedWanted && this.stage === 'once-per-file') {
 			this.switchToSliceStage()
 			while(this.hasNextStep()) {
 				await this.nextStep()

src/core/steps.ts

@@ -95,7 +95,8 @@ export const STEPS_PER_SLICE = {
 } as const
 
 export const STEPS = { ...STEPS_PER_FILE, ...STEPS_PER_SLICE } as const
-export const LAST_STEP: keyof typeof STEPS = 'reconstruct' as const
+export const LAST_PER_FILE_STEP = 'dataflow' as const
+export const LAST_STEP = 'reconstruct' as const
 
 export type SubStepName = keyof typeof STEPS
 export type SubStep<name extends SubStepName> = typeof STEPS[name]

src/slicing/criterion/parse.ts

@@ -83,6 +83,13 @@ function conventionalCriteriaToId<OtherInfo>(line: number, name: string, dataflo
 	return id
 }
 
-export function convertAllSlicingCriteriaToIds(criteria: SlicingCriteria, decorated: DecoratedAst): { criterion: SingleSlicingCriterion, id: NodeId }[] {
+export interface DecodedCriterion {
+	criterion: SingleSlicingCriterion,
+	id:        NodeId
+}
+
+export type DecodedCriteria = DecodedCriterion[]
+
+export function convertAllSlicingCriteriaToIds(criteria: SlicingCriteria, decorated: DecoratedAst): DecodedCriteria {
 	return criteria.map(l => ({ criterion: l, id: slicingCriterionToId(l, decorated) }))
 }

src/slicing/reconstruct.ts

@@ -378,7 +378,8 @@ function reconstructFunctionCall(call: RFunctionCall<ParentInformation>, functio
 	}
 }
 
-type AutoSelectPredicate = (node: RNode<ParentInformation>) => boolean
+/** The structure of the predicate that should be used to determine if a given normalized node should be included in the reconstructed code independent of if it is selected by the slice or not */
+export type AutoSelectPredicate = (node: RNode<ParentInformation>) => boolean
 
 
 interface ReconstructionConfiguration extends MergeableRecord {
@@ -392,6 +393,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 !== Type.FunctionCall || node.flavor !== 'named') {
 		return false
@@ -468,7 +470,7 @@ export interface ReconstructionResult {
  *
  * @returns Number of times `autoSelectIf` triggered
  */
-export function reconstructToCode<Info>(ast: DecoratedAst<Info>, selection: Selection, autoSelectIf: (node: RNode<ParentInformation>) => boolean = autoSelectLibrary): ReconstructionResult {
+export function reconstructToCode<Info>(ast: DecoratedAst<Info>, selection: Selection, autoSelectIf: AutoSelectPredicate = autoSelectLibrary): ReconstructionResult {
 	reconstructLogger.trace(`reconstruct ast with ids: ${JSON.stringify([...selection])}`)
 	let autoSelected = 0
 	const autoSelectIfWrapper = (node: RNode<ParentInformation>) => {

test/helper/shell.ts

@@ -2,7 +2,6 @@ import { it } from 'mocha'
 import { testRequiresNetworkConnection } from './network'
 import { DeepPartial } from 'ts-essentials'
 import {
-	decorateAst,
 	DecoratedAstMap,
 	deterministicCountingIdGenerator,
 	getStoredTokenMap,
@@ -12,30 +11,33 @@ import {
 	RExpressionList,
 	RNode,
 	RNodeWithParent, RParseRequest,
-	RShell,
+	RShell, TokenMap,
 	XmlParserHooks
 } from '../../src/r-bridge'
 import { assert } from 'chai'
-import { DataflowGraph, diffGraphsToMermaidUrl, graphToMermaidUrl, produceDataFlowGraph } from '../../src/dataflow'
-import { reconstructToCode, SlicingCriteria, slicingCriterionToId, staticSlicing } from '../../src/slicing'
+import { DataflowGraph, diffGraphsToMermaidUrl, graphToMermaidUrl } from '../../src/dataflow'
+import { SlicingCriteria } from '../../src/slicing'
 import { testRequiresRVersion } from './version'
 import { deepMergeObject, MergeableRecord } from '../../src/util/objects'
-import { executeSingleSubStep, SteppingSlicer } from '../../src/core'
+import { executeSingleSubStep, LAST_STEP, SteppingSlicer } from '../../src/core'
 
-let defaultTokenMap: Record<string, string>
+let _defaultTokenMap: TokenMap | undefined
 
 // we want the token map only once (to speed up tests)!
-before(async function() {
-	this.timeout('15min')
-	const shell = new RShell()
-	try {
-		shell.tryToInjectHomeLibPath()
-		await shell.ensurePackageInstalled('xmlparsedata')
-		defaultTokenMap = await getStoredTokenMap(shell)
-	} finally {
-		shell.close()
+async function defaultTokenMap(): Promise<TokenMap> {
+	if(_defaultTokenMap === undefined) {
+		const shell = new RShell()
+		try {
+			shell.tryToInjectHomeLibPath()
+			await shell.ensurePackageInstalled('xmlparsedata')
+			_defaultTokenMap = await getStoredTokenMap(shell)
+		} finally {
+			shell.close()
+		}
 	}
-})
+	return _defaultTokenMap
+}
+
 
 export const testWithShell = (msg: string, fn: (shell: RShell, test: Mocha.Context) => void | Promise<void>): Mocha.Test => {
 	return it(msg, async function(): Promise<void> {
@@ -109,7 +111,7 @@ export const retrieveAst = async(shell: RShell, input: `file://${string}` | stri
 		stepOfInterest: 'normalize ast',
 		shell,
 		request,
-		tokenMap:       defaultTokenMap,
+		tokenMap:       await defaultTokenMap(),
 		hooks
 	}).allRemainingSteps())['normalize ast']
 }
@@ -153,7 +155,7 @@ export function assertDecoratedAst<Decorated>(name: string, shell: RShell, input
 			stepOfInterest: 'decorate',
 			getId:          deterministicCountingIdGenerator(startIndexForDeterministicIds),
 			shell,
-			tokenMap:       defaultTokenMap,
+			tokenMap:       await defaultTokenMap(),
 			request:        requestFromInput(input),
 		}).allRemainingSteps()
 
@@ -171,7 +173,7 @@ export function assertDataflow(name: string, shell: RShell, input: string, expec
 			stepOfInterest: 'dataflow',
 			request:        requestFromInput(input),
 			shell,
-			tokenMap:       defaultTokenMap,
+			tokenMap:       await defaultTokenMap(),
 			getId:          deterministicCountingIdGenerator(startIndexForDeterministicIds),
 		}).allRemainingSteps()
 
@@ -210,7 +212,7 @@ export function assertReconstructed(name: string, shell: RShell, input: string,
 			getId:          getId,
 			request:        requestFromInput(input),
 			shell,
-			tokenMap:       defaultTokenMap,
+			tokenMap:       await defaultTokenMap(),
 		}).allRemainingSteps()
 		const reconstructed = executeSingleSubStep('reconstruct', result.decorate,  new Set(selectedIds))
 		assert.strictEqual(reconstructed.code, expected, `got: ${reconstructed.code}, vs. expected: ${expected}, for input ${input} (ids: ${printIdMapping(selectedIds, result.decorate.idMap)})`)
@@ -220,20 +222,20 @@ export function assertReconstructed(name: string, shell: RShell, input: string,
 
 export function assertSliced(name: string, shell: RShell, input: string, criteria: SlicingCriteria, expected: string, getId: IdGenerator<NoInfo> = deterministicCountingIdGenerator(0)): Mocha.Test {
 	return it(`${JSON.stringify(criteria)} ${name}`, async function() {
-		const ast = await retrieveAst(shell, input)
-		const decoratedAst = decorateAst(ast, getId)
+		const result = await new SteppingSlicer({
+			stepOfInterest: LAST_STEP,
+			getId,
+			request:        requestFromInput(input),
+			shell,
+			tokenMap:       await defaultTokenMap(),
+			criterion:      criteria,
+		}).allRemainingSteps()
 
-		const dataflow = produceDataFlowGraph(decoratedAst)
 
 		try {
-			const mappedIds = criteria.map(c => slicingCriterionToId(c, decoratedAst))
-
-			const { result: sliced } = staticSlicing(dataflow.graph, decoratedAst.idMap, mappedIds.slice())
-			const reconstructed = reconstructToCode<NoInfo>(decoratedAst, sliced)
-
-			assert.strictEqual(reconstructed.code, expected, `got: ${reconstructed.code}, vs. expected: ${expected}, for input ${input} (slice: ${printIdMapping(mappedIds, decoratedAst.idMap)}), url: ${graphToMermaidUrl(dataflow.graph, decoratedAst.idMap, sliced)}`)
+			assert.strictEqual(result.reconstruct.code, expected, `got: ${result.reconstruct.code}, vs. expected: ${expected}, for input ${input} (slice: ${printIdMapping(result['decode criteria'].map(({ id }) => id), result.decorate.idMap)}), url: ${graphToMermaidUrl(result.dataflow.graph, result.decorate.idMap, result.slice.result)}`)
 		} catch(e) {
-			console.error('vis-got:\n', graphToMermaidUrl(dataflow.graph, decoratedAst.idMap))
+			console.error('vis-got:\n', graphToMermaidUrl(result.dataflow.graph, result.decorate.idMap))
 			throw e
 		}
 	})