Merge branch 'master' into next

palantir · Feb 8, 2016 · 027988b · 027988b
2 parents a35071b + 0bdb454
commit 027988b
Show file tree

Hide file tree

Showing 40 changed files with 450 additions and 77 deletions.
diff --git a/.gitattributes b/.gitattributes
@@ -0,0 +1,5 @@
+# lint files can't get an extra \r character before \n, otherwise
+# they'll make some rule tests fail on windows, due to the lint error
+# being reported to be 1 character longer
+
+*.lint text eol=lf
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/configuration.ts b/src/configuration.ts
@@ -39,17 +39,17 @@ export const DEFAULT_CONFIG = {
             "index-signature": "nospace",
             "parameter": "nospace",
             "property-declaration": "nospace",
-            "variable-declaration": "nospace"
-        }],
+            "variable-declaration": "nospace",
+        }, ],
         "variable-name": [true, "ban-keywords"],
         "whitespace": [true,
             "check-branch",
             "check-decl",
             "check-operator",
             "check-separator",
-            "check-type"
+            "check-type",
         ],
-    }
+    },
 };
 
 export function findConfiguration(configFile: string, inputFileLocation: string): any {
@@ -110,7 +110,7 @@ function getHomeDir() {
         environment.USERPROFILE,
         environment.HOME,
         environment.HOMEPATH,
-        environment.HOMEDRIVE + environment.HOMEPATH
+        environment.HOMEDRIVE + environment.HOMEPATH,
     ];
 
     for (const homePath of paths) {

diff --git a/src/enableDisableRules.ts b/src/enableDisableRules.ts
@@ -64,7 +64,7 @@ export class EnableDisableRulesWalker extends SkippableTokenAwareRuleWalker {
                     }
                     this.enableDisableRuleMap[ruleToAdd].push({
                         isEnabled: isEnabled,
-                        position: startingPosition
+                        position: startingPosition,
                     });
                 }
             }

diff --git a/src/formatters/msbuildFormatter.ts b/src/formatters/msbuildFormatter.ts
@@ -0,0 +1,34 @@
+/**
+ * @license
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {AbstractFormatter} from "../language/formatter/abstractFormatter";
+import {RuleFailure} from "../language/rule/rule";
+
+export class Formatter extends AbstractFormatter {
+    public format(failures: RuleFailure[]): string {
+        const outputLines = failures.map((failure: RuleFailure) => {
+            const fileName = failure.getFileName();
+            const failureString = failure.getFailure();
+
+            const lineAndCharacter = failure.getStartPosition().getLineAndCharacter();
+            const positionTuple = `(${lineAndCharacter.line + 1},${lineAndCharacter.character + 1})`;
+
+            return `${fileName}${positionTuple}: warning: ${failureString}`;
+        });
+
+        return outputLines.join("\n") + "\n";
+    }
+}
diff --git a/src/language/languageServiceHost.ts b/src/language/languageServiceHost.ts
@@ -26,7 +26,7 @@ export function createLanguageServiceHost(fileName: string, source: string): ts.
         getScriptFileNames: () => [fileName],
         getScriptSnapshot: (name: string) => ts.ScriptSnapshot.fromString(name === fileName ? source : ""),
         getScriptVersion: () => "1",
-        log: () => { /* */ }
+        log: () => { /* */ },
     };
 }
 

diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts
@@ -35,7 +35,7 @@ export abstract class AbstractRule implements IRule {
         this.options = {
             disabledIntervals: disabledIntervals,
             ruleArguments: ruleArguments,
-            ruleName: ruleName
+            ruleName: ruleName,
         };
     }
 

diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts
@@ -57,7 +57,7 @@ export class RuleFailurePosition {
         return {
             character: this.lineAndCharacter.character,
             line: this.lineAndCharacter.line,
-            position: this.position
+            position: this.position,
         };
     }
 
@@ -119,7 +119,7 @@ export class RuleFailure {
             failure: this.failure,
             name: this.fileName,
             ruleName: this.ruleName,
-            startPosition: this.startPosition.toJson()
+            startPosition: this.startPosition.toJson(),
         };
     }
 

diff --git a/src/language/utils.ts b/src/language/utils.ts
@@ -36,7 +36,7 @@ export function getSourceFile(fileName: string, source: string): ts.SourceFile {
         },
         readFile: () => null,
         useCaseSensitiveFileNames: () => true,
-        writeFile: () => null
+        writeFile: () => null,
     };
 
     const program = ts.createProgram([normalizedName], compilerOptions, compilerHost);
@@ -47,7 +47,7 @@ export function getSourceFile(fileName: string, source: string): ts.SourceFile {
 export function createCompilerOptions(): ts.CompilerOptions {
     return {
         noResolve: true,
-        target: ts.ScriptTarget.ES5
+        target: ts.ScriptTarget.ES5,
     };
 }
 

diff --git a/src/ruleLoader.ts b/src/ruleLoader.ts
@@ -33,11 +33,15 @@ export function loadRules(ruleConfiguration: {[name: string]: any},
                           enableDisableRuleMap: {[rulename: string]: IEnableDisablePosition[]},
                           rulesDirectories?: string | string[]): IRule[] {
     const rules: IRule[] = [];
+    const notFoundRules: string[] = [];
+
     for (const ruleName in ruleConfiguration) {
         if (ruleConfiguration.hasOwnProperty(ruleName)) {
             const ruleValue = ruleConfiguration[ruleName];
             const Rule = findRule(ruleName, rulesDirectories);
-            if (Rule !== undefined) {
+            if (Rule == null) {
+                notFoundRules.push(ruleName);
+            } else {
                 const all = "all"; // make the linter happy until we can turn it on and off
                 const allList = (all in enableDisableRuleMap ? enableDisableRuleMap[all] : []);
                 const ruleSpecificList = (ruleName in enableDisableRuleMap ? enableDisableRuleMap[ruleName] : []);
@@ -47,7 +51,12 @@ export function loadRules(ruleConfiguration: {[name: string]: any},
         }
     }
 
-    return rules;
+    if (notFoundRules.length > 0) {
+        const errorMessage = `Could not find the following rules specified in the configuration:\n${notFoundRules.join("\n")}`;
+        throw new Error(errorMessage);
+    } else {
+        return rules;
+    }
 }
 
 export function findRule(name: string, rulesDirectories?: string | string[]) {
@@ -134,7 +143,7 @@ function buildDisabledIntervalsFromSwitches(ruleSpecificList: IEnableDisablePosi
                 // we're currently disabled and about to enable -- end the interval
                 disabledIntervalList.push({
                     endPosition: newPositionToCheck.position,
-                    startPosition: disabledStartPosition
+                    startPosition: disabledStartPosition,
                 });
                 isCurrentlyDisabled = false;
             }
@@ -145,7 +154,7 @@ function buildDisabledIntervalsFromSwitches(ruleSpecificList: IEnableDisablePosi
         // we started an interval but didn't finish one -- so finish it with an Infinity
         disabledIntervalList.push({
             endPosition: Infinity,
-            startPosition: disabledStartPosition
+            startPosition: disabledStartPosition,
         });
     }
 

diff --git a/src/rules/memberOrderingRule.ts b/src/rules/memberOrderingRule.ts
@@ -37,7 +37,7 @@ function getModifiers(isMethod: boolean, modifiers?: ts.ModifiersArray): IModifi
     return {
         isInstance: !Lint.hasModifier(modifiers, ts.SyntaxKind.StaticKeyword),
         isMethod: isMethod,
-        isPrivate: Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)
+        isPrivate: Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword),
     };
 }
 
@@ -46,7 +46,7 @@ function toString(modifiers: IModifiers): string {
         modifiers.isPrivate ? "private" : "public",
         modifiers.isInstance ? "instance" : "static",
         "member",
-        modifiers.isMethod ? "function" : "variable"
+        modifiers.isMethod ? "function" : "variable",
     ].join(" ");
 }
 
@@ -95,7 +95,7 @@ export class MemberOrderingWalker extends Lint.RuleWalker {
         this.previousMember = {
             isInstance: false,
             isMethod: false,
-            isPrivate: false
+            isPrivate: false,
         };
     }
 

diff --git a/src/rules/noConstructRule.ts b/src/rules/noConstructRule.ts
@@ -30,7 +30,7 @@ class NoConstructWalker extends Lint.RuleWalker {
     private static FORBIDDEN_CONSTRUCTORS = [
         "Boolean",
         "Number",
-        "String"
+        "String",
     ];
 
     public visitNewExpression(node: ts.NewExpression) {

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/src/rules/trailingCommaRule.ts b/src/rules/trailingCommaRule.ts
@@ -53,7 +53,7 @@ class TrailingCommaWalker extends Lint.RuleWalker {
     private lintNode(node: ts.Node) {
         const child = node.getChildAt(1);
         if (child != null && child.kind === ts.SyntaxKind.SyntaxList) {
-            const isMultiline = child.getText().match(/\n|\r/);
+            const isMultiline = node.getText().match(/\n|\r/);
             const option = this.getOption(isMultiline ? "multiline" : "singleline");
             const grandChildren = child.getChildren();
 

diff --git a/src/test.ts b/src/test.ts
@@ -64,12 +64,12 @@ export function runTest(testDirectory: string): TestResult {
             return {
                 endPos: {
                     col: endLineAndCharacter.character,
-                    line: endLineAndCharacter.line
+                    line: endLineAndCharacter.line,
                 },
                 message: failure.getFailure(),
                 startPos: {
                     col: startLineAndCharacter.character,
-                    line: startLineAndCharacter.line
+                    line: startLineAndCharacter.line,
                 },
             };
         });
@@ -105,7 +105,7 @@ export function consoleTestResultHandler(testResult: TestResult): boolean {
             didAllTestsPass = false;
 
             for (const diffResult of diffResults) {
-                let color = colors.gray;
+                let color = colors.grey;
                 if (diffResult.added) {
                     color = colors.green;
                 } else if (diffResult.removed) {

diff --git a/src/test/parse.ts b/src/test/parse.ts
@@ -15,8 +15,16 @@
  */
 
 import {LintError, errorComparator, lintSyntaxError} from "./lintError";
-import {Line, ErrorLine, CodeLine, MultilineErrorLine, EndErrorLine, MessageSubstitutionLine,
-        parseLine, printLine} from "./lines";
+import {
+    Line,
+    ErrorLine,
+    CodeLine,
+    MultilineErrorLine,
+    EndErrorLine,
+    MessageSubstitutionLine,
+    parseLine,
+    printLine,
+} from "./lines";
 
 /**
  * Takes the full text of a .lint file and returns the contents of the file
@@ -65,7 +73,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
                 lintErrors.push({
                     startPos: errorStartPos,
                     endPos: { line: lineNo, col: errorLine.endCol },
-                    message: messageSubstitutions[errorLine.message] || errorLine.message
+                    message: messageSubstitutions[errorLine.message] || errorLine.message,
                 });
 
             // if the error is the start of a multiline error
@@ -85,7 +93,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
                             lintErrors.push({
                                 startPos: errorStartPos,
                                 endPos: { line: nextLineNo, col: nextErrorLine.endCol },
-                                message: messageSubstitutions[nextErrorLine.message] || nextErrorLine.message
+                                message: messageSubstitutions[nextErrorLine.message] || nextErrorLine.message,
                             });
                             break;
                         }

diff --git a/src/tsconfig.json b/src/tsconfig.json
@@ -41,6 +41,7 @@
         "tslint.ts",
         "formatters/index.ts",
         "formatters/jsonFormatter.ts",
+        "formatters/msbuildFormatter.ts",
         "formatters/pmdFormatter.ts",
         "formatters/proseFormatter.ts",
         "formatters/verboseFormatter.ts",