Brace style now only lints valid atrule types

lib/rules/brace-style.js

@@ -30,8 +30,20 @@ var getPreviousNode = function (node) {
     return previousParent && previousParent.contains('atkeyword') ? previousParent.last('atkeyword') : false;
   }
 
-  // Functions, Mixins, Loops
-  if (node.is('atrule') || node.is('mixin') || node.is('loop')) {
+  // Loops
+  if (node.is('loop')) {
+    return node.contains('atkeyword') ? node.last('atkeyword') : false;
+  }
+
+  // Mixins and atrules (functions etc)
+  if (node.is('mixin') || node.is('atrule')) {
+    if (node.contains('function')) {
+      return node.last('function');
+    }
+
+    if (node.contains('arguments')) {
+      return node.last('arguments');
+    }
     return node.contains('atkeyword') ? node.last('atkeyword') : false;
   }
 
@@ -107,11 +119,18 @@ var isClosingBraceOnNewLine = function (node) {
  * @returns {bool|null} True or false if relevant else null
  */
 var isConditionOnNewLine = function (node, parentNode, j) {
+  var currentNode = node;
+
+  // Check node is part of an `else if` and if it is, use the else node instead
+  if (node.first('ident').content === 'if') {
+    var initialNode = parentNode.get(j);
+    currentNode = initialNode.contains('atkeyword') ? initialNode.first('atkeyword') : false;
+  }
+
   // Only check if it's an @else condition
-  if (node.contains('ident') && node.first('ident').content === 'else') {
+  if (currentNode && currentNode.contains('ident') && currentNode.first('ident').content === 'else') {
     // Reverse back up tree
     var previousChild = parentNode.get(--j) || false;
-
     if (previousChild) {
       // Determine if we have a leading new line
       if (previousChild.is('space') && helpers.hasEOL(previousChild.content)) {
@@ -156,6 +175,24 @@ var runRuleChecks = function (node, currentNode, previousNode, parentNode, index
   return checks;
 };
 
+/**
+ * Filter at-rules
+ *
+ * @param {Object} node - The node to filter
+ * @param {Array} accepted - The array of accepted at-rule types
+ * @returns {bool} true if we should ignore, false to continue
+ */
+var filterAtrule = function (node, accepted) {
+  if (node.is('atrule')) {
+    if (node.contains('atkeyword') && node.first('atkeyword').contains('ident')) {
+      if (accepted.indexOf(node.first('atkeyword').first('ident').content) === -1) {
+        return true;
+      }
+    }
+  }
+  return false;
+};
+
 /**
  * Create an issue using the supplied information
  *
@@ -167,8 +204,8 @@ var runRuleChecks = function (node, currentNode, previousNode, parentNode, index
 var createIssue = function (parser, node, message) {
   return {
     'ruleId': parser.rule.name,
-    'line': node.end.line,
-    'column': node.end.column,
+    'line': node.line,
+    'column': node.column,
     'message': message,
     'severity': parser.severity
   };
@@ -181,7 +218,12 @@ module.exports = {
     'allow-single-line': true
   },
   'detect': function (ast, parser) {
-    var result = [];
+    var result = [],
+        acceptedAtrules = [
+          'function',
+          'if',
+          'else'
+        ];
 
     ast.traverseByTypes(['conditionalStatement', 'atrule', 'ruleset', 'mixin', 'loop'], function (node, i, parent) {
       var currentNode = false,
@@ -206,6 +248,11 @@ module.exports = {
         return false;
       }
 
+      // Filter at-rule types
+      if (filterAtrule(node, acceptedAtrules)) {
+        return false;
+      }
+
       // Assign current & previous nodes based on node type
       currentNode = getCurrentNode(node);
       previousNode = getPreviousNode(node);
@@ -218,12 +265,18 @@ module.exports = {
 
         // Build single-line statement results
         if (checks.singleLineStatement === false && checks.closingBraceOnNewLine === false) {
-          result = helpers.addUnique(result, createIssue(parser, currentNode, messages[5]));
+          result = helpers.addUnique(result, createIssue(parser, {
+            line: currentNode.end.line,
+            column: currentNode.end.column
+          }, messages[5]));
         }
 
         if (checks.singleLineStatement === true) {
           if (parser.options['allow-single-line'] === false) {
-            result = helpers.addUnique(result, createIssue(parser, node, messages[0]));
+            result = helpers.addUnique(result, createIssue(parser, {
+              line: node.start.line,
+              column: node.start.column
+            }, messages[0]));
           }
           return false;
         }
@@ -232,28 +285,46 @@ module.exports = {
         if (previousNode && currentNode) {
           if (parser.options.style === '1tbs') {
             if (checks.openingBraceOnNewLine === false) {
-              result = helpers.addUnique(result, createIssue(parser, currentNode, messages[1]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: currentNode.start.line,
+                column: currentNode.start.column
+              }, messages[1]));
             }
             if (checks.conditionOnNewLine === true) {
-              result = helpers.addUnique(result, createIssue(parser, previousNode, messages[3]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: previousNode.start.line,
+                column: previousNode.start.column
+              }, messages[3]));
             }
           }
 
           if (parser.options.style === 'stroustrup') {
             if (checks.openingBraceOnNewLine === false) {
-              result = helpers.addUnique(result, createIssue(parser, currentNode, messages[1]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: currentNode.start.line,
+                column: currentNode.start.column
+              }, messages[1]));
             }
             if (checks.conditionOnNewLine === false) {
-              result = helpers.addUnique(result, createIssue(parser, previousNode, messages[4]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: previousNode.start.line,
+                column: previousNode.start.column
+              }, messages[4]));
             }
           }
 
           if (parser.options.style === 'allman') {
             if (checks.openingBraceOnNewLine === true) {
-              result = helpers.addUnique(result, createIssue(parser, currentNode, messages[2]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: currentNode.end.line,
+                column: currentNode.end.column
+              }, messages[2]));
             }
             if (checks.conditionOnNewLine === false) {
-              result = helpers.addUnique(result, createIssue(parser, previousNode, messages[4]));
+              result = helpers.addUnique(result, createIssue(parser, {
+                line: previousNode.start.line,
+                column: previousNode.start.column
+              }, messages[4]));
             }
           }
         }

tests/rules/brace-style.js

@@ -12,7 +12,7 @@ describe('brace style - scss', function () {
     lint.test(file, {
       'brace-style': 1
     }, function (data) {
-      lint.assert.equal(36, data.warningCount);
+      lint.assert.equal(40, data.warningCount);
       done();
     });
   });
@@ -27,7 +27,7 @@ describe('brace style - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(58, data.warningCount);
+      lint.assert.equal(62, data.warningCount);
       done();
     });
   });
@@ -42,7 +42,7 @@ describe('brace style - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(40, data.warningCount);
+      lint.assert.equal(44, data.warningCount);
       done();
     });
   });
@@ -57,7 +57,7 @@ describe('brace style - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(62, data.warningCount);
+      lint.assert.equal(66, data.warningCount);
       done();
     });
   });
@@ -72,7 +72,7 @@ describe('brace style - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(77, data.warningCount);
+      lint.assert.equal(81, data.warningCount);
       done();
     });
   });
@@ -87,7 +87,7 @@ describe('brace style - scss', function () {
         }
       ]
     }, function (data) {
-      lint.assert.equal(99, data.warningCount);
+      lint.assert.equal(103, data.warningCount);
       done();
     });
   });

tests/sass/brace-style.scss

@@ -471,3 +471,70 @@ $total: 4;
 
 @mixin bar() {
   content: 'bar'; }
+
+
+
+// =======================
+//  Multi-line parameters
+// =======================
+
+@function foo(
+  $bar
+) {
+  content: $bar;
+}
+
+
+@function foo(
+  $bar
+)
+{
+  content: $bar;
+}
+
+
+
+@function bar(
+  $bar,
+  $baz
+) {
+  content: $bar;
+}
+
+@function bar(
+  $bar,
+  $baz
+)
+{
+  content: $bar;
+}
+
+
+@mixin foo(
+  $bar
+) {
+  content: 'foo bar';
+}
+
+@mixin foo(
+  $bar
+)
+{
+  content: 'foo bar';
+}
+
+
+@mixin bar(
+  $bar,
+  $baz
+) {
+  content: 'foo bar';
+}
+
+@mixin bar(
+  $bar,
+  $baz
+)
+{
+  content: 'foo bar';
+}