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

New: Add no-var-requires #114

Merged
merged 2 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Then configure the rules you want to use under the rules section.
* [`typescript/no-type-alias`](./docs/rules/no-type-alias.md) — Disallow the use of type aliases (`interface-over-type-literal` from TSLint)
* [`typescript/no-unused-vars`](./docs/rules/no-unused-vars.md) — Prevent TypeScript-specific constructs from being erroneously flagged as unused
* [`typescript/no-use-before-define`](./docs/rules/no-use-before-define.md) — Disallow the use of variables before they are defined
* [`typescript/no-var-requires`](./docs/rules/no-var-requires.md) — Disallows the use of require statements except in import statements (`no-var-requires` from TSLint)
* [`typescript/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) — Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules. (`no-internal-module` from TSLint)
* [`typescript/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) — Require consistent spacing around type annotations
<!-- end rule list -->
28 changes: 28 additions & 0 deletions docs/rules/no-var-requires.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Disallows the use of require statements except in import statements (no-var-requires)

In other words, the use of forms such as `var foo = require("foo")` are banned. Instead use ES6 style imports or `import foo = require("foo")` imports.

## Rule Details

Examples of **incorrect** code for this rule:

```ts
require("foo");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be allowed IMO since there isn’t an equivalent with the import = syntax.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I could see how introducing this as-is would be a breaking change, since all standalone require() calls would be flagged by this rule.

I will push up a fix that allows this. If we ultimately want to disallow it before merging, I will just revert the commit. 👍

var foo = require("foo");
const foo = require("foo");
let foo = require("foo");
```

Examples of **correct** code for this rule:

```ts
import foo = require("foo");
```

## When Not To Use It

If you don't care about TypeScript module syntax, then you will not need this rule.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These make me giggle each time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha yeah, I agree 😂


## Compatibility

* TSLint: [no-var-requires](https://palantir.github.io/tslint/rules/no-var-requires/)
42 changes: 42 additions & 0 deletions lib/rules/no-var-requires.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* @fileoverview Disallows the use of require statements except in import statements.
* @author Macklin Underdown
*/
"use strict";

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

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description:
"Disallows the use of require statements except in import statements",
extraDescription: [util.tslintRule("no-var-requires")],
category: "TypeScript",
url:
"https://github.com/nzakas/eslint-plugin-typescript/blob/master/docs/rules/no-var-requires.md"
},
schema: []
},
create(context) {
//----------------------------------------------------------------------
// Public
//----------------------------------------------------------------------

return {
CallExpression(node) {
if (node.callee.name === "require") {
context.report({
node,
message:
"Require statement not part of import statement"
});
}
}
};
}
};
66 changes: 66 additions & 0 deletions tests/lib/rules/no-var-requires.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* @fileoverview Disallows the use of require statements except in import statements.
* @author Macklin Underdown
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const rule = require("../../../lib/rules/no-var-requires"),
RuleTester = require("eslint").RuleTester;

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parser: "typescript-eslint-parser"
});

ruleTester.run("no-var-requires", rule, {
valid: ["import foo = require('foo')"],
invalid: [
{
code: "var foo = require('foo')",
errors: [
{
message: "Require statement not part of import statement",
line: 1,
column: 11
}
]
},
{
code: "const foo = require('foo')",
errors: [
{
message: "Require statement not part of import statement",
line: 1,
column: 13
}
]
},
{
code: "let foo = require('foo')",
errors: [
{
message: "Require statement not part of import statement",
line: 1,
column: 11
}
]
},
{
code: "require('foo')",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe an unassigned call to require() is invalid, but I'm not 100% sure about this based on TSLint's no-var-requires tests. Just calling this out in case anyone has more experience with TSLint's no-var-requires rule and if this truly an invalid case.

Copy link

@ajafff ajafff Mar 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can replace require("foo"); with import "foo";. Hence an error on this require call is expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about that, thanks for pointing that out. I’ll push up a commit next week when I get access to a computer

errors: [
{
message: "Require statement not part of import statement",
line: 1,
column: 1
}
]
}
]
});