Merge pull request #627 from vmmitev/fix-no-internal-module

Tests and improvement for no-internal-module rule on next branch

src/rules/noInternalModuleRule.ts

@@ -24,12 +24,23 @@ export class Rule extends Lint.Rules.AbstractRule {
 
 class NoInternalModuleWalker extends Lint.RuleWalker {
     public visitModuleDeclaration(node: ts.ModuleDeclaration) {
-        if (Lint.isNodeFlagSet(node, ts.NodeFlags.Namespace)) {
-            // ok namespace
-        } else if (node.name.kind === ts.SyntaxKind.Identifier) {
-            // for external modules, node.name will be a LiteralExpression instead of Identifier
+        if (this.isInternalModuleDeclaration(node)) {
             this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
         }
         super.visitModuleDeclaration(node);
     }
+
+    private isInternalModuleDeclaration(node: ts.ModuleDeclaration) {
+        // an internal module declaration is not a namespace or a nested declaration
+        // for external modules, node.name.kind will be a LiteralExpression instead of Identifier
+        return !Lint.isNodeFlagSet(node, ts.NodeFlags.Namespace)
+            && !this.isNestedDeclaration(node)
+            && node.name.kind === ts.SyntaxKind.Identifier;
+    }
+
+    private isNestedDeclaration(node: ts.ModuleDeclaration) {
+        // in a declaration expression like 'module a.b.c' - 'a' is the top level module declaration node and 'b' and 'c' are nested
+        // therefore we can depend that a node's position will only match with its name's position for nested nodes
+        return node.name.pos === node.pos;
+    }
 }

test/files/rules/nointernalmodule.test.ts

@@ -11,3 +11,42 @@ declare module "hoge" {
 }
 declare module 'fuga' {
 }
+
+namespace foo.bar {
+}
+namespace foo.bar.baz {
+}
+namespace foo {
+    namespace bar.baz {
+    }
+}
+
+namespace foo.bar {
+    module baz {
+        namespace buzz {
+        }
+    }
+}
+
+module foo.bar {
+    namespace baz {
+        module buzz {
+        }
+    }
+}
+
+namespace name.namespace {
+}
+namespace namespace.name {
+}
+
+// intentionally malformed for test cases, do not format
+declare module declare
+.dec{}
+declare  module dec . declare  {
+}
+
+module  mod.module{}
+module module.mod
+{
+}

test/rules/noInternalModuleTests.ts

@@ -23,7 +23,14 @@ describe("<no-internal-module>", () => {
         const actualFailures = Lint.Test.applyRuleOnFile(fileName, NoInternalModule);
         const expectedFailures = [
             Lint.Test.createFailure(fileName, [4, 1], [4, 15], failureString),
-            Lint.Test.createFailure(fileName, [7, 1], [7, 24], failureString)
+            Lint.Test.createFailure(fileName, [7, 1], [7, 24], failureString),
+            Lint.Test.createFailure(fileName, [25, 5], [28, 6], failureString),
+            Lint.Test.createFailure(fileName, [31, 1], [36, 2], failureString),
+            Lint.Test.createFailure(fileName, [33, 9], [33, 32], failureString),
+            Lint.Test.createFailure(fileName, [44, 1], [44, 30], failureString),
+            Lint.Test.createFailure(fileName, [46, 1], [46, 35], failureString),
+            Lint.Test.createFailure(fileName, [49, 1], [49, 21], failureString),
+            Lint.Test.createFailure(fileName, [50, 1], [50, 22], failureString)
         ];
 
         Lint.Test.assertFailuresEqual(actualFailures, expectedFailures);