Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: mermaid printing, assign ids, and environment references #811

Merged
merged 7 commits into from
May 21, 2024
44 changes: 22 additions & 22 deletions src/dataflow/environments/built-in.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,29 +149,29 @@ registerBuiltInConstant('FALSE', false)
registerBuiltInConstant('F', false)
registerSimpleFunctions('~', '+', '-', '*', '/', '^', '!', '?', '**', '==', '!=', '>', '<', '>=', '<=', '%%', '%/%', '%*%', ':', 'list')
registerBuiltInFunctions(['cat', 'switch'], defaultBuiltInProcessor, {}) /* returns null */
registerBuiltInFunctions(['print', '('], defaultBuiltInProcessor, { returnsNthArgument: 0 }, )
registerBuiltInFunctions(['return'], defaultBuiltInProcessor, { returnsNthArgument: 0, cfg: ExitPointType.Return }, )
registerBuiltInFunctions(['break'], defaultBuiltInProcessor, { cfg: ExitPointType.Break }, )
registerBuiltInFunctions(['next'], defaultBuiltInProcessor, { cfg: ExitPointType.Next }, )
registerBuiltInFunctions(['{'], processExpressionList, {}, )
registerBuiltInFunctions(['source'], processSourceCall, {}, )
registerBuiltInFunctions(['[', '[['], processAccess, { treatIndicesAsString: false }, )
registerBuiltInFunctions(['$', '@'], processAccess, { treatIndicesAsString: true }, )
registerBuiltInFunctions(['if'], processIfThenElse, {}, )
registerBuiltInFunctions(['get'], processGet, {}, )
registerBuiltInFunctions(['library'], processLibrary, {}, )
registerBuiltInFunctions(['<-', '='], processAssignment, { canBeReplacement: true }, )
registerBuiltInFunctions([':=', 'assign'], processAssignment, {}, )
registerBuiltInFunctions(['delayedAssign'], processAssignment, { quoteSource: true }, )
registerBuiltInFunctions(['<<-'], processAssignment, { superAssignment: true, canBeReplacement: true }, )
registerBuiltInFunctions(['->'], processAssignment, { swapSourceAndTarget: true, canBeReplacement: true }, )
registerBuiltInFunctions(['print', '('], defaultBuiltInProcessor, { returnsNthArgument: 0 } )
registerBuiltInFunctions(['return'], defaultBuiltInProcessor, { returnsNthArgument: 0, cfg: ExitPointType.Return } )
registerBuiltInFunctions(['break'], defaultBuiltInProcessor, { cfg: ExitPointType.Break } )
registerBuiltInFunctions(['next'], defaultBuiltInProcessor, { cfg: ExitPointType.Next } )
registerBuiltInFunctions(['{'], processExpressionList, {} )
registerBuiltInFunctions(['source'], processSourceCall, {} )
registerBuiltInFunctions(['[', '[['], processAccess, { treatIndicesAsString: false } )
registerBuiltInFunctions(['$', '@'], processAccess, { treatIndicesAsString: true } )
registerBuiltInFunctions(['if'], processIfThenElse, {} )
registerBuiltInFunctions(['get'], processGet, {} )
registerBuiltInFunctions(['library'], processLibrary, {} )
registerBuiltInFunctions(['<-', '='], processAssignment, { canBeReplacement: true } )
registerBuiltInFunctions([':=', 'assign'], processAssignment, {} )
registerBuiltInFunctions(['delayedAssign'], processAssignment, { quoteSource: true } )
registerBuiltInFunctions(['<<-'], processAssignment, { superAssignment: true, canBeReplacement: true } )
registerBuiltInFunctions(['->'], processAssignment, { swapSourceAndTarget: true, canBeReplacement: true } )
registerBuiltInFunctions(['->>'], processAssignment, { superAssignment: true, swapSourceAndTarget: true, canBeReplacement: true } )
registerBuiltInFunctions(['&&', '||', '&', '|'], processSpecialBinOp, { lazy: true } )
registerBuiltInFunctions(['|>', '%>%'], processPipe, {}, )
registerBuiltInFunctions(['function', '\\'], processFunctionDefinition, {}, )
registerBuiltInFunctions(['quote', 'substitute', 'bquote'], processQuote, { quoteArgumentsWithIndex: 0 }, )
registerBuiltInFunctions(['for'], processForLoop, {}, )
registerBuiltInFunctions(['repeat'], processRepeatLoop, {}, )
registerBuiltInFunctions(['while'], processWhileLoop, {}, )
registerBuiltInFunctions(['|>', '%>%'], processPipe, {} )
registerBuiltInFunctions(['function', '\\'], processFunctionDefinition, {} )
registerBuiltInFunctions(['quote', 'substitute', 'bquote'], processQuote, { quoteArgumentsWithIndex: 0 } )
registerBuiltInFunctions(['for'], processForLoop, {} )
registerBuiltInFunctions(['repeat'], processRepeatLoop, {} )
registerBuiltInFunctions(['while'], processWhileLoop, {} )
/* they are all mapped to `<-` but we separate super assignments */
registerReplacementFunctions({ makeMaybe: true }, ['<-', '<<-'], '[', '[[', '$', '@', 'names', 'dimnames', 'attributes', 'attr', 'class', 'levels', 'rownames', 'colnames')
4 changes: 3 additions & 1 deletion src/dataflow/environments/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@

export function diffEnvironmentInformation<Report extends WriteableDifferenceReport>(a: REnvironmentInformation | undefined, b: REnvironmentInformation | undefined, info: GenericDifferenceInformation<Report>): void {
if(a === undefined || b === undefined) {
info.report.addComment(`${info.position}Different environments: ${JSON.stringify(a, jsonReplacer)} vs. ${JSON.stringify(b, jsonReplacer)}`)
if(a !== b) {
info.report.addComment(`${info.position}Different environments: ${JSON.stringify(a, jsonReplacer)} vs. ${JSON.stringify(b, jsonReplacer)}`)

Check warning on line 84 in src/dataflow/environments/diff.ts

View check run for this annotation

Codecov / codecov/patch

src/dataflow/environments/diff.ts#L84

Added line #L84 was not covered by tests
}
return
}
diffEnvironment(a.current, b.current, info)
Expand Down
8 changes: 8 additions & 0 deletions src/dataflow/graph/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,18 @@ function diffOutgoingEdges(ctx: DataflowDiffContext): void {
}

for(const [id, edge] of lEdges) {
if(!ctx.left.hasVertex(id)) {
ctx.report.addComment(`The source ${id} of edges ${JSON.stringify(edge, jsonReplacer)} is not present in ${ctx.leftname}. This means that the graph contains an edge but not the corresponding vertex.`)
continue
}
diffEdges(ctx, id, edge, rEdges.get(id))
}
// just to make it both ways in case the length differs
for(const [id, edge] of rEdges) {
if(!ctx.right.hasVertex(id)) {
ctx.report.addComment(`The source ${id} of edges ${JSON.stringify(edge, jsonReplacer)} is not present in ${ctx.rightname}. This means that the graph contains an edge but not the corresponding vertex.`)
continue
}
if(!lEdges.has(id)) {
diffEdges(ctx, id, undefined, edge)
}
Expand Down
24 changes: 19 additions & 5 deletions src/dataflow/graph/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ type EdgeData<Edge extends DataflowGraphEdge> = Omit<Edge, 'from' | 'to' | 'type
*/
export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGraphVertexInfo, Edge extends DataflowGraphEdge = DataflowGraphEdge> {
private static DEFAULT_ENVIRONMENT: REnvironmentInformation | undefined = undefined
public readonly idMap: AstIdMap | undefined
private _idMap: AstIdMap | undefined

constructor(idMap: AstIdMap | undefined) {
DataflowGraph.DEFAULT_ENVIRONMENT ??= initializeCleanEnvironments()
this.idMap = idMap
this._idMap = idMap
}

/** Contains the vertices of the root level graph (i.e., included those vertices from the complete graph, that are nested within function definitions) */
Expand Down Expand Up @@ -134,6 +134,17 @@ export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGrap
}


/** Retrieves the id-map to the normalized AST attached to the dataflow graph */
public get idMap(): AstIdMap | undefined {
return this._idMap
}

/** Allows setting the id-map explicitly (which should only be used when, e.g., you plan to compare two dataflow graphs on the same AST-basis) */
public setIdMap(idMap: AstIdMap): void {
this._idMap = idMap
}


/**
* @param includeDefinedFunctions - If true this will iterate over function definitions as well and not just the toplevel
* @returns the ids of all toplevel vertices in the graph together with their vertex information
Expand Down Expand Up @@ -165,7 +176,7 @@ export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGrap
* @param id - The id to check for
* @param includeDefinedFunctions - If true this will check function definitions as well and not just the toplevel
*/
public hasVertex(id: NodeId, includeDefinedFunctions: boolean): boolean {
public hasVertex(id: NodeId, includeDefinedFunctions = true): boolean {
return includeDefinedFunctions ? this.vertexInformation.has(id) : this.rootVertices.has(id)
}

Expand Down Expand Up @@ -196,9 +207,12 @@ export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGrap
return this
}

const fallback = vertex.tag === VertexType.VariableDefinition || vertex.tag === VertexType.Use || vertex.tag === VertexType.Value ? undefined : DataflowGraph.DEFAULT_ENVIRONMENT
// keep a clone of the original environment
const environment = vertex.environment === undefined ? DataflowGraph.DEFAULT_ENVIRONMENT : cloneEnvironmentInformation(vertex.environment)
const environment = vertex.environment === undefined ? fallback : cloneEnvironmentInformation(vertex.environment)



this.vertexInformation.set(vertex.id, {
...vertex,
when: vertex.controlDependencies ?? 'always',
Expand All @@ -224,7 +238,7 @@ export class DataflowGraph<Vertex extends DataflowGraphVertexInfo = DataflowGrap
public addEdge(from: NodeId | ReferenceForEdge, to: NodeId | ReferenceForEdge, edgeInfo: EdgeData<Edge>): this {
const { fromId, toId } = extractEdgeIds(from, to)
const { type, ...rest } = edgeInfo

if(fromId === toId) {
return this
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ function processAssignmentToString<OtherInfo>(
data,
reverseOrder: !config.swapSourceAndTarget
})

return processAssignmentToSymbol<OtherInfo & ParentInformation>({
...config,
name,
Expand Down Expand Up @@ -200,7 +201,7 @@ function processAssignmentToSymbol<OtherInfo>({

// we drop the first arg which we use to pass along arguments :D
const readFromSourceWritten = sourceArg.out.slice(1)
const readTargets: readonly IdentifierReference[] = [{ nodeId: name.info.id, name: name.content, controlDependencies: data.controlDependencies }, ...sourceArg.unknownReferences, ...sourceArg.in, ...targetArg.in.filter(i => i.nodeId !== target.info.id), ...readFromSourceWritten]
const readTargets: readonly IdentifierReference[] = [{ nodeId: rootId, name: name.content, controlDependencies: data.controlDependencies }, ...sourceArg.unknownReferences, ...sourceArg.in, ...targetArg.in.filter(i => i.nodeId !== target.info.id), ...readFromSourceWritten]
const writeTargets = [...writeNodes, ...writeNodes, ...readFromSourceWritten]

information.environment = overwriteEnvironment(targetArg.environment, sourceArg.environment)
Expand All @@ -223,7 +224,7 @@ function processAssignmentToSymbol<OtherInfo>({
}
}

information.graph.addEdge(name.info.id, targetArg.entryPoint, { type: EdgeType.Returns })
information.graph.addEdge(rootId, targetArg.entryPoint, { type: EdgeType.Returns })

if(quoteSource) {
information.graph.addEdge(rootId, source.info.id, { type: EdgeType.NonStandardEvaluation })
Expand Down
6 changes: 6 additions & 0 deletions test/functionality/_helper/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ export function assertDataflow(
getId: deterministicCountingIdGenerator(startIndexForDeterministicIds)
}).allRemainingSteps()

// assign the same id map to the expected graph, so that resolves work as expected
expected.setIdMap(info.normalize.idMap)

const report: DataflowDifferenceReport = expected.equals(info.dataflow.graph, true, { left: 'expected', right: 'got' })
// with the try catch the diff graph is not calculated if everything is fine
try {
Expand All @@ -239,6 +242,9 @@ export function assertDataflow(
{ label: 'got', graph: info.dataflow.graph, mark: mapProblematicNodesToIds(report.problematic()) },
`%% ${input.replace(/\n/g, '\n%% ')}\n` + report.comments()?.map(n => `%% ${n}\n`).join('') ?? '' + '\n'
)

console.error('ast', normalizedAstToMermaidUrl(info.normalize.ast))

console.error('best-effort reconstruction:\n', printAsBuilder(info.dataflow.graph))

console.error('diff:\n', diff)
Expand Down
21 changes: 9 additions & 12 deletions test/functionality/util/quads-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,21 @@ describe('Quads', withShell(shell => {
<${idPrefix}1> <${domain}tag> "use" <${context}> .
<${idPrefix}1> <${domain}id> "1"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}1> <${domain}when> "always" <${context}> .
<${idPrefix}1> <${domain}environment> <${idPrefix}3> <${context}> .
<${idPrefix}3> <${domain}current> <${idPrefix}4> <${context}> .
<${idPrefix}3> <${domain}level> "0"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}0> <${domain}vertices> <${idPrefix}2> <${context}> .
<${idPrefix}2> <${domain}tag> "function-call" <${context}> .
<${idPrefix}2> <${domain}id> "3"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}2> <${domain}environment> <${idPrefix}5> <${context}> .
<${idPrefix}5> <${domain}current> <${idPrefix}6> <${context}> .
<${idPrefix}5> <${domain}level> "0"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}2> <${domain}environment> <${idPrefix}3> <${context}> .
<${idPrefix}3> <${domain}current> <${idPrefix}4> <${context}> .
<${idPrefix}3> <${domain}level> "0"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}2> <${domain}name> "foo" <${context}> .
<${idPrefix}2> <${domain}onlyBuiltin> "false"^^<http://www.w3.org/2001/XMLSchema#boolean> <${context}> .
<${idPrefix}2> <${domain}args> <${idPrefix}7> <${context}> .
<${idPrefix}7> <${domain}nodeId> "1"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}2> <${domain}args> <${idPrefix}5> <${context}> .
<${idPrefix}5> <${domain}nodeId> "1"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}2> <${domain}when> "always" <${context}> .
<${idPrefix}0> <${domain}edges> <${idPrefix}8> <${context}> .
<${idPrefix}8> <${domain}from> "3"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}8> <${domain}to> "1"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}8> <${domain}type> "argument" <${context}> .
<${idPrefix}0> <${domain}edges> <${idPrefix}6> <${context}> .
<${idPrefix}6> <${domain}from> "3"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}6> <${domain}to> "1"^^<http://www.w3.org/2001/XMLSchema#integer> <${context}> .
<${idPrefix}6> <${domain}type> "argument" <${context}> .
`)
})
}))
Loading