From c5d80fae71507a9bd1bf3d676b0e018fba7a7c42 Mon Sep 17 00:00:00 2001 From: Alexandre Hajjar Date: Tue, 4 May 2021 19:40:10 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20Publicodes=20static=20cycles=20c?= =?UTF-8?q?heck=20using=20AST=20iterator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * AST API: add AST visitor & iterator * cycles detection: use AST iterator * remove dagrejs/graphlib * cycle extraction: smallest cycle & print in Graphviz dot format WARNING: a cycle still exists around `entreprise . chiffre d'affaires` see issue #1524 for a definitive fix. --- mon-entreprise/test/cycles.test.js | 39 ++- publicodes/core/package.json | 3 +- publicodes/core/source/AST/findCycles.ts | 330 ++++++++++++++++++ publicodes/core/source/AST/graph.ts | 193 +++++++--- publicodes/core/source/AST/index.ts | 98 ++++-- publicodes/core/source/AST/types.ts | 7 +- publicodes/core/source/index.ts | 2 +- publicodes/core/source/parsePublicodes.ts | 4 +- publicodes/core/source/replacement.tsx | 30 +- publicodes/core/source/rule.ts | 4 +- publicodes/core/source/types/dagres.d.ts | 8 - publicodes/core/test/cycles.test.js | 20 +- publicodes/core/test/mécanismes/remplace.yaml | 15 +- yarn.lock | 9 +- 14 files changed, 604 insertions(+), 158 deletions(-) create mode 100644 publicodes/core/source/AST/findCycles.ts delete mode 100644 publicodes/core/source/types/dagres.d.ts diff --git a/mon-entreprise/test/cycles.test.js b/mon-entreprise/test/cycles.test.js index 83ba3d309..58d56781f 100644 --- a/mon-entreprise/test/cycles.test.js +++ b/mon-entreprise/test/cycles.test.js @@ -4,31 +4,34 @@ import { cyclicDependencies } from '../../publicodes/core/source/AST/graph' describe('DottedNames graph', () => { it("shouldn't have cycles", () => { - let cyclesDependencies = cyclicDependencies(rules) + const [cyclesDependencies, dotGraphs] = cyclicDependencies(rules) + + const dotGraphsToLog = dotGraphs + .map( + (dotGraph) => + `🌀🌀🌀🌀🌀🌀🌀🌀🌀🌀🌀\n A cycle graph to stare at with Graphviz:\n${dotGraph}\n\n` + ) + .join('\n\n') expect( cyclesDependencies, - `\nThe cycles have been found in the rules dependencies graph.\nSee below for a representation of each cycle.\n⬇️ is a node of the cycle.\n\t- ${cyclesDependencies + `${dotGraphsToLog}\nAT LEAST the following cycles have been found in the rules dependencies graph.\nSee below for a representation of each cycle.\n⬇️ is a node of the cycle.\n\t- ${cyclesDependencies .map( (cycleDependencies, idx) => - '#' + - idx + - ':\n\t\t⬇️ ' + - cycleDependencies - // .map( - // ([ruleName, dependencies]) => - // ruleName + '\n\t\t\t↘️ ' + dependencies.join('\n\t\t\t↘️ ') - // ) - .join('\n\t\t⬇️ ') + '#' + idx + ':\n\t\t⬇️ ' + cycleDependencies.join('\n\t\t⬇️ ') ) .join('\n\t- ')}\n\n` + ).to.deep.equal([ + [ + "entreprise . chiffre d'affaires", + 'dirigeant . rémunération . impôt', + 'dirigeant . rémunération . imposable', + 'dirigeant . auto-entrepreneur . impôt . revenu imposable', + "entreprise . chiffre d'affaires . vente restauration hébergement", + ], + ]) + console.warn( + "[ WARNING ] A cycle still exists around `entreprise . chiffre d'affaires` see issue #1524 for a definitive fix." ) - .to.be.an('array') - .of.length(1) - - // Cycle doesn't occur in real life. - // ⬇️ entreprise . chiffre d'affaires - // ⬇️ dirigeant . rémunération totale - // ⬇️ entreprise . chiffre d'affaires }) }) diff --git a/publicodes/core/package.json b/publicodes/core/package.json index c1408a8db..646ca39b9 100644 --- a/publicodes/core/package.json +++ b/publicodes/core/package.json @@ -28,8 +28,7 @@ "chai": "^4.2.0", "intl": "^1.2.5", "typescript": "^4.2.4", - "dedent-js": "1.0.1", - "@dagrejs/graphlib": "^2.1.4" + "dedent-js": "1.0.1" }, "dependencies": { "moo": "^0.5.1", diff --git a/publicodes/core/source/AST/findCycles.ts b/publicodes/core/source/AST/findCycles.ts new file mode 100644 index 000000000..009f05bbd --- /dev/null +++ b/publicodes/core/source/AST/findCycles.ts @@ -0,0 +1,330 @@ +/* eslint-disable prefer-rest-params */ +/* eslint-disable @typescript-eslint/no-this-alias */ +// Adapted from https://github.com/dagrejs/graphlib (MIT license) +// and https://github.com/lodash/lodash (MIT license) + +// TODO: type this + +function has(obj, key) { + return obj != null && Object.prototype.hasOwnProperty.call(obj, key) +} +function constant(value) { + return function (...args) { + return value + } +} + +const DEFAULT_EDGE_NAME = '\x00' +const EDGE_KEY_DELIM = '\x01' + +const incrementOrInitEntry = (map, k) => { + if (map[k]) { + map[k]++ + } else { + map[k] = 1 + } +} + +const decrementOrRemoveEntry = (map, k) => { + if (!--map[k]) { + delete map[k] + } +} + +const edgeArgsToId = (isDirected, v_, w_, name) => { + let v = '' + v_ + let w = '' + w_ + if (!isDirected && v > w) { + const tmp = v + v = w + w = tmp + } + return ( + v + + EDGE_KEY_DELIM + + w + + EDGE_KEY_DELIM + + (name === undefined ? DEFAULT_EDGE_NAME : name) + ) +} + +const edgeArgsToObj = (isDirected, v_, w_, name) => { + let v = '' + v_ + let w = '' + w_ + if (!isDirected && v > w) { + const tmp = v + v = w + w = tmp + } + const edgeObj: any = { v: v, w: w } + if (name) { + edgeObj.name = name + } + return edgeObj +} + +const edgeObjToId = (isDirected, edgeObj) => { + return edgeArgsToId(isDirected, edgeObj.v, edgeObj.w, edgeObj.name) +} +export class Graph { + private _nodeCount = 0 + private _edgeCount = 0 + + private _isDirected: any + + private _label: undefined + private _defaultNodeLabelFn: (...args: any[]) => any + private _defaultEdgeLabelFn: (...args: any[]) => any + private _nodes: Record + private _in: Record + private _preds: Record> + private _out: Record> + private _sucs: Record> + private _edgeObjs: Record + private _edgeLabels: Record + + constructor(opts: Record = {}) { + this._isDirected = has(opts, 'directed') ? opts.directed : true + + // Label for the graph itself + this._label = undefined + + // Defaults to be set when creating a new node + this._defaultNodeLabelFn = constant(undefined) + + // Defaults to be set when creating a new edge + this._defaultEdgeLabelFn = constant(undefined) + + // v -> label + this._nodes = {} + + // v -> edgeObj + this._in = {} + + // u -> v -> Number + this._preds = {} + + // v -> edgeObj + this._out = {} as Record> + + // v -> w -> Number + this._sucs = {} + + // e -> edgeObj + this._edgeObjs = {} + + // e -> label + this._edgeLabels = {} + } + + /* === Graph functions ========= */ + + isDirected() { + return this._isDirected + } + setGraph(label) { + this._label = label + return this + } + graph() { + return this._label + } + + /* === Node functions ========== */ + + nodeCount() { + return this._nodeCount + } + nodes() { + return Object.keys(this._nodes) + } + setNode(v, value: any = undefined) { + if (has(this._nodes, v)) { + if (arguments.length > 1) { + this._nodes[v] = value + } + return this + } + + this._nodes[v] = arguments.length > 1 ? value : this._defaultNodeLabelFn(v) + this._in[v] = {} + this._preds[v] = {} + this._out[v] = {} + this._sucs[v] = {} + ++this._nodeCount + return this + } + setNodes(vs, value) { + vs.forEach((v) => { + if (value !== undefined) { + this.setNode(v, value) + } else { + this.setNode(v) + } + }) + return this + } + node(v) { + return this._nodes[v] + } + hasNode(v) { + return has(this._nodes, v) + } + successors(v) { + const sucsV = this._sucs[v] + if (sucsV) { + return Object.keys(sucsV) + } + } + + /* === Edge functions ========== */ + + edgeCount() { + return this._edgeCount + } + edges() { + return Object.values(this._edgeObjs) + } + setEdge( + v: string, + w: string, + value: any = undefined, + name: string | undefined = undefined + ) { + v = '' + v + w = '' + w + + const e = edgeArgsToId(this._isDirected, v, w, name) + if (has(this._edgeLabels, e)) { + if (value !== undefined) { + this._edgeLabels[e] = value + } + return this + } + + // It didn't exist, so we need to create it. + // First ensure the nodes exist. + this.setNode(v) + this.setNode(w) + + this._edgeLabels[e] = + value !== undefined ? value : this._defaultEdgeLabelFn(v, w, name) + + const edgeObj = edgeArgsToObj(this._isDirected, v, w, name) + // Ensure we add undirected edges in a consistent way. + v = edgeObj.v + w = edgeObj.w + + Object.freeze(edgeObj) + this._edgeObjs[e] = edgeObj + incrementOrInitEntry(this._preds[w], v) + incrementOrInitEntry(this._sucs[v], w) + this._in[w][e] = edgeObj + this._out[v][e] = edgeObj + this._edgeCount++ + return this + } + + edge(v, w, name) { + const e = + arguments.length === 1 + ? edgeObjToId(this._isDirected, arguments[0]) + : edgeArgsToId(this._isDirected, v, w, name) + return this._edgeLabels[e] + } + + hasEdge(v, w, name) { + const e = + arguments.length === 1 + ? edgeObjToId(this._isDirected, arguments[0]) + : edgeArgsToId(this._isDirected, v, w, name) + return has(this._edgeLabels, e) + } + + removeEdge(v, w, name) { + const e = + arguments.length === 1 + ? edgeObjToId(this._isDirected, arguments[0]) + : edgeArgsToId(this._isDirected, v, w, name) + const edge = this._edgeObjs[e] + if (edge) { + v = edge.v + w = edge.w + delete this._edgeLabels[e] + delete this._edgeObjs[e] + decrementOrRemoveEntry(this._preds[w], v) + decrementOrRemoveEntry(this._sucs[v], w) + delete this._in[w][e] + delete this._out[v][e] + this._edgeCount-- + } + return this + } + + outEdges(v: string, w: string | undefined = undefined) { + const outV = this._out[v] + if (outV) { + const edges: any = Object.values(outV) + if (w === undefined) { + return edges + } + return edges.filter(function (edge) { + return edge.w === w + }) + } + } +} + +/** Cycles stuff **/ + +function tarjan(graph) { + let index = 0 + const stack: any[] = [] + const visited = {} // node id -> { onStack, lowlink, index } + const results: any[] = [] + + function dfs(v) { + const entry = (visited[v] = { + onStack: true, + lowlink: index, + index: index++, + }) + stack.push(v) + + graph.successors(v).forEach(function (w) { + if (!Object.prototype.hasOwnProperty.call(visited, w)) { + dfs(w) + entry.lowlink = Math.min(entry.lowlink, visited[w].lowlink) + } else if (visited[w].onStack) { + entry.lowlink = Math.min(entry.lowlink, visited[w].index) + } + }) + + if (entry.lowlink === entry.index) { + const cmpt: any[] = [] + let w + do { + w = stack.pop() + visited[w].onStack = false + cmpt.push(w) + } while (v !== w) + results.push(cmpt) + } + } + + graph.nodes().forEach(function (v) { + if (!Object.prototype.hasOwnProperty.call(visited, v)) { + dfs(v) + } + }) + + return results +} + +export function findCycles(graph): string[][] { + return tarjan(graph).filter(function (cmpt) { + return ( + cmpt.length > 1 || (cmpt.length === 1 && graph.hasEdge(cmpt[0], cmpt[0])) + ) + }) +} diff --git a/publicodes/core/source/AST/graph.ts b/publicodes/core/source/AST/graph.ts index 5429346ae..97a078afc 100644 --- a/publicodes/core/source/AST/graph.ts +++ b/publicodes/core/source/AST/graph.ts @@ -1,10 +1,11 @@ -import graphlib from '@dagrejs/graphlib' +import { ASTNode } from './types' import parsePublicodes from '../parsePublicodes' import { RuleNode } from '../rule' -import { reduceAST } from './index' -type RulesDependencies = Array<[string, Array]> -type GraphCycles = Array> -type GraphCyclesWithDependencies = Array +import { getChildrenNodes, iterAST } from './index' +import { findCycles, Graph } from './findCycles' + +type RulesDependencies = [string, string[]][] +type GraphCycles = string[][] function buildRulesDependencies( parsedRules: Record @@ -12,42 +13,56 @@ function buildRulesDependencies( const uniq = (arr: Array): Array => [...new Set(arr)] return Object.entries(parsedRules).map(([name, node]) => [ name, - uniq(buildRuleDependancies(node)), + uniq(getDependencies(node)), ]) } -function buildRuleDependancies(rule: RuleNode): Array { - return reduceAST( - (acc, node, fn) => { - switch (node.nodeKind) { - case 'replacementRule': - case 'inversion': - case 'une possibilité': - return acc - case 'recalcul': - return node.explanation.amendedSituation.flatMap((s) => fn(s[1])) - case 'reference': - return [...acc, node.dottedName as string] - case 'résoudre référence circulaire': - return [] - case 'rule': - // Cycle from parent dependancies are ignored at runtime, - // so we don' detect them statically - return fn(rule.explanation.valeur) - case 'variations': - // a LOT of cycles with replacements... we disactivate them until we see clearer, - if (node.rawNode && typeof node.rawNode === 'string') { - return [...acc, node.rawNode] - } - } - }, - [], - rule - ) +function getReferenceName(node: ASTNode): string | undefined { + switch (node.nodeKind) { + case 'reference': + return node.dottedName as string + } +} +/** + * Recursively selects the children nodes that have the ability to include a reference + * to a rule. + */ +function getReferencingDescendants(node: ASTNode): ASTNode[] { + return iterAST((node) => { + switch (node.nodeKind) { + case 'replacementRule': + case 'inversion': + case 'une possibilité': + case 'reference': + case 'résoudre référence circulaire': + // "résoudre référence circulaire" is a chained mechanism. When returning `[]` we prevent + // iteration inside of the rule's `valeur`, meaning the rule returns no descendants at all. + return [] + case 'recalcul': + return node.explanation.amendedSituation.map(([, astNode]) => astNode) + case 'rule': + return [node.explanation.valeur] + case 'variations': + if (node.visualisationKind === 'replacement') { + return node.explanation + .filter(({ condition }) => condition.isDefault) + .map(({ consequence }) => consequence) + .filter((consequence) => consequence.nodeKind === 'reference') + } + } + return getChildrenNodes(node) + }, node) +} +function getDependencies(node: ASTNode): string[] { + const descendantNodes = Array.from(getReferencingDescendants(node)) + const descendantsReferences = descendantNodes + .map(getReferenceName) + .filter((refName): refName is string => refName !== undefined) + return descendantsReferences } -function buildDependenciesGraph(rulesDeps: RulesDependencies): graphlib.Graph { - const g = new (graphlib as any).Graph() +function buildDependenciesGraph(rulesDeps: RulesDependencies) { + const g = new Graph() rulesDeps.forEach(([ruleDottedName, dependencies]) => { dependencies.forEach((depDottedName) => { g.setEdge(ruleDottedName, depDottedName) @@ -62,40 +77,106 @@ export function cyclesInDependenciesGraph(rawRules: RawRules): GraphCycles { const parsedRules = parsePublicodes(rawRules) const rulesDependencies = buildRulesDependencies(parsedRules) const dependenciesGraph = buildDependenciesGraph(rulesDependencies) - const cycles = (graphlib as any).alg.findCycles(dependenciesGraph) + const cycles = findCycles(dependenciesGraph) return cycles.map((c) => c.reverse()) } +/** + * Make the cycle as small as possible. + */ +export function squashCycle( + rulesDependenciesObject: Record, + cycle: string[] +): string[] { + function* loopFrom(i: number) { + let j = i + while (true) { + yield cycle[j++ % cycle.length] + } + } + const smallCycleStartingAt: string[][] = [] + for (let i = 0; i < cycle.length; i++) { + const smallCycle: string[] = [] + let previousVertex: string | undefined = undefined + for (const vertex of loopFrom(i)) { + if (previousVertex === undefined) { + smallCycle.push(vertex) + previousVertex = vertex + } else if (rulesDependenciesObject[previousVertex].includes(vertex)) { + if (smallCycle.includes(vertex)) { + smallCycle.splice(0, smallCycle.lastIndexOf(vertex)) + break + } + smallCycle.push(vertex) + previousVertex = vertex + } + } + smallCycleStartingAt.push(smallCycle) + } + + const smallest = smallCycleStartingAt.reduce((minCycle, someCycle) => + someCycle.length > minCycle.length ? minCycle : someCycle + ) + return smallest +} + /** * This function is useful so as to print the dependencies at each node of the * cycle. - * ⚠️ Indeed, the graphlib.findCycles function returns the cycle found using the + * ⚠️ Indeed, the findCycles function returns the cycle found using the * Tarjan method, which is **not necessarily the smallest cycle**. However, the - * smallest cycle would be the most legibe one… + * smallest cycle is more readable. */ export function cyclicDependencies( rawRules: RawRules -): GraphCyclesWithDependencies { +): [GraphCycles, string[]] { const parsedRules = parsePublicodes(rawRules) const rulesDependencies = buildRulesDependencies(parsedRules) const dependenciesGraph = buildDependenciesGraph(rulesDependencies) - const cycles = (graphlib as any).alg.findCycles(dependenciesGraph) - const rulesDependenciesObject = Object.fromEntries(rulesDependencies) + const cycles = findCycles(dependenciesGraph) - return cycles.map((cycle) => { - const c = cycle.reverse() + const reversedCycles = cycles.map((c) => c.reverse()) + const rulesDependenciesObject = Object.fromEntries( + rulesDependencies + ) as Record + const smallCycles = reversedCycles.map((cycle) => + squashCycle(rulesDependenciesObject, cycle) + ) - return [...c, c[0]].reduce((acc, current) => { - const previous = acc.slice(-1)[0] - if (previous && !rulesDependenciesObject[previous].includes(current)) { - return acc - } - return [...acc, current] - }, []) - // .map(name => [ - // name, - // rulesDependenciesObject[name].filter(name => c.includes(name)) - // ]) - }) + const printableStronglyConnectedComponents = reversedCycles.map((c, i) => + printInDotFormat(dependenciesGraph, c, smallCycles[i]) + ) + + return [smallCycles, printableStronglyConnectedComponents] +} + +/** + * Is edge in the cycle, in the same order? + */ +const edgeIsInCycle = (cycle: string[], v: string, w: string): boolean => { + for (let i = 0; i < cycle.length + 1; i++) { + if (v === cycle[i] && w === cycle[(i + 1) % cycle.length]) return true + } + return false +} + +export function printInDotFormat( + dependenciesGraph: Graph, + cycle: string[], + subCycleToHighlight: string[] +) { + const edgesSet = new Set() + cycle.forEach((vertex) => { + dependenciesGraph + .outEdges(vertex) + .filter(({ w }) => cycle.includes(w)) + .forEach(({ v, w }) => { + edgesSet.add( + `"${v}" -> "${w}"` + + (edgeIsInCycle(subCycleToHighlight, v, w) ? ' [color=red]' : '') + ) + }) + }) + return `digraph Cycle {\n\t${[...edgesSet].join(';\n\t')};\n}` } diff --git a/publicodes/core/source/AST/index.ts b/publicodes/core/source/AST/index.ts index f9f42880a..bbbdcf15f 100644 --- a/publicodes/core/source/AST/index.ts +++ b/publicodes/core/source/AST/index.ts @@ -2,9 +2,14 @@ import { InternalError } from '../error' import { TrancheNodes } from '../mecanisms/trancheUtils' import { ReplacementRule } from '../replacement' import { RuleNode } from '../rule' -import { ASTNode, NodeKind, TraverseFunction } from './types' +import { + ASTNode, + ASTVisitor, + ASTTransformer, + NodeKind, + TraverseFunction, +} from './types' -type TransformASTFunction = (n: ASTNode) => ASTNode /** This function creates a transormation of the AST from on a simpler callback function `fn` @@ -20,42 +25,68 @@ type TransformASTFunction = (n: ASTNode) => ASTNode by using the function passed as second argument. The returned value will be the transformed version of the node. */ -export function transformAST( - fn: ( - node: ASTNode, - updateFn: TransformASTFunction - ) => ASTNode | undefined | false -): TransformASTFunction { - function traverseFn(node: ASTNode) { - const updatedNode = fn(node, traverseFn) +export function makeASTTransformer( + fn: (node: ASTNode, transform: ASTTransformer) => ASTNode | undefined | false +): ASTTransformer { + function transform(node: ASTNode): ASTNode { + const updatedNode = fn(node, transform) if (updatedNode === false) { return node } if (updatedNode === undefined) { - return traverseASTNode(traverseFn, node) + return traverseASTNode(transform, node) } return updatedNode } - return traverseFn + return transform +} +export function makeASTVisitor( + fn: (node: ASTNode, visit: ASTVisitor) => 'continue' | 'stop' +): ASTVisitor { + function visit(node: ASTNode) { + switch (fn(node, visit)) { + case 'continue': + traverseASTNode(transformizedVisit, node) + return + case 'stop': + return + } + } + const transformizedVisit: ASTTransformer = (node) => { + visit(node) + return node + } + return visit +} + +// Can be made more flexible with other args like a filter function (ASTNode -> Bool). +export function iterAST( + childrenSelector: (node: ASTNode) => Iterable, + node: ASTNode +): ASTNode[] { + function* iterate(node: ASTNode): IterableIterator { + yield node + const selectedSubNodes = childrenSelector(node) + for (const subNode of selectedSubNodes) yield* iterate(subNode) + } + return [...iterate(node)] } /** - This function allows to construct a specific value while exploring the AST with - a simple reducing function as argument. - - `fn` will be called with the currently reduced value `acc` and the current node of the AST - - - The outcome of the callback function has an influence on the exploration of the AST : - - `undefined`, the exploration continues further down and all the child are reduced - successively to a single value - - `T`, the reduced value - - `reduceFn` : It is possible to specifically use the reduced value of a child - by using the function passed as second argument. The returned value will be the reduced version - of the node - */ - + * This function allows to construct a specific value while exploring the AST with + * a simple reducing function as argument. + * + * `fn` will be called with the currently reduced value `acc` and the current node of the AST + * + * If the callback function returns: + * - `undefined`, the exploration continues further down and all the children are reduced + * successively to a single value + * - `T`, the reduced value is returned + * + * `reduceFn` : It is possible to specifically use the reduced value of a child + * by using the function passed as second argument. The returned value will be the reduced version + * of the node + */ export function reduceAST( fn: (acc: T, n: ASTNode, reduceFn: (n: ASTNode) => T) => T | undefined, start: T, @@ -64,14 +95,14 @@ export function reduceAST( function traverseFn(acc: T, node: ASTNode): T { const result = fn(acc, node, traverseFn.bind(null, start)) if (result === undefined) { - return gatherNodes(node).reduce(traverseFn, acc) + return getChildrenNodes(node).reduce(traverseFn, acc) } return result } return traverseFn(start, node) } -function gatherNodes(node: ASTNode): ASTNode[] { +export function getChildrenNodes(node: ASTNode): ASTNode[] { const nodes: ASTNode[] = [] traverseASTNode((node) => { nodes.push(node) @@ -81,7 +112,7 @@ function gatherNodes(node: ASTNode): ASTNode[] { } export function traverseParsedRules( - fn: (n: ASTNode) => ASTNode, + fn: ASTTransformer, parsedRules: Record ): Record { return Object.fromEntries( @@ -89,7 +120,10 @@ export function traverseParsedRules( ) as Record } -const traverseASTNode: TraverseFunction = (fn, node) => { +/** + * Apply a transform function on children. Not recursive. + */ +export const traverseASTNode: TraverseFunction = (fn, node) => { switch (node.nodeKind) { case 'rule': return traverseRuleNode(fn, node) diff --git a/publicodes/core/source/AST/types.ts b/publicodes/core/source/AST/types.ts index e732c828a..66886e52a 100644 --- a/publicodes/core/source/AST/types.ts +++ b/publicodes/core/source/AST/types.ts @@ -85,10 +85,13 @@ export type MecanismNode = Exclude< ASTNode, RuleNode | ConstantNode | ReferenceNode > -export type NodeKind = ASTNode['nodeKind'] +export type ASTTransformer = (n: ASTNode) => ASTNode +export type ASTVisitor = (n: ASTNode) => void + +export type NodeKind = ASTNode['nodeKind'] export type TraverseFunction = ( - fn: (n: ASTNode) => ASTNode, + fn: ASTTransformer, node: ASTNode & { nodeKind: Kind } ) => ASTNode & { nodeKind: Kind } diff --git a/publicodes/core/source/index.ts b/publicodes/core/source/index.ts index 7098a5cc2..807f83643 100644 --- a/publicodes/core/source/index.ts +++ b/publicodes/core/source/index.ts @@ -40,7 +40,7 @@ export type EvaluationOptions = Partial<{ unit: string }> -export { reduceAST, transformAST } from './AST/index' +export { reduceAST, makeASTTransformer as transformAST } from './AST/index' export { Evaluation, Unit } from './AST/types' export { capitalise0, formatValue } from './format' export { simplifyNodeUnit } from './nodeUnits' diff --git a/publicodes/core/source/parsePublicodes.ts b/publicodes/core/source/parsePublicodes.ts index 08d7e0a61..f90f80b6d 100644 --- a/publicodes/core/source/parsePublicodes.ts +++ b/publicodes/core/source/parsePublicodes.ts @@ -1,6 +1,6 @@ import yaml from 'yaml' import { ParsedRules, Logger } from '.' -import { transformAST, traverseParsedRules } from './AST' +import { makeASTTransformer, traverseParsedRules } from './AST' import parse from './parse' import { getReplacements, inlineReplacements } from './replacement' import { Rule, RuleNode } from './rule' @@ -105,7 +105,7 @@ function transpileRef(object: Record | string | Array) { } export const disambiguateReference = (parsedRules: Record) => - transformAST((node) => { + makeASTTransformer((node) => { if (node.nodeKind === 'reference') { const dottedName = disambiguateRuleReference( parsedRules, diff --git a/publicodes/core/source/replacement.tsx b/publicodes/core/source/replacement.tsx index 04dcac90c..e781a0eaf 100644 --- a/publicodes/core/source/replacement.tsx +++ b/publicodes/core/source/replacement.tsx @@ -1,5 +1,5 @@ import { Logger } from '.' -import { transformAST } from './AST' +import { makeASTTransformer } from './AST' import { ASTNode } from './AST/types' import { InternalError, warning } from './error' import { defaultNode } from './evaluation' @@ -25,7 +25,7 @@ export type ReplacementRule = { // // The implementation works by first attributing an identifier for each // replacementRule. We then use this identifier to create a cache key that -// represent the combinaison of applicables replacements for a given reference. +// represents the combinaison of applicables replacements for a given reference. // For example if replacements 12, 13 et 643 are applicable we use the key // `12-13-643` as the cache identifier in the `inlineReplacements` function. let remplacementRuleId = 0 @@ -104,31 +104,31 @@ export function inlineReplacements( replacements: Record>, logger: Logger ): (n: ASTNode) => ASTNode { - return transformAST((n, fn) => { + return makeASTTransformer((node, transform) => { if ( - n.nodeKind === 'replacementRule' || - n.nodeKind === 'inversion' || - n.nodeKind === 'une possibilité' + node.nodeKind === 'replacementRule' || + node.nodeKind === 'inversion' || + node.nodeKind === 'une possibilité' ) { return false } - if (n.nodeKind === 'recalcul') { + if (node.nodeKind === 'recalcul') { // We don't replace references in recalcul keys return { - ...n, + ...node, explanation: { - recalcul: fn(n.explanation.recalcul), - amendedSituation: n.explanation.amendedSituation.map( - ([name, value]) => [name, fn(value)] + recalcul: transform(node.explanation.recalcul), + amendedSituation: node.explanation.amendedSituation.map( + ([name, value]) => [name, transform(value)] ), }, } } - if (n.nodeKind === 'reference') { - if (!n.dottedName) { - throw new InternalError(n) + if (node.nodeKind === 'reference') { + if (!node.dottedName) { + throw new InternalError(node) } - return replace(n, replacements[n.dottedName] ?? [], logger) + return replace(node, replacements[node.dottedName] ?? [], logger) } }) } diff --git a/publicodes/core/source/rule.ts b/publicodes/core/source/rule.ts index 134ff24b3..cf16a7425 100644 --- a/publicodes/core/source/rule.ts +++ b/publicodes/core/source/rule.ts @@ -137,7 +137,9 @@ registerEvaluationFunction('rule', function evaluate(node) { if ( this.cache._meta.evaluationRuleStack.filter( (dottedName) => dottedName === node.dottedName - ).length > 15 // I don't know why this magic number, but below, cycle are detected "too early", which leads to blank value in brut-net simulator + ).length > 15 + // I don't know why this magic number, but below, cycle are + // detected "too early", which leads to blank value in brut-net simulator ) { warning( this.options.logger, diff --git a/publicodes/core/source/types/dagres.d.ts b/publicodes/core/source/types/dagres.d.ts deleted file mode 100644 index 31c1d22e4..000000000 --- a/publicodes/core/source/types/dagres.d.ts +++ /dev/null @@ -1,8 +0,0 @@ -declare module '@dagrejs/graphlib' { - export interface Graph { - setEdge(n1: string, n2: string): void - } - export type alg = { - findCycles: (g: Graph) => Array> - } -} diff --git a/publicodes/core/test/cycles.test.js b/publicodes/core/test/cycles.test.js index 98e09c009..f7e475c33 100644 --- a/publicodes/core/test/cycles.test.js +++ b/publicodes/core/test/cycles.test.js @@ -27,7 +27,7 @@ describe('Cyclic dependencies detectron 3000 ™', () => { expect(cycles).to.deep.equal([['a', 'b', 'c', 'd']]) }) - it('should not detect formule cycles due to parent dependancy', () => { + it('should not detect formule cycles due to parent dependency', () => { const rules = dedent` a: formule: b + 1 @@ -38,17 +38,29 @@ describe('Cyclic dependencies detectron 3000 ™', () => { expect(cycles).to.deep.equal([]) }) - it('should not detect cycles due to replacement', () => { + it('should not detect cycles due to replacements', () => { const rules = dedent` a: formule: b + 1 a . b: formule: 3 - a . c: + a . c: remplace: b formule: a ` const cycles = cyclesInDependenciesGraph(rules) - expect(cycles).to.deep.equal([['a', 'a . c']]) + expect(cycles).to.deep.equal([]) + }) + + it('should not detect cycles when résoudre référence circulaire', () => { + const rules = dedent` + fx: + 200 - x + x: + résoudre la référence circulaire: oui + valeur: fx + ` + const cycles = cyclesInDependenciesGraph(rules) + expect(cycles).to.deep.equal([]) }) }) diff --git a/publicodes/core/test/mécanismes/remplace.yaml b/publicodes/core/test/mécanismes/remplace.yaml index 7b2b8f000..4c43ea2da 100644 --- a/publicodes/core/test/mécanismes/remplace.yaml +++ b/publicodes/core/test/mécanismes/remplace.yaml @@ -113,8 +113,8 @@ exemple5: par: 10€ - règle: cotisations . maladie par: 0 - -remplacements : + +remplacements: formule: cotisations exemples: - nom: sans boucle infinie si il n'y a pas de dépendances cycliques @@ -125,8 +125,8 @@ remplacements : situation: exemple2: oui valeur attendue: 250 - - nom: avec plusieurs remplacements existant pour une même variables - # ici, le remplacement de l'exemple 2 doit être effectué car plus précis que celui de l'exemple 1 + - nom: avec plusieurs remplacements existant pour une même variables + # ici, le remplacement de l'exemple 2 doit être effectué car plus précis que celui de l'exemple 1 situation: exemple1: oui exemple2: oui @@ -149,8 +149,6 @@ remplacements : exemple5: oui valeur attendue: 110 - - A: formule: 1 B: @@ -181,19 +179,18 @@ remplacement non applicable car branche desactivée: exemples: - valeur attendue: 1 -# Remplacement non effectué dans la formule du remplacement +# Remplacement effectué dans la bonne variable espace: oui espace . valeur: formule: 20 espace . remplacement: remplace: valeur formule: valeur + 10 -test remplacement non effectué dans la formule du remplacement: +test remplacement effectué dans la variable à remplacer: formule: espace . valeur exemples: - valeur attendue: 30 - frais de repas: formule: 5 €/repas diff --git a/yarn.lock b/yarn.lock index 242a90581..dac84ed1c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1666,13 +1666,6 @@ debug "^3.1.0" lodash.once "^4.1.1" -"@dagrejs/graphlib@^2.1.4": - version "2.1.4" - resolved "https://registry.yarnpkg.com/@dagrejs/graphlib/-/graphlib-2.1.4.tgz#86c70e4f073844a2f4ada254c8c7b88a6bdacdb1" - integrity sha512-QCg9sL4uhjn468FDEsb/S9hS2xUZSrv/+dApb1Ze5VKO96pTXKNJZ6MGhIpgWkc1TVhbVGH9/7rq/Mf8/jWicw== - dependencies: - lodash "^4.11.1" - "@emotion/is-prop-valid@^0.8.8": version "0.8.8" resolved "https://registry.yarnpkg.com/@emotion/is-prop-valid/-/is-prop-valid-0.8.8.tgz#db28b1c4368a259b60a97311d6a952d4fd01ac1a" @@ -9073,7 +9066,7 @@ lodash.uniq@^4.5.0: resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773" integrity sha1-0CJTc662Uq3BvILklFM5qEJ1R3M= -lodash@^4.0.1, lodash@^4.11.1, lodash@^4.15.0, lodash@^4.17.11, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.3, lodash@^4.17.4, lodash@^4.17.5, lodash@^4.3.0, lodash@~4.17.4: +lodash@^4.0.1, lodash@^4.15.0, lodash@^4.17.11, lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.3, lodash@^4.17.4, lodash@^4.17.5, lodash@^4.3.0, lodash@~4.17.4: version "4.17.20" resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52" integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==