removed: badly broken self-referencing check, #47

CHANGELOG.md

@@ -2,6 +2,11 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+[Unreleased]
+---------------------
+##### Removed
+- Badly broken self-reference check.
+
 [1.5.1] - 2018-12-22
 ---------------------
 ##### Fixed

src/validator.js

@@ -65,15 +65,7 @@ let validate = ( xformStr, options = {} ) => {
                     try {
                         xform.enketoEvaluate( logicExpr, ( calculation ? 'string' : 'boolean' ), path );
 
-                        // After checking that the non-constraint expression is valid, check for self-references.
-                        // Make an exception for a calculate="." as it does no harm.
-                        if ( logicName !== 'constraint' && !( logicName === 'calculate' && logicExpr.trim() === '.' ) ) {
-                            if ( xform.hasSelfReference( logicExpr, path ) ) {
-                                throw new Error( 'refers to itself' );
-                                //errors.push( `${friendlyLogicName} formula for "${nodeName}" refers to itself` );
-                            }
-                        }
-                        // TODO: check for cyclic dependencies between calculations, e.g. triangular calculation dependencies
+                        // TODO: check for cyclic dependencies within single expression and between calculations, e.g. triangular calculation dependencies
                     } catch ( e ) {
                         errors.push( `${friendlyLogicName} formula for "${nodeName}": ${e}` );
                     }

src/xform.js

@@ -121,29 +121,6 @@ class XForm {
         }
     }
 
-    /**
-     * A function that performs a basic check for references to a path or self-as-dot.
-     * Note that this could easily be circumvented e.g. with a triangular dependency cycle 
-     * between nodes with expressions or using paths with predicates.
-     * It is just a start with detecting the most obvious dependency errors.
-     * 
-     * @param {string} expr 
-     * @param {string} selfPath 
-     */
-    hasSelfReference( expr, selfPath ) {
-        if ( !expr || !selfPath ) {
-            throw new Error( 'hasSelfReference function requires 2 parameters', expr, selfPath );
-        }
-        const self = this.enketoEvaluate( selfPath, 'node' );
-
-        return this._extractNodeReferences( expr )
-            .some( path => {
-                // The path could return multiple nodes, and self cannot be one of them.
-                const nodes = this.enketoEvaluate( path, 'nodes', selfPath );
-                return nodes.includes( self );
-            } );
-    }
-
     checkStructure( warnings, errors ) {
         const rootEl = this.doc.documentElement;
         const rootElNodeName = rootEl.nodeName;
@@ -435,33 +412,6 @@ class XForm {
         return parts.join( ', ' ).replace( /\.\s*,/g, ',' );
     }
 
-    /**
-     * [EXPERIMENTAL] Extracts node references.
-     * It excludes any references with predicates which is a big limitation but
-     * the goal is a better-safe-than-sorry approach to not have any false positives.
-     * 
-     * @param {string} expr 
-     */
-    _extractNodeReferences( expr ) {
-        return expr
-            // functions
-            .replace( /[a-z-:]+\(/g, ' ' )
-            .replace( /\)/g, ' ' )
-            .replace( /,/g, ' ' ) // VERY WRONG: a string could contain a comma!
-            // * character when used for multiplication only
-            .replace( /(?<!\/)\*/g, ' ' )
-            // other operators (- character only when used for deduction)
-            .replace( /(\+| -|\|| and | or | mod | div |!=|=|<=|>=|<|>)/g, ' ' )
-            // remaining brackets that were not part of function calls
-            .replace( /(\(|\))/g, ' ' )
-            .split( /\s+/ )
-            // filter out empty strings, string literals, numbers
-            // Note this also filters out the path-as-string literal argument in jr:choice-name but who cares?
-            .filter( n => n && !( /".*"/.test( n ) ) && !( /'.*'/.test( n ) ) && isNaN( n ) )
-            // filter out anything with predicates as it is too difficult
-            .filter( n => !( /(\[|\])/.test( n ) ) );
-    }
-
 }
 
 module.exports = {

test/spec/xform.spec.js

@@ -139,7 +139,7 @@ describe( 'XForm', () => {
         } );
     } );
 
-    describe( 'with disallowed self-referencing', () => {
+    xdescribe( 'with disallowed self-referencing', () => {
         // Unit tests are in xpath.spec.js
         const xf = loadXForm( 'self-reference.xml' );
         const result = validator.validate( xf );

test/spec/xpath.spec.js

@@ -72,7 +72,7 @@ describe( 'XPath expressions', () => {
     } );
 
 
-    describe( 'with expressions that refer to self when not allowed', () => {
+    xdescribe( 'with expressions that refer to self when not allowed', () => {
         // There is an integration test in xform.spec.js
         const FULL_PATH_TO_SELF = '/data/a';
         [