Enhance NoEmptyRule (#2061)

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;
 }

src/rules/noEmptyRule.ts

@@ -43,26 +43,29 @@ 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);
             }
         }
 
         super.visitBlock(node);
     }
 }
 
-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,

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]
+}