Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

[require|disallow]SpaceBeforeBlockStatements #291

Merged
merged 1 commit into from

2 participants

@rxin

This resolves #284.

@mikesherov
Owner

Can you rebase and squash this pull request so it no longer has a merge commit in it? Thanks again for everything!

@rxin

Ok done.

@mikesherov mikesherov commented on the diff
lib/rules/disallow-space-before-block-statements.js
((10 lines not shown))
+ );
+ assert(
+ disallowSpaceBeforeBlockStatements === true,
+ 'disallowSpaceBeforeBlockStatements option requires true value or should be removed'
+ );
+ },
+
+ getOptionName: function() {
+ return 'disallowSpaceBeforeBlockStatements';
+ },
+
+ check: function(file, errors) {
+ var tokens = file.getTokens();
+
+ file.iterateNodesByType('BlockStatement', function(node) {
+ var tokenBeforeBodyPos = file.getTokenPosByRangeStart(node.range[0] - 1);
@mikesherov Owner

I really like this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mikesherov
Owner

@rxin, thanks again! I'm ready to land this, I just would like one more test for before require and disallow. Can you just make sure allman style bracing (http://en.wikipedia.org/wiki/Indent_style#Allman_style) is not an error under the require conditions and is an error on the disallow conditions?

After that, I can land this. Thanks again for contributing!

@rxin

Thanks for the comment. The Allman style should be tested already in test cases where there are "\n"s.

@mikesherov
Owner

I see. Merging! Thanks again!

@mikesherov mikesherov merged commit cd6d37d into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 8, 2014
  1. @rxin
This page is out of date. Refresh to see the latest.
View
95 README.md
@@ -141,6 +141,101 @@ if(x > y) {
}
```
+
+### requireSpaceBeforeBlockStatements
+
+Requires space before block statements (for loops, control structures).
+
+Type: `Boolean`
+
+Values: `true`
+
+#### Example
+
+```js
+"requireSpaceBeforeBlockStatements": true
+```
+
+##### Valid
+
+```js
+if (cond) {
+ foo();
+}
+
+for (var e in elements) {
+ bar(e);
+}
+
+while (cond) {
+ foo();
+}
+```
+
+##### Invalid
+
+```js
+if (cond){
+ foo();
+}
+
+for (var e in elements){
+ bar(e);
+}
+
+while (cond){
+ foo();
+}
+```
+
+
+### disallowSpaceBeforeBlockStatements
+
+Disallows space before block statements (for loops, control structures).
+
+Type: `Boolean`
+
+Values: `true`
+
+#### Example
+
+```js
+"disallowSpaceBeforeBlockStatements": true
+```
+
+##### Valid
+
+```js
+if (cond){
+ foo();
+}
+
+for (var e in elements){
+ bar(e);
+}
+
+while (cond){
+ foo();
+}
+```
+
+##### Invalid
+
+```js
+if (cond) {
+ foo();
+}
+
+for (var e in elements) {
+ bar(e);
+}
+
+while (cond) {
+ foo();
+}
+```
+
+
### requireParenthesesAroundIIFE
Requires parentheses around immediately invoked function expressions.
View
36 lib/rules/disallow-space-before-block-statements.js
@@ -0,0 +1,36 @@
+var assert = require('assert');
+
+module.exports = function() {};
+
+module.exports.prototype = {
+ configure: function(disallowSpaceBeforeBlockStatements) {
+ assert(
+ typeof disallowSpaceBeforeBlockStatements === 'boolean',
+ 'disallowSpaceBeforeBlockStatements option requires boolean value'
+ );
+ assert(
+ disallowSpaceBeforeBlockStatements === true,
+ 'disallowSpaceBeforeBlockStatements option requires true value or should be removed'
+ );
+ },
+
+ getOptionName: function() {
+ return 'disallowSpaceBeforeBlockStatements';
+ },
+
+ check: function(file, errors) {
+ var tokens = file.getTokens();
+
+ file.iterateNodesByType('BlockStatement', function(node) {
+ var tokenBeforeBodyPos = file.getTokenPosByRangeStart(node.range[0] - 1);
@mikesherov Owner

I really like this approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ var tokenBeforeBody = tokens[tokenBeforeBodyPos];
+ if (!tokenBeforeBody) {
+ errors.add(
+ 'Extra space before opening curly brace for block expressions',
+ node.loc.start
+ );
+ }
+ });
+ }
+
+};
View
36 lib/rules/require-space-before-block-statements.js
@@ -0,0 +1,36 @@
+var assert = require('assert');
+
+module.exports = function() {};
+
+module.exports.prototype = {
+ configure: function(requireSpaceBeforeBlockStatements) {
+ assert(
+ typeof requireSpaceBeforeBlockStatements === 'boolean',
+ 'requireSpaceBeforeBlockStatements option requires boolean value'
+ );
+ assert(
+ requireSpaceBeforeBlockStatements === true,
+ 'requireSpaceBeforeBlockStatements option requires true value or should be removed'
+ );
+ },
+
+ getOptionName: function() {
+ return 'requireSpaceBeforeBlockStatements';
+ },
+
+ check: function(file, errors) {
+ var tokens = file.getTokens();
+
+ file.iterateNodesByType('BlockStatement', function(node) {
+ var tokenBeforeBodyPos = file.getTokenPosByRangeStart(node.range[0] - 1);
+ var tokenBeforeBody = tokens[tokenBeforeBodyPos];
+ if (tokenBeforeBody) {
+ errors.add(
+ 'Missing space before opening curly brace for block expressions',
+ tokenBeforeBody.loc.start
+ );
+ }
+ });
+ }
+
+};
View
3  lib/string-checker.js
@@ -80,6 +80,9 @@ StringChecker.prototype = {
this.registerRule(new (require('./rules/disallow-comma-before-line-break'))());
this.registerRule(new (require('./rules/require-comma-before-line-break'))());
+ this.registerRule(new (require('./rules/disallow-space-before-block-statements.js'))());
+ this.registerRule(new (require('./rules/require-space-before-block-statements.js'))());
+
this.registerRule(new (require('./rules/disallow-space-before-postfix-unary-operators.js'))());
this.registerRule(new (require('./rules/require-space-before-postfix-unary-operators.js'))());
View
39 test/test.disallow-space-before-block-statements.js
@@ -0,0 +1,39 @@
+var Checker = require('../lib/checker');
+var assert = require('assert');
+
+describe('rules/disallow-space-before-block-statements', function() {
+ var checker;
+
+ beforeEach(function() {
+ checker = new Checker();
+ checker.registerDefaultRules();
+ checker.configure({ disallowSpaceBeforeBlockStatements: true });
+ });
+
+ it('should report missing space for control structures', function() {
+ assert(checker.checkString('if (true) { var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('if (true)\n{ var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('if (true){ var a = false; }').getErrorCount() === 0);
+ });
+
+ it('should report missing space for loops', function() {
+ assert(checker.checkString('while (true) { var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('while (true)\n{ var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('while (true){ var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('for (var e in es) { var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('for (var e in es)\n{ var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('for (var e in es){ var a = false; }').getErrorCount() === 0);
+ });
+
+ it('should report missing space for function declarations', function() {
+ assert(checker.checkString('function foo(i) { var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('function foo(i){ var a = false; }').getErrorCount() === 0);
+ });
+
+ it('should not affect object decls', function() {
+ assert(checker.checkString('var a ={};').getErrorCount() === 0);
+ assert(checker.checkString('var a ={id:5};').getErrorCount() === 0);
+ assert(checker.checkString('var a = {};').getErrorCount() === 0);
+ assert(checker.checkString('var a = {id:5};').getErrorCount() === 0);
+ });
+});
View
39 test/test.require-space-before-block-statements.js
@@ -0,0 +1,39 @@
+var Checker = require('../lib/checker');
+var assert = require('assert');
+
+describe('rules/require-space-before-block-statements', function() {
+ var checker;
+
+ beforeEach(function() {
+ checker = new Checker();
+ checker.registerDefaultRules();
+ checker.configure({ requireSpaceBeforeBlockStatements: true });
+ });
+
+ it('should report missing space for control structures', function() {
+ assert(checker.checkString('if (true) { var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('if (true)\n{ var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('if (true){ var a = false; }').getErrorCount() === 1);
+ });
+
+ it('should report missing space for loops', function() {
+ assert(checker.checkString('while (true) { var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('while (true)\n{ var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('while (true){ var a = false; }').getErrorCount() === 1);
+ assert(checker.checkString('for (var e in es) { var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('for (var e in es)\n{ var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('for (var e in es){ var a = false; }').getErrorCount() === 1);
+ });
+
+ it('should report missing space for function declarations', function() {
+ assert(checker.checkString('function foo(i) { var a = false; }').getErrorCount() === 0);
+ assert(checker.checkString('function foo(i){ var a = false; }').getErrorCount() === 1);
+ });
+
+ it('should not affect object decls', function() {
+ assert(checker.checkString('var a ={};').getErrorCount() === 0);
+ assert(checker.checkString('var a ={id:5};').getErrorCount() === 0);
+ assert(checker.checkString('var a = {};').getErrorCount() === 0);
+ assert(checker.checkString('var a = {id:5};').getErrorCount() === 0);
+ });
+});
Something went wrong with that request. Please try again.