From d65114566808959c3d7ce423b68b4474f4edd232 Mon Sep 17 00:00:00 2001
From: Klaus Meinhardt <klaus.meinhardt1@gmail.com>
Date: Fri, 20 Jan 2017 14:57:47 +0100
Subject: [PATCH] Enhance NoEmptyRule (#2061)

---
 src/language/utils.ts            |  2 +-
 src/rules/noEmptyRule.ts         | 25 ++++++++++++++-----------
 test/rules/no-empty/test.ts.lint | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/src/language/utils.ts b/src/language/utils.ts
index fd122a9910a..93ee4091a3e 100644
--- a/src/language/utils.ts
+++ b/src/language/utils.ts
@@ -413,5 +413,5 @@ function canHaveTrailingTrivia(tokenKind: ts.SyntaxKind, parent: ts.Node): boole
  */
 export function hasCommentAfterPosition(text: string, position: number): boolean {
     return ts.getTrailingCommentRanges(text, position) !== undefined ||
-           ts.getTrailingCommentRanges(text, position) !== undefined;
+           ts.getLeadingCommentRanges(text, position) !== undefined;
 }
diff --git a/src/rules/noEmptyRule.ts b/src/rules/noEmptyRule.ts
index 1b384227b40..bd1bef5ce0e 100644
--- a/src/rules/noEmptyRule.ts
+++ b/src/rules/noEmptyRule.ts
@@ -43,17 +43,13 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 class BlockWalker extends Lint.RuleWalker {
     public visitBlock(node: ts.Block) {
-        if (node.statements.length === 0 && !isConstructorWithParameterProperties(node.parent!)) {
+        if (node.statements.length === 0 && !isExcludedConstructor(node.parent!)) {
             const sourceFile = this.getSourceFile();
-            const children = node.getChildren(sourceFile);
-            const openBrace = children[0];
-            const closeBrace = children[children.length - 1];
-            const sourceFileText = sourceFile.text;
-
-            if (ts.getLeadingCommentRanges(sourceFileText, closeBrace.getFullStart()) === undefined &&
-                ts.getTrailingCommentRanges(sourceFileText, openBrace.getEnd()) === undefined) {
-
-                this.addFailureAtNode(node, Rule.FAILURE_STRING);
+            const start = node.getStart(sourceFile);
+            // Block always starts with open brace. Adding 1 to its start gives us the end of the brace,
+            // which can be used to conveniently check for comments between braces
+            if (!Lint.hasCommentAfterPosition(sourceFile.text, start + 1)) {
+                this.addFailureFromStartToEnd(start , node.getEnd(), Rule.FAILURE_STRING);
             }
         }
 
@@ -61,8 +57,15 @@ class BlockWalker extends Lint.RuleWalker {
     }
 }
 
-function isConstructorWithParameterProperties(node: ts.Node): boolean {
+function isExcludedConstructor(node: ts.Node): boolean {
     if (node.kind === ts.SyntaxKind.Constructor) {
+        if (Lint.hasModifier(node.modifiers, ts.SyntaxKind.PrivateKeyword, ts.SyntaxKind.ProtectedKeyword)) {
+            /* If constructor is private or protected, the block is allowed to be empty.
+               The constructor is there on purpose to disallow instantiation from outside the class */
+            /* The public modifier does not serve a purpose here. It can only be used to allow instantiation of a base class where
+               the super constructor is protected. But then the block would not be empty, because of the call to super() */
+            return true;
+        }
         for (const parameter of (node as ts.ConstructorDeclaration).parameters) {
             if (Lint.hasModifier(parameter.modifiers,
                                  ts.SyntaxKind.PrivateKeyword,
diff --git a/test/rules/no-empty/test.ts.lint b/test/rules/no-empty/test.ts.lint
index 9abc44e9d88..1846cc5ff6f 100644
--- a/test/rules/no-empty/test.ts.lint
+++ b/test/rules/no-empty/test.ts.lint
@@ -23,6 +23,11 @@ for (var x = 0; x < 1; ++x) { }
 for (var y = 0; y < 1; ++y) {
     // empty here
 }
+{ // empty block allowed
+}
+{
+    /* this block is also empty, but allowed to be */
+}
 
 class testClass {
     constructor(private allowed: any, private alsoAllowed: any) {
@@ -45,3 +50,16 @@ class testClass4 {
     constructor(readonly allowed: any) {
     }
 }
+
+class PrivateClassConstructor {
+    private constructor() {}
+}
+
+class ProtectedClassConstructor {
+    protected constructor() {}
+}
+
+class PublicClassConstructor {
+    public constructor() {}
+                         ~~ [block is empty]
+}