Skip to content
This repository was archived by the owner on Jan 19, 2019. It is now read-only.

Commit

Permalink
[FIX] [no-namespace] Refactor rule (#257)
Browse files Browse the repository at this point in the history 
This PR changes logic in no-namespace rule and fixing some annoying bugs

- improve definition file detection
- migrate messages to messageId
- correct reporting `namespace Foo.Bar {}`
- add unit tests for isDefinitionFile & isTypescript
- add tests for allowDefinitionFiles
- [x] I’ve added tests for my changes
  • Loading branch information
@armano2 @bradzacher
armano2 authored and bradzacher committed Dec 21, 2018
1 parent 63311f3 commit a3b0a73
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 69 deletions.
66 changes: 14 additions & 52 deletions lib/rules/no-namespace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ module.exports = {
category: "TypeScript",
url: util.metaDocsUrl("no-namespace"),
},
messages: {
moduleSyntaxIsPreferred:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
},
schema: [
{
type: "object",
Expand All @@ -37,70 +41,28 @@ module.exports = {
},

create(context) {
const allowDeclarations = context.options[0]
? context.options[0].allowDeclarations
: false;
const allowDefinitionFiles = context.options[0]
? context.options[0].allowDefinitionFiles
: false;

//----------------------------------------------------------------------
// Helpers
//----------------------------------------------------------------------

/**
* Determines if node is a TypeScript module declaration (instead of a namespace/module).
* @param {ASTNode} node the node to be evaluated.
* @returns {boolean} true when node is an external declaration.
* @private
*/
function isTypeScriptModuleDeclaration(node) {
return node.id && node.id.type === "Literal";
}

/**
* Determines if node is a module/namespace declaration.
* @param {ASTNode} node the node to be evaluated.
* @returns {boolean} true when dealing with declarations.
* @private
*/
function isDeclaration(node) {
return (
node.declare === true && !isTypeScriptModuleDeclaration(node)
);
}

/**
* Determines if node is part of a declaration file (d.ts).
* @returns {boolean} true when dealing with declaration files.
* @private
*/
function isDefinitionFile() {
const filename = context.getFilename();

return filename
? filename.slice(-5).toLowerCase() === ".d.ts"
: false;
}
const options = context.options[0] || {};
const allowDeclarations = options.allowDeclarations || false;
const allowDefinitionFiles = options.allowDefinitionFiles || false;
const filename = context.getFilename();

//----------------------------------------------------------------------
// Public
//----------------------------------------------------------------------
return {
TSModuleDeclaration(node) {
"TSModuleDeclaration[global!=true][id.type='Identifier']"(node) {
if (
node.global ||
isTypeScriptModuleDeclaration(node) ||
(allowDefinitionFiles && isDefinitionFile()) ||
(allowDeclarations && isDeclaration(node))
(node.parent &&
node.parent.type === "TSModuleDeclaration") ||
(allowDefinitionFiles && util.isDefinitionFile(filename)) ||
(allowDeclarations && node.declare === true)
) {
return;
}

context.report({
node,
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
});
},
};
Expand Down
10 changes: 9 additions & 1 deletion lib/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,15 @@ exports.metaDocsUrl = name =>
* @returns {boolean} `true` if the file name ends in *.ts or *.tsx
* @private
*/
exports.isTypescript = fileName => /\.tsx?$/.test(fileName);
exports.isTypescript = fileName => /\.tsx?$/i.test(fileName || "");

/**
* Check if the context file name is *.d.ts or *.d.ts
* @param {string} fileName The context file name
* @returns {boolean} `true` if the file name ends in *.d.ts or *.d.ts
* @private
*/
exports.isDefinitionFile = fileName => /\.d\.tsx?$/i.test(fileName || "");

/**
* Pure function - doesn't mutate either parameter!
Expand Down
114 changes: 98 additions & 16 deletions tests/lib/rules/no-namespace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,23 @@ ruleTester.run("no-namespace", rule, {
code: "declare namespace foo { }",
options: [{ allowDeclarations: true }],
},
{
filename: "test.d.ts",
code: "namespace foo { }",
options: [{ allowDefinitionFiles: true }],
},
{
filename: "test.d.ts",
code: "module foo { }",
options: [{ allowDefinitionFiles: true }],
},
],
invalid: [
{
code: "module foo {}",
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -48,8 +57,7 @@ ruleTester.run("no-namespace", rule, {
code: "namespace foo {}",
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -60,8 +68,7 @@ ruleTester.run("no-namespace", rule, {
options: [{ allowDeclarations: false }],
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -72,8 +79,7 @@ ruleTester.run("no-namespace", rule, {
options: [{ allowDeclarations: false }],
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -83,8 +89,7 @@ ruleTester.run("no-namespace", rule, {
code: "declare module foo { }",
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -94,8 +99,7 @@ ruleTester.run("no-namespace", rule, {
code: "declare namespace foo { }",
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
Expand All @@ -106,24 +110,102 @@ ruleTester.run("no-namespace", rule, {
options: [{ allowDeclarations: false }],
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
code: "declare namespace foo {}",
options: [{ allowDeclarations: false }],
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
filename: "test.d.ts",
code: "namespace foo { }",
options: [{ allowDefinitionFiles: false }],
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
filename: "test.d.ts",
code: "module foo { }",
options: [{ allowDefinitionFiles: false }],
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
filename: "test.d.ts",
code: "declare module foo {}",
options: [{ allowDefinitionFiles: false }],
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
filename: "test.d.ts",
code: "declare namespace foo {}",
options: [{ allowDefinitionFiles: false }],
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
code: "namespace Foo.Bar {}",
options: [{ allowDeclarations: false }],
errors: [
{
message:
"ES2015 module syntax is preferred over custom TypeScript modules and namespaces.",
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 1,
},
],
},
{
code: `
namespace Foo.Bar {
namespace Baz.Bas {
interface X {}
}
}
`,
errors: [
{
messageId: "moduleSyntaxIsPreferred",
row: 1,
column: 17,
},
{
messageId: "moduleSyntaxIsPreferred",
row: 2,
column: 21,
},
],
},
],
});
52 changes: 52 additions & 0 deletions tests/lib/util.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
"use strict";
const assert = require("assert");

const util = require("../../lib/util");

describe("isTypescript", () => {
it("should return false", () => {
assert.strictEqual(util.isTypescript("test.js"), false);
assert.strictEqual(util.isTypescript("test.jsx"), false);
assert.strictEqual(util.isTypescript("README.md"), false);
assert.strictEqual(util.isTypescript("test.d.js"), false);
assert.strictEqual(util.isTypescript("test.ts.js"), false);
assert.strictEqual(util.isTypescript("test.ts.map"), false);
assert.strictEqual(util.isTypescript("test.ts-js"), false);
assert.strictEqual(util.isTypescript("ts"), false);
});

it("should return true", () => {
assert.strictEqual(util.isTypescript("test.ts"), true);
assert.strictEqual(util.isTypescript("test.tsx"), true);
assert.strictEqual(util.isTypescript("test.TS"), true);
assert.strictEqual(util.isTypescript("test.TSX"), true);
assert.strictEqual(util.isTypescript("test.d.ts"), true);
assert.strictEqual(util.isTypescript("test.d.tsx"), true);
assert.strictEqual(util.isTypescript("test.D.TS"), true);
assert.strictEqual(util.isTypescript("test.D.TSX"), true);
});
});

describe("isDefinitionFile", () => {
it("should return false", () => {
assert.strictEqual(util.isDefinitionFile("test.js"), false);
assert.strictEqual(util.isDefinitionFile("test.jsx"), false);
assert.strictEqual(util.isDefinitionFile("README.md"), false);
assert.strictEqual(util.isDefinitionFile("test.d.js"), false);
assert.strictEqual(util.isDefinitionFile("test.ts.js"), false);
assert.strictEqual(util.isDefinitionFile("test.ts.map"), false);
assert.strictEqual(util.isDefinitionFile("test.ts-js"), false);
assert.strictEqual(util.isDefinitionFile("test.ts"), false);
assert.strictEqual(util.isDefinitionFile("ts"), false);
assert.strictEqual(util.isDefinitionFile("test.tsx"), false);
assert.strictEqual(util.isDefinitionFile("test.TS"), false);
assert.strictEqual(util.isDefinitionFile("test.TSX"), false);
});

it("should return true", () => {
assert.strictEqual(util.isDefinitionFile("test.d.ts"), true);
assert.strictEqual(util.isDefinitionFile("test.d.tsx"), true);
assert.strictEqual(util.isDefinitionFile("test.D.TS"), true);
assert.strictEqual(util.isDefinitionFile("test.D.TSX"), true);
});
});

0 comments on commit a3b0a73

Please sign in to comment.