[#302] new rule: react-tsx-curly-spacing

- The ESLint version: https://github.com/yannickcr/eslint-plugin-react/blob/master/lib/rules/jsx-curly-spacing.js
- The rule options are the same: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-curly-spacing.md

closes #302

README.md

@@ -153,6 +153,7 @@ Rule Name   | Description | Since
 `react-anchor-blank-noopener`   | For security reasons, anchor tags with target="_blank" should also include rel="noopener noreferrer". In order to restrict the behavior window.opener access, the original page needs to add a rel="noopener" attribute to any link that has target="_blank". However, Firefox does not support that tag, so you should actually use rel="noopener noreferrer" for full coverage. For more info see: [The target="_blank" vulnerability by example](https://dev.to/ben/the-targetblank-vulnerability-by-example)| 2.0.11
 `react-no-dangerous-html`       | Do not use React's dangerouslySetInnerHTML API. This rule finds usages of the dangerouslySetInnerHTML API (but not any JSX references). For more info see the [react-no-dangerous-html Rule wiki page](https://github.com/Microsoft/tslint-microsoft-contrib/wiki/react-no-dangerous-html-Rule). | 0.0.2
 `react-this-binding-issue`      | Several errors can occur when using React and React.Component subclasses. When using React components you must be careful to correctly bind the 'this' reference on any methods that you pass off to child components as callbacks. For example, it is common to define a private method called 'onClick' and then specify `onClick={this.onClick}` as a JSX attribute. If you do this then the 'this' reference will be undefined when your private method is invoked. The React documentation suggests that you bind the 'this' reference on all of your methods within the constructor: `this.onClick = this.onClick.bind(this);`. This rule will create a violation if 1) a method reference is passed to a JSX attribute without being bound in the constructor. And 2) a method is bound multiple times in the constructor. Another issue that can occur is binding the 'this' reference to a function within the render() method. For example, many people will create an anonymous lambda within the JSX attribute to avoid the 'this' binding issue: `onClick={() => { this.onClick(); }}`. The problem with this is that a new instance of an anonymous function is created every time render() is invoked. When React compares virutal DOM properties within shouldComponentUpdate() then the onClick property will look like a new property and force a re-render. You should avoid this pattern because creating function instances within render methods breaks any logic within shouldComponentUpdate() methods. This rule creates violations if 1) an anonymous function is passed as a JSX attribute. And 2) if a function instantiated in local scope is passed as a JSX attribute. This rule can be configured via the "allow-anonymous-listeners" parameter. If you want to suppress violations for the anonymous listener scenarios then configure that rule like this: `"react-this-binding-issue": [ true, { 'allow-anonymous-listeners': true } ]` | 2.0.8, 2.0.9
+`react-tsx-curly-spacing`       | Consistently use spaces around the brace characters of JSX attributes.You can either allow or bad spaces between the braces and the values they enclose. <br/><br/>One of the two following options are required:<br/>* "always" enforces a space inside of curly braces (default)<br/>* "never" disallows spaces inside of curly braces<br/><br/>By default, braces spanning multiple lines are not allowed with either setting. If you want to allow them you can specify an additional allowMultiline property with the value false. <br/><br/>Examples: <br/>* "react-tsx-curly-spacing": [true, "always"]<br/>* "react-tsx-curly-spacing": [true, "never"]<br/>* "react-tsx-curly-spacing": [true, "never", {"allowMultiline": false}]<br/><br/>References<br/>* [eslint-plugin-react jsx-curly-spacing rule](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-curly-spacing.md)  | 2.0.14
 `react-unused-props-and-state`  | Remove unneeded properties defined in React Props and State interfaces. Any interface named Props or State is defined as a React interface. All fields in these interfaces must be referenced. | 2.0.10
 `underscore-consistent-invocation` | Enforce a consistent usage of the _ functions. By default, invoking underscore functions should begin with wrapping a variable in an underscore instance: `_(list).map(...)`. An alternative is to prefer using the static methods on the _ variable: `_.map(list, ...)`. The rule accepts single parameter called 'style' which can be the value 'static' or 'instance': `[true, { "style": "static" }]`| 2.0.10
 `use-isnan`                     | Deprecated - This rule is now part of the base TSLint product. Ensures that you use the isNaN() function to check for NaN references instead of a comparison to the NaN constant. Similar to the [use-isnan ESLint rule](http://eslint.org/docs/rules/use-isnan).| 1.0

recommended_ruleset.js

@@ -170,6 +170,7 @@ module.exports = {
         "object-literal-key-quotes": [true, "as-needed"],
         "one-line": [true, "check-open-brace", "check-catch", "check-else", "check-whitespace"],
         "quotemark": [true, "single"],
+        "react-tsx-curly-spacing": true,
         "semicolon": [true, "always"],
         "trailing-comma": [true, {"singleline": "never", "multiline": "never"}], // forcing trailing commas for multi-line
                     // lists results in lists that are easier to reorder and version control diffs that are more clear.

src/reactTsxCurlySpacingRule.ts

@@ -0,0 +1,129 @@
+import * as ts from 'typescript';
+import * as Lint from 'tslint/lib/lint';
+import {SyntaxKind} from './utils/SyntaxKind';
+import {ExtendedMetadata} from './utils/ExtendedMetadata';
+
+/**
+ * TSX curly spacing rule.
+ *
+ * Allows you to specify how spacing works around JSX Expressions.
+ */
+export class Rule extends Lint.Rules.AbstractRule {
+
+    public static metadata: ExtendedMetadata = {
+        ruleName: 'react-tsx-curly-spacing',
+        type: 'style',
+        description: 'Consistently use spaces around the brace characters of JSX attributes.',
+        options: null,
+        issueClass: 'Non-SDL',
+        issueType: 'Warning',
+        severity: 'Low',
+        level: 'Opportunity for Excellence',
+        group: 'Whitespace'
+    };
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithWalker(new TsxCurlySpacingWalker(sourceFile, this.getOptions()));
+    }
+}
+
+enum Spacing {
+    always,
+    never
+}
+
+class TsxCurlySpacingWalker extends Lint.RuleWalker {
+
+    private spacing: Spacing;
+    private allowMultiline: boolean;
+
+    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
+        super(sourceFile, options);
+        // default value is always
+        this.spacing = options.ruleArguments[0] === 'never' ? Spacing.never : Spacing.always;
+        // default value is to not allow multiline
+        this.allowMultiline = false;
+        if (options.ruleArguments[1] != null) {
+            this.allowMultiline = !(options.ruleArguments[1].allowMultiline === false);
+        }
+    }
+
+    public visitJsxExpression(node: ts.JsxExpression): void {
+        const childrenCount: number = node.getChildCount();
+        const first: ts.Node = node.getFirstToken(); // '{' sign
+        const last: ts.Node = node.getLastToken(); // '}' sign
+        const second: ts.Node = node.getChildAt(1); // after '{' sign
+        const penultimate: ts.Node = node.getChildAt(childrenCount - 2); // before '}' sign
+        this.validateBraceSpacing(node, first, second, first);
+        this.validateBraceSpacing(node, penultimate, last, last);
+    }
+
+    protected visitNode(node: ts.Node): void {
+        // This is hacked to visit JSX Expression. See https://github.com/palantir/tslint/pull/1292
+        // newer versions of tslint have a public visitJsxExpression but older versions do not
+        if (node.kind === SyntaxKind.current().JsxExpression) {
+            this.visitJsxExpression(<ts.JsxExpression> node);
+            this.walkChildren(node);
+        } else {
+            super.visitNode(node);
+        }
+    }
+
+    private validateBraceSpacing(node: ts.Node, first: ts.Node, second: ts.Node, violationRoot: ts.Node): void {
+
+        if (this.isMultiline(first, second)) {
+            if (!this.allowMultiline) {
+                this.reportFailure(node, violationRoot, this.getFailureForNewLine(first, violationRoot));
+            }
+        } else if (this.spacing === Spacing.always) {
+            if (!this.isSpaceBetweenTokens(first, second)) {
+                this.reportFailure(node, violationRoot, this.getFailureForSpace(first, violationRoot));
+            }
+        } else { // never space
+            if (this.isSpaceBetweenTokens(first, second)) {
+                this.reportFailure(node, violationRoot, this.getFailureForSpace(first, violationRoot));
+            }
+        }
+    }
+
+    private getFailureForSpace(first: ts.Node, violationRoot: ts.Node): string {
+        if (this.spacing === Spacing.always) {
+            if (first === violationRoot) {
+                return `A space is required after '${violationRoot.getText()}'`;
+            } else {
+                return `A space is required before '${violationRoot.getText()}'`;
+            }
+        } else {
+            if (first === violationRoot) {
+                return `There should be no space after '${violationRoot.getText()}'`;
+            } else {
+                return `There should be no space before '${violationRoot.getText()}'`;
+            }
+        }
+    }
+
+    private getFailureForNewLine(first: ts.Node, violationRoot: ts.Node): string {
+        if (first === violationRoot) {
+            return `There should be no newline after '${violationRoot.getText()}'`;
+        } else {
+            return `There should be no newline before '${violationRoot.getText()}'`;
+        }
+    }
+
+    private reportFailure(start: ts.Node, endNode: ts.Node, failure: string): void {
+        this.addFailure(this.createFailure(start.getStart(), endNode.getStart() - start.getStart(), failure));
+    }
+
+    private isSpaceBetweenTokens(left: ts.Node, right: ts.Node): boolean {
+        // Inspired from https://github.com/eslint/eslint/blob/master/lib/util/source-code.js#L296
+        const text: string = this.getSourceFile().getText().slice(left.getEnd(), right.getStart());
+        return /\s/.test(text.replace(/\/\*.*?\*\//g, ''));
+    }
+
+    private isMultiline(left: ts.Node, right: ts.Node): boolean {
+        const sourceFile: ts.SourceFile = this.getSourceFile();
+        const leftLine: number = sourceFile.getLineAndCharacterOfPosition(left.getStart()).line;
+        const rightLine: number = sourceFile.getLineAndCharacterOfPosition(right.getStart()).line;
+        return leftLine !== rightLine;
+    }
+}

src/tests/ReactTsxCurlySpacingRuleTests.ts

@@ -0,0 +1,172 @@
+import { TestHelper } from './TestHelper';
+
+/**
+ * Unit tests.
+ */
+describe('reactTsxCurlySpacing', () => {
+    const ruleName: string = 'react-tsx-curly-spacing';
+
+    describe('should pass', () => {
+
+        describe('on single line expressions', () => {
+            it('when always set', () => {
+                const script: string = `
+                    import React = require('react');
+                    const a = <Hello name={ firstname } />;
+                `;
+                TestHelper.assertNoViolation(ruleName, script);
+            });
+
+            it('when never set', () => {
+                const script: string = `
+                    import React = require('react');
+                    const a = <Hello name={firstname} />;
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'never' ], script, []);
+            });
+        });
+
+        describe('on multi-line expressions', () => {
+            it('when always set', () => {
+                const script: string = `
+                    import React = require('react');
+                    <Hello name={
+                      firstname
+                    } />
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'always', { allowMultiline: true } ], script, []);
+            });
+
+            it('when never set', () => {
+                const script: string = `
+                    import React = require('react');
+                    <Hello name={
+                      firstname
+                    } />
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'never', { allowMultiline: true } ], script, []);
+            });
+        });
+    });
+
+    describe('should fail', () => {
+
+        describe('on single line expressions', () => {
+
+            it('when always set', () => {
+                const script: string = `
+                    import React = require('react');
+
+                    const a = <Hello name={firstname} />;
+                    const b = <Hello name={ firstname} />;
+                    const c = <Hello name={firstname } />;
+                `;
+                TestHelper.assertViolations(ruleName, script, [
+                    {
+                        "failure": "A space is required after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 4 }
+                    },
+                    {
+                        "failure": "A space is required before '}'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 4 }
+                    },
+                    {
+                        "failure": "A space is required before '}'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 5 }
+                    },
+                    {
+                        "failure": "A space is required after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 6 }
+                    }
+
+                ]);
+            });
+
+            it('when never set', () => {
+                const script: string = `
+                    import React = require('react');
+                    const a = <Hello name={ firstname} />;
+                    const b = <Hello name={firstname } />;
+                    const c = <Hello name={ firstname} />;
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'never' ], script, [
+                    {
+                        "failure": "There should be no space after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 3 }
+                    },
+                    {
+                        "failure": "There should be no space before '}'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 4 }
+                    },
+                    {
+                        "failure": "There should be no space after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 43, "line": 5 }
+                    }
+                ]);
+            });
+
+        });
+
+        describe('on multi-line expressions', () => {
+            it('when always set', () => {
+                const script: string = `
+                    import React = require('react');
+                    <Hello name={
+                      firstname
+                    } />
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'always', { allowMultiline: false } ], script, [
+                    {
+                        "failure": "There should be no newline after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 33, "line": 3 }
+                    },
+                    {
+                        "failure": "There should be no newline before '}'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 33, "line": 3 }
+                    }
+                ]);
+            });
+
+            it('when never set', () => {
+                const script: string = `
+                    import React = require('react');
+                    <Hello name={
+                      firstname
+                    } />
+                `;
+                TestHelper.assertViolationsWithOptions(ruleName, [ 'never' ], script, [
+                    {
+                        "failure": "There should be no newline after '{'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 33, "line": 3 }
+                    },
+                    {
+                        "failure": "There should be no newline before '}'",
+                        "name": "file.tsx",
+                        "ruleName": "react-tsx-curly-spacing",
+                        "startPosition": { "character": 33, "line": 3 }
+                    }
+                ]);
+            });
+        });
+    });
+});

tslint-warnings.csv

@@ -219,6 +219,7 @@ react-no-dangerous-html,Do not use React's dangerouslySetInnerHTML API.,TSLINTPH
 CWE 85 - Doubled Character XSS Manipulations
 CWE 710 - Coding Standards Violation"
 react-this-binding-issue,When using React components you must be careful to correctly bind the `this` reference on any methods that you pass off to child components as callbacks.,TSLINTPH85PH,tslint,Non-SDL,Error,Critical,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
+react-tsx-curly-spacing,Consistently use spaces around the brace characters of JSX attributes.,TSLINT1L70KC2,tslint,Non-SDL,Warning,Low,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,,
 react-unused-props-and-state,Remove unneeded properties defined in React Props and State interfaces,TSLINT9FI4LK,tslint,Non-SDL,Warning,Low,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,398,"CWE 398 - Indicator of Poor Code Quality"
 restrict-plus-operands,"When adding two variables, operands must both be of type number or of type string.",TSLINT15S62ML,tslint,Non-SDL,Warning,Moderate,Opportunity for Excellence,See description on the tslint or tslint-microsoft-contrib website,TSLint Procedure,"398, 597, 351, 480, 704, 710","CWE 398 - Indicator of Poor Code Quality
 CWE 597 - Use of Wrong Operator in String Comparison

tslint.json

@@ -173,6 +173,7 @@
     "react-iframe-missing-sandbox": true,
     "react-no-dangerous-html": true,
     "react-this-binding-issue": true,
+    "react-tsx-curly-spacing": true,
     "react-unused-props-and-state": true,
     "restrict-plus-operands": false,
     "semicolon": true,