This repository has been archived by the owner on Mar 25, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 885
Add no-submodule-imports rule #3091
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
99e309e
lockfile
a6b53f0
init implementation
3289577
test.lint ~~~ now more specific
1fb4d85
like import-blacklist rule, no sensible default; added to tslint:all …
fdbf7f6
template literal requirement in rule's source ts
9994fc9
additional tests
77cc105
forEach -> for of; naming updates; dynamic import checking
8199561
ruleName metadata updated
0242b89
reverting to commit before permissions thing happened
baa9386
changes for readability; isSubmodulePath() updates
eabbc33
whitespace
1510446
listed correctly in all.ts
e392f7c
additional tests for relative paths; dynamic import syntax correction
f659a53
require relative path tests
ec9673f
added [typescript]: >=2.4.0 to dynamic import test file
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
/** | ||
* @license | ||
* Copyright 2017 Palantir Technologies, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import { | ||
isCallExpression, | ||
isExternalModuleReference, | ||
isIdentifier, | ||
isImportDeclaration, | ||
isImportEqualsDeclaration, | ||
isTextualLiteral, | ||
} from "tsutils"; | ||
import * as ts from "typescript"; | ||
import * as Lint from "../index"; | ||
|
||
export class Rule extends Lint.Rules.AbstractRule { | ||
/* tslint:disable:object-literal-sort-keys */ | ||
public static metadata: Lint.IRuleMetadata = { | ||
ruleName: "no-submodule-imports", | ||
description: Lint.Utils.dedent` | ||
Disallows importing any submodule.`, | ||
rationale: Lint.Utils.dedent` | ||
Submodules of some packages are treated as private APIs and the import | ||
paths may change without deprecation periods. It's best to stick with | ||
top-level package exports.`, | ||
optionsDescription: "A list of packages whose submodules are whitelisted.", | ||
options: { | ||
type: "array", | ||
items: { | ||
type: "string", | ||
}, | ||
minLength: 0, | ||
}, | ||
optionExamples: [true, [true, "rxjs", "@angular/core"]], | ||
type: "functionality", | ||
typescriptOnly: false, | ||
}; | ||
|
||
public static FAILURE_STRING = "Submodule import paths from this package are disallowed; import from the root instead"; | ||
|
||
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { | ||
return this.applyWithWalker(new NoSubmoduleImportsWalker(sourceFile, this.ruleName, this.ruleArguments)); | ||
} | ||
} | ||
|
||
class NoSubmoduleImportsWalker extends Lint.AbstractWalker<string[]> { | ||
public walk(sourceFile: ts.SourceFile) { | ||
const findDynamicImport = (node: ts.Node): void => { | ||
if (isCallExpression(node) && node.arguments.length === 1 && | ||
(isIdentifier(node.expression) && node.expression.text === "require" || | ||
node.expression.kind === ts.SyntaxKind.ImportKeyword)) { | ||
this.checkForBannedImport(node.arguments[0]); | ||
} | ||
return ts.forEachChild(node, findDynamicImport); | ||
}; | ||
|
||
for (const statement of sourceFile.statements) { | ||
if (isImportDeclaration(statement)) { | ||
this.checkForBannedImport(statement.moduleSpecifier); | ||
} else if (isImportEqualsDeclaration(statement)) { | ||
if (isExternalModuleReference(statement.moduleReference) && statement.moduleReference.expression !== undefined) { | ||
this.checkForBannedImport(statement.moduleReference.expression); | ||
} | ||
} else { | ||
ts.forEachChild(statement, findDynamicImport); | ||
} | ||
} | ||
} | ||
|
||
private checkForBannedImport(expression: ts.Expression) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a static function (doesn't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only if |
||
if (isTextualLiteral(expression)) { | ||
if (isAbsoluteOrRelativePath(expression.text) || !isSubmodulePath(expression.text)) { | ||
return; | ||
} | ||
|
||
/** | ||
* A submodule is being imported. | ||
* Check if its path contains any | ||
* of the whitelist packages. | ||
*/ | ||
for (const option of this.options) { | ||
if (expression.text.startsWith(`${option}/`)) { | ||
return; | ||
} | ||
} | ||
|
||
this.addFailureAtNode(expression, Rule.FAILURE_STRING); | ||
} | ||
} | ||
} | ||
|
||
function isAbsoluteOrRelativePath(path: string): boolean { | ||
return /^(..?(\/|$)|\/)/.test(path); | ||
} | ||
|
||
function isScopedPath(path: string): boolean { | ||
return path[0] === "@"; | ||
} | ||
|
||
function isSubmodulePath(path: string): boolean { | ||
return path.split("/").length > (isScopedPath(path) ? 2 : 1); | ||
} |
28 changes: 28 additions & 0 deletions
28
test/rules/no-submodule-imports/dynamic-imports/test.ts.lint
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
[typescript]: >=2.4.0 | ||
|
||
const dynamicImport = import("lodash"); | ||
|
||
const dynamicImport = import("lodash/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
const dynamicImport = import("lodash/a/b/c/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
|
||
const dynamicImport = import("@blueprintjs/core"); | ||
|
||
const dynamicImport = import("@blueprintjs/core/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
const dynamicImport = import("@blueprintjs/core/a/b/c/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
const dynamicImport = import("rxjs"); | ||
const dynamicImport = import("rxjs/sub-module"); | ||
const dynamicImport = import("@angular/core"); | ||
const dynamicImport = import("@angular/core/forms/directives"); | ||
|
||
const dynamicImport = import("./myModule"); | ||
const dynamicImport = import("./myModule/a/b/c/sub-module"); | ||
|
||
[0]: Submodule import paths from this package are disallowed; import from the root instead |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"rules": { | ||
"no-submodule-imports": [true, "@angular/core", "rxjs"] | ||
} | ||
} |
65 changes: 65 additions & 0 deletions
65
test/rules/no-submodule-imports/static-imports/test.ts.lint
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import { submodule } from "@blueprintjs/core"; | ||
|
||
import { submodule } from "@blueprintjs/core/sub-module"; | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import submodule = require("@blueprintjs/core"); | ||
|
||
import submodule = require("@blueprintjs/core/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import { submodule } from "@blueprintjs/core"; | ||
|
||
import { submodule } from "@blueprintjs/core/a/b/c/sub-module"; | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import submodule = require("@blueprintjs/core"); | ||
|
||
import submodule = require("@blueprintjs/core/a/b/c/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import { submodule } from "@angular/core"; | ||
import { submodule } from "@angular/core/sub-module"; | ||
import submodule = require("@angular/core"); | ||
import submodule = require("@angular/core/sub-module"); | ||
import { submodule } from "@angular/core"; | ||
import { submodule } from "@angular/core/a/b/c/sub-module"; | ||
import submodule = require("@angular/core"); | ||
import submodule = require("@angular/core/a/b/c/sub-module"); | ||
|
||
|
||
import { submodule } from "lodash"; | ||
|
||
import { submodule } from "lodash/sub-module"; | ||
~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import submodule = require("lodash"); | ||
|
||
import submodule = require("lodash/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import { submodule } from "lodash"; | ||
|
||
import { submodule } from "lodash/a/b/c/sub-module"; | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import submodule = require("lodash"); | ||
|
||
import submodule = require("lodash/a/b/c/sub-module"); | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ [0] | ||
|
||
import { submodule } from "rxjs"; | ||
import { submodule } from "rxjs/sub-module"; | ||
import submodule = require("rxjs"); | ||
import submodule = require("rxjs/sub-module"); | ||
import { submodule } from "rxjs"; | ||
import { submodule } from "rxjs/a/b/c/sub-module"; | ||
import submodule = require("rxjs"); | ||
import submodule = require("rxjs/a/b/c/sub-module"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test with a relative path |
||
|
||
import { submodule } from "./../node_modules/package"; | ||
import { submodule } from "../myModule/a/package"; | ||
import submodule = require("./../node_modules/package"); | ||
import submodule = require("../myModule/a/package"); | ||
|
||
[0]: Submodule import paths from this package are disallowed; import from the root instead |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"rules": { | ||
"no-submodule-imports": [true, "@angular/core", "rxjs"] | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't need to be a
const
, please make it a plain helperfunction
at the end of the fileThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can't. It uses the lexical this binding