Merge pull request palantir#957 from palantir/finally

Various fixes to one-line rule
sunnywp · Feb 5, 2016 · 0bdb454 · 0bdb454
2 parents 8c58ef2 + b588952
commit 0bdb454
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 3 deletions.
diff --git a/README.md b/README.md
@@ -206,6 +206,7 @@ A sample configuration file with all options is available [here](https://github.
 * `one-line` enforces the specified tokens to be on the same line as the expression preceding it. Rule options:
   * `"check-catch"` checks that `catch` is on the same line as the closing brace for `try`.
   * `"check-else"` checks that `else` is on the same line as the closing brace for `if`.
+  * `"check-finally"` checks that `finally` is on the same line as the closing brace for the preceding `try` or `catch`.
   * `"check-open-brace"` checks that an open brace falls on the same line as its preceding expression.
   * `"check-whitespace"` checks preceding whitespace for the specified tokens.
 * `quotemark` enforces consistent single or double quoted string literals. Rule options (at least one of `"double"` or `"single"` is required):

diff --git a/docs/sample.tslint.json b/docs/sample.tslint.json
@@ -75,6 +75,7 @@
       "check-open-brace",
       "check-catch",
       "check-else",
+      "check-finally",
       "check-whitespace"
     ],
     "quotemark": [

diff --git a/src/rules/oneLineRule.ts b/src/rules/oneLineRule.ts
@@ -21,12 +21,14 @@ import * as Lint from "../lint";
 const OPTION_BRACE = "check-open-brace";
 const OPTION_CATCH = "check-catch";
 const OPTION_ELSE = "check-else";
+const OPTION_FINALLY = "check-finally";
 const OPTION_WHITESPACE = "check-whitespace";
 
 export class Rule extends Lint.Rules.AbstractRule {
     public static BRACE_FAILURE_STRING = "misplaced opening brace";
     public static CATCH_FAILURE_STRING = "misplaced 'catch'";
     public static ELSE_FAILURE_STRING = "misplaced 'else'";
+    public static FINALLY_FAILURE_STRING = "misplaced 'finally'";
     public static WHITESPACE_FAILURE_STRING = "missing whitespace";
 
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
@@ -67,15 +69,17 @@ class OneLineWalker extends Lint.RuleWalker {
     }
 
     public visitCatchClause(node: ts.CatchClause) {
-        const catchKeyword = node.getChildAt(0);
+        const catchClosingParen = node.getChildren().filter((n) => n.kind === ts.SyntaxKind.CloseParenToken)[0];
         const catchOpeningBrace = node.block.getChildAt(0);
-        this.handleOpeningBrace(catchKeyword, catchOpeningBrace);
+        this.handleOpeningBrace(catchClosingParen, catchOpeningBrace);
         super.visitCatchClause(node);
     }
 
     public visitTryStatement(node: ts.TryStatement) {
         const sourceFile = node.getSourceFile();
         const catchClause = node.catchClause;
+        const finallyBlock = node.finallyBlock;
+        const finallyKeyword = node.getChildren().filter((n) => n.kind === ts.SyntaxKind.FinallyKeyword)[0];
 
         // "visit" try block
         const tryKeyword = node.getChildAt(0);
@@ -93,6 +97,23 @@ class OneLineWalker extends Lint.RuleWalker {
                 this.addFailure(failure);
             }
         }
+
+        if (finallyBlock != null && finallyKeyword != null) {
+            const finallyOpeningBrace = finallyBlock.getChildAt(0);
+            this.handleOpeningBrace(finallyKeyword, finallyOpeningBrace);
+
+            if (this.hasOption(OPTION_FINALLY)) {
+                const previousBlock = catchClause != null ? catchClause.block : node.tryBlock;
+                const closingBrace = previousBlock.getChildAt(previousBlock.getChildCount() - 1);
+                const closingBraceLine = sourceFile.getLineAndCharacterOfPosition(closingBrace.getEnd()).line;
+                const finallyKeywordLine = sourceFile.getLineAndCharacterOfPosition(finallyKeyword.getStart()).line;
+                if (closingBraceLine !== finallyKeywordLine) {
+                    const failure = this.createFailure(finallyKeyword.getStart(), finallyKeyword.getWidth(), Rule.FINALLY_FAILURE_STRING);
+                    this.addFailure(failure);
+                }
+            }
+        }
+
         super.visitTryStatement(node);
     }
 

diff --git a/test/rules/one-line/all/test.ts.lint b/test/rules/one-line/all/test.ts.lint
@@ -67,6 +67,33 @@ catch (e)
 ~ [misplaced opening brace]
     throw(e);
 }
+finally
+~~~~~~~ [misplaced 'finally']
+{
+~ [misplaced opening brace]
+    // do something
+}
+
+try {
+    foo();
+}
+finally
+~~~~~~~ [misplaced 'finally']
+{
+~ [misplaced opening brace]
+    bar();
+}
+
+try{
+   ~ [missing whitespace]
+    foo();
+} catch(e){
+          ~ [missing whitespace]
+    bar();
+} finally{
+         ~ [missing whitespace]
+    baz();
+}
 
 while(x < 10){
              ~ [missing whitespace]

diff --git a/test/rules/one-line/all/tslint.json b/test/rules/one-line/all/tslint.json
@@ -1,5 +1,5 @@
 {
   "rules": {
-    "one-line": [true, "check-open-brace", "check-catch", "check-else", "check-whitespace"]
+    "one-line": [true, "check-open-brace", "check-catch", "check-else", "check-finally", "check-whitespace"]
   }
 }
diff --git a/test/rules/one-line/none/test.ts.lint b/test/rules/one-line/none/test.ts.lint
@@ -55,6 +55,26 @@ catch (e)
 {
     throw(e);
 }
+finally
+{
+    // do something
+}
+
+try {
+    foo();
+}
+finally
+{
+    bar();
+}
+
+try{
+    foo();
+} catch(e){
+    bar();
+} finally{
+    baz();
+}
 
 while(x < 10){
     x++;

diff --git a/tslint.json b/tslint.json
@@ -45,6 +45,7 @@
       "check-open-brace",
       "check-catch",
       "check-else",
+      "check-finally",
       "check-whitespace"
     ],
     "quotemark": [true, "double", "avoid-escape"],