Merge pull request #46 from publicodes/fix-import

Fix(compilation): don't import two times rules defined in 'avec'

package.json

@@ -60,7 +60,7 @@
   "dependencies": {
     "@types/node": "^18.11.18",
     "glob": "^10.4.1",
-    "publicodes": "^1.3.0"
+    "publicodes": "^1.3.3"
   },
   "devDependencies": {
     "@types/jest": "^29.2.5",

src/compilation/resolveImports.ts

@@ -253,9 +253,47 @@ Supprimez une des deux définitions de la règle '${ruleName}' dans la macro 'im
           return acc
         }
 
-        let rule
         try {
-          rule = engine.getRule(ruleName)
+          const rule = engine.getRule(ruleName)
+          const getUpdatedRule = (ruleName: RuleName, rule: Rule) => {
+            const ruleWithUpdatedDescription = addSourceModelInfomation(
+              importMacro,
+              rule,
+            )
+            // Rules defined in a [avec] mechanism are already resolved in the
+            // engine (as we use [getRule]) so we don't want to duplicate them.
+            if (ruleWithUpdatedDescription['avec'] !== undefined) {
+              delete ruleWithUpdatedDescription['avec']
+            }
+
+            utils
+              .ruleParents(ruleName)
+              .forEach((rule) => neededNamespaces.add(`${namespace} . ${rule}`))
+            return [`${namespace} . ${ruleName}`, ruleWithUpdatedDescription]
+          }
+
+          const ruleWithOverridenAttributes = { ...rule.rawNode, ...attrs }
+
+          acc.push(getUpdatedRule(ruleName, ruleWithOverridenAttributes))
+          const ruleDeps = getDependencies(engine, rule)
+            .filter(([ruleDepName, _]) => {
+              // Avoid to overwrite the updatedRawNode
+              return (
+                !accFind(acc, ruleDepName) &&
+                // The dependency is part of the rule to import so we don't want
+                // to handle it now
+                !rulesToImport.find(({ ruleName: ruleToImportName }) => {
+                  const theDepIsARuleToImport =
+                    ruleName !== ruleToImportName &&
+                    ruleToImportName === ruleDepName
+                  return theDepIsARuleToImport
+                })
+              )
+            })
+            .map(([ruleName, ruleNode]) => {
+              return getUpdatedRule(ruleName, ruleNode)
+            })
+          acc.push(...ruleDeps)
         } catch (e) {
           throw new Error(`[ Erreur dans la macro 'importer!' ]
 La règle '${ruleName}' n'existe pas dans '${importMacro.depuis.nom}'.
@@ -264,39 +302,6 @@ La règle '${ruleName}' n'existe pas dans '${importMacro.depuis.nom}'.
 - Vérifiez que le nom de la règle est correct.
 - Assurez-vous que la règle '${ruleName}' existe dans '${importMacro.depuis.nom}'.`)
         }
-
-        const getUpdatedRule = (ruleName: RuleName, rule: Rule) => {
-          const ruleWithUpdatedDescription = addSourceModelInfomation(
-            importMacro,
-            rule,
-          )
-          utils
-            .ruleParents(ruleName)
-            .forEach((rule) => neededNamespaces.add(`${namespace} . ${rule}`))
-          return [`${namespace} . ${ruleName}`, ruleWithUpdatedDescription]
-        }
-
-        const ruleWithOverridenAttributes = { ...rule.rawNode, ...attrs }
-
-        acc.push(getUpdatedRule(ruleName, ruleWithOverridenAttributes))
-        const ruleDeps = getDependencies(engine, rule)
-          .filter(([ruleDepName, _]) => {
-            // Avoid to overwrite the updatedRawNode
-            return (
-              !accFind(acc, ruleDepName) &&
-              // The dependency is part of the rule to import so we don't want to handle it now
-              !rulesToImport.find(({ ruleName: ruleToImportName }) => {
-                const theDepIsARuleToImport =
-                  ruleName !== ruleToImportName &&
-                  ruleToImportName === ruleDepName
-                return theDepIsARuleToImport
-              })
-            )
-          })
-          .map(([ruleName, ruleNode]) => {
-            return getUpdatedRule(ruleName, ruleNode)
-          })
-        acc.push(...ruleDeps)
       })
     } else {
       let doubleDefinition = accFind(acc, name)

src/serializeParsedRules.ts

@@ -400,7 +400,6 @@ export function serializeParsedRules(
    * Consequently, we need to remove them from the rawNode in order to avoid
    * duplicate mecanisms.
    *
-   *
    * NOTE: for now, the [avec] mecanism is unfolded as full rules. Therefore,
    * we need to remove the [avec] mecanism from the rawNode in order to
    * avoid duplicate rule definitions.

test/compilation/data/import-avec-mechanism.publicodes

@@ -0,0 +1,6 @@
+importer!:
+  depuis:
+    nom: 'my-external-package'
+    source: './my-external-package.model.json'
+  les règles:
+    - rule with avec

test/compilation/data/my-external-package.model.json

@@ -31,5 +31,13 @@
   "e": {
     "question": "Question ?",
     "formule": 10
+  },
+  "rule with avec": {
+    "formule": "F1 + F2 + F3",
+    "avec": {
+      "F1": 1,
+      "F2": 2,
+      "F3": 3
+    }
   }
 }

test/compilation/getModelFromSource.test.ts

@@ -8,7 +8,7 @@ const updatedDescription = `> ℹ️ Cette règle provient du modèle \`my-exter
 describe('getModelFromSource › rules import', () => {
   it('should import a rule from a package', () => {
     expect(
-      getModelFromSource(join(testDataDir, 'simple-import.publicodes')),
+      getModelFromSource(join(testDataDir, 'import-simple.publicodes')),
     ).toEqual({
       'my-external-package': null,
       'my-external-package . root': null,
@@ -21,7 +21,7 @@ describe('getModelFromSource › rules import', () => {
 
   it('should import a rule from a package with its needed dependency', () => {
     expect(
-      getModelFromSource(join(testDataDir, 'deps-import.publicodes')),
+      getModelFromSource(join(testDataDir, 'import-deps.publicodes')),
     ).toEqual({
       'my-external-package': null,
       'my-external-package . root': null,
@@ -38,7 +38,7 @@ describe('getModelFromSource › rules import', () => {
 
   it('should import a rule from a package with all its needed dependencies', () => {
     expect(
-      getModelFromSource(join(testDataDir, 'multiple-deps-import.publicodes')),
+      getModelFromSource(join(testDataDir, 'import-multiple-deps.publicodes')),
     ).toEqual({
       'my-external-package': null,
       'my-external-package . root': {
@@ -62,7 +62,7 @@ describe('getModelFromSource › rules import', () => {
 
   it('should import a rule from a package with all updated attributes', () => {
     expect(
-      getModelFromSource(join(testDataDir, 'updated-attrs-import.publicodes')),
+      getModelFromSource(join(testDataDir, 'import-updated-attrs.publicodes')),
     ).toEqual({
       'my-external-package': null,
       'my-external-package . root': null,
@@ -96,7 +96,7 @@ Ajout d'une description`,
   it('should import a rule from a package with all updated attributes even in imported rule deps', () => {
     expect(
       getModelFromSource(
-        join(testDataDir, 'updated-attrs-from-deps-import.publicodes'),
+        join(testDataDir, 'import-updated-attrs-from-deps.publicodes'),
       ),
     ).toEqual({
       'my-external-package': null,
@@ -115,7 +115,7 @@ Ajout d'une description`,
 
   it('should import a rule from a package with all dependencies from a complex formula', () => {
     expect(
-      getModelFromSource(join(testDataDir, 'complex-deps-import.publicodes')),
+      getModelFromSource(join(testDataDir, 'import-complex-deps.publicodes')),
     ).toEqual({
       'my-external-package': null,
       'my-external-package . complex': {
@@ -156,7 +156,7 @@ Ajout d'une description`,
 
   it('should throw an error when trying to import an unknown rule', () => {
     expect(() => {
-      getModelFromSource(join(testDataDir, 'unknown-import.publicodes'))
+      getModelFromSource(join(testDataDir, 'import-unknown.publicodes'))
     }).toThrow(
       `[ Erreur dans la macro 'importer!' ]
 La règle 'root . unknown' n'existe pas dans 'my-external-package'.
@@ -168,12 +168,12 @@ La règle 'root . unknown' n'existe pas dans 'my-external-package'.
   })
 
   it('should throw an error if there is no package name specified', () => {
-    const path = join(testDataDir, 'no-name-import.publicodes')
+    const path = join(testDataDir, 'import-no-name.publicodes')
     expect(() => {
       getModelFromSource(path)
     }).toThrow(
       `[ Erreur dans la macro 'importer!' ]
-Le nom du package est manquant dans la macro 'importer!' dans le fichier: no-name-import.publicodes.
+Le nom du package est manquant dans la macro 'importer!' dans le fichier: import-no-name.publicodes.
 
 [ Solution ]
 Ajoutez le nom du package dans la macro 'importer!'.
@@ -191,7 +191,7 @@ importer!:
 
   it('should throw an error if there is a conflict between to imported rules', () => {
     expect(() => {
-      getModelFromSource(join(testDataDir, 'doublon-import.publicodes'))
+      getModelFromSource(join(testDataDir, 'import-doublon.publicodes'))
     }).toThrow(
       "La règle 'root . a' est définie deux fois dans my-external-package",
     )
@@ -216,7 +216,7 @@ importer!:
   it('should import a rule from a package with all dependencies from a complex formula in a custom namespace', () => {
     expect(
       getModelFromSource(
-        join(testDataDir, 'complex-deps-with-namespace-import.publicodes'),
+        join(testDataDir, 'import-complex-deps-with-namespace.publicodes'),
       ),
     ).toEqual({
       pkg: null,
@@ -256,6 +256,30 @@ importer!:
     })
   })
 
+  it('should correctly import rules with [avec] mechanism', () => {
+    expect(
+      getModelFromSource(join(testDataDir, 'import-avec-mechanism.publicodes')),
+    ).toEqual({
+      'my-external-package': null,
+      'my-external-package . rule with avec': {
+        formule: 'F1 + F2 + F3',
+        description: updatedDescription,
+      },
+      'my-external-package . rule with avec . F1': {
+        valeur: '1',
+        description: updatedDescription,
+      },
+      'my-external-package . rule with avec . F2': {
+        valeur: '2',
+        description: updatedDescription,
+      },
+      'my-external-package . rule with avec . F3': {
+        valeur: '3',
+        description: updatedDescription,
+      },
+    })
+  })
+
   it('should not add namespace if it is already present in the model', () => {
     expect(
       getModelFromSource(join(testDataDir, 'namespace-conflicts')),

yarn.lock

@@ -2543,10 +2543,10 @@ prompts@^2.0.1:
     kleur "^3.0.3"
     sisteransi "^1.0.5"
 
-publicodes@^1.3.0:
-  version "1.3.0"
-  resolved "https://registry.yarnpkg.com/publicodes/-/publicodes-1.3.0.tgz#5530e029ef6aaea2373c56a846d78732c56abc66"
-  integrity sha512-VBicg3+/SZUe49LzM77CpxWEjPra7sWDnbJyGVShDd2Tuo/p3Krb7ebe/kNM5Z1WDwuqCYLT+X75Co7fi/tMAA==
+publicodes@^1.3.3:
+  version "1.3.3"
+  resolved "https://registry.yarnpkg.com/publicodes/-/publicodes-1.3.3.tgz#8e9491b282a408e918458dfedabc4512aac05cb1"
+  integrity sha512-bRGB4M2HfgfwSI0ysHptn6pq3iYqos1/z2x7DghzMkK5uaoSMgvaeuIwjiX2SI1kjTh11zdFdwYB8Swyr8oZdA==
 
 punycode@^2.1.0:
   version "2.3.1"