Misc: do not modify CST in check mode (#2252)

* Slightly changes token-assert API by providing result of the check
with return values

* Apply fixes only in fix mode

* Modify couple rules to adjust to new strategy

* Make "fixed" property of the Error instance consistent

* Introduce "fix" property of the Error class

* Divide fix behaviour on "common" and "specific" actions

Consequently should speed up linting

Fixes #2244

lib/errors.js

@@ -87,7 +87,8 @@ Errors.prototype = {
             element: errorInfo.element,
             offset: errorInfo.offset,
             additional: errorInfo.additional,
-            fixed: errorInfo.fixed
+            fixed: false,
+            fix: errorInfo.fix
         });
     },
 

lib/rules/disallow-spaces-inside-parentheses.js

@@ -89,6 +89,7 @@ module.exports.prototype = {
         var only = this._only;
         var singleQuote = this._onlySingleQuote;
         var doubleQuote = this._onlyDoubleQuote;
+        var alreadyAdded = [];
 
         file.iterateTokensByTypeAndValue('Punctuator', '(', function(token) {
             var nextToken = file.getNextToken(token, {includeComments: true});
@@ -115,18 +116,27 @@ module.exports.prototype = {
                 return;
             }
 
-            errors.assert.noWhitespaceBetween({
+            var isAdded = errors.assert.noWhitespaceBetween({
                 token: token,
                 nextToken: nextToken,
                 message: 'Illegal space after opening round bracket'
             });
+
+            if (isAdded) {
+                alreadyAdded.push(token);
+            }
         });
 
         file.iterateTokensByTypeAndValue('Punctuator', ')', function(token) {
             var prevToken = file.getPrevToken(token, {includeComments: true});
             var value = prevToken.getSourceCode();
             var shouldReturn = true;
 
+            // Do not check already found errors, like "function ( ) { ..."
+            if (alreadyAdded.indexOf(prevToken) > -1) {
+                return;
+            }
+
             if (doubleQuote && prevToken.type === 'String' && value[value.length - 1] === '"') {
                 shouldReturn = false;
             }

lib/rules/require-aligned-multiline-params.js

@@ -141,8 +141,7 @@ module.exports.prototype = {
                             token: param.getFirstToken(),
                             actual: paramColumn,
                             expected: referenceColumn,
-                            indentChar: ' ',
-                            silent: false
+                            indentChar: ' '
                         });
                     }
 

lib/string-checker.js

@@ -95,41 +95,75 @@ StringChecker.prototype = {
     },
 
     /**
-     * Fix provided error.
+     * Apply fix for common errors.
+     *
+     * @param {Error} error
+     * @return {Boolean} whether the correction was carried out
+     * @private
+     */
+    _fixCommonError: function(error) {
+        if (error.fix) {
+            // "error.fixed = true" should go first, so rule can
+            // decide for itself (with "error.fixed = false")
+            // if it can fix this particular error
+            error.fixed = true;
+            error.fix();
+        }
+
+        return !!error.fixed;
+    },
+
+    /**
+     * Apply fix for specific error.
+     *
+     * @param {Error} error
+     * @return {Boolean} whether the correction was carried out
+     * @private
+     */
+    _fixSpecificError: function(file, error) {
+        var configuration = this.getConfiguration();
+        var instance = configuration.getConfiguredRule(error.rule);
+
+        if (instance && instance._fix) {
+            // "error.fixed = true" should go first, so rule can
+            // decide for itself (with "error.fixed = false")
+            // if it can fix this particular error
+            error.fixed = true;
+            instance._fix(file, error);
+        }
+
+        return !!error.fixed;
+    },
+
+    /**
+     * Apply specific and common fixes.
      *
      * @param {JsFile} file
      * @param {Errors} errors
      * @protected
      */
     _fixJsFile: function(file, errors) {
-        var list = errors.getErrorList();
-        var configuration = this.getConfiguration();
-
-        list.forEach(function(error) {
+        errors.getErrorList().forEach(function(error) {
             if (error.fixed) {
                 return;
             }
 
-            var instance = configuration.getConfiguredRule(error.rule);
-
-            if (instance && instance._fix) {
-                try {
-
-                    // "error.fixed = true" should go first, so rule can
-                    // decide for itself (with "error.fixed = false")
-                    // if it can fix this particular error
-                    error.fixed = true;
-                    instance._fix(file, error);
+            try {
+                // Try to apply fixes for common errors
+                var isFixed = this._fixCommonError(error);
 
-                } catch (e) {
-                    error.fixed = undefined;
-                    errors.add(
-                        getInternalErrorMessage(error.rule, e),
-                        file.getProgram()
-                    );
+                // Apply specific fix
+                if (!isFixed) {
+                    this._fixSpecificError(file, error);
                 }
+            } catch (e) {
+                error.fixed = false;
+                errors.add(
+                    getInternalErrorMessage(error.rule, e),
+                    file.getProgram()
+                );
             }
-        });
+        }, this);
     },
 
     /**
@@ -232,7 +266,7 @@ StringChecker.prototype = {
     },
 
     /**
-     * Checks file provided with a string.
+     * Checks and fix file provided with a string.
      *
      * @param {String} source
      * @param {String} [filename='input']
@@ -254,11 +288,11 @@ StringChecker.prototype = {
         } else {
             var attempt = 0;
             do {
-                // Changes to current sources are made in rules through assertions.
+
+                // Fill in errors list
                 this._checkJsFile(file, errors);
 
-                // If assertions weren't used but rule has "fix" method,
-                // which we could use.
+                // Apply fixes
                 this._fixJsFile(file, errors);
 
                 var hasFixes = errors.getErrorList().some(function(err) {